diff --git a/shell/browser/api/electron_api_app.cc b/shell/browser/api/electron_api_app.cc index 189e2655c55e..e55db8832adb 100644 --- a/shell/browser/api/electron_api_app.cc +++ b/shell/browser/api/electron_api_app.cc @@ -1197,10 +1197,10 @@ void App::ImportCertificate(gin_helper::ErrorThrower thrower, return; } - auto browser_context = ElectronBrowserContext::From("", false); + auto* browser_context = ElectronBrowserContext::From("", false); if (!certificate_manager_model_) { CertificateManagerModel::Create( - browser_context.get(), + browser_context, base::BindOnce(&App::OnCertificateManagerModelCreated, base::Unretained(this), std::move(options), std::move(callback))); diff --git a/shell/browser/api/electron_api_session.cc b/shell/browser/api/electron_api_session.cc index 7cee6468a9e6..8bb13bc12c00 100644 --- a/shell/browser/api/electron_api_session.cc +++ b/shell/browser/api/electron_api_session.cc @@ -276,7 +276,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); } @@ -289,7 +289,7 @@ Session::~Session() { #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER) SpellcheckService* service = - SpellcheckServiceFactory::GetForContext(browser_context_.get()); + SpellcheckServiceFactory::GetForContext(browser_context_); if (service) { service->SetHunspellObserver(nullptr); } @@ -351,7 +351,7 @@ v8::Local Session::GetCacheSize() { gin_helper::Promise promise(isolate); auto handle = promise.GetHandle(); - content::BrowserContext::GetDefaultStoragePartition(browser_context_.get()) + content::BrowserContext::GetDefaultStoragePartition(browser_context_) ->GetNetworkContext() ->ComputeHttpCacheSize( base::Time(), base::Time::Max(), @@ -375,7 +375,7 @@ v8::Local Session::ClearCache() { gin_helper::Promise 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::ResolvePromise, @@ -471,17 +471,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()); } @@ -501,7 +501,7 @@ void Session::SetCertVerifyProc(v8::Local val, std::make_unique(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)); @@ -553,7 +553,7 @@ v8::Local Session::ClearHostResolverCache(gin::Arguments* args) { gin_helper::Promise promise(isolate); v8::Local handle = promise.GetHandle(); - content::BrowserContext::GetDefaultStoragePartition(browser_context_.get()) + content::BrowserContext::GetDefaultStoragePartition(browser_context_) ->GetNetworkContext() ->ClearHostCache(nullptr, base::BindOnce(gin_helper::Promise::ResolvePromise, @@ -567,7 +567,7 @@ v8::Local Session::ClearAuthCache() { gin_helper::Promise promise(isolate); v8::Local handle = promise.GetHandle(); - content::BrowserContext::GetDefaultStoragePartition(browser_context_.get()) + content::BrowserContext::GetDefaultStoragePartition(browser_context_) ->GetNetworkContext() ->ClearHttpAuthCache( base::Time(), @@ -593,9 +593,9 @@ void Session::AllowNTLMCredentialsForDomains(const std::string& domains) { void Session::SetUserAgent(const std::string& user_agent, gin::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; @@ -781,10 +781,9 @@ v8::Local Session::NetLog(v8::Isolate* isolate) { return net_log_.Get(isolate); } -static void StartPreconnectOnUI( - scoped_refptr 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 requests = { {url::Origin::Create(url), num_sockets_to_preconnect, net::NetworkIsolationKey()}}; @@ -815,7 +814,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)); } @@ -860,7 +859,7 @@ v8::Local Session::ListWordsInSpellCheckerDictionary() { v8::Local handle = promise.GetHandle(); SpellcheckService* spellcheck = - SpellcheckServiceFactory::GetForContext(browser_context_.get()); + SpellcheckServiceFactory::GetForContext(browser_context_); if (!spellcheck) promise.RejectWithErrorMessage( @@ -886,7 +885,7 @@ bool Session::AddWordToSpellCheckerDictionary(const std::string& word) { return false; SpellcheckService* service = - SpellcheckServiceFactory::GetForContext(browser_context_.get()); + SpellcheckServiceFactory::GetForContext(browser_context_); if (!service) return false; @@ -907,7 +906,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; @@ -953,7 +952,7 @@ gin::Handle Session::CreateFrom( gin::Handle Session::FromPartition(v8::Isolate* isolate, const std::string& partition, base::DictionaryValue options) { - scoped_refptr browser_context; + ElectronBrowserContext* browser_context; if (partition.empty()) { browser_context = ElectronBrowserContext::From("", false, std::move(options)); @@ -966,7 +965,7 @@ gin::Handle 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); } gin::ObjectTemplateBuilder Session::GetObjectTemplateBuilder( diff --git a/shell/browser/api/electron_api_session.h b/shell/browser/api/electron_api_session.h index f74f148fd5ff..c6c63c57328a 100644 --- a/shell/browser/api/electron_api_session.h +++ b/shell/browser/api/electron_api_session.h @@ -71,9 +71,7 @@ class Session : public gin::Wrappable, 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::Wrappable static gin::WrapperInfo kWrapperInfo; @@ -159,7 +157,7 @@ class Session : public gin::Wrappable, // The client id to enable the network throttler. base::UnguessableToken network_emulation_token_; - scoped_refptr browser_context_; + ElectronBrowserContext* browser_context_; DISALLOW_COPY_AND_ASSIGN(Session); }; diff --git a/shell/browser/common_web_contents_delegate.cc b/shell/browser/common_web_contents_delegate.cc index 641ea7976c75..01297e80482e 100644 --- a/shell/browser/common_web_contents_delegate.cc +++ b/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 browser_context, - std::unique_ptr 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(); } diff --git a/shell/browser/common_web_contents_delegate.h b/shell/browser/common_web_contents_delegate.h index 9cfe0b55d556..f1dc3b58a67a 100644 --- a/shell/browser/common_web_contents_delegate.h +++ b/shell/browser/common_web_contents_delegate.h @@ -171,8 +171,7 @@ class CommonWebContentsDelegate : public content::WebContentsDelegate, scoped_refptr devtools_file_system_indexer_; - // Make sure BrowserContext is alwasys destroyed after WebContents. - scoped_refptr browser_context_; + ElectronBrowserContext* browser_context_; // The stored InspectableWebContents object. // Notice that web_contents_ must be placed after dialog_manager_, so we can diff --git a/shell/browser/electron_browser_context.cc b/shell/browser/electron_browser_context.cc index b658879b8922..1eb2a4a9591d 100644 --- a/shell/browser/electron_browser_context.cc +++ b/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" @@ -91,15 +92,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 + browser_context_map; + return *browser_context_map; +} ElectronBrowserContext::ElectronBrowserContext(const std::string& partition, bool in_memory, base::DictionaryValue options) - : base::RefCountedDeleteOnSequence( - base::ThreadTaskRunnerHandle::Get()), - in_memory_pref_store_(nullptr), + : in_memory_pref_store_(nullptr), storage_policy_(new SpecialStoragePolicy), protocol_registry_(new ProtocolRegistry), in_memory_(in_memory), @@ -444,19 +447,21 @@ ResolveProxyHelper* ElectronBrowserContext::GetResolveProxyHelper() { } // static -scoped_refptr 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(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(new_context); + browser_context_map()[key] = + std::unique_ptr(new_context); + return new_context; } } // namespace electron diff --git a/shell/browser/electron_browser_context.h b/shell/browser/electron_browser_context.h index e0a17ad47219..fd5d1d84fedc 100644 --- a/shell/browser/electron_browser_context.h +++ b/shell/browser/electron_browser_context.h @@ -10,7 +10,6 @@ #include #include -#include "base/memory/ref_counted_delete_on_sequence.h" #include "base/memory/weak_ptr.h" #include "chrome/browser/predictors/preconnect_manager.h" #include "content/public/browser/browser_context.h" @@ -50,8 +49,7 @@ class WebViewManager; class ProtocolRegistry; class ElectronBrowserContext - : public base::RefCountedDeleteOnSequence, - 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>; + std::map>; // 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 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; @@ -151,15 +147,12 @@ class ElectronBrowserContext return protocol_registry_.get(); } - protected: - ElectronBrowserContext(const std::string& partition, - bool in_memory, - base::DictionaryValue options); ~ElectronBrowserContext() override; private: - friend class base::RefCountedDeleteOnSequence; - friend class base::DeleteHelper; + ElectronBrowserContext(const std::string& partition, + bool in_memory, + base::DictionaryValue options); void OnLoaderCreated(int32_t request_id, mojo::PendingReceiver @@ -168,8 +161,6 @@ class ElectronBrowserContext // Initialize pref registry. void InitPrefs(); - static BrowserContextMap browser_context_map_; - ValueMapPrefStore* in_memory_pref_store_; std::unique_ptr resource_context_; diff --git a/shell/browser/electron_browser_main_parts.cc b/shell/browser/electron_browser_main_parts.cc index e805a7d6aa07..f70997008e15 100644 --- a/shell/browser/electron_browser_main_parts.cc +++ b/shell/browser/electron_browser_main_parts.cc @@ -553,6 +553,8 @@ void ElectronBrowserMainParts::PostMainMessageLoopRun() { js_env_->OnMessageLoopDestroying(); node_env_.reset(); + ElectronBrowserContext::browser_context_map().clear(); + fake_browser_process_->PostMainMessageLoopRun(); content::DevToolsAgentHost::StopRemoteDebuggingPipeHandler(); } diff --git a/shell/browser/extensions/electron_extensions_browser_client.cc b/shell/browser/extensions/electron_extensions_browser_client.cc index e7a8cfe5788e..edd6ce71e878 100644 --- a/shell/browser/extensions/electron_extensions_browser_client.cc +++ b/shell/browser/extensions/electron_extensions_browser_client.cc @@ -85,7 +85,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; @@ -113,7 +113,7 @@ BrowserContext* ElectronExtensionsBrowserClient::GetOriginalContext( BrowserContext* context) { DCHECK(context); if (context->IsOffTheRecord()) { - return ElectronBrowserContext::From("", false).get(); + return ElectronBrowserContext::From("", false); } else { return context; } @@ -311,7 +311,7 @@ void ElectronExtensionsBrowserClient::BroadcastEventToRenderers( std::unique_ptr 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()) diff --git a/shell/browser/net/electron_url_loader_factory.cc b/shell/browser/net/electron_url_loader_factory.cc index 795a10cf33b4..fa624a692685 100644 --- a/shell/browser/net/electron_url_loader_factory.cc +++ b/shell/browser/net/electron_url_loader_factory.cc @@ -419,7 +419,7 @@ void ElectronURLLoaderFactory::StartLoadingHttp( if (request->method != "GET" && request->method != "HEAD") dict.Get("uploadData", &upload_data); - scoped_refptr browser_context = + ElectronBrowserContext* browser_context = ElectronBrowserContext::From("", false); v8::Local value; if (dict.Get("session", &value)) { diff --git a/spec-main/fixtures/crash-cases/in-memory-session-double-free/index.js b/spec-main/fixtures/crash-cases/in-memory-session-double-free/index.js new file mode 100644 index 000000000000..e2445690cd2f --- /dev/null +++ b/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,'); + setTimeout(() => app.quit()); +});