From fbbb7041466b7e5d476b65fa9d0b2df49e060dca Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Tue, 23 Oct 2018 10:45:41 +0200 Subject: [PATCH] refactor: eliminate brightray::BrowserClient (#15320) --- atom/browser/atom_browser_client.cc | 58 ++++++++++++- atom/browser/atom_browser_client.h | 16 ++-- atom/browser/atom_browser_main_parts.cc | 7 +- .../browser/net/url_request_context_getter.cc | 2 +- brightray/BUILD.gn | 2 - brightray/browser/browser_client.cc | 85 ------------------- brightray/browser/browser_client.h | 50 ----------- brightray/browser/browser_main_parts.cc | 5 -- brightray/browser/browser_main_parts.h | 3 +- brightray/common/main_delegate.cc | 10 --- brightray/common/main_delegate.h | 10 --- 11 files changed, 74 insertions(+), 174 deletions(-) delete mode 100644 brightray/browser/browser_client.cc delete mode 100644 brightray/browser/browser_client.h diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 81dd96338e0e..950c6e2d238d 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -38,7 +38,9 @@ #include "base/environment.h" #include "base/files/file_util.h" #include "base/json/json_reader.h" +#include "base/lazy_instance.h" #include "base/no_destructor.h" +#include "base/path_service.h" #include "base/stl_util.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" @@ -121,6 +123,18 @@ bool IsSameWebSite(content::BrowserContext* browser_context, src_url; } +AtomBrowserClient* g_browser_client = nullptr; + +base::LazyInstance::DestructorAtExit + g_io_thread_application_locale = LAZY_INSTANCE_INITIALIZER; + +base::NoDestructor g_application_locale; + +void SetApplicationLocaleOnIOThread(const std::string& locale) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + g_io_thread_application_locale.Get() = locale; +} + } // namespace // static @@ -133,9 +147,32 @@ void AtomBrowserClient::SetCustomServiceWorkerSchemes( *g_custom_service_worker_schemes = base::JoinString(schemes, ","); } -AtomBrowserClient::AtomBrowserClient() {} +AtomBrowserClient* AtomBrowserClient::Get() { + return g_browser_client; +} -AtomBrowserClient::~AtomBrowserClient() {} +// static +void AtomBrowserClient::SetApplicationLocale(const std::string& locale) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + + if (!BrowserThread::IsThreadInitialized(BrowserThread::IO) || + !BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::BindOnce(&SetApplicationLocaleOnIOThread, locale))) { + g_io_thread_application_locale.Get() = locale; + } + *g_application_locale = locale; +} + +AtomBrowserClient::AtomBrowserClient() { + DCHECK(!g_browser_client); + g_browser_client = this; +} + +AtomBrowserClient::~AtomBrowserClient() { + DCHECK(g_browser_client); + g_browser_client = nullptr; +} content::WebContents* AtomBrowserClient::GetWebContentsFromProcessID( int process_id) { @@ -579,7 +616,7 @@ net::NetLog* AtomBrowserClient::GetNetLog() { return AtomBrowserMainParts::Get()->net_log(); } -brightray::BrowserMainParts* AtomBrowserClient::OverrideCreateBrowserMainParts( +content::BrowserMainParts* AtomBrowserClient::CreateBrowserMainParts( const content::MainFunctionParams& params) { return new AtomBrowserMainParts(params); } @@ -708,4 +745,19 @@ AtomBrowserClient::GetPlatformNotificationService() { return notification_service_.get(); } +base::FilePath AtomBrowserClient::GetDefaultDownloadDirectory() { + // ~/Downloads + base::FilePath path; + if (base::PathService::Get(base::DIR_HOME, &path)) + path = path.Append(FILE_PATH_LITERAL("Downloads")); + + return path; +} + +std::string AtomBrowserClient::GetApplicationLocale() { + if (BrowserThread::CurrentlyOn(BrowserThread::IO)) + return g_io_thread_application_locale.Get(); + return *g_application_locale; +} + } // namespace atom diff --git a/atom/browser/atom_browser_client.h b/atom/browser/atom_browser_client.h index 520b9e032e5f..7a36efea72f5 100644 --- a/atom/browser/atom_browser_client.h +++ b/atom/browser/atom_browser_client.h @@ -11,7 +11,7 @@ #include #include -#include "brightray/browser/browser_client.h" +#include "content/public/browser/content_browser_client.h" #include "content/public/browser/render_process_host_observer.h" #include "net/ssl/client_cert_identity.h" @@ -30,9 +30,12 @@ class AtomResourceDispatcherHostDelegate; class NotificationPresenter; class PlatformNotificationService; -class AtomBrowserClient : public brightray::BrowserClient, +class AtomBrowserClient : public content::ContentBrowserClient, public content::RenderProcessHostObserver { public: + static AtomBrowserClient* Get(); + static void SetApplicationLocale(const std::string& locale); + AtomBrowserClient(); ~AtomBrowserClient() override; @@ -58,8 +61,10 @@ class AtomBrowserClient : public brightray::BrowserClient, std::vector> CreateThrottlesForNavigation(content::NavigationHandle* handle) override; - protected: // content::ContentBrowserClient: + std::string GetApplicationLocale() override; + + protected: void RenderProcessWillLaunch( content::RenderProcessHost* host, service_manager::mojom::ServiceRequest* service_request) override; @@ -131,10 +136,9 @@ class AtomBrowserClient : public brightray::BrowserClient, content::DevToolsManagerDelegate* GetDevToolsManagerDelegate() override; content::PlatformNotificationService* GetPlatformNotificationService() override; - - // brightray::BrowserClient: - brightray::BrowserMainParts* OverrideCreateBrowserMainParts( + content::BrowserMainParts* CreateBrowserMainParts( const content::MainFunctionParams&) override; + base::FilePath GetDefaultDownloadDirectory() override; // content::RenderProcessHostObserver: void RenderProcessHostDestroyed(content::RenderProcessHost* host) override; diff --git a/atom/browser/atom_browser_main_parts.cc b/atom/browser/atom_browser_main_parts.cc index 2f17c9992d13..6d227bf9300d 100644 --- a/atom/browser/atom_browser_main_parts.cc +++ b/atom/browser/atom_browser_main_parts.cc @@ -173,9 +173,14 @@ void AtomBrowserMainParts::PostEarlyInitialization() { int AtomBrowserMainParts::PreCreateThreads() { const int result = brightray::BrowserMainParts::PreCreateThreads(); + + // Initialize the app locale. + AtomBrowserClient::SetApplicationLocale( + l10n_util::GetApplicationLocale(custom_locale_)); + if (!result) { fake_browser_process_->SetApplicationLocale( - brightray::BrowserClient::Get()->GetApplicationLocale()); + AtomBrowserClient::Get()->GetApplicationLocale()); } // Force MediaCaptureDevicesDispatcher to be created on UI thread. diff --git a/atom/browser/net/url_request_context_getter.cc b/atom/browser/net/url_request_context_getter.cc index 056fe8416640..ec719e73b3c8 100644 --- a/atom/browser/net/url_request_context_getter.cc +++ b/atom/browser/net/url_request_context_getter.cc @@ -81,7 +81,7 @@ network::mojom::NetworkContextParamsPtr CreateDefaultNetworkContextParams( network_context_params->http_cache_enabled = use_cache; network_context_params->accept_language = net::HttpUtil::GenerateAcceptLanguageHeader( - brightray::BrowserClient::Get()->GetApplicationLocale()); + AtomBrowserClient::Get()->GetApplicationLocale()); network_context_params->enable_data_url_support = false; network_context_params->proxy_resolver_factory = ChromeMojoProxyResolverFactory::CreateWithStrongBinding().PassInterface(); diff --git a/brightray/BUILD.gn b/brightray/BUILD.gn index 9db8ac0dfcd8..492c935d8606 100644 --- a/brightray/BUILD.gn +++ b/brightray/BUILD.gn @@ -19,8 +19,6 @@ static_library("brightray") { sources = [ "browser/brightray_paths.h", - "browser/browser_client.cc", - "browser/browser_client.h", "browser/browser_main_parts.cc", "browser/browser_main_parts.h", "browser/browser_main_parts_mac.mm", diff --git a/brightray/browser/browser_client.cc b/brightray/browser/browser_client.cc deleted file mode 100644 index 7c7a7974f940..000000000000 --- a/brightray/browser/browser_client.cc +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE-CHROMIUM file. - -#include "brightray/browser/browser_client.h" - -#include "base/lazy_instance.h" -#include "base/no_destructor.h" -#include "base/path_service.h" -#include "brightray/browser/browser_main_parts.h" -#include "content/public/browser/browser_thread.h" -#include "content/public/common/url_constants.h" - -using content::BrowserThread; - -namespace brightray { - -namespace { - -BrowserClient* g_browser_client; - -base::LazyInstance::DestructorAtExit - g_io_thread_application_locale = LAZY_INSTANCE_INITIALIZER; - -base::NoDestructor g_application_locale; - -void SetApplicationLocaleOnIOThread(const std::string& locale) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - g_io_thread_application_locale.Get() = locale; -} - -} // namespace - -// static -void BrowserClient::SetApplicationLocale(const std::string& locale) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - - if (!BrowserThread::IsThreadInitialized(BrowserThread::IO) || - !BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::BindOnce(&SetApplicationLocaleOnIOThread, locale))) { - g_io_thread_application_locale.Get() = locale; - } - *g_application_locale = locale; -} - -BrowserClient* BrowserClient::Get() { - return g_browser_client; -} - -BrowserClient::BrowserClient() : browser_main_parts_(nullptr) { - DCHECK(!g_browser_client); - g_browser_client = this; -} - -BrowserClient::~BrowserClient() {} - -BrowserMainParts* BrowserClient::OverrideCreateBrowserMainParts( - const content::MainFunctionParams&) { - return new BrowserMainParts; -} - -content::BrowserMainParts* BrowserClient::CreateBrowserMainParts( - const content::MainFunctionParams& parameters) { - DCHECK(!browser_main_parts_); - browser_main_parts_ = OverrideCreateBrowserMainParts(parameters); - return browser_main_parts_; -} - -base::FilePath BrowserClient::GetDefaultDownloadDirectory() { - // ~/Downloads - base::FilePath path; - if (base::PathService::Get(base::DIR_HOME, &path)) - path = path.Append(FILE_PATH_LITERAL("Downloads")); - - return path; -} - -std::string BrowserClient::GetApplicationLocale() { - if (BrowserThread::CurrentlyOn(BrowserThread::IO)) - return g_io_thread_application_locale.Get(); - return *g_application_locale; -} - -} // namespace brightray diff --git a/brightray/browser/browser_client.h b/brightray/browser/browser_client.h deleted file mode 100644 index 587157c37d03..000000000000 --- a/brightray/browser/browser_client.h +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE-CHROMIUM file. - -#ifndef BRIGHTRAY_BROWSER_BROWSER_CLIENT_H_ -#define BRIGHTRAY_BROWSER_BROWSER_CLIENT_H_ - -#include -#include -#include - -#include "content/public/browser/content_browser_client.h" - -namespace brightray { - -class BrowserMainParts; - -class BrowserClient : public content::ContentBrowserClient { - public: - static BrowserClient* Get(); - static void SetApplicationLocale(const std::string& locale); - - BrowserClient(); - ~BrowserClient() override; - - BrowserMainParts* browser_main_parts() { return browser_main_parts_; } - - // Subclasses that override this (e.g., to provide their own protocol - // handlers) should call this implementation after doing their own work. - content::BrowserMainParts* CreateBrowserMainParts( - const content::MainFunctionParams&) override; - base::FilePath GetDefaultDownloadDirectory() override; - std::string GetApplicationLocale() override; - - protected: - // Subclasses should override this to provide their own BrowserMainParts - // implementation. The lifetime of the returned instance is managed by the - // caller. - virtual BrowserMainParts* OverrideCreateBrowserMainParts( - const content::MainFunctionParams&); - - private: - BrowserMainParts* browser_main_parts_; - - DISALLOW_COPY_AND_ASSIGN(BrowserClient); -}; - -} // namespace brightray - -#endif // BRIGHTRAY_BROWSER_BROWSER_CLIENT_H_ diff --git a/brightray/browser/browser_main_parts.cc b/brightray/browser/browser_main_parts.cc index e6fb9342758e..9c9875c923c6 100644 --- a/brightray/browser/browser_main_parts.cc +++ b/brightray/browser/browser_main_parts.cc @@ -22,7 +22,6 @@ #include "base/message_loop/message_loop.h" #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" -#include "brightray/browser/browser_client.h" #include "brightray/common/application_info.h" #include "brightray/common/main_delegate.h" #include "content/public/browser/browser_thread.h" @@ -305,10 +304,6 @@ int BrowserMainParts::PreCreateThreads() { if (!views::LayoutProvider::Get()) layout_provider_.reset(new views::LayoutProvider()); - // Initialize the app locale. - BrowserClient::SetApplicationLocale( - l10n_util::GetApplicationLocale(custom_locale_)); - return 0; } diff --git a/brightray/browser/browser_main_parts.h b/brightray/browser/browser_main_parts.h index 505689e6704c..9c39fed898f0 100644 --- a/brightray/browser/browser_main_parts.h +++ b/brightray/browser/browser_main_parts.h @@ -41,6 +41,8 @@ class BrowserMainParts : public content::BrowserMainParts { void InitializeFeatureList(); + std::string custom_locale_; + private: #if defined(OS_MACOSX) void InitializeMainNib(); @@ -52,7 +54,6 @@ class BrowserMainParts : public content::BrowserMainParts { #endif std::unique_ptr layout_provider_; - std::string custom_locale_; DISALLOW_COPY_AND_ASSIGN(BrowserMainParts); }; diff --git a/brightray/common/main_delegate.cc b/brightray/common/main_delegate.cc index be1e6418640d..fa687ee193c1 100644 --- a/brightray/common/main_delegate.cc +++ b/brightray/common/main_delegate.cc @@ -9,7 +9,6 @@ #include "base/command_line.h" #include "base/mac/bundle_locations.h" #include "base/path_service.h" -#include "brightray/browser/browser_client.h" #include "content/public/common/content_switches.h" #include "electron/buildflags/buildflags.h" #include "services/service_manager/embedder/switches.h" @@ -97,13 +96,4 @@ void MainDelegate::PreSandboxStartup() { } } -content::ContentBrowserClient* MainDelegate::CreateContentBrowserClient() { - browser_client_ = CreateBrowserClient(); - return browser_client_.get(); -} - -std::unique_ptr MainDelegate::CreateBrowserClient() { - return std::unique_ptr(new BrowserClient); -} - } // namespace brightray diff --git a/brightray/common/main_delegate.h b/brightray/common/main_delegate.h index 569248a1bb30..9b281ac40f00 100644 --- a/brightray/common/main_delegate.h +++ b/brightray/common/main_delegate.h @@ -21,8 +21,6 @@ class ResourceBundle; namespace brightray { -class BrowserClient; - void LoadResourceBundle(const std::string& locale); void LoadCommonResources(); @@ -32,10 +30,6 @@ class MainDelegate : public content::ContentMainDelegate { ~MainDelegate() override; protected: - // Subclasses can override this to provide their own BrowserClient - // implementation. - virtual std::unique_ptr CreateBrowserClient(); - #if defined(OS_MACOSX) // Subclasses can override this to custom the paths of child process and // framework bundle. @@ -47,10 +41,6 @@ class MainDelegate : public content::ContentMainDelegate { void PreSandboxStartup() override; private: - content::ContentBrowserClient* CreateContentBrowserClient() override; - - std::unique_ptr browser_client_; - DISALLOW_COPY_AND_ASSIGN(MainDelegate); };