From cee2c2ceeb604fd2f5c352d39ca6b00e606495c4 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:42:12 -0500 Subject: [PATCH] refactor: Add `ElectronBrowserContext::BrowserContexts()` (#46158) * refactor: add ElectronBrowserContext::BrowserContexts() Co-authored-by: Charles Kerr * refactor: use ElectronBrowserContext::BrowserContexts() in ElectronBrowserMainParts::PostMainMessageLoopRun() Co-authored-by: Charles Kerr * refactor: use ElectronBrowserContext::BrowserContexts() in ElectronExtensionsBrowserClient::IsValidContext() Co-authored-by: Charles Kerr * refactor: use ElectronBrowserContext::BrowserContexts() in ElectronExtensionsBrowserClient::BroadcastEventToRenderers() Co-authored-by: Charles Kerr * refactor: move PartitionKey, BrowserContextMap private Co-authored-by: Charles Kerr * refactor: add ElectronBrowserContext::IsValidContext() decouple ElectronExtensionsBrowserClient from the internals of ElectronBrowserContext Co-authored-by: Charles Kerr --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Charles Kerr --- shell/browser/electron_browser_context.cc | 48 +++++++++++++++---- shell/browser/electron_browser_context.h | 25 +--------- shell/browser/electron_browser_main_parts.cc | 6 +-- .../electron_extensions_browser_client.cc | 18 ++----- 4 files changed, 49 insertions(+), 48 deletions(-) diff --git a/shell/browser/electron_browser_context.cc b/shell/browser/electron_browser_context.cc index 908d23ed5ad2..24eba93bd7b5 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,19 +309,50 @@ 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 = browser_context_map(); + auto& map = ContextMap(); // Avoid UAF by destroying the default context last. See ba629e3 for info. const auto extracted = map.extract(PartitionKey{"", false}); map.clear(); @@ -841,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)}); @@ -858,7 +890,7 @@ ElectronBrowserContext* ElectronBrowserContext::GetDefaultBrowserContext( 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 3ae8b2e2f13f..298586182511 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 default BrowserContext. static ElectronBrowserContext* GetDefaultBrowserContext( @@ -101,8 +82,6 @@ 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); diff --git a/shell/browser/electron_browser_main_parts.cc b/shell/browser/electron_browser_main_parts.cc index 5e5a8b6ae765..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 diff --git a/shell/browser/extensions/electron_extensions_browser_client.cc b/shell/browser/extensions/electron_extensions_browser_client.cc index f74f1a7e61a3..1ada759c73d9 100644 --- a/shell/browser/extensions/electron_extensions_browser_client.cc +++ b/shell/browser/extensions/electron_extensions_browser_client.cc @@ -84,12 +84,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, @@ -340,13 +335,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())); } }