x11/ozone: set error handlers

Extends OzonePlatform and adds pre and early initialize
steps that are used to set error handlers from
ChromeBrowserMainExtraPartsOzone (used to be
CBMEPX11).

At the moment, there is only X11 that uses that, but
Wayland will also use that in the follow up CL.

Bug: 1093008
Change-Id: I32c56d0ed888c6e965068776c37529aade81fc10
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2238071
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779219}

chrome/browser/BUILD.gn

@@ -4563,13 +4563,18 @@ static_library("browser") {
 
   if (use_x11) {
     sources += [
-      "chrome_browser_main_extra_parts_x11.cc",
-      "chrome_browser_main_extra_parts_x11.h",
       "password_manager/password_store_x.cc",
       "password_manager/password_store_x.h",
     ]
   }
 
+  if (use_x11 || use_ozone) {
+    sources += [
+      "chrome_browser_main_extra_parts_ozone.cc",
+      "chrome_browser_main_extra_parts_ozone.h",
+    ]
+  }
+
   if (enable_background_mode) {
     sources += [
       "background/background_application_list_model.cc",

chrome/browser/chrome_browser_main_extra_parts_ozone.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chrome_browser_main_extra_parts_ozone.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "chrome/browser/lifetime/application_lifetime.h"
+
+#if defined(USE_X11)
+#include "ui/base/x/x11_error_handler.h"
+#else
+#include "ui/ozone/public/ozone_platform.h"
+#endif
+
+ChromeBrowserMainExtraPartsOzone::ChromeBrowserMainExtraPartsOzone() = default;
+
+ChromeBrowserMainExtraPartsOzone::~ChromeBrowserMainExtraPartsOzone() = default;
+
+void ChromeBrowserMainExtraPartsOzone::PreEarlyInitialization() {
+#if defined(USE_X11)
+  ui::SetNullErrorHandlers();
+#else
+  ui::OzonePlatform::PreEarlyInitialization();
+#endif
+}
+
+void ChromeBrowserMainExtraPartsOzone::PostMainMessageLoopStart() {
+  auto shutdown_cb = base::BindOnce(&chrome::SessionEnding);
+#if defined(USE_X11)
+  ui::SetErrorHandlers(std::move(shutdown_cb));
+#else
+  ui::OzonePlatform::GetInstance()->PostMainMessageLoopStart(
+      std::move(shutdown_cb));
+#endif
+}
+
+void ChromeBrowserMainExtraPartsOzone::PostMainMessageLoopRun() {
+#if defined(USE_X11)
+  ui::SetEmptyErrorHandlers();
+#else
+  ui::OzonePlatform::GetInstance()->PostMainMessageLoopRun();
+#endif
+}

chrome/browser/chrome_browser_main_extra_parts_x11.h → chrome/browser/chrome_browser_main_extra_parts_ozone.h

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright 2020 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Contains functions used by BrowserMain() that are gtk-specific.
+#ifndef CHROME_BROWSER_CHROME_BROWSER_MAIN_EXTRA_PARTS_OZONE_H_
+#define CHROME_BROWSER_CHROME_BROWSER_MAIN_EXTRA_PARTS_OZONE_H_
 
-#ifndef CHROME_BROWSER_CHROME_BROWSER_MAIN_EXTRA_PARTS_X11_H_
-#define CHROME_BROWSER_CHROME_BROWSER_MAIN_EXTRA_PARTS_X11_H_
-
-#include "base/compiler_specific.h"
-#include "base/macros.h"
 #include "chrome/browser/chrome_browser_main_extra_parts.h"
 
-class ChromeBrowserMainExtraPartsX11 : public ChromeBrowserMainExtraParts {
+// Temporarily used by both Ozone and non-Ozone/X11. Once Ozone becomes default
+// on Linux, this class will be used purely by Ozone.
+class ChromeBrowserMainExtraPartsOzone : public ChromeBrowserMainExtraParts {
  public:
-  ChromeBrowserMainExtraPartsX11();
-  ~ChromeBrowserMainExtraPartsX11() override;
+  ChromeBrowserMainExtraPartsOzone();
+  ChromeBrowserMainExtraPartsOzone(const ChromeBrowserMainExtraPartsOzone&) =
+      delete;
+  ChromeBrowserMainExtraPartsOzone& operator=(
+      const ChromeBrowserMainExtraPartsOzone&) = delete;
+  ~ChromeBrowserMainExtraPartsOzone() override;
 
  private:
   // ChromeBrowserMainExtraParts overrides.
   void PreEarlyInitialization() override;
   void PostMainMessageLoopStart() override;
   void PostMainMessageLoopRun() override;
-
-  DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainExtraPartsX11);
 };
 
-#endif  // CHROME_BROWSER_CHROME_BROWSER_MAIN_EXTRA_PARTS_X11_H_
+#endif  // CHROME_BROWSER_CHROME_BROWSER_MAIN_EXTRA_PARTS_OZONE_H_

chrome/browser/chrome_content_browser_client.cc

@@ -508,8 +508,8 @@
 #include "ui/ozone/public/ozone_platform.h"
 #endif
 
-#if defined(USE_X11)
-#include "chrome/browser/chrome_browser_main_extra_parts_x11.h"
+#if defined(USE_X11) || defined(USE_OZONE)
+#include "chrome/browser/chrome_browser_main_extra_parts_ozone.h"
 #endif
 
 #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
@@ -1358,8 +1358,8 @@ ChromeContentBrowserClient::CreateBrowserMainParts(
   main_parts->AddParts(new ChromeBrowserMainExtraPartsAsh());
 #endif
 
-#if defined(USE_X11)
-  main_parts->AddParts(new ChromeBrowserMainExtraPartsX11());
+#if defined(USE_X11) || defined(USE_OZONE)
+  main_parts->AddParts(new ChromeBrowserMainExtraPartsOzone());
 #endif
 
   main_parts->AddParts(new ChromeBrowserMainExtraPartsPerformanceManager);

ui/base/x/BUILD.gn

@@ -21,6 +21,8 @@ jumbo_component("x") {
     "x11_display_manager.h",
     "x11_display_util.cc",
     "x11_display_util.h",
+    "x11_error_handler.cc",
+    "x11_error_handler.h",
     "x11_menu_list.cc",
     "x11_menu_list.h",
     "x11_menu_registrar.cc",

chrome/browser/chrome_browser_main_extra_parts_x11.cc → ui/base/x/x11_error_handler.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2020 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chrome_browser_main_extra_parts_x11.h"
+#include "ui/base/x/x11_error_handler.h"
 
-#include "base/bind.h"
-#include "base/debug/debugger.h"
-#include "base/location.h"
+#include "base/compiler_specific.h"
+#include "base/lazy_instance.h"
+#include "base/message_loop/message_loop_current.h"
 #include "base/sequenced_task_runner.h"
 #include "base/threading/sequenced_task_runner_handle.h"
-#include "chrome/browser/lifetime/application_lifetime.h"
-#include "chrome/common/chrome_result_codes.h"
-#include "content/public/browser/browser_thread.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/base/x/x11_util_internal.h"
 #include "ui/gfx/x/xproto_util.h"
 
-using content::BrowserThread;
+namespace ui {
 
 namespace {
 
 // Indicates that we're currently responding to an IO error (by shutting down).
 bool g_in_x11_io_error_handler = false;
 
+base::LazyInstance<base::OnceClosure>::Leaky g_shutdown_cb =
+    LAZY_INSTANCE_INITIALIZER;
+
 // Number of seconds to wait for UI thread to get an IO error if we get it on
 // the background thread.
 const int kWaitForUIThreadSeconds = 10;
@@ -35,7 +35,6 @@ int BrowserX11ErrorHandler(Display* d, XErrorEvent* error) {
   return 0;
 }
 
-
 // This function is used to help us diagnose crash dumps that happen
 // during the shutdown process.
 NOINLINE void WaitingForUIThreadToHandleIOError() {
@@ -45,7 +44,7 @@ NOINLINE void WaitingForUIThreadToHandleIOError() {
 }
 
 int BrowserX11IOErrorHandler(Display* d) {
-  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+  if (!base::MessageLoopCurrentForUI::IsSet()) {
     // Wait for the UI thread (which has a different connection to the X server)
     // to get the error. We can't call shutdown from this thread without
     // tripping an error. Doing it through a function so that we'll be able
@@ -61,7 +60,8 @@ int BrowserX11IOErrorHandler(Display* d) {
 
   g_in_x11_io_error_handler = true;
   LOG(ERROR) << "X IO error received (X server probably went away)";
-  chrome::SessionEnding();
+  DCHECK(!g_shutdown_cb.Get().is_null());
+  std::move(g_shutdown_cb.Get()).Run();
 
   return 0;
 }
@@ -76,29 +76,27 @@ int X11EmptyIOErrorHandler(Display* d) {
 
 }  // namespace
 
-ChromeBrowserMainExtraPartsX11::ChromeBrowserMainExtraPartsX11() {
-}
-
-ChromeBrowserMainExtraPartsX11::~ChromeBrowserMainExtraPartsX11() {
-}
-
-void ChromeBrowserMainExtraPartsX11::PreEarlyInitialization() {
+void SetNullErrorHandlers() {
   // Installs the X11 error handlers for the browser process used during
   // startup. They simply print error messages and exit because
   // we can't shutdown properly while creating and initializing services.
-  ui::SetX11ErrorHandlers(NULL, NULL);
+  ui::SetX11ErrorHandlers(nullptr, nullptr);
 }
 
-void ChromeBrowserMainExtraPartsX11::PostMainMessageLoopStart() {
+void SetErrorHandlers(base::OnceCallback<void()> shutdown_cb) {
   // Installs the X11 error handlers for the browser process after the
   // main message loop has started. This will allow us to exit cleanly
-  // if X exits before us.
+  // if X exits before we do.
+  DCHECK(g_shutdown_cb.Get().is_null());
+  g_shutdown_cb.Get() = std::move(shutdown_cb);
   ui::SetX11ErrorHandlers(BrowserX11ErrorHandler, BrowserX11IOErrorHandler);
 }
 
-void ChromeBrowserMainExtraPartsX11::PostMainMessageLoopRun() {
+void SetEmptyErrorHandlers() {
   // Unset the X11 error handlers. The X11 error handlers log the errors using a
   // |PostTask()| on the message-loop. But since the message-loop is in the
   // process of terminating, this can cause errors.
   ui::SetX11ErrorHandlers(X11EmptyErrorHandler, X11EmptyIOErrorHandler);
 }
+
+}  // namespace ui

ui/base/x/x11_error_handler.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UI_BASE_X_X11_ERROR_HANDLER_H_
+#define UI_BASE_X_X11_ERROR_HANDLER_H_
+
+#include "base/callback.h"
+#include "base/component_export.h"
+
+namespace ui {
+
+// Sets null error handlers that just catch error messages.
+COMPONENT_EXPORT(UI_BASE_X) void SetNullErrorHandlers();
+
+// Sets error handlers that catch the error messages on ui thread, waits until
+// errors are received on io thread, and stops the browser.
+COMPONENT_EXPORT(UI_BASE_X)
+void SetErrorHandlers(base::OnceCallback<void()> shutdown_cb);
+
+// Unsets the error handlers.
+COMPONENT_EXPORT(UI_BASE_X) void SetEmptyErrorHandlers();
+
+}  // namespace ui
+
+#endif  // UI_BASE_X_X11_ERROR_HANDLER_H_

ui/ozone/platform/x11/ozone_platform_x11.cc

@@ -14,6 +14,7 @@
 #include "ui/base/cursor/cursor_factory.h"
 #include "ui/base/dragdrop/os_exchange_data_provider_factory.h"
 #include "ui/base/dragdrop/os_exchange_data_provider_factory_ozone.h"
+#include "ui/base/x/x11_error_handler.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/display/fake/fake_display_delegate.h"
 #include "ui/events/devices/x11/touch_factory_x11.h"
@@ -201,6 +202,28 @@ class OzonePlatformX11 : public OzonePlatform,
     gl_egl_utility_ = std::make_unique<GLEGLUtilityX11>();
   }
 
+  void PostMainMessageLoopStart(
+      base::OnceCallback<void()> shutdown_cb) override {
+    // Installs the X11 error handlers for the UI process after the
+    // main message loop has started. This will allow us to exit cleanly
+    // if X exits before we do.
+    SetErrorHandlers(std::move(shutdown_cb));
+  }
+
+  void PostMainMessageLoopRun() override {
+    // Unset the X11 error handlers. The X11 error handlers log the errors using
+    // a |PostTask()| on the message-loop. But since the message-loop is in the
+    // process of terminating, this can cause errors.
+    SetEmptyErrorHandlers();
+  }
+
+  void PreEarlyInitialize() override {
+    // Installs the X11 error handlers for the browser process used during
+    // startup. They simply print error messages and exit because
+    // we can't shutdown properly while creating and initializing services.
+    SetNullErrorHandlers();
+  }
+
  private:
   // Performs initialization steps need by both UI and GPU.
   void InitializeCommon(const InitParams& params) {

ui/ozone/public/ozone_platform.cc

@@ -44,6 +44,15 @@ OzonePlatform::OzonePlatform() {
 
 OzonePlatform::~OzonePlatform() = default;
 
+// static
+void OzonePlatform::PreEarlyInitialization() {
+  EnsureInstance();
+  if (g_instance->prearly_initialized_)
+    return;
+  g_instance->prearly_initialized_ = true;
+  g_instance->PreEarlyInitialize();
+}
+
 // static
 void OzonePlatform::InitializeForUI(const InitParams& args) {
   EnsureInstance();
@@ -115,4 +124,11 @@ void OzonePlatform::AfterSandboxEntry() {
   DCHECK(!single_process_);
 }
 
+void OzonePlatform::PostMainMessageLoopStart(
+    base::OnceCallback<void()> shutdown_cb) {}
+
+void OzonePlatform::PostMainMessageLoopRun() {}
+
+void OzonePlatform::PreEarlyInitialize() {}
+
 }  // namespace ui

ui/ozone/public/ozone_platform.h

@@ -103,6 +103,30 @@ class COMPONENT_EXPORT(OZONE) OzonePlatform {
     bool supports_overlays = false;
   };
 
+  // Corresponds to chrome_browser_main_extra_parts.h.
+  //
+  // The browser process' initialization involves several steps -
+  // PreEarlyInitialization, PostMainMessageLoopStart, PostMainMessageLoopRun,
+  // etc. In order to be consistent with that and allow platform specific
+  // initialization steps, the OzonePlatform has three methods - one static
+  // PreEarlyInitialization that is expected to do some early non-ui
+  // initialization (like error handlers that X11 sets), and two non-static
+  // methods - PostMainmessageLoopStart and PostMainMessageLoopRun. The latter
+  // two are supposed to be called on a post start and a post-run of the
+  // MessageLoop. Please note that this methods must be run on the browser' UI
+  // thread.
+  //
+  // Creates OzonePlatform and does pre-early initialization (internally, sets
+  // error handlers if supported so that we can print errors during the browser
+  // process' start up).
+  static void PreEarlyInitialization();
+  // Sets error handlers if supported for the browser process after the message
+  // loop started. It's required to call this so that we can exit cleanly if the
+  // server can exit before we do.
+  virtual void PostMainMessageLoopStart(base::OnceCallback<void()> shutdown_cb);
+  // Resets the error handlers if set.
+  virtual void PostMainMessageLoopRun();
+
   // Initializes the subsystems/resources necessary for the UI process (e.g.
   // events) with additional properties to customize the ozone platform
   // implementation. Ozone will not retain InitParams after returning from
@@ -188,11 +212,16 @@ class COMPONENT_EXPORT(OZONE) OzonePlatform {
   bool single_process() const { return single_process_; }
 
  private:
+  // Optional method for pre-early initialization. In case of X11, sets X11
+  // error handlers so that errors can be caught if early initialization fails.
+  virtual void PreEarlyInitialize();
+
   virtual void InitializeUI(const InitParams& params) = 0;
   virtual void InitializeGPU(const InitParams& params) = 0;
 
   bool initialized_ui_ = false;
   bool initialized_gpu_ = false;
+  bool prearly_initialized_ = false;
 
   bool single_process_ = false;