From 0c5b1f54e388f3ca762abfa4b34aafad1389158c Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Fri, 25 Aug 2023 06:47:48 +0000 Subject: [PATCH] [headless] Shutdown gracefully upon Ctrl-C, Ctrl-break, and console closure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate //headless to using //content's console control handler facilities. This reduces complexity in //headless and should slow the tide of crashes coming from child procs, where //headless was not previously installing a console control handler. Bug: 1473516 Change-Id: I320058d4806f3e80d82adefa72014ba727170629 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4807610 Auto-Submit: Greg Thompson Reviewed-by: Sami Kyöstilä Reviewed-by: Rakina Zata Amni Reviewed-by: Bruce Dawson Reviewed-by: Peter Kvitek Reviewed-by: Andrey Kosyakov Commit-Queue: Rakina Zata Amni Cr-Commit-Position: refs/heads/main@{#1188201} --- .../browser/chrome_content_browser_client.cc | 3 +- .../browser/chrome_content_browser_client.h | 2 +- content/app/content_main_runner_impl.cc | 2 +- .../public/browser/content_browser_client.h | 6 +- headless/BUILD.gn | 4 - .../lib/browser/headless_browser_main_parts.h | 2 +- .../headless_browser_main_parts_win.cc | 86 ------------------- .../headless_content_browser_client.cc | 8 ++ .../browser/headless_content_browser_client.h | 3 + .../lib/headless_content_main_delegate.cc | 8 ++ headless/lib/headless_content_main_delegate.h | 3 + 11 files changed, 31 insertions(+), 96 deletions(-) delete mode 100644 headless/lib/browser/headless_browser_main_parts_win.cc diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index 72839015502473..1eb294de625f86 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -4897,7 +4897,8 @@ bool ChromeContentBrowserClient::IsUtilityCetCompatible( return true; } -void ChromeContentBrowserClient::SessionEnding() { +void ChromeContentBrowserClient::SessionEnding( + absl::optional control_type) { chrome::SessionEnding(); } diff --git a/chrome/browser/chrome_content_browser_client.h b/chrome/browser/chrome_content_browser_client.h index 36d90dc7b528bd..46d348b5703a64 100644 --- a/chrome/browser/chrome_content_browser_client.h +++ b/chrome/browser/chrome_content_browser_client.h @@ -494,7 +494,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient { std::wstring GetLPACCapabilityNameForNetworkService() override; bool IsUtilityCetCompatible(const std::string& utility_sub_type) override; bool IsRendererCodeIntegrityEnabled() override; - void SessionEnding() override; + void SessionEnding(absl::optional control_type) override; bool ShouldEnableAudioProcessHighPriority() override; #endif void ExposeInterfacesToRenderer( diff --git a/content/app/content_main_runner_impl.cc b/content/app/content_main_runner_impl.cc index 6cb42330a794c8..5a28e805be7211 100644 --- a/content/app/content_main_runner_impl.cc +++ b/content/app/content_main_runner_impl.cc @@ -463,7 +463,7 @@ mojo::ScopedMessagePipeHandle MaybeAcceptMojoInvitation() { #if BUILDFLAG(IS_WIN) void HandleConsoleControlEventOnBrowserUiThread(DWORD control_type) { - GetContentClient()->browser()->SessionEnding(); + GetContentClient()->browser()->SessionEnding(control_type); } // A console control event handler for browser processes that initiates end diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h index 4d89822c5cabc1..23d172929b465d 100644 --- a/content/public/browser/content_browser_client.h +++ b/content/public/browser/content_browser_client.h @@ -1495,8 +1495,10 @@ class CONTENT_EXPORT ContentBrowserClient { // This is called on the UI thread. virtual bool IsRendererCodeIntegrityEnabled(); - // Performs a fast and orderly shutdown of the browser. - virtual void SessionEnding() {} + // Performs a fast and orderly shutdown of the browser. If present, + // `control_type` is a CTRL_* value from a Windows console control handler; + // see https://learn.microsoft.com/en-us/windows/console/handlerroutine. + virtual void SessionEnding(absl::optional control_type) {} // Returns true if the audio process should run with high priority. false // otherwise. diff --git a/headless/BUILD.gn b/headless/BUILD.gn index 8939ea6af417e2..e0ec74566e0474 100644 --- a/headless/BUILD.gn +++ b/headless/BUILD.gn @@ -383,10 +383,6 @@ component("headless_non_renderer") { sources += [ "lib/browser/headless_browser_main_parts_posix.cc" ] } - if (is_win) { - sources += [ "lib/browser/headless_browser_main_parts_win.cc" ] - } - if (use_aura) { sources += [ "lib/browser/headless_browser_impl_aura.cc", diff --git a/headless/lib/browser/headless_browser_main_parts.h b/headless/lib/browser/headless_browser_main_parts.h index d48e3f95fbef13..bd04765a2aaa96 100644 --- a/headless/lib/browser/headless_browser_main_parts.h +++ b/headless/lib/browser/headless_browser_main_parts.h @@ -48,7 +48,7 @@ class HEADLESS_EXPORT HeadlessBrowserMainParts #if BUILDFLAG(IS_MAC) void PreCreateMainMessageLoop() override; #endif -#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_WIN) +#if BUILDFLAG(IS_POSIX) void PostCreateMainMessageLoop() override; #endif void QuitMainMessageLoop(); diff --git a/headless/lib/browser/headless_browser_main_parts_win.cc b/headless/lib/browser/headless_browser_main_parts_win.cc deleted file mode 100644 index 0121a8b64a6d4c..00000000000000 --- a/headless/lib/browser/headless_browser_main_parts_win.cc +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright 2023 The Chromium Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "headless/lib/browser/headless_browser_main_parts.h" - -#include - -#include "base/functional/bind.h" -#include "base/functional/callback.h" -#include "base/logging.h" -#include "base/memory/scoped_refptr.h" -#include "base/no_destructor.h" -#include "base/task/single_thread_task_runner.h" -#include "content/public/browser/browser_task_traits.h" -#include "content/public/browser/browser_thread.h" -#include "headless/lib/browser/headless_browser_impl.h" - -namespace headless { - -namespace { - -class BrowserShutdownHandler { - public: - typedef base::OnceCallback ShutdownCallback; - - BrowserShutdownHandler(const BrowserShutdownHandler&) = delete; - BrowserShutdownHandler& operator=(const BrowserShutdownHandler&) = delete; - - static void Install(ShutdownCallback shutdown_callback) { - GetInstance().Init(std::move(shutdown_callback)); - - PCHECK(::SetConsoleCtrlHandler(ConsoleCtrlHandler, true) != 0); - } - - private: - friend class base::NoDestructor; - - BrowserShutdownHandler() = default; - ~BrowserShutdownHandler() = default; - - static BrowserShutdownHandler& GetInstance() { - static base::NoDestructor instance; - return *instance; - } - - void Init(ShutdownCallback shutdown_callback) { - task_runner_ = content::GetUIThreadTaskRunner({}); - shutdown_callback_ = std::move(shutdown_callback); - } - - bool Shutdown(DWORD ctrl_type) { - // If the callback is already consumed, let the default handler do its - // thing. - if (!shutdown_callback_) { - return false; - } - - DCHECK_LT(ctrl_type, 0x7fu); - int exit_code = 0x80u + ctrl_type; - if (!task_runner_->PostTask( - FROM_HERE, - base::BindOnce(std::move(shutdown_callback_), exit_code))) { - RAW_LOG(WARNING, "No valid task runner, exiting ungracefully."); - return false; - } - - return true; - } - - static BOOL WINAPI ConsoleCtrlHandler(DWORD ctrl_type) { - return GetInstance().Shutdown(ctrl_type); - } - - scoped_refptr task_runner_; - ShutdownCallback shutdown_callback_; -}; - -} // namespace - -void HeadlessBrowserMainParts::PostCreateMainMessageLoop() { - BrowserShutdownHandler::Install(base::BindOnce( - &HeadlessBrowserImpl::ShutdownWithExitCode, browser_->GetWeakPtr())); -} - -} // namespace headless diff --git a/headless/lib/browser/headless_content_browser_client.cc b/headless/lib/browser/headless_content_browser_client.cc index eb4a7fdec40d74..c8fa0a0379b448 100644 --- a/headless/lib/browser/headless_content_browser_client.cc +++ b/headless/lib/browser/headless_content_browser_client.cc @@ -348,6 +348,14 @@ HeadlessContentBrowserClient::GetGeolocationManager() { #endif } +#if BUILDFLAG(IS_WIN) +void HeadlessContentBrowserClient::SessionEnding( + absl::optional control_type) { + DCHECK_LT(control_type.value_or(0), 0x7fu); + browser_->ShutdownWithExitCode(control_type.value_or(0) + 0x80u); +} +#endif + #if defined(HEADLESS_USE_POLICY) std::vector> HeadlessContentBrowserClient::CreateThrottlesForNavigation( diff --git a/headless/lib/browser/headless_content_browser_client.h b/headless/lib/browser/headless_content_browser_client.h index aee1b9b88171b0..dfdef1bdae542a 100644 --- a/headless/lib/browser/headless_content_browser_client.h +++ b/headless/lib/browser/headless_content_browser_client.h @@ -94,6 +94,9 @@ class HeadlessContentBrowserClient : public content::ContentBrowserClient { bool CanAcceptUntrustedExchangesIfNeeded() override; device::GeolocationManager* GetGeolocationManager() override; +#if BUILDFLAG(IS_WIN) + void SessionEnding(absl::optional control_type) override; +#endif #if defined(HEADLESS_USE_POLICY) std::vector> diff --git a/headless/lib/headless_content_main_delegate.cc b/headless/lib/headless_content_main_delegate.cc index 2f1021c81526f7..6066e28b0fd320 100644 --- a/headless/lib/headless_content_main_delegate.cc +++ b/headless/lib/headless_content_main_delegate.cc @@ -464,6 +464,14 @@ absl::optional HeadlessContentMainDelegate::PreBrowserMain() { return absl::nullopt; } +#if BUILDFLAG(IS_WIN) +bool HeadlessContentMainDelegate::ShouldHandleConsoleControlEvents() { + // Handle console control events so that orderly shutdown can be performed by + // HeadlessContentBrowserClient's override of SessionEnding. + return true; +} +#endif + content::ContentClient* HeadlessContentMainDelegate::CreateContentClient() { return &content_client_; } diff --git a/headless/lib/headless_content_main_delegate.h b/headless/lib/headless_content_main_delegate.h index e9c4064e3a04b2..4ac94ed3529d69 100644 --- a/headless/lib/headless_content_main_delegate.h +++ b/headless/lib/headless_content_main_delegate.h @@ -46,6 +46,9 @@ class HEADLESS_EXPORT HeadlessContentMainDelegate const std::string& process_type, content::MainFunctionParams main_function_params) override; absl::optional PreBrowserMain() override; +#if BUILDFLAG(IS_WIN) + bool ShouldHandleConsoleControlEvents() override; +#endif content::ContentClient* CreateContentClient() override; content::ContentBrowserClient* CreateContentBrowserClient() override; content::ContentUtilityClient* CreateContentUtilityClient() override;