Reland "Set Xlib error handler after GTK X11 initialization"

This is a reland of 2ed82a41

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}

Bug: 1145929
Change-Id: Ib65f3add91181b3bf82dd71f7b9b87a9a681a53d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2533199
Auto-Submit: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826572}

ui/gfx/x/connection.cc

@@ -189,8 +189,7 @@ 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_->xlib_loader_.get(),
-                            xlib_display_->display_, type);
+  return XlibDisplayWrapper(xlib_display_->display_, type);
 }
 
 Connection::Request::Request(unsigned int sequence,

ui/gfx/x/xlib_support.cc

@@ -7,6 +7,7 @@
 #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 {
@@ -18,41 +19,58 @@ int XlibErrorHandler(void*, void*) {
   return 0;
 }
 
+XlibLoader* GetXlibLoader() {
+  static base::NoDestructor<XlibLoader> xlib_loader;
+  return xlib_loader.get();
+}
+
 }  // namespace
 
 DISABLE_CFI_ICALL
-XlibDisplay::XlibDisplay(const std::string& address) {
-  xlib_loader_ = std::make_unique<XlibLoader>();
-  CHECK(xlib_loader_->Load("libX11.so.6"));
+void InitXlib() {
+  auto* xlib_loader = GetXlibLoader();
+  if (xlib_loader->loaded())
+    return;
+
+  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.
-  xlib_loader_->XSetErrorHandler(XlibErrorHandler);
+  SetXlibErrorHandler();
+}
+
+DISABLE_CFI_ICALL
+void SetXlibErrorHandler() {
+  GetXlibLoader()->XSetErrorHandler(XlibErrorHandler);
+}
+
+DISABLE_CFI_ICALL
+XlibDisplay::XlibDisplay(const std::string& address) {
+  InitXlib();
 
-  display_ =
-      xlib_loader_->XOpenDisplay(address.empty() ? nullptr : address.c_str());
+  display_ = GetXlibLoader()->XOpenDisplay(address.empty() ? nullptr
+                                                           : address.c_str());
 }
 
 DISABLE_CFI_ICALL
 XlibDisplay::~XlibDisplay() {
   if (display_)
-    xlib_loader_->XCloseDisplay(display_);
+    GetXlibLoader()->XCloseDisplay(display_);
 }
 
 DISABLE_CFI_ICALL
-XlibDisplayWrapper::XlibDisplayWrapper(XlibLoader* xlib_loader,
-                                       struct _XDisplay* display,
+XlibDisplayWrapper::XlibDisplayWrapper(struct _XDisplay* display,
                                        XlibDisplayType type)
-    : xlib_loader_(xlib_loader), display_(display), type_(type) {
+    : display_(display), type_(type) {
   if (!display_)
     return;
   if (type == XlibDisplayType::kSyncing)
-    xlib_loader_->XSynchronize(display_, true);
+    GetXlibLoader()->XSynchronize(display_, true);
 }
 
 DISABLE_CFI_ICALL
@@ -60,25 +78,21 @@ XlibDisplayWrapper::~XlibDisplayWrapper() {
   if (!display_)
     return;
   if (type_ == XlibDisplayType::kFlushing)
-    xlib_loader_->XFlush(display_);
+    GetXlibLoader()->XFlush(display_);
   else if (type_ == XlibDisplayType::kSyncing)
-    xlib_loader_->XSynchronize(display_, false);
+    GetXlibLoader()->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,7 +10,6 @@
 #include "base/component_export.h"
 
 struct _XDisplay;
-class XlibLoader;
 
 namespace x11 {
 
@@ -26,6 +25,12 @@ 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:
@@ -38,7 +43,6 @@ 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
@@ -56,13 +60,10 @@ class COMPONENT_EXPORT(X11) XlibDisplayWrapper {
   XlibDisplayWrapper& operator=(XlibDisplayWrapper&& other);
 
  private:
-  XlibDisplayWrapper(XlibLoader* xlib_loader,
-                     struct _XDisplay* display,
-                     XlibDisplayType type);
+  XlibDisplayWrapper(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,6 +10,7 @@
 #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"
@@ -31,6 +32,7 @@ GtkUiDelegateX11::GtkUiDelegateX11(x11::Connection* connection)
     : connection_(connection) {
   DCHECK(connection_);
   gdk_set_allowed_backends("x11");
+  x11::InitXlib();
 }
 
 GtkUiDelegateX11::~GtkUiDelegateX11() = default;
@@ -38,6 +40,11 @@ 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() {