chrome: Fix shutdown crashes with multi-threaded-message-loop (fixes issue #3277)

This commit is contained in:
Marshall Greenblatt
2022-03-09 14:45:39 -05:00
parent a513e01b01
commit 3474e7abc1
4 changed files with 89 additions and 8 deletions

View File

@@ -5,11 +5,13 @@
#include "libcef/common/chrome/chrome_main_runner_delegate.h" #include "libcef/common/chrome/chrome_main_runner_delegate.h"
#include "libcef/common/app_manager.h"
#include "libcef/common/chrome/chrome_main_delegate_cef.h" #include "libcef/common/chrome/chrome_main_delegate_cef.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "chrome/browser/browser_process_impl.h" #include "chrome/browser/browser_process_impl.h"
#include "chrome/browser/chrome_content_browser_client.h"
#include "chrome/common/profiler/main_thread_stack_sampling_profiler.h" #include "chrome/common/profiler/main_thread_stack_sampling_profiler.h"
#include "components/keep_alive_registry/keep_alive_types.h" #include "components/keep_alive_registry/keep_alive_types.h"
#include "components/keep_alive_registry/scoped_keep_alive.h" #include "components/keep_alive_registry/scoped_keep_alive.h"
@@ -68,6 +70,12 @@ bool ChromeMainRunnerDelegate::HandleMainMessageLoopQuit() {
return true; return true;
} }
void ChromeMainRunnerDelegate::AfterUIThreadShutdown() {
static_cast<ChromeContentBrowserClient*>(
CefAppManager::Get()->GetContentClient()->browser())
->CleanupOnUIThread();
}
void ChromeMainRunnerDelegate::AfterMainThreadShutdown() { void ChromeMainRunnerDelegate::AfterMainThreadShutdown() {
sampling_profiler_.reset(); sampling_profiler_.reset();
} }

View File

@@ -35,6 +35,7 @@ class ChromeMainRunnerDelegate : public CefMainRunnerDelegate {
void BeforeMainThreadInitialize(const CefMainArgs& args) override; void BeforeMainThreadInitialize(const CefMainArgs& args) override;
void BeforeMainMessageLoopRun(base::RunLoop* run_loop) override; void BeforeMainMessageLoopRun(base::RunLoop* run_loop) override;
bool HandleMainMessageLoopQuit() override; bool HandleMainMessageLoopQuit() override;
void AfterUIThreadShutdown() override;
void AfterMainThreadShutdown() override; void AfterMainThreadShutdown() override;
void BeforeExecuteProcess(const CefMainArgs& args) override; void BeforeExecuteProcess(const CefMainArgs& args) override;
void AfterExecuteProcess() override; void AfterExecuteProcess() override;

View File

@@ -170,7 +170,7 @@ index 831d7173873d1..594aee58331a7 100644
+#endif +#endif
} }
diff --git chrome/browser/chrome_content_browser_client.cc chrome/browser/chrome_content_browser_client.cc diff --git chrome/browser/chrome_content_browser_client.cc chrome/browser/chrome_content_browser_client.cc
index d590b2d42f416..e1039ef8b78b4 100644 index d590b2d42f416..0e8067890c203 100644
--- chrome/browser/chrome_content_browser_client.cc --- chrome/browser/chrome_content_browser_client.cc
+++ chrome/browser/chrome_content_browser_client.cc +++ chrome/browser/chrome_content_browser_client.cc
@@ -28,6 +28,7 @@ @@ -28,6 +28,7 @@
@@ -181,7 +181,28 @@ index d590b2d42f416..e1039ef8b78b4 100644
#include "chrome/browser/accessibility/accessibility_labels_service.h" #include "chrome/browser/accessibility/accessibility_labels_service.h"
#include "chrome/browser/accessibility/accessibility_labels_service_factory.h" #include "chrome/browser/accessibility/accessibility_labels_service_factory.h"
#include "chrome/browser/after_startup_task_utils.h" #include "chrome/browser/after_startup_task_utils.h"
@@ -3705,9 +3706,11 @@ void ChromeContentBrowserClient::BrowserURLHandlerCreated( @@ -1252,6 +1253,8 @@ bool IsTopChromeWebUIURL(const GURL& url) {
} // namespace
ChromeContentBrowserClient::ChromeContentBrowserClient() {
+ keepalive_timer_.reset(new base::OneShotTimer());
+
#if BUILDFLAG(ENABLE_PLUGINS)
extra_parts_.push_back(new ChromeContentBrowserClientPluginsPart);
#endif
@@ -1277,6 +1280,11 @@ ChromeContentBrowserClient::~ChromeContentBrowserClient() {
extra_parts_.clear();
}
+void ChromeContentBrowserClient::CleanupOnUIThread() {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ keepalive_timer_.reset();
+}
+
// static
void ChromeContentBrowserClient::RegisterLocalStatePrefs(
PrefRegistrySimple* registry) {
@@ -3705,9 +3713,11 @@ void ChromeContentBrowserClient::BrowserURLHandlerCreated(
&search::HandleNewTabURLReverseRewrite); &search::HandleNewTabURLReverseRewrite);
#endif // BUILDFLAG(IS_ANDROID) #endif // BUILDFLAG(IS_ANDROID)
@@ -193,7 +214,7 @@ index d590b2d42f416..e1039ef8b78b4 100644
} }
base::FilePath ChromeContentBrowserClient::GetDefaultDownloadDirectory() { base::FilePath ChromeContentBrowserClient::GetDefaultDownloadDirectory() {
@@ -5340,7 +5343,7 @@ void ChromeContentBrowserClient::OnNetworkServiceCreated( @@ -5340,7 +5350,7 @@ void ChromeContentBrowserClient::OnNetworkServiceCreated(
network_service); network_service);
} }
@@ -202,7 +223,7 @@ index d590b2d42f416..e1039ef8b78b4 100644
content::BrowserContext* context, content::BrowserContext* context,
bool in_memory, bool in_memory,
const base::FilePath& relative_partition_path, const base::FilePath& relative_partition_path,
@@ -5358,6 +5361,8 @@ void ChromeContentBrowserClient::ConfigureNetworkContextParams( @@ -5358,6 +5368,8 @@ void ChromeContentBrowserClient::ConfigureNetworkContextParams(
network_context_params->user_agent = GetUserAgentBasedOnPolicy(context); network_context_params->user_agent = GetUserAgentBasedOnPolicy(context);
network_context_params->accept_language = GetApplicationLocale(); network_context_params->accept_language = GetApplicationLocale();
} }
@@ -211,11 +232,52 @@ index d590b2d42f416..e1039ef8b78b4 100644
} }
std::vector<base::FilePath> std::vector<base::FilePath>
@@ -6202,10 +6214,10 @@ void ChromeContentBrowserClient::OnKeepaliveRequestStarted(
const auto now = base::TimeTicks::Now();
const auto timeout = GetKeepaliveTimerTimeout(context);
keepalive_deadline_ = std::max(keepalive_deadline_, now + timeout);
- if (keepalive_deadline_ > now && !keepalive_timer_.IsRunning()) {
+ if (keepalive_deadline_ > now && !keepalive_timer_->IsRunning()) {
DVLOG(1) << "Starting a keepalive timer(" << timeout.InSecondsF()
<< " seconds)";
- keepalive_timer_.Start(
+ keepalive_timer_->Start(
FROM_HERE, keepalive_deadline_ - now,
base::BindOnce(
&ChromeContentBrowserClient::OnKeepaliveTimerFired,
@@ -6224,7 +6236,8 @@ void ChromeContentBrowserClient::OnKeepaliveRequestFinished() {
--num_keepalive_requests_;
if (num_keepalive_requests_ == 0) {
DVLOG(1) << "Stopping the keepalive timer";
- keepalive_timer_.Stop();
+ if (keepalive_timer_)
+ keepalive_timer_->Stop();
// This deletes the keep alive handle attached to the timer function and
// unblock the shutdown sequence.
}
@@ -6333,7 +6346,7 @@ void ChromeContentBrowserClient::OnKeepaliveTimerFired(
const auto now = base::TimeTicks::Now();
const auto then = keepalive_deadline_;
if (now < then) {
- keepalive_timer_.Start(
+ keepalive_timer_->Start(
FROM_HERE, then - now,
base::BindOnce(&ChromeContentBrowserClient::OnKeepaliveTimerFired,
weak_factory_.GetWeakPtr(),
diff --git chrome/browser/chrome_content_browser_client.h chrome/browser/chrome_content_browser_client.h diff --git chrome/browser/chrome_content_browser_client.h chrome/browser/chrome_content_browser_client.h
index f2a7fdf291652..f106b11e61ab0 100644 index f2a7fdf291652..d086be4c88f56 100644
--- chrome/browser/chrome_content_browser_client.h --- chrome/browser/chrome_content_browser_client.h
+++ chrome/browser/chrome_content_browser_client.h +++ chrome/browser/chrome_content_browser_client.h
@@ -557,7 +557,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient { @@ -121,6 +121,8 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
~ChromeContentBrowserClient() override;
+ virtual void CleanupOnUIThread();
+
// TODO(https://crbug.com/787567): This file is about calls from content/ out
// to chrome/ to get values or notify about events, but both of these
// functions are from chrome/ to chrome/ and don't involve content/ at all.
@@ -557,7 +559,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
override; override;
void OnNetworkServiceCreated( void OnNetworkServiceCreated(
network::mojom::NetworkService* network_service) override; network::mojom::NetworkService* network_service) override;
@@ -224,6 +286,15 @@ index f2a7fdf291652..f106b11e61ab0 100644
content::BrowserContext* context, content::BrowserContext* context,
bool in_memory, bool in_memory,
const base::FilePath& relative_partition_path, const base::FilePath& relative_partition_path,
@@ -909,7 +911,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
#if !BUILDFLAG(IS_ANDROID)
uint64_t num_keepalive_requests_ = 0;
- base::OneShotTimer keepalive_timer_;
+ std::unique_ptr<base::OneShotTimer> keepalive_timer_;
base::TimeTicks keepalive_deadline_;
#endif
diff --git chrome/browser/prefs/browser_prefs.cc chrome/browser/prefs/browser_prefs.cc diff --git chrome/browser/prefs/browser_prefs.cc chrome/browser/prefs/browser_prefs.cc
index 94cf3615137ad..369983be86323 100644 index 94cf3615137ad..369983be86323 100644
--- chrome/browser/prefs/browser_prefs.cc --- chrome/browser/prefs/browser_prefs.cc

View File

@@ -100,7 +100,7 @@ index 332b7d026e036..0d63ad05fde4e 100644
} }
diff --git content/app/content_main_runner_impl.cc content/app/content_main_runner_impl.cc diff --git content/app/content_main_runner_impl.cc content/app/content_main_runner_impl.cc
index 9d22238878e32..e0833ed4572e9 100644 index 9d22238878e32..dd231b0d55422 100644
--- content/app/content_main_runner_impl.cc --- content/app/content_main_runner_impl.cc
+++ content/app/content_main_runner_impl.cc +++ content/app/content_main_runner_impl.cc
@@ -44,6 +44,7 @@ @@ -44,6 +44,7 @@
@@ -111,12 +111,13 @@ index 9d22238878e32..e0833ed4572e9 100644
#include "base/time/time.h" #include "base/time/time.h"
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
#include "build/build_config.h" #include "build/build_config.h"
@@ -1205,6 +1206,11 @@ void ContentMainRunnerImpl::Shutdown() { @@ -1205,6 +1206,12 @@ void ContentMainRunnerImpl::Shutdown() {
is_shutdown_ = true; is_shutdown_ = true;
} }
+void ContentMainRunnerImpl::ShutdownOnUIThread() { +void ContentMainRunnerImpl::ShutdownOnUIThread() {
+ base::ScopedAllowBaseSyncPrimitivesForTesting allow_wait; + base::ScopedAllowBaseSyncPrimitivesForTesting allow_wait;
+ unregister_thread_closure_.RunAndReset();
+ discardable_shared_memory_manager_.reset(); + discardable_shared_memory_manager_.reset();
+} +}
+ +