fix: browser contexts live forever (#25002)

shell/browser/api/electron_api_app.cc

@@ -1094,12 +1094,12 @@ Browser::LoginItemSettings App::GetLoginItemSettings(
 #if defined(USE_NSS_CERTS)
 void App::ImportCertificate(const base::DictionaryValue& options,
                             net::CompletionRepeatingCallback callback) {
-  auto browser_context = ElectronBrowserContext::From("", false);
+  auto* browser_context = ElectronBrowserContext::From("", false);
   if (!certificate_manager_model_) {
     auto copy = base::DictionaryValue::From(
         base::Value::ToUniquePtrValue(options.Clone()));
     CertificateManagerModel::Create(
-        browser_context.get(),
+        browser_context,
         base::BindRepeating(&App::OnCertificateManagerModelCreated,
                             base::Unretained(this), base::Passed(&copy),
                             callback));

shell/browser/api/electron_api_cookies.cc

@@ -188,8 +188,8 @@ v8::Local<v8::Promise> Cookies::Get(const gin_helper::Dictionary& filter) {
   gin_helper::Promise<net::CookieList> promise(isolate());
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
-  auto* storage_partition = content::BrowserContext::GetDefaultStoragePartition(
-      browser_context_.get());
+  auto* storage_partition =
+      content::BrowserContext::GetDefaultStoragePartition(browser_context_);
   auto* manager = storage_partition->GetCookieManagerForBrowserProcess();
 
   base::DictionaryValue dict;
@@ -224,8 +224,8 @@ v8::Local<v8::Promise> Cookies::Remove(const GURL& url,
   cookie_deletion_filter->url = url;
   cookie_deletion_filter->cookie_name = name;
 
-  auto* storage_partition = content::BrowserContext::GetDefaultStoragePartition(
-      browser_context_.get());
+  auto* storage_partition =
+      content::BrowserContext::GetDefaultStoragePartition(browser_context_);
   auto* manager = storage_partition->GetCookieManagerForBrowserProcess();
 
   manager->DeleteCookies(
@@ -280,8 +280,8 @@ v8::Local<v8::Promise> Cookies::Set(base::DictionaryValue details) {
     options.set_include_httponly();
   }
 
-  auto* storage_partition = content::BrowserContext::GetDefaultStoragePartition(
-      browser_context_.get());
+  auto* storage_partition =
+      content::BrowserContext::GetDefaultStoragePartition(browser_context_);
   auto* manager = storage_partition->GetCookieManagerForBrowserProcess();
   manager->SetCanonicalCookie(
       *canonical_cookie, url.scheme(), options,
@@ -303,8 +303,8 @@ v8::Local<v8::Promise> Cookies::FlushStore() {
   gin_helper::Promise<void> promise(isolate());
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
-  auto* storage_partition = content::BrowserContext::GetDefaultStoragePartition(
-      browser_context_.get());
+  auto* storage_partition =
+      content::BrowserContext::GetDefaultStoragePartition(browser_context_);
   auto* manager = storage_partition->GetCookieManagerForBrowserProcess();
 
   manager->FlushCookieStore(base::BindOnce(

shell/browser/api/electron_api_cookies.h

@@ -58,7 +58,7 @@ class Cookies : public gin_helper::TrackableObject<Cookies> {
   std::unique_ptr<base::CallbackList<void(
       const net::CookieChangeInfo& change)>::Subscription>
       cookie_change_subscription_;
-  scoped_refptr<ElectronBrowserContext> browser_context_;
+  ElectronBrowserContext* browser_context_;
 
   DISALLOW_COPY_AND_ASSIGN(Cookies);
 };

shell/browser/api/electron_api_session.cc

@@ -284,7 +284,7 @@ Session::Session(v8::Isolate* isolate, ElectronBrowserContext* browser_context)
 
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
   SpellcheckService* service =
-      SpellcheckServiceFactory::GetForContext(browser_context_.get());
+      SpellcheckServiceFactory::GetForContext(browser_context_);
   if (service) {
     service->SetHunspellObserver(this);
   }
@@ -297,7 +297,7 @@ Session::~Session() {
 
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
   SpellcheckService* service =
-      SpellcheckServiceFactory::GetForContext(browser_context_.get());
+      SpellcheckServiceFactory::GetForContext(browser_context_);
   if (service) {
     service->SetHunspellObserver(nullptr);
   }
@@ -366,7 +366,7 @@ v8::Local<v8::Promise> Session::GetCacheSize() {
   gin_helper::Promise<int64_t> promise(isolate);
   auto handle = promise.GetHandle();
 
-  content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
+  content::BrowserContext::GetDefaultStoragePartition(browser_context_)
       ->GetNetworkContext()
       ->ComputeHttpCacheSize(
           base::Time(), base::Time::Max(),
@@ -390,7 +390,7 @@ v8::Local<v8::Promise> Session::ClearCache() {
   gin_helper::Promise<void> promise(isolate);
   auto handle = promise.GetHandle();
 
-  content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
+  content::BrowserContext::GetDefaultStoragePartition(browser_context_)
       ->GetNetworkContext()
       ->ClearHttpCache(base::Time(), base::Time::Max(), nullptr,
                        base::BindOnce(gin_helper::Promise<void>::ResolvePromise,
@@ -486,17 +486,17 @@ void Session::EnableNetworkEmulation(const gin_helper::Dictionary& options) {
     conditions->latency = base::TimeDelta::FromMillisecondsD(latency);
   }
 
-  auto* network_context = content::BrowserContext::GetDefaultStoragePartition(
-                              browser_context_.get())
-                              ->GetNetworkContext();
+  auto* network_context =
+      content::BrowserContext::GetDefaultStoragePartition(browser_context_)
+          ->GetNetworkContext();
   network_context->SetNetworkConditions(network_emulation_token_,
                                         std::move(conditions));
 }
 
 void Session::DisableNetworkEmulation() {
-  auto* network_context = content::BrowserContext::GetDefaultStoragePartition(
-                              browser_context_.get())
-                              ->GetNetworkContext();
+  auto* network_context =
+      content::BrowserContext::GetDefaultStoragePartition(browser_context_)
+          ->GetNetworkContext();
   network_context->SetNetworkConditions(
       network_emulation_token_, network::mojom::NetworkConditions::New());
 }
@@ -516,7 +516,7 @@ void Session::SetCertVerifyProc(v8::Local<v8::Value> val,
         std::make_unique<CertVerifierClient>(proc),
         cert_verifier_client_remote.InitWithNewPipeAndPassReceiver());
   }
-  content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
+  content::BrowserContext::GetDefaultStoragePartition(browser_context_)
       ->GetNetworkContext()
       ->SetCertVerifierClient(std::move(cert_verifier_client_remote));
 
@@ -569,7 +569,7 @@ v8::Local<v8::Promise> Session::ClearHostResolverCache(
   gin_helper::Promise<void> promise(isolate);
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
-  content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
+  content::BrowserContext::GetDefaultStoragePartition(browser_context_)
       ->GetNetworkContext()
       ->ClearHostCache(nullptr,
                        base::BindOnce(gin_helper::Promise<void>::ResolvePromise,
@@ -583,7 +583,7 @@ v8::Local<v8::Promise> Session::ClearAuthCache() {
   gin_helper::Promise<void> promise(isolate);
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
-  content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
+  content::BrowserContext::GetDefaultStoragePartition(browser_context_)
       ->GetNetworkContext()
       ->ClearHttpAuthCache(
           base::Time(),
@@ -609,9 +609,9 @@ void Session::AllowNTLMCredentialsForDomains(const std::string& domains) {
 void Session::SetUserAgent(const std::string& user_agent,
                            gin_helper::Arguments* args) {
   browser_context_->SetUserAgent(user_agent);
-  auto* network_context = content::BrowserContext::GetDefaultStoragePartition(
-                              browser_context_.get())
-                              ->GetNetworkContext();
+  auto* network_context =
+      content::BrowserContext::GetDefaultStoragePartition(browser_context_)
+          ->GetNetworkContext();
   network_context->SetUserAgent(user_agent);
 
   std::string accept_lang;
@@ -790,10 +790,9 @@ v8::Local<v8::Value> Session::NetLog(v8::Isolate* isolate) {
   return v8::Local<v8::Value>::New(isolate, net_log_);
 }
 
-static void StartPreconnectOnUI(
-    scoped_refptr<ElectronBrowserContext> browser_context,
-    const GURL& url,
-    int num_sockets_to_preconnect) {
+static void StartPreconnectOnUI(ElectronBrowserContext* browser_context,
+                                const GURL& url,
+                                int num_sockets_to_preconnect) {
   std::vector<predictors::PreconnectRequest> requests = {
       {url::Origin::Create(url), num_sockets_to_preconnect,
        net::NetworkIsolationKey()}};
@@ -823,7 +822,7 @@ void Session::Preconnect(const gin_helper::Dictionary& options,
   DCHECK_GT(num_sockets_to_preconnect, 0);
   base::PostTask(
       FROM_HERE, {content::BrowserThread::UI},
-      base::BindOnce(&StartPreconnectOnUI, base::RetainedRef(browser_context_),
+      base::BindOnce(&StartPreconnectOnUI, base::Unretained(browser_context_),
                      url, num_sockets_to_preconnect));
 }
 
@@ -867,7 +866,7 @@ v8::Local<v8::Promise> Session::ListWordsInSpellCheckerDictionary() {
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
   SpellcheckService* spellcheck =
-      SpellcheckServiceFactory::GetForContext(browser_context_.get());
+      SpellcheckServiceFactory::GetForContext(browser_context_);
 
   if (!spellcheck)
     promise.RejectWithErrorMessage(
@@ -893,7 +892,7 @@ bool Session::AddWordToSpellCheckerDictionary(const std::string& word) {
     return false;
 
   SpellcheckService* spellcheck =
-      SpellcheckServiceFactory::GetForContext(browser_context_.get());
+      SpellcheckServiceFactory::GetForContext(browser_context_);
   if (!spellcheck)
     return false;
 
@@ -915,7 +914,7 @@ bool Session::RemoveWordFromSpellCheckerDictionary(const std::string& word) {
     return false;
 
   SpellcheckService* service =
-      SpellcheckServiceFactory::GetForContext(browser_context_.get());
+      SpellcheckServiceFactory::GetForContext(browser_context_);
   if (!service)
     return false;
 
@@ -952,7 +951,7 @@ gin::Handle<Session> Session::CreateFrom(
 gin::Handle<Session> Session::FromPartition(v8::Isolate* isolate,
                                             const std::string& partition,
                                             base::DictionaryValue options) {
-  scoped_refptr<ElectronBrowserContext> browser_context;
+  ElectronBrowserContext* browser_context;
   if (partition.empty()) {
     browser_context =
         ElectronBrowserContext::From("", false, std::move(options));
@@ -965,7 +964,7 @@ gin::Handle<Session> Session::FromPartition(v8::Isolate* isolate,
     browser_context =
         ElectronBrowserContext::From(partition, true, std::move(options));
   }
-  return CreateFrom(isolate, browser_context.get());
+  return CreateFrom(isolate, browser_context);
 }
 
 // static

shell/browser/api/electron_api_session.h

@@ -57,9 +57,7 @@ class Session : public gin_helper::TrackableObject<Session>,
       const std::string& partition,
       base::DictionaryValue options = base::DictionaryValue());
 
-  ElectronBrowserContext* browser_context() const {
-    return browser_context_.get();
-  }
+  ElectronBrowserContext* browser_context() const { return browser_context_; }
 
   // gin_helper::TrackableObject:
   static void BuildPrototype(v8::Isolate* isolate,
@@ -146,7 +144,7 @@ class Session : public gin_helper::TrackableObject<Session>,
   // The client id to enable the network throttler.
   base::UnguessableToken network_emulation_token_;
 
-  scoped_refptr<ElectronBrowserContext> browser_context_;
+  ElectronBrowserContext* browser_context_;
 
   DISALLOW_COPY_AND_ASSIGN(Session);
 };

shell/browser/common_web_contents_delegate.cc

@@ -250,14 +250,8 @@ void CommonWebContentsDelegate::ResetManagedWebContents(bool async) {
     // is destroyed.
     // //electron/patches/chromium/content_browser_main_loop.patch
     // is required to get the right quit closure for the main message loop.
-    base::ThreadTaskRunnerHandle::Get()->PostNonNestableTask(
-        FROM_HERE,
-        base::BindOnce(
-            [](scoped_refptr<ElectronBrowserContext> browser_context,
-               std::unique_ptr<InspectableWebContents> web_contents) {
-              web_contents.reset();
-            },
-            base::RetainedRef(browser_context_), std::move(web_contents_)));
+    base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE,
+                                                    web_contents_.release());
   } else {
     web_contents_.reset();
   }

shell/browser/common_web_contents_delegate.h

@@ -172,8 +172,7 @@ class CommonWebContentsDelegate : public content::WebContentsDelegate,
 
   scoped_refptr<DevToolsFileSystemIndexer> devtools_file_system_indexer_;
 
-  // Make sure BrowserContext is alwasys destroyed after WebContents.
-  scoped_refptr<ElectronBrowserContext> browser_context_;
+  ElectronBrowserContext* browser_context_;
 
   // The stored InspectableWebContents object.
   // Notice that web_contents_ must be placed after dialog_manager_, so we can

shell/browser/electron_browser_context.cc

@@ -10,6 +10,7 @@
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "base/no_destructor.h"
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/task/post_task.h"
@@ -90,15 +91,17 @@ std::string MakePartitionName(const std::string& input) {
 }  // namespace
 
 // static
-ElectronBrowserContext::BrowserContextMap
-    ElectronBrowserContext::browser_context_map_;
+ElectronBrowserContext::BrowserContextMap&
+ElectronBrowserContext::browser_context_map() {
+  static base::NoDestructor<ElectronBrowserContext::BrowserContextMap>
+      browser_context_map;
+  return *browser_context_map;
+}
 
 ElectronBrowserContext::ElectronBrowserContext(const std::string& partition,
                                                bool in_memory,
                                                base::DictionaryValue options)
-    : base::RefCountedDeleteOnSequence<ElectronBrowserContext>(
-          base::ThreadTaskRunnerHandle::Get()),
-      in_memory_pref_store_(nullptr),
+    : in_memory_pref_store_(nullptr),
       storage_policy_(new SpecialStoragePolicy),
       in_memory_(in_memory),
       weak_factory_(this) {
@@ -444,19 +447,21 @@ ResolveProxyHelper* ElectronBrowserContext::GetResolveProxyHelper() {
 }
 
 // static
-scoped_refptr<ElectronBrowserContext> ElectronBrowserContext::From(
+ElectronBrowserContext* ElectronBrowserContext::From(
     const std::string& partition,
     bool in_memory,
     base::DictionaryValue options) {
   PartitionKey key(partition, in_memory);
-  auto* browser_context = browser_context_map_[key].get();
-  if (browser_context)
-    return scoped_refptr<ElectronBrowserContext>(browser_context);
+  ElectronBrowserContext* browser_context = browser_context_map()[key].get();
+  if (browser_context) {
+    return browser_context;
+  }
 
   auto* new_context =
       new ElectronBrowserContext(partition, in_memory, std::move(options));
-  browser_context_map_[key] = new_context->GetWeakPtr();
-  return scoped_refptr<ElectronBrowserContext>(new_context);
+  browser_context_map()[key] =
+      std::unique_ptr<ElectronBrowserContext>(new_context);
+  return new_context;
 }
 
 }  // namespace electron

shell/browser/electron_browser_context.h

@@ -10,7 +10,6 @@
 #include <string>
 #include <vector>
 
-#include "base/memory/ref_counted_delete_on_sequence.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/net/proxy_config_monitor.h"
 #include "chrome/browser/predictors/preconnect_manager.h"
@@ -50,8 +49,7 @@ class SpecialStoragePolicy;
 class WebViewManager;
 
 class ElectronBrowserContext
-    : public base::RefCountedDeleteOnSequence<ElectronBrowserContext>,
-      public content::BrowserContext,
+    : public content::BrowserContext,
       public network::mojom::TrustedURLLoaderAuthClient {
  public:
   // partition_id => browser_context
@@ -73,19 +71,17 @@ class ElectronBrowserContext
     }
   };
   using BrowserContextMap =
-      std::map<PartitionKey, base::WeakPtr<ElectronBrowserContext>>;
+      std::map<PartitionKey, std::unique_ptr<ElectronBrowserContext>>;
 
   // 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.
-  static scoped_refptr<ElectronBrowserContext> From(
+  static ElectronBrowserContext* From(
       const std::string& partition,
       bool in_memory,
       base::DictionaryValue options = base::DictionaryValue());
 
-  static BrowserContextMap browser_context_map() {
-    return browser_context_map_;
-  }
+  static BrowserContextMap& browser_context_map();
 
   void SetUserAgent(const std::string& user_agent);
   std::string GetUserAgent() const;
@@ -149,16 +145,12 @@ class ElectronBrowserContext
   }
 #endif
 
- protected:
   ElectronBrowserContext(const std::string& partition,
                          bool in_memory,
                          base::DictionaryValue options);
   ~ElectronBrowserContext() override;
 
  private:
-  friend class base::RefCountedDeleteOnSequence<ElectronBrowserContext>;
-  friend class base::DeleteHelper<ElectronBrowserContext>;
-
   void OnLoaderCreated(int32_t request_id,
                        mojo::PendingReceiver<network::mojom::TrustedAuthClient>
                            header_client) override;
@@ -166,8 +158,6 @@ class ElectronBrowserContext
   // Initialize pref registry.
   void InitPrefs();
 
-  static BrowserContextMap browser_context_map_;
-
   ValueMapPrefStore* in_memory_pref_store_;
 
   std::unique_ptr<content::ResourceContext> resource_context_;

shell/browser/electron_browser_main_parts.cc

@@ -555,6 +555,8 @@ void ElectronBrowserMainParts::PostMainMessageLoopRun() {
   node_env_.reset();
   js_env_->OnMessageLoopDestroying();
 
+  ElectronBrowserContext::browser_context_map().clear();
+
   fake_browser_process_->PostMainMessageLoopRun();
 }
 

shell/browser/extensions/electron_extensions_browser_client.cc

@@ -84,7 +84,7 @@ bool ElectronExtensionsBrowserClient::AreExtensionsDisabled(
 }
 
 bool ElectronExtensionsBrowserClient::IsValidContext(BrowserContext* context) {
-  auto context_map = ElectronBrowserContext::browser_context_map();
+  auto& context_map = ElectronBrowserContext::browser_context_map();
   for (auto const& entry : context_map) {
     if (entry.second && entry.second.get() == context)
       return true;
@@ -112,7 +112,7 @@ BrowserContext* ElectronExtensionsBrowserClient::GetOriginalContext(
     BrowserContext* context) {
   DCHECK(context);
   if (context->IsOffTheRecord()) {
-    return ElectronBrowserContext::From("", false).get();
+    return ElectronBrowserContext::From("", false);
   } else {
     return context;
   }
@@ -310,7 +310,7 @@ void ElectronExtensionsBrowserClient::BroadcastEventToRenderers(
 
   std::unique_ptr<extensions::Event> event(
       new extensions::Event(histogram_value, event_name, std::move(args)));
-  auto context_map = ElectronBrowserContext::browser_context_map();
+  auto& context_map = ElectronBrowserContext::browser_context_map();
   for (auto const& entry : context_map) {
     if (entry.second) {
       extensions::EventRouter::Get(entry.second.get())

shell/browser/net/electron_url_loader_factory.cc

@@ -401,7 +401,7 @@ void ElectronURLLoaderFactory::StartLoadingHttp(
   if (request->method != "GET" && request->method != "HEAD")
     dict.Get("uploadData", &upload_data);
 
-  scoped_refptr<ElectronBrowserContext> browser_context =
+  ElectronBrowserContext* browser_context =
       ElectronBrowserContext::From("", false);
   v8::Local<v8::Value> value;
   if (dict.Get("session", &value)) {

spec-main/fixtures/crash-cases/in-memory-session-double-free/index.js

@@ -0,0 +1,7 @@
+const { app, BrowserWindow } = require('electron');
+
+app.on('ready', async () => {
+  const win = new BrowserWindow({ show: false, webPreferences: { partition: '123321' } });
+  await win.loadURL('data:text/html,<body>');
+  setTimeout(() => app.quit());
+});