Split UI thread shutdown into before and after stages (see #3609)

Existing UI thread shutdown tasks need to be executed before the UI
thread RunLoop is terminated. Those tasks have now been moved to
CefMainRunner::StartShutdownOnUIThread and FinishShutdownOnUIThread is
now called after the RunLoop is terminated.

This fixes a shutdown crash in ChromeProcessSingleton::DeleteInstance.
DeleteInstance needs to be called on the UI thread near the end of the
shutdown process (after ChromeProcessSingleton::Cleanup is called via
PostMainMessageLoopRun).

libcef/browser/main_runner.cc

@@ -162,6 +162,10 @@ class CefUIThread : public base::PlatformThread::Delegate {
     CHECK_EQ(exit_code, -1);
   }
 
+  void set_shutdown_callback(base::OnceClosure shutdown_callback) {
+    shutdown_callback_ = std::move(shutdown_callback);
+  }
+
  protected:
   void ThreadMain() override {
     base::PlatformThread::SetName("CefUIThread");
@@ -182,6 +186,8 @@ class CefUIThread : public base::PlatformThread::Delegate {
 
     content::BrowserTaskExecutor::Shutdown();
 
+    std::move(shutdown_callback_).Run();
+
     // Run exit callbacks on the UI thread to avoid sequence check failures.
     base::AtExitManager::ProcessCallbacksNow();
 
@@ -194,6 +200,7 @@ class CefUIThread : public base::PlatformThread::Delegate {
 
   CefMainRunner* const runner_;
   base::OnceClosure setup_callback_;
+  base::OnceClosure shutdown_callback_;
 
   std::unique_ptr<content::BrowserMainRunner> browser_runner_;
 
@@ -252,29 +259,45 @@ bool CefMainRunner::Initialize(CefSettings* settings,
 void CefMainRunner::Shutdown(base::OnceClosure shutdown_on_ui_thread,
                              base::OnceClosure finalize_shutdown) {
   if (multi_threaded_message_loop_) {
-    // Events that will be used to signal when shutdown is complete. Start in
-    // non-signaled mode so that the event will block.
-    base::WaitableEvent uithread_shutdown_event(
-        base::WaitableEvent::ResetPolicy::AUTOMATIC,
-        base::WaitableEvent::InitialState::NOT_SIGNALED);
+    // Start shutdown on the UI thread. This is guaranteed to run before the
+    // thread RunLoop has stopped.
+    CEF_POST_TASK(CEF_UIT,
+                  base::BindOnce(&CefMainRunner::StartShutdownOnUIThread,
+                                 base::Unretained(this),
+                                 std::move(shutdown_on_ui_thread)));
 
-    // Finish shutdown on the UI thread.
-    CEF_POST_TASK(
-        CEF_UIT,
-        base::BindOnce(&CefMainRunner::FinishShutdownOnUIThread,
-                       base::Unretained(this), std::move(shutdown_on_ui_thread),
-                       &uithread_shutdown_event));
+    // Finish shutdown on the UI thread after the thread RunLoop has stopped and
+    // before running exit callbacks.
+    ui_thread_->set_shutdown_callback(base::BindOnce(
+        &CefMainRunner::FinishShutdownOnUIThread, base::Unretained(this)));
 
-    /// Block until UI thread shutdown is complete.
-    uithread_shutdown_event.Wait();
-
-    FinalizeShutdown(std::move(finalize_shutdown));
-  } else {
-    // Finish shutdown on the current thread, which should be the UI thread.
-    FinishShutdownOnUIThread(std::move(shutdown_on_ui_thread), nullptr);
-
-    FinalizeShutdown(std::move(finalize_shutdown));
+    // Blocks until the thread has stopped.
+    ui_thread_->Stop();
+    ui_thread_.reset();
   }
+
+  main_delegate_->BeforeMainThreadShutdown();
+
+  if (!multi_threaded_message_loop_) {
+    // Main thread and UI thread are the same.
+    StartShutdownOnUIThread(std::move(shutdown_on_ui_thread));
+
+    browser_runner_->Shutdown();
+    browser_runner_.reset();
+
+    FinishShutdownOnUIThread();
+  }
+
+  // Shut down the content runner.
+  content::ContentMainShutdown(main_runner_.get());
+
+  main_runner_.reset();
+
+  std::move(finalize_shutdown).Run();
+  main_delegate_->AfterMainThreadShutdown();
+
+  main_delegate_.reset();
+  g_runtime_type = RuntimeType::UNINITIALIZED;
 }
 
 void CefMainRunner::RunMessageLoop() {
@@ -493,9 +516,8 @@ void CefMainRunner::OnContextInitialized(
   std::move(context_initialized).Run();
 }
 
-void CefMainRunner::FinishShutdownOnUIThread(
-    base::OnceClosure shutdown_on_ui_thread,
-    base::WaitableEvent* uithread_shutdown_event) {
+void CefMainRunner::StartShutdownOnUIThread(
+    base::OnceClosure shutdown_on_ui_thread) {
   CEF_REQUIRE_UIT();
 
   // Execute all pending tasks now before proceeding with shutdown. Otherwise,
@@ -512,37 +534,11 @@ void CefMainRunner::FinishShutdownOnUIThread(
       ->ShutdownOnUIThread();
 
   std::move(shutdown_on_ui_thread).Run();
-  main_delegate_->AfterUIThreadShutdown();
-
-  if (uithread_shutdown_event) {
-    uithread_shutdown_event->Signal();
-  }
+  main_delegate_->BeforeUIThreadShutdown();
 }
 
-void CefMainRunner::FinalizeShutdown(base::OnceClosure finalize_shutdown) {
-  main_delegate_->BeforeMainThreadShutdown();
-
-  if (browser_runner_.get()) {
-    browser_runner_->Shutdown();
-    browser_runner_.reset();
-  }
-
-  if (ui_thread_.get()) {
-    // Blocks until the thread has stopped.
-    ui_thread_->Stop();
-    ui_thread_.reset();
-  }
-
-  // Shut down the content runner.
-  content::ContentMainShutdown(main_runner_.get());
-
-  main_runner_.reset();
-
-  std::move(finalize_shutdown).Run();
-  main_delegate_->AfterMainThreadShutdown();
-
-  main_delegate_.reset();
-  g_runtime_type = RuntimeType::UNINITIALIZED;
+void CefMainRunner::FinishShutdownOnUIThread() {
+  main_delegate_->AfterUIThreadShutdown();
 }
 
 // From libcef/features/runtime.h:

libcef/browser/main_runner.h

@@ -71,13 +71,13 @@ class CefMainRunner : public CefMainRunnerHandler {
   // Called on the UI thread after the context is initialized.
   void OnContextInitialized(base::OnceClosure context_initialized);
 
-  // Performs shutdown actions that need to occur on the UI thread before any
-  // threads are destroyed.
-  void FinishShutdownOnUIThread(base::OnceClosure shutdown_on_ui_thread,
-                                base::WaitableEvent* uithread_shutdown_event);
+  // Performs shutdown actions that need to occur on the UI thread before the
+  // thread RunLoop has stopped.
+  void StartShutdownOnUIThread(base::OnceClosure shutdown_on_ui_thread);
 
-  // Destroys the runtime and related objects.
-  void FinalizeShutdown(base::OnceClosure finalize_shutdown);
+  // Performs shutdown actions that need to occur on the UI thread after the
+  // thread RunLoop has stopped and before running exit callbacks.
+  void FinishShutdownOnUIThread();
 
   const bool multi_threaded_message_loop_;
   const bool external_message_pump_;

libcef/common/alloy/alloy_main_runner_delegate.cc

@@ -9,6 +9,7 @@
 #include "libcef/common/alloy/alloy_main_delegate.h"
 #include "libcef/renderer/alloy/alloy_content_renderer_client.h"
 
+#include "chrome/browser/chrome_process_singleton.h"
 #include "content/public/browser/render_process_host.h"
 #include "ui/base/resource/resource_bundle.h"
 
@@ -42,13 +43,17 @@ void AlloyMainRunnerDelegate::AfterUIThreadInitialize() {
       ->OnContextInitialized();
 }
 
-void AlloyMainRunnerDelegate::AfterUIThreadShutdown() {
+void AlloyMainRunnerDelegate::BeforeUIThreadShutdown() {
   static_cast<ChromeBrowserProcessAlloy*>(g_browser_process)
       ->CleanupOnUIThread();
 
   ui::ResourceBundle::GetSharedInstance().CleanupOnUIThread();
 }
 
+void AlloyMainRunnerDelegate::AfterUIThreadShutdown() {
+  ChromeProcessSingleton::DeleteInstance();
+}
+
 void AlloyMainRunnerDelegate::BeforeMainThreadShutdown() {
   if (content::RenderProcessHost::run_renderer_in_process()) {
     // Blocks until RenderProcess cleanup is complete.

libcef/common/alloy/alloy_main_runner_delegate.h

@@ -33,6 +33,7 @@ class AlloyMainRunnerDelegate : public CefMainRunnerDelegate {
   void BeforeMainThreadInitialize(const CefMainArgs& args) override;
   void BeforeMainThreadRun(bool multi_threaded_message_loop) override;
   void AfterUIThreadInitialize() override;
+  void BeforeUIThreadShutdown() override;
   void AfterUIThreadShutdown() override;
   void BeforeMainThreadShutdown() override;
   void AfterMainThreadShutdown() override;

libcef/common/chrome/chrome_main_runner_delegate.cc

@@ -12,6 +12,7 @@
 #include "base/run_loop.h"
 #include "chrome/browser/browser_process_impl.h"
 #include "chrome/browser/chrome_content_browser_client.h"
+#include "chrome/browser/chrome_process_singleton.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"
@@ -46,6 +47,8 @@ void ChromeMainRunnerDelegate::BeforeMainThreadInitialize(
 void ChromeMainRunnerDelegate::BeforeMainThreadRun(
     bool multi_threaded_message_loop) {
   if (multi_threaded_message_loop) {
+    multi_threaded_message_loop_ = true;
+
     // Detach from the main thread so that these objects can be attached and
     // modified from the UI thread going forward.
     metrics::GlobalPersistentSystemProfile::GetInstance()
@@ -83,7 +86,7 @@ void ChromeMainRunnerDelegate::BeforeUIThreadInitialize() {
   sampling_profiler_ = std::make_unique<MainThreadStackSamplingProfiler>();
 }
 
-void ChromeMainRunnerDelegate::AfterUIThreadShutdown() {
+void ChromeMainRunnerDelegate::BeforeUIThreadShutdown() {
   static_cast<ChromeContentBrowserClient*>(
       CefAppManager::Get()->GetContentClient()->browser())
       ->CleanupOnUIThread();
@@ -92,6 +95,14 @@ void ChromeMainRunnerDelegate::AfterUIThreadShutdown() {
   sampling_profiler_.reset();
 }
 
+void ChromeMainRunnerDelegate::AfterUIThreadShutdown() {
+  if (multi_threaded_message_loop_) {
+    // Don't wait for this to be called in ChromeMainDelegate::ProcessExiting.
+    // It is safe to call multiple times.
+    ChromeProcessSingleton::DeleteInstance();
+  }
+}
+
 void ChromeMainRunnerDelegate::BeforeExecuteProcess(const CefMainArgs& args) {
   BeforeMainThreadInitialize(args);
 }

libcef/common/chrome/chrome_main_runner_delegate.h

@@ -37,6 +37,7 @@ class ChromeMainRunnerDelegate : public CefMainRunnerDelegate {
   void BeforeMainMessageLoopRun(base::RunLoop* run_loop) override;
   bool HandleMainMessageLoopQuit() override;
   void BeforeUIThreadInitialize() override;
+  void BeforeUIThreadShutdown() override;
   void AfterUIThreadShutdown() override;
   void BeforeExecuteProcess(const CefMainArgs& args) override;
   void AfterExecuteProcess() override;
@@ -50,6 +51,8 @@ class ChromeMainRunnerDelegate : public CefMainRunnerDelegate {
   CefMainRunnerHandler* const runner_;
   CefSettings* const settings_;
   CefRefPtr<CefApp> application_;
+
+  bool multi_threaded_message_loop_ = false;
 };
 
 #endif  // CEF_LIBCEF_COMMON_CHROME_CHROME_MAIN_RUNNER_DELEGATE_CEF_

libcef/common/main_runner_delegate.h

@@ -26,6 +26,7 @@ class CefMainRunnerDelegate {
   virtual bool HandleMainMessageLoopQuit() { return false; }
   virtual void BeforeUIThreadInitialize() {}
   virtual void AfterUIThreadInitialize() {}
+  virtual void BeforeUIThreadShutdown() {}
   virtual void AfterUIThreadShutdown() {}
   virtual void BeforeMainThreadShutdown() {}
   virtual void AfterMainThreadShutdown() {}