Browse Source

refactor: add `ElectronBrowserContext::GetDefaultBrowserContext()` (#46065)

* refactor: add ElectronBrowserContext::DestroyAllContexts()

Simpler semantics than previous implementation; also hides the
"default context must be destroyed last" implementation detail.

* refactor: add ElectronBrowserContext::GetDefaultBrowserContext()

clearer semantics than everyone calling From("", false)
Charles Kerr 1 month ago
parent
commit
4bf99c9bea

+ 1 - 1
shell/browser/api/electron_api_global_shortcut.cc

@@ -119,7 +119,7 @@ bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
   }
 
   if (instance->IsRegistrationHandledExternally()) {
-    auto* context = ElectronBrowserContext::From("", false);
+    auto* context = ElectronBrowserContext::GetDefaultBrowserContext();
     PrefService* prefs = context->prefs();
 
     // Need a unique profile id. Set one if not generated yet, otherwise re-use

+ 1 - 1
shell/browser/api/electron_api_session.cc

@@ -1679,7 +1679,7 @@ gin::Handle<Session> Session::FromPartition(v8::Isolate* isolate,
   ElectronBrowserContext* browser_context;
   if (partition.empty()) {
     browser_context =
-        ElectronBrowserContext::From("", false, std::move(options));
+        ElectronBrowserContext::GetDefaultBrowserContext(std::move(options));
   } else if (partition.starts_with(kPersistPrefix)) {
     std::string name = partition.substr(8);
     browser_context =

+ 14 - 0
shell/browser/electron_browser_context.cc

@@ -318,6 +318,14 @@ ElectronBrowserContext::browser_context_map() {
   return *browser_context_map;
 }
 
+// static
+void ElectronBrowserContext::DestroyAllContexts() {
+  auto& map = browser_context_map();
+  // Avoid UAF by destroying the default context last. See ba629e3 for info.
+  const auto extracted = map.extract(PartitionKey{"", false});
+  map.clear();
+}
+
 ElectronBrowserContext::ElectronBrowserContext(
     const PartitionOrPath partition_location,
     bool in_memory,
@@ -841,6 +849,12 @@ ElectronBrowserContext* ElectronBrowserContext::From(
   return context.get();
 }
 
+// static
+ElectronBrowserContext* ElectronBrowserContext::GetDefaultBrowserContext(
+    base::Value::Dict options) {
+  return ElectronBrowserContext::From("", false, std::move(options));
+}
+
 ElectronBrowserContext* ElectronBrowserContext::FromPath(
     const base::FilePath& path,
     base::Value::Dict options) {

+ 6 - 0
shell/browser/electron_browser_context.h

@@ -84,6 +84,10 @@ class ElectronBrowserContext : public content::BrowserContext {
   using BrowserContextMap =
       std::map<PartitionKey, std::unique_ptr<ElectronBrowserContext>>;
 
+  // Get or create the default BrowserContext.
+  static ElectronBrowserContext* GetDefaultBrowserContext(
+      base::Value::Dict options = {});
+
   // Get or create the BrowserContext according to its |partition| and
   // |in_memory|. The |options| will be passed to constructor when there is no
   // existing BrowserContext.
@@ -99,6 +103,8 @@ class ElectronBrowserContext : public content::BrowserContext {
 
   static BrowserContextMap& browser_context_map();
 
+  static void DestroyAllContexts();
+
   void SetUserAgent(const std::string& user_agent);
   std::string GetUserAgent() const;
   bool can_use_http_cache() const { return use_cache_; }

+ 1 - 5
shell/browser/electron_browser_main_parts.cc

@@ -598,11 +598,7 @@ void ElectronBrowserMainParts::PostMainMessageLoopRun() {
   node_bindings_->set_uv_env(nullptr);
   node_env_.reset();
 
-  auto default_context_key = ElectronBrowserContext::PartitionKey("", false);
-  std::unique_ptr<ElectronBrowserContext> default_context = std::move(
-      ElectronBrowserContext::browser_context_map()[default_context_key]);
-  ElectronBrowserContext::browser_context_map().clear();
-  default_context.reset();
+  ElectronBrowserContext::DestroyAllContexts();
 
   fake_browser_process_->PostMainMessageLoopRun();
   content::DevToolsAgentHost::StopRemoteDebuggingPipeHandler();

+ 1 - 1
shell/browser/extensions/electron_extensions_browser_client.cc

@@ -112,7 +112,7 @@ BrowserContext* ElectronExtensionsBrowserClient::GetOriginalContext(
     BrowserContext* context) {
   DCHECK(context);
   if (context->IsOffTheRecord()) {
-    return ElectronBrowserContext::From("", false);
+    return ElectronBrowserContext::GetDefaultBrowserContext();
   } else {
     return context;
   }

+ 1 - 2
shell/browser/net/electron_url_loader_factory.cc

@@ -544,8 +544,7 @@ void ElectronURLLoaderFactory::StartLoadingHttp(
       request->method != net::HttpRequestHeaders::kHeadMethod)
     dict.Get("uploadData", &upload_data);
 
-  ElectronBrowserContext* browser_context =
-      ElectronBrowserContext::From("", false);
+  auto* browser_context = ElectronBrowserContext::GetDefaultBrowserContext();
   v8::Local<v8::Value> value;
   if (dict.Get("session", &value)) {
     if (value->IsNull()) {

+ 1 - 1
shell/browser/ui/devtools_manager_delegate.cc

@@ -138,7 +138,7 @@ bool DevToolsManagerDelegate::HasBundledFrontendResources() {
 }
 
 content::BrowserContext* DevToolsManagerDelegate::GetDefaultBrowserContext() {
-  return ElectronBrowserContext::From("", false);
+  return ElectronBrowserContext::GetDefaultBrowserContext();
 }
 
 }  // namespace electron