From f5587b74f0aa3c15721401736990e0d7a2bdb8e1 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Sat, 27 Jun 2020 16:43:23 -0400 Subject: [PATCH] Set enable_background_mode=false by default (see issue #2969) This mode doesn't make sense with CEF, and it was causing strange shutdown crashes when running with Chrome mode enabled. --- BUILD.gn | 1 + libcef/browser/chrome_browser_process_stub.cc | 2 + libcef/browser/chrome_browser_process_stub.h | 2 + patch/patch.cfg | 5 + ...rome_browser_background_mode_1100085.patch | 108 ++++++++++++++++++ tools/gn_args.py | 6 + 6 files changed, 124 insertions(+) create mode 100644 patch/patches/chrome_browser_background_mode_1100085.patch diff --git a/BUILD.gn b/BUILD.gn index 30699646d..9bdda78bf 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -812,6 +812,7 @@ static_library("libcef_static") { "//chrome:packed_resources", "//chrome:resources", "//chrome:strings", + "//chrome/common:buildflags", "//chrome/services/printing:lib", "//components/cdm/renderer", "//components/certificate_transparency", diff --git a/libcef/browser/chrome_browser_process_stub.cc b/libcef/browser/chrome_browser_process_stub.cc index 8912530c2..4559b3604 100644 --- a/libcef/browser/chrome_browser_process_stub.cc +++ b/libcef/browser/chrome_browser_process_stub.cc @@ -289,6 +289,7 @@ DownloadRequestLimiter* ChromeBrowserProcessStub::download_request_limiter() { return nullptr; } +#if BUILDFLAG(ENABLE_BACKGROUND_MODE) BackgroundModeManager* ChromeBrowserProcessStub::background_mode_manager() { NOTREACHED(); return nullptr; @@ -298,6 +299,7 @@ void ChromeBrowserProcessStub::set_background_mode_manager_for_test( std::unique_ptr manager) { NOTREACHED(); } +#endif StatusTray* ChromeBrowserProcessStub::status_tray() { NOTREACHED(); diff --git a/libcef/browser/chrome_browser_process_stub.h b/libcef/browser/chrome_browser_process_stub.h index aa5b34998..45f89afec 100644 --- a/libcef/browser/chrome_browser_process_stub.h +++ b/libcef/browser/chrome_browser_process_stub.h @@ -73,9 +73,11 @@ class ChromeBrowserProcessStub : public BrowserProcess { void SetApplicationLocale(const std::string& locale) override; DownloadStatusUpdater* download_status_updater() override; DownloadRequestLimiter* download_request_limiter() override; +#if BUILDFLAG(ENABLE_BACKGROUND_MODE) BackgroundModeManager* background_mode_manager() override; void set_background_mode_manager_for_test( std::unique_ptr manager) override; +#endif StatusTray* status_tray() override; safe_browsing::SafeBrowsingService* safe_browsing_service() override; subresource_filter::RulesetService* subresource_filter_ruleset_service() diff --git a/patch/patch.cfg b/patch/patch.cfg index 1f573279f..80ceedad5 100644 --- a/patch/patch.cfg +++ b/patch/patch.cfg @@ -512,5 +512,10 @@ patches = [ # ChromePageInfoClient::GetJavaResourceId. # https://bugs.chromium.org/p/chromium/issues/detail?id=1099927 'name': 'linux_chrome_page_info_1099927', + }, + { + # Fix build errors with enable_background_mode=false. + # https://bugs.chromium.org/p/chromium/issues/detail?id=1100085 + 'name': 'chrome_browser_background_mode_1100085', } ] diff --git a/patch/patches/chrome_browser_background_mode_1100085.patch b/patch/patches/chrome_browser_background_mode_1100085.patch new file mode 100644 index 000000000..153f538ed --- /dev/null +++ b/patch/patches/chrome_browser_background_mode_1100085.patch @@ -0,0 +1,108 @@ +diff --git chrome/browser/browser_process.h chrome/browser/browser_process.h +index 6332572ac884..bf555124ce59 100644 +--- chrome/browser/browser_process.h ++++ chrome/browser/browser_process.h +@@ -197,10 +197,12 @@ class BrowserProcess { + virtual DownloadStatusUpdater* download_status_updater() = 0; + virtual DownloadRequestLimiter* download_request_limiter() = 0; + ++#if BUILDFLAG(ENABLE_BACKGROUND_MODE) + // Returns the object that manages background applications. + virtual BackgroundModeManager* background_mode_manager() = 0; + virtual void set_background_mode_manager_for_test( + std::unique_ptr manager) = 0; ++#endif + + // Returns the StatusTray, which provides an API for displaying status icons + // in the system status tray. Returns NULL if status icons are not supported +diff --git chrome/browser/browser_process_impl.cc chrome/browser/browser_process_impl.cc +index 13958187a750..0017954a32da 100644 +--- chrome/browser/browser_process_impl.cc ++++ chrome/browser/browser_process_impl.cc +@@ -963,24 +963,19 @@ DownloadRequestLimiter* BrowserProcessImpl::download_request_limiter() { + return download_request_limiter_.get(); + } + ++#if BUILDFLAG(ENABLE_BACKGROUND_MODE) + BackgroundModeManager* BrowserProcessImpl::background_mode_manager() { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); +-#if BUILDFLAG(ENABLE_BACKGROUND_MODE) + if (!background_mode_manager_) + CreateBackgroundModeManager(); + return background_mode_manager_.get(); +-#else +- NOTIMPLEMENTED(); +- return NULL; +-#endif + } + + void BrowserProcessImpl::set_background_mode_manager_for_test( + std::unique_ptr manager) { +-#if BUILDFLAG(ENABLE_BACKGROUND_MODE) + background_mode_manager_ = std::move(manager); +-#endif + } ++#endif + + StatusTray* BrowserProcessImpl::status_tray() { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); +diff --git chrome/browser/browser_process_impl.h chrome/browser/browser_process_impl.h +index 12c76fb96762..60637b2b0044 100644 +--- chrome/browser/browser_process_impl.h ++++ chrome/browser/browser_process_impl.h +@@ -164,9 +164,11 @@ class BrowserProcessImpl : public BrowserProcess, + void SetApplicationLocale(const std::string& actual_locale) override; + DownloadStatusUpdater* download_status_updater() override; + DownloadRequestLimiter* download_request_limiter() override; ++#if BUILDFLAG(ENABLE_BACKGROUND_MODE) + BackgroundModeManager* background_mode_manager() override; + void set_background_mode_manager_for_test( + std::unique_ptr manager) override; ++#endif + StatusTray* status_tray() override; + safe_browsing::SafeBrowsingService* safe_browsing_service() override; + subresource_filter::RulesetService* subresource_filter_ruleset_service() +diff --git chrome/browser/lifetime/browser_close_manager.cc chrome/browser/lifetime/browser_close_manager.cc +index bc7995a47fbd..28d3658d2885 100644 +--- chrome/browser/lifetime/browser_close_manager.cc ++++ chrome/browser/lifetime/browser_close_manager.cc +@@ -147,12 +147,14 @@ void BrowserCloseManager::CloseBrowsers() { + // exit can restore all browsers open before exiting. + ProfileManager::ShutdownSessionServices(); + #endif ++#if BUILDFLAG(ENABLE_BACKGROUND_MODE) + if (!browser_shutdown::IsTryingToQuit()) { + BackgroundModeManager* background_mode_manager = + g_browser_process->background_mode_manager(); + if (background_mode_manager) + background_mode_manager->SuspendBackgroundMode(); + } ++#endif + + // Make a copy of the BrowserList to simplify the case where we need to + // destroy a Browser during the loop. +diff --git chrome/browser/sessions/session_service.cc chrome/browser/sessions/session_service.cc +index f5079f6d2189..a43a981d471b 100644 +--- chrome/browser/sessions/session_service.cc ++++ chrome/browser/sessions/session_service.cc +@@ -942,12 +942,19 @@ void SessionService::MaybeDeleteSessionOnlyData() { + if (!profile() || profile()->AsTestingProfile()) + return; + ++#if BUILDFLAG(ENABLE_BACKGROUND_MODE) ++ const bool background_mode_active = ++ g_browser_process->background_mode_manager()->IsBackgroundModeActive(); ++#else ++ const bool background_mode_active = false; ++#endif ++ + // Clear session data if the last window for a profile has been closed and + // closing the last window would normally close Chrome, unless background mode + // is active. Tests don't have a background_mode_manager. + if (has_open_trackable_browsers_ || + browser_defaults::kBrowserAliveWithNoWindows || +- g_browser_process->background_mode_manager()->IsBackgroundModeActive()) { ++ background_mode_active) { + return; + } + diff --git a/tools/gn_args.py b/tools/gn_args.py index f4b3d7568..33acf5973 100644 --- a/tools/gn_args.py +++ b/tools/gn_args.py @@ -215,6 +215,12 @@ def GetRecommendedDefaultArgs(): # and macOS builds. Currently turned off for CEF because it requires # additional setup and is not yet tested. See issue #2956. 'chrome_pgo_phase': 0, + + # Disable support for background apps, which don't make sense with CEF. + # Default is enabled on desktop platforms. This feature was also causing + # strange shutdown crashes when using the Chrome runtime with a Debug + # component build on Windows. + 'enable_background_mode': False, } if platform == 'linux':