alloy: Move ExtensionsBrowserClient ownership to BrowserProcess (fixes issue #3247)

Fixes a shutdown crash due to `ExtensionsBrowserClient::Set(nullptr)` being
called too early. Some code that may occasionally be triggered via
`content::ContentMainShutdown()` is expecting the extensions objects to still
be valid.

This new ownership pattern matches the code in chrome/.
This commit is contained in:
Marshall Greenblatt 2022-01-28 16:16:29 -05:00
parent 81e7748fb5
commit 9eb0954cde
4 changed files with 30 additions and 23 deletions

View File

@ -13,12 +13,10 @@
#include "libcef/browser/context.h"
#include "libcef/browser/devtools/devtools_manager_delegate.h"
#include "libcef/browser/extensions/extension_system_factory.h"
#include "libcef/browser/extensions/extensions_browser_client.h"
#include "libcef/browser/net/chrome_scheme_handler.h"
#include "libcef/browser/printing/constrained_window_views_client.h"
#include "libcef/browser/thread_util.h"
#include "libcef/common/app_manager.h"
#include "libcef/common/extensions/extensions_client.h"
#include "libcef/common/extensions/extensions_util.h"
#include "libcef/common/net/net_resource_provider.h"
@ -35,7 +33,7 @@
#include "content/public/browser/gpu_data_manager.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/common/result_codes.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/extensions_browser_client.h"
#include "extensions/common/constants.h"
#include "net/base/net_module.h"
#include "third_party/widevine/cdm/buildflags.h"
@ -205,14 +203,10 @@ int AlloyBrowserMainParts::PreMainMessageLoopRun() {
#endif
if (extensions::ExtensionsEnabled()) {
// This should be set in ChromeBrowserProcessAlloy::Initialize.
DCHECK(extensions::ExtensionsBrowserClient::Get());
// Initialize extension global objects before creating the global
// BrowserContext.
extensions_client_.reset(new extensions::CefExtensionsClient());
extensions::ExtensionsClient::Set(extensions_client_.get());
extensions_browser_client_.reset(
new extensions::CefExtensionsBrowserClient);
extensions::ExtensionsBrowserClient::Set(extensions_browser_client_.get());
extensions::CefExtensionSystemFactory::GetInstance();
}
@ -283,11 +277,6 @@ void AlloyBrowserMainParts::PostMainMessageLoopRun() {
}
void AlloyBrowserMainParts::PostDestroyThreads() {
if (extensions::ExtensionsEnabled()) {
extensions::ExtensionsBrowserClient::Set(nullptr);
extensions_browser_client_.reset();
}
#if defined(TOOLKIT_VIEWS)
views_delegate_.reset();
#if BUILDFLAG(IS_MAC)

View File

@ -15,11 +15,6 @@
#include "content/public/browser/browser_main_parts.h"
#include "content/public/common/main_function_params.h"
namespace extensions {
class ExtensionsBrowserClient;
class ExtensionsClient;
} // namespace extensions
#if defined(USE_AURA)
namespace display {
class Screen;
@ -84,10 +79,6 @@ class AlloyBrowserMainParts : public content::BrowserMainParts {
CefRefPtr<CefRequestContextImpl> global_request_context_;
CefDevToolsDelegate* devtools_delegate_ = nullptr; // Deletes itself.
std::unique_ptr<extensions::ExtensionsClient> extensions_client_;
std::unique_ptr<extensions::ExtensionsBrowserClient>
extensions_browser_client_;
// Blocking task runners exposed via CefTaskRunner. For consistency with
// previous named thread behavior always execute all pending tasks before
// shutdown (e.g. to make sure critical data is saved to disk).

View File

@ -8,9 +8,12 @@
#include "libcef/browser/alloy/chrome_profile_manager_alloy.h"
#include "libcef/browser/browser_context.h"
#include "libcef/browser/context.h"
#include "libcef/browser/extensions/extensions_browser_client.h"
#include "libcef/browser/prefs/browser_prefs.h"
#include "libcef/browser/thread_util.h"
#include "libcef/common/cef_switches.h"
#include "libcef/common/extensions/extensions_client.h"
#include "libcef/common/extensions/extensions_util.h"
#include "base/command_line.h"
#include "chrome/browser/component_updater/chrome_component_updater_configurator.h"
@ -38,6 +41,11 @@ ChromeBrowserProcessAlloy::ChromeBrowserProcessAlloy()
ChromeBrowserProcessAlloy::~ChromeBrowserProcessAlloy() {
DCHECK((!initialized_ && !context_initialized_) || shutdown_);
if (extensions::ExtensionsEnabled()) {
extensions::ExtensionsBrowserClient::Set(nullptr);
extensions_browser_client_.reset();
}
}
void ChromeBrowserProcessAlloy::Initialize() {
@ -49,6 +57,16 @@ void ChromeBrowserProcessAlloy::Initialize() {
// Initialize this early before any code tries to check feature flags.
field_trial_list_ = content::SetUpFieldTrialsAndFeatureList();
if (extensions::ExtensionsEnabled()) {
// Initialize extension global objects before creating the global
// BrowserContext.
extensions_client_.reset(new extensions::CefExtensionsClient());
extensions::ExtensionsClient::Set(extensions_client_.get());
extensions_browser_client_.reset(
new extensions::CefExtensionsBrowserClient);
extensions::ExtensionsBrowserClient::Set(extensions_browser_client_.get());
}
initialized_ = true;
}

View File

@ -18,6 +18,11 @@
#include "chrome/browser/extensions/event_router_forwarder.h"
#include "media/media_buildflags.h"
namespace extensions {
class ExtensionsBrowserClient;
class ExtensionsClient;
} // namespace extensions
class ChromeProfileManagerAlloy;
class BackgroundModeManager {
@ -112,6 +117,10 @@ class ChromeBrowserProcessAlloy : public BrowserProcess {
bool context_initialized_;
bool shutdown_;
std::unique_ptr<extensions::ExtensionsClient> extensions_client_;
std::unique_ptr<extensions::ExtensionsBrowserClient>
extensions_browser_client_;
std::string locale_;
std::unique_ptr<printing::PrintJobManager> print_job_manager_;
std::unique_ptr<ChromeProfileManagerAlloy> profile_manager_;