Revert "Set Xlib error handler after GTK X11 initialization"

This reverts commit 2ed82a41.

Reason for revert: Looks like this is causing the Linux perf tests to crash in xlib_support.cc: https://ci.chromium.org/p/chrome/builders/ci/linux-perf/13312

Original change's description:
> Set Xlib error handler after GTK X11 initialization
>
> The default GTK error handler exits the process, which we don't want.
> We used to reset the error handler at some point after GTK
> initialization, but this became lost after r815319.
>
> Also refactor the XlibLoader instance to be global so it doesn't have
> to be created multiple times for each connection.
>
> R=​sky
>
> Bug: 1145929
> Change-Id: I73a3f27551534c2262bd8994d33e30fc2a0a4e27
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527743
> Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#825585}

TBR=sky@chromium.org,thomasanderson@chromium.org

Change-Id: Ie9f3614a8773220e0d6d6596d14c858a879a6704
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1145929
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2529152Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825761}

ui/gfx/x/connection.cc

@@ -189,7 +189,8 @@ XlibDisplayWrapper Connection::GetXlibDisplay(XlibDisplayType type) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!xlib_display_)
     xlib_display_ = base::WrapUnique(new XlibDisplay(display_string_));
-  return XlibDisplayWrapper(xlib_display_->display_, type);
+  return XlibDisplayWrapper(xlib_display_->xlib_loader_.get(),
+                            xlib_display_->display_, type);
 }
 
 Connection::Request::Request(unsigned int sequence,

ui/gfx/x/xlib_support.cc

@@ -7,7 +7,6 @@
 #include "base/check.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
-#include "base/no_destructor.h"
 #include "library_loaders/xlib_loader.h"
 
 namespace x11 {
@@ -19,56 +18,41 @@ int XlibErrorHandler(void*, void*) {
   return 0;
 }
 
-XlibLoader* GetXlibLoader() {
-  static base::NoDestructor<XlibLoader> xlib_loader;
-  return xlib_loader.get();
-}
-
 }  // namespace
 
-void InitXlib() {
-  auto* xlib_loader = GetXlibLoader();
-  if (xlib_loader->loaded())
-    return;
-
-  CHECK(xlib_loader->Load("libX11.so.6"));
+DISABLE_CFI_ICALL
+XlibDisplay::XlibDisplay(const std::string& address) {
+  xlib_loader_ = std::make_unique<XlibLoader>();
+  CHECK(xlib_loader_->Load("libX11.so.6"));
 
-  CHECK(xlib_loader->XInitThreads());
+  CHECK(xlib_loader_->XInitThreads());
 
   // The default Xlib error handler calls exit(1), which we don't want.  This
   // shouldn't happen in the browser process since only XProto requests are
   // made, but in the GPU process, GLX can make Xlib requests, so setting an
   // error handler is necessary.  Importantly, there's also an IO error handler,
   // and Xlib always calls exit(1) with no way to change this behavior.
-  SetXlibErrorHandler();
-}
-
-void SetXlibErrorHandler() {
-  GetXlibLoader()->XSetErrorHandler(XlibErrorHandler);
-}
-
-DISABLE_CFI_ICALL
-XlibDisplay::XlibDisplay(const std::string& address) {
-  InitXlib();
+  xlib_loader_->XSetErrorHandler(XlibErrorHandler);
 
-  display_ = GetXlibLoader()->XOpenDisplay(address.empty() ? nullptr
-                                                           : address.c_str());
+  display_ =
+      xlib_loader_->XOpenDisplay(address.empty() ? nullptr : address.c_str());
 }
 
 DISABLE_CFI_ICALL
 XlibDisplay::~XlibDisplay() {
   if (display_)
-    GetXlibLoader()->XCloseDisplay(display_);
+    xlib_loader_->XCloseDisplay(display_);
 }
 
 DISABLE_CFI_ICALL
-XlibDisplayWrapper::XlibDisplayWrapper(struct _XDisplay* display,
+XlibDisplayWrapper::XlibDisplayWrapper(XlibLoader* xlib_loader,
+                                       struct _XDisplay* display,
                                        XlibDisplayType type)
-    : display_(display), type_(type) {
+    : xlib_loader_(xlib_loader), display_(display), type_(type) {
   if (!display_)
     return;
   if (type == XlibDisplayType::kSyncing)
-    GetXlibLoader()->XSynchronize(display_, true);
+    xlib_loader_->XSynchronize(display_, true);
 }
 
 DISABLE_CFI_ICALL
@@ -76,21 +60,25 @@ XlibDisplayWrapper::~XlibDisplayWrapper() {
   if (!display_)
     return;
   if (type_ == XlibDisplayType::kFlushing)
-    GetXlibLoader()->XFlush(display_);
+    xlib_loader_->XFlush(display_);
   else if (type_ == XlibDisplayType::kSyncing)
-    GetXlibLoader()->XSynchronize(display_, false);
+    xlib_loader_->XSynchronize(display_, false);
 }
 
 XlibDisplayWrapper::XlibDisplayWrapper(XlibDisplayWrapper&& other) {
+  xlib_loader_ = other.xlib_loader_;
   display_ = other.display_;
   type_ = other.type_;
+  other.xlib_loader_ = nullptr;
   other.display_ = nullptr;
   other.type_ = XlibDisplayType::kNormal;
 }
 
 XlibDisplayWrapper& XlibDisplayWrapper::operator=(XlibDisplayWrapper&& other) {
+  xlib_loader_ = other.xlib_loader_;
   display_ = other.display_;
   type_ = other.type_;
+  other.xlib_loader_ = nullptr;
   other.display_ = nullptr;
   other.type_ = XlibDisplayType::kNormal;
   return *this;

ui/gfx/x/xlib_support.h

@@ -10,6 +10,7 @@
 #include "base/component_export.h"
 
 struct _XDisplay;
+class XlibLoader;
 
 namespace x11 {
 
@@ -25,12 +26,6 @@ enum class XlibDisplayType {
   kSyncing,
 };
 
-// Loads Xlib, initializes threads, and sets a default error handler.
-COMPONENT_EXPORT(X11) void InitXlib();
-
-// Sets an async error handler which only logs an error message.
-COMPONENT_EXPORT(X11) void SetXlibErrorHandler();
-
 // A scoped Xlib display.
 class COMPONENT_EXPORT(X11) XlibDisplay {
  public:
@@ -43,6 +38,7 @@ class COMPONENT_EXPORT(X11) XlibDisplay {
   explicit XlibDisplay(const std::string& address);
 
   struct _XDisplay* display_ = nullptr;
+  std::unique_ptr<XlibLoader> xlib_loader_;
 };
 
 // A temporary wrapper around an unowned Xlib display that adds behavior
@@ -60,10 +56,13 @@ class COMPONENT_EXPORT(X11) XlibDisplayWrapper {
   XlibDisplayWrapper& operator=(XlibDisplayWrapper&& other);
 
  private:
-  XlibDisplayWrapper(struct _XDisplay* display, XlibDisplayType type);
+  XlibDisplayWrapper(XlibLoader* xlib_loader,
+                     struct _XDisplay* display,
+                     XlibDisplayType type);
 
   friend class Connection;
 
+  XlibLoader* xlib_loader_;
   struct _XDisplay* display_;
   XlibDisplayType type_;
 };

ui/gtk/x/gtk_ui_delegate_x11.cc

@@ -10,7 +10,6 @@
 #include "ui/base/x/x11_util.h"
 #include "ui/events/platform/x11/x11_event_source.h"
 #include "ui/gfx/native_widget_types.h"
-#include "ui/gfx/x/xlib_support.h"
 #include "ui/gfx/x/xproto.h"
 #include "ui/gtk/x/gtk_event_loop_x11.h"
 #include "ui/platform_window/x11/x11_window.h"
@@ -32,7 +31,6 @@ GtkUiDelegateX11::GtkUiDelegateX11(x11::Connection* connection)
     : connection_(connection) {
   DCHECK(connection_);
   gdk_set_allowed_backends("x11");
-  x11::InitXlib();
 }
 
 GtkUiDelegateX11::~GtkUiDelegateX11() = default;
@@ -40,11 +38,6 @@ GtkUiDelegateX11::~GtkUiDelegateX11() = default;
 void GtkUiDelegateX11::OnInitialized() {
   // Ensure the singleton instance of GtkEventLoopX11 is created and started.
   GtkEventLoopX11::EnsureInstance();
-
-  // GTK sets an Xlib error handler that exits the process on any async errors.
-  // We don't want this behavior, so reset the error handler to something that
-  // just logs the error.
-  x11::SetXlibErrorHandler();
 }
 
 GdkKeymap* GtkUiDelegateX11::GetGdkKeymap() {