From d3a483ef59f8f40c624967361e05edda413bbf5b Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Tue, 30 Jan 2024 16:33:39 -0500 Subject: [PATCH] Fix singleton relaunch issues with multi-threaded-message-loop (fixes #3635) - Fix UI thread shutdown issues on early app exit. - views: cefclient: Fix threading requirements in RootWindowViews::Init when called on the UI thread via OnAlreadyRunningAppRelaunch. --- libcef/browser/main_runner.cc | 26 ++++++++++++++++--- .../chrome/chrome_main_runner_delegate.cc | 12 ++++++++- tests/cefclient/browser/root_window_views.cc | 16 ++++++++++-- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/libcef/browser/main_runner.cc b/libcef/browser/main_runner.cc index 24540b56b..94822ebf8 100644 --- a/libcef/browser/main_runner.cc +++ b/libcef/browser/main_runner.cc @@ -183,12 +183,20 @@ class CefUIThread : public base::PlatformThread::Delegate { runner_->RunMessageLoop(); - browser_runner_->Shutdown(); - browser_runner_.reset(); + // Stop may be called before InitializeBrowserRunner if + // content::ContentMainRun was not successful (for example, due to process + // singleton relaunch). + if (browser_runner_) { + browser_runner_->Shutdown(); + browser_runner_.reset(); + } + // This will be a no-op if there is no BrowserTaskExecutor. content::BrowserTaskExecutor::Shutdown(); - std::move(shutdown_callback_).Run(); + if (!shutdown_callback_.is_null()) { + std::move(shutdown_callback_).Run(); + } // Run exit callbacks on the UI thread to avoid sequence check failures. base::AtExitManager::ProcessCallbacksNow(); @@ -435,6 +443,11 @@ int CefMainRunner::ContentMainRun(bool* initialized, int* exit_code) { runner->main_delegate_->BeforeUIThreadInitialize(); *exit_code = content::ContentMainRun(runner->main_runner_.get()); + + if (*exit_code != content::RESULT_CODE_NORMAL_EXIT) { + runner->FinishShutdownOnUIThread(); + } + event->Signal(); }, base::Unretained(this), base::Unretained(&uithread_startup_event), @@ -446,6 +459,13 @@ int CefMainRunner::ContentMainRun(bool* initialized, // We need to wait until content::ContentMainRun has finished. uithread_startup_event.Wait(); + + if (exit_code != content::RESULT_CODE_NORMAL_EXIT) { + // content::ContentMainRun was not successful (for example, due to process + // singleton relaunch). Stop the UI thread and block until done. + ui_thread_->Stop(); + ui_thread_.reset(); + } } else { *initialized = true; main_delegate_->BeforeUIThreadInitialize(); diff --git a/libcef/common/chrome/chrome_main_runner_delegate.cc b/libcef/common/chrome/chrome_main_runner_delegate.cc index ce44e62e6..c639e1f60 100644 --- a/libcef/common/chrome/chrome_main_runner_delegate.cc +++ b/libcef/common/chrome/chrome_main_runner_delegate.cc @@ -58,6 +58,11 @@ void ChromeMainRunnerDelegate::BeforeMainThreadRun( void ChromeMainRunnerDelegate::BeforeMainMessageLoopRun( base::RunLoop* run_loop) { + // May be nullptr if content::ContentMainRun exits early. + if (!g_browser_process) { + return; + } + // The ScopedKeepAlive instance triggers shutdown logic when released on the // UI thread before terminating the message loop (e.g. from CefQuitMessageLoop // or FinishShutdownOnUIThread when running with multi-threaded message loop). @@ -67,12 +72,17 @@ void ChromeMainRunnerDelegate::BeforeMainMessageLoopRun( // The QuitClosure will be executed from BrowserProcessImpl::Unpin() via // KeepAliveRegistry when the last ScopedKeepAlive is released. // ScopedKeepAlives are also held by Browser objects. - DCHECK(g_browser_process); static_cast(g_browser_process) ->SetQuitClosure(run_loop->QuitClosure()); } bool ChromeMainRunnerDelegate::HandleMainMessageLoopQuit() { + // May be nullptr if content::ContentMainRun exits early. + if (!g_browser_process) { + // Proceed with direct execution of the QuitClosure(). + return false; + } + // May be called multiple times. See comments in RunMainMessageLoopBefore. keep_alive_.reset(); diff --git a/tests/cefclient/browser/root_window_views.cc b/tests/cefclient/browser/root_window_views.cc index 27156e10c..b302abc7c 100644 --- a/tests/cefclient/browser/root_window_views.cc +++ b/tests/cefclient/browser/root_window_views.cc @@ -58,8 +58,20 @@ void RootWindowViews::Init(RootWindow::Delegate* delegate, CreateClientHandler(config_->url); initialized_ = true; - // Continue initialization on the UI thread. - InitOnUIThread(settings, delegate_->GetRequestContext(this)); + if (!CURRENTLY_ON_MAIN_THREAD()) { + // Execute GetRequestContext() on the main thread. + MAIN_POST_CLOSURE(base::BindOnce( + [](scoped_refptr self, + const CefBrowserSettings& settings) { + // Continue initialization on the UI thread. + self->InitOnUIThread(settings, + self->delegate_->GetRequestContext(self.get())); + }, + scoped_refptr(this), settings)); + } else { + // Continue initialization on the UI thread. + InitOnUIThread(settings, delegate_->GetRequestContext(this)); + } } void RootWindowViews::InitAsPopup(RootWindow::Delegate* delegate,