Repository navigation
fix(extensions): crash when using chrome.system.storage - #55624
Conversation
Electron never created the global StorageMonitor that the API dereferences without a null check. Create it like Chrome does, and destroy it after browser contexts are gone. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Samuel Maddock <samuel.maddock@gmail.com>
Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Samuel Maddock <samuel.maddock@gmail.com>
There was a problem hiding this comment.
I reviewed this backport and didn't find any bugs. It changes C++ startup and shutdown order in the browser process and leaves a known Linux DCHECK path in place, so a human look would still be worthwhile.
What was reviewed:
- Shutdown order:
ElectronBrowserMainParts::PostMainMessageLoopRuncallsElectronBrowserContext::DestroyAllContexts()(electron_browser_main_parts.cc:724) beforefake_browser_process_->PostMainMessageLoopRun()(:726). SoStorageMonitor::Destroy()runs after the contexts are gone, as the new comment says. StorageMonitor::Create(),Destroy(), the include and the//components/storage_monitordep are all insideENABLE_ELECTRON_EXTENSIONSguards. The BUILD.gn dep is at line 948.- The new suite is skipped on Linux because device scanning hits
DCHECK(!unique_id.empty()). Debug builds on Linux can still reach that DCHECK, yet docs/api/extensions.md:162 says all features of the API are supported.
Extended reasoning...
This backport adds a //components/storage_monitor dep, creates and destroys the global StorageMonitor in shell/browser/browser_process_impl.cc under the extensions buildflag, adds a docs section and adds a 3-test spec suite that is skipped on Linux. There is no IPC, permission or webPreferences change, so the security-sensitive surface is limited to which extensions can use the new API. I confirmed the teardown order claim from electron_browser_main_parts.cc. This is a standalone clone, so I couldn't check upstream StorageMonitor behavior, and that is part of why I deferred. The other reason is the Linux DCHECK path that the spec skip works around rather than fixes.
|
Release Notes Persisted
|
Backport of #55612
See that PR for details.
Notes: Fixed a crash when extensions access
chrome.system.storageAPIs.