Repository navigation
feat: support chrome.storage.sync in extensions - #55253
samuelmaddock merged 4 commits into
Conversation
Register a ValueStoreCache for the SYNC settings namespace from ElectronExtensionsAPIClient::AddAdditionalValueStoreCaches. Until now Electron never overrode that hook, so StorageFrontend only knew about LOCAL (and the in-memory SESSION area) and every chrome.storage.sync call failed with '"sync" is not available in this instance of Chrome'. ElectronSyncValueStoreCache mirrors Chromium's LocalValueStoreCache but uses settings_namespace::SYNC, which gives it its own LevelDB directory under the partition path (kSyncExtensionSettingsDirectoryName), so sync and local never alias. It enforces the chrome.storage.sync quotas from extensions/common/api/storage.json (QUOTA_BYTES 102400, QUOTA_BYTES_PER_ITEM 8192, MAX_ITEMS 512); the per-minute and per-hour write limits are already applied to kSync by storage_api.cc. There is no sync service in Electron, so the area behaves like Chrome with sync disabled: a second local store with no cross-device sync. chrome.storage.managed stays unsupported because there is no policy source to back it. This answers the question raised on #53995 about supporting chrome.storage instead of dropping sync from the test fixture. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFucMLGyVVqSUAKXrqbaUT Co-authored-by: Claude <noreply@anthropic.com>
The linux-x64 and linux-x64-ubsan builds failed compiling
electron_extensions_api_client.cc:
../../base/memory/scoped_refptr.h:398:6: error: member access into
incomplete type 'value_store::ValueStoreFactory'
../../base/memory/scoped_refptr.h:405:6: error: member access into
incomplete type 'value_store::ValueStoreFactory'
note: in instantiation of ... requested here
electron_extensions_api_client.cc:97:39:
new ElectronSyncValueStoreCache(factory);
AddAdditionalValueStoreCaches copies the
scoped_refptr<value_store::ValueStoreFactory> into the new
ElectronSyncValueStoreCache, which instantiates AddRef/Release on the
pointee. extensions_api_client.h and electron_sync_value_store_cache.h
only forward-declare ValueStoreFactory, so the translation unit needs
the full definition. Include components/value_store/value_store_factory.h
the same way chrome_extensions_api_client.cc does.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AFucMLGyVVqSUAKXrqbaUT
Co-authored-by: Claude <noreply@anthropic.com>
main dropped chai in favour of vitest's own matchers and now lints against chai-style chains, so convert the chrome.storage.sync spec's assertions to the equivalent vitest matchers. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFucMLGyVVqSUAKXrqbaUT Co-authored-by: Claude <noreply@anthropic.com>
The spec file on this branch does not import randomUUID from node:crypto, so the backported test failed the spec typecheck. Use the global instead, which needs no import. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
The latest push fixes the spec issue I raised earlier: the test now calls the global crypto.randomUUID(). I found no new bugs. A human should still look, because this adds a new C++ storage backend that relies on upstream contracts I can't see.
What was reviewed (from a standalone clone, so no Chromium sources were available):
ElectronSyncValueStoreCache: it creates stores per extension from the shared factory, wraps each one in the sync quota enforcer, and handlesDeleteStorageSoon, which deletes both the APP and EXTENSION stores.- How it is registered in
ElectronExtensionsAPIClient::AddAdditionalValueStoreCaches, and the new sources infilenames.gni. - The new spec and fixture: separation from local storage,
onChangedwith areasync, the quota error, andclear.
I can't see ValueStoreCache, SettingsStorageQuotaEnforcer or value_store_util from this checkout. Can you confirm that the cache map takes ownership of the raw new'd cache and that it is destroyed on the backend sequence, as the destructor's DCHECK expects?
Extended reasoning...
This backport adds a new C++ class, ElectronSyncValueStoreCache, that backs chrome.storage.sync with a quota-limited local store. It registers the class in ElectronExtensionsAPIClient and adds a spec and fixture. It doesn't touch IPC handlers, webPreferences, or permission or protocol handlers. The one finding from the earlier review (the undefined randomUUID in the spec) is fixed in the latest commit. I'm deferring rather than approving because this is new feature code whose ownership and threading depend on upstream extensions storage APIs that aren't in this standalone checkout.
|
Release Notes Persisted
|
Backport of #54088
See that PR for details.
Notes: Added support for
chrome.storage.syncin extensions. It behaves as a second local storage area with Chrome's sync quotas; there is no cross-device sync.