From 998de7aa6c9e575d6dc23724c4ab2a3bb1eb5a22 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 21 Mar 2025 10:50:16 -0500 Subject: [PATCH] refactor: Add `ElectronBrowserContext::BrowserContexts()` (35-x-y backport) (#46161) refactor: Add `ElectronBrowserContext::BrowserContexts()` * refactor: add ElectronBrowserContext::BrowserContexts() * refactor: use ElectronBrowserContext::BrowserContexts() in ElectronBrowserMainParts::PostMainMessageLoopRun() * refactor: use ElectronBrowserContext::BrowserContexts() in ElectronExtensionsBrowserClient::IsValidContext() * refactor: use ElectronBrowserContext::BrowserContexts() in ElectronExtensionsBrowserClient::BroadcastEventToRenderers() * refactor: move PartitionKey, BrowserContextMap private * refactor: add ElectronBrowserContext::IsValidContext() decouple ElectronExtensionsBrowserClient from the internals of ElectronBrowserContext --- shell/browser/electron_browser_context.cc | 54 ++++++++++++++++--- shell/browser/electron_browser_context.h | 25 ++------- shell/browser/electron_browser_main_parts.cc | 12 ++--- .../electron_extensions_browser_client.cc | 18 ++----- 4 files changed, 58 insertions(+), 51 deletions(-) diff --git a/shell/browser/electron_browser_context.cc b/shell/browser/electron_browser_context.cc index fd89254e23b3..757fbb29ad11 100644 --- a/shell/browser/electron_browser_context.cc +++ b/shell/browser/electron_browser_context.cc @@ -12,6 +12,7 @@ #include "base/barrier_closure.h" #include "base/base_paths.h" #include "base/command_line.h" +#include "base/containers/to_vector.h" #include "base/files/file_path.h" #include "base/no_destructor.h" #include "base/path_service.h" @@ -308,14 +309,53 @@ bool DoesDeviceMatch(const base::Value& device, return false; } +// partition_id => browser_context +struct PartitionKey { + PartitionKey(const std::string_view partition, bool in_memory) + : type_{Type::Partition}, location_{partition}, in_memory_{in_memory} {} + + explicit PartitionKey(const base::FilePath& file_path) + : type_{Type::Path}, + location_{file_path.AsUTF8Unsafe()}, + in_memory_{false} {} + + friend auto operator<=>(const PartitionKey&, const PartitionKey&) = default; + + private: + enum class Type { Partition, Path }; + + Type type_; + std::string location_; + bool in_memory_; +}; + +[[nodiscard]] auto& ContextMap() { + static base::NoDestructor< + std::map>> + map; + return *map; +} + } // namespace // static -ElectronBrowserContext::BrowserContextMap& -ElectronBrowserContext::browser_context_map() { - static base::NoDestructor - browser_context_map; - return *browser_context_map; +std::vector ElectronBrowserContext::BrowserContexts() { + return base::ToVector(ContextMap(), + [](auto& iter) { return iter.second.get(); }); +} + +bool ElectronBrowserContext::IsValidContext(const void* context) { + return std::ranges::any_of(ContextMap(), [context](const auto& iter) { + return iter.second.get() == context; + }); +} + +// static +void ElectronBrowserContext::DestroyAllContexts() { + auto& map = ContextMap(); + // Avoid UAF by destroying the default context last. See ba629e3 for info. + const auto extracted = map.extract(PartitionKey{"", false}); + map.clear(); } ElectronBrowserContext::ElectronBrowserContext( @@ -833,7 +873,7 @@ ElectronBrowserContext* ElectronBrowserContext::From( const std::string& partition, bool in_memory, base::Value::Dict options) { - auto& context = browser_context_map()[PartitionKey(partition, in_memory)]; + auto& context = ContextMap()[PartitionKey(partition, in_memory)]; if (!context) { context.reset(new ElectronBrowserContext{std::cref(partition), in_memory, std::move(options)}); @@ -844,7 +884,7 @@ ElectronBrowserContext* ElectronBrowserContext::From( ElectronBrowserContext* ElectronBrowserContext::FromPath( const base::FilePath& path, base::Value::Dict options) { - auto& context = browser_context_map()[PartitionKey(path)]; + auto& context = ContextMap()[PartitionKey(path)]; if (!context) { context.reset( new ElectronBrowserContext{std::cref(path), false, std::move(options)}); diff --git a/shell/browser/electron_browser_context.h b/shell/browser/electron_browser_context.h index 29c2c119655b..782cc8fc2073 100644 --- a/shell/browser/electron_browser_context.h +++ b/shell/browser/electron_browser_context.h @@ -61,28 +61,9 @@ class ElectronBrowserContext : public content::BrowserContext { ElectronBrowserContext(const ElectronBrowserContext&) = delete; ElectronBrowserContext& operator=(const ElectronBrowserContext&) = delete; - // partition_id => browser_context - struct PartitionKey { - PartitionKey(const std::string_view partition, bool in_memory) - : type_{Type::Partition}, location_{partition}, in_memory_{in_memory} {} + [[nodiscard]] static std::vector BrowserContexts(); - explicit PartitionKey(const base::FilePath& file_path) - : type_{Type::Path}, - location_{file_path.AsUTF8Unsafe()}, - in_memory_{false} {} - - friend auto operator<=>(const PartitionKey&, const PartitionKey&) = default; - - private: - enum class Type { Partition, Path }; - - Type type_; - std::string location_; - bool in_memory_; - }; - - using BrowserContextMap = - std::map>; + [[nodiscard]] static bool IsValidContext(const void* context); // Get or create the BrowserContext according to its |partition| and // |in_memory|. The |options| will be passed to constructor when there is no @@ -97,7 +78,7 @@ class ElectronBrowserContext : public content::BrowserContext { static ElectronBrowserContext* FromPath(const base::FilePath& path, base::Value::Dict options = {}); - static BrowserContextMap& browser_context_map(); + static void DestroyAllContexts(); void SetUserAgent(const std::string& user_agent); std::string GetUserAgent() const; diff --git a/shell/browser/electron_browser_main_parts.cc b/shell/browser/electron_browser_main_parts.cc index dfdec38f1609..1603230fbb62 100644 --- a/shell/browser/electron_browser_main_parts.cc +++ b/shell/browser/electron_browser_main_parts.cc @@ -554,11 +554,9 @@ void ElectronBrowserMainParts::PostMainMessageLoopRun() { // Shutdown the DownloadManager before destroying Node to prevent // DownloadItem callbacks from crashing. - for (auto& iter : ElectronBrowserContext::browser_context_map()) { - auto* download_manager = iter.second.get()->GetDownloadManager(); - if (download_manager) { + for (auto* browser_context : ElectronBrowserContext::BrowserContexts()) { + if (auto* download_manager = browser_context->GetDownloadManager()) download_manager->Shutdown(); - } } // Shutdown utility process created with Electron API before @@ -598,11 +596,7 @@ void ElectronBrowserMainParts::PostMainMessageLoopRun() { node_bindings_->set_uv_env(nullptr); node_env_.reset(); - auto default_context_key = ElectronBrowserContext::PartitionKey("", false); - std::unique_ptr 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(); diff --git a/shell/browser/extensions/electron_extensions_browser_client.cc b/shell/browser/extensions/electron_extensions_browser_client.cc index 03fc80d215c7..e146cbaab3bf 100644 --- a/shell/browser/extensions/electron_extensions_browser_client.cc +++ b/shell/browser/extensions/electron_extensions_browser_client.cc @@ -85,12 +85,7 @@ bool ElectronExtensionsBrowserClient::AreExtensionsDisabled( } bool ElectronExtensionsBrowserClient::IsValidContext(void* context) { - auto& context_map = ElectronBrowserContext::browser_context_map(); - for (auto const& entry : context_map) { - if (entry.second && entry.second.get() == context) - return true; - } - return false; + return ElectronBrowserContext::IsValidContext(context); } bool ElectronExtensionsBrowserClient::IsSameContext(BrowserContext* first, @@ -341,13 +336,10 @@ void ElectronExtensionsBrowserClient::BroadcastEventToRenderers( return; } - for (auto const& [key, browser_context] : - ElectronBrowserContext::browser_context_map()) { - if (browser_context) { - extensions::EventRouter::Get(browser_context.get()) - ->BroadcastEvent(std::make_unique( - histogram_value, event_name, args.Clone())); - } + for (auto* browser_context : ElectronBrowserContext::BrowserContexts()) { + extensions::EventRouter::Get(browser_context) + ->BroadcastEvent(std::make_unique( + histogram_value, event_name, args.Clone())); } }