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 cfa9cb381f
commit c30e52c620
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/app_manager.h"
#include "libcef/common/chrome/chrome_main_delegate_cef.h"
#include "base/command_line.h"
#include "base/run_loop.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 "components/keep_alive_registry/keep_alive_types.h"
#include "components/keep_alive_registry/scoped_keep_alive.h"
@ -68,6 +70,12 @@ bool ChromeMainRunnerDelegate::HandleMainMessageLoopQuit() {
return true;
}
void ChromeMainRunnerDelegate::AfterUIThreadShutdown() {
static_cast<ChromeContentBrowserClient*>(
CefAppManager::Get()->GetContentClient()->browser())
->CleanupOnUIThread();
}
void ChromeMainRunnerDelegate::AfterMainThreadShutdown() {
sampling_profiler_.reset();
}

View File

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

View File

@ -170,7 +170,7 @@ index 831d7173873d1..594aee58331a7 100644
+#endif
}
diff --git chrome/browser/chrome_content_browser_client.cc chrome/browser/chrome_content_browser_client.cc
index 2d3d64f06e330..a936eba6c8349 100644
index d590b2d42f416..0e8067890c203 100644
--- chrome/browser/chrome_content_browser_client.cc
+++ chrome/browser/chrome_content_browser_client.cc
@@ -28,6 +28,7 @@
@ -181,7 +181,28 @@ index 2d3d64f06e330..a936eba6c8349 100644
#include "chrome/browser/accessibility/accessibility_labels_service.h"
#include "chrome/browser/accessibility/accessibility_labels_service_factory.h"
#include "chrome/browser/after_startup_task_utils.h"
@@ -3718,9 +3719,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);
#endif // BUILDFLAG(IS_ANDROID)
@ -193,7 +214,7 @@ index 2d3d64f06e330..a936eba6c8349 100644
}
base::FilePath ChromeContentBrowserClient::GetDefaultDownloadDirectory() {
@@ -5353,7 +5356,7 @@ void ChromeContentBrowserClient::OnNetworkServiceCreated(
@@ -5340,7 +5350,7 @@ void ChromeContentBrowserClient::OnNetworkServiceCreated(
network_service);
}
@ -202,7 +223,7 @@ index 2d3d64f06e330..a936eba6c8349 100644
content::BrowserContext* context,
bool in_memory,
const base::FilePath& relative_partition_path,
@@ -5371,6 +5374,8 @@ void ChromeContentBrowserClient::ConfigureNetworkContextParams(
@@ -5358,6 +5368,8 @@ void ChromeContentBrowserClient::ConfigureNetworkContextParams(
network_context_params->user_agent = GetUserAgentBasedOnPolicy(context);
network_context_params->accept_language = GetApplicationLocale();
}
@ -211,11 +232,52 @@ index 2d3d64f06e330..a936eba6c8349 100644
}
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
index 79b637d77174d..15d5483eeea7a 100644
index f2a7fdf291652..d086be4c88f56 100644
--- chrome/browser/chrome_content_browser_client.h
+++ chrome/browser/chrome_content_browser_client.h
@@ -561,7 +561,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;
void OnNetworkServiceCreated(
network::mojom::NetworkService* network_service) override;
@ -224,6 +286,15 @@ index 79b637d77174d..15d5483eeea7a 100644
content::BrowserContext* context,
bool in_memory,
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
index 94cf3615137ad..369983be86323 100644
--- 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
index 9d22238878e32..e0833ed4572e9 100644
index 9d22238878e32..dd231b0d55422 100644
--- content/app/content_main_runner_impl.cc
+++ content/app/content_main_runner_impl.cc
@@ -44,6 +44,7 @@
@ -111,12 +111,13 @@ index 9d22238878e32..e0833ed4572e9 100644
#include "base/time/time.h"
#include "base/trace_event/trace_event.h"
#include "build/build_config.h"
@@ -1205,6 +1206,11 @@ void ContentMainRunnerImpl::Shutdown() {
@@ -1205,6 +1206,12 @@ void ContentMainRunnerImpl::Shutdown() {
is_shutdown_ = true;
}
+void ContentMainRunnerImpl::ShutdownOnUIThread() {
+ base::ScopedAllowBaseSyncPrimitivesForTesting allow_wait;
+ unregister_thread_closure_.RunAndReset();
+ discardable_shared_memory_manager_.reset();
+}
+