Introduce x11::Connection to replace XDisplay

x11::Connection::Get() should replace gfx::GetXDisplay().

R=sky
CC=​msisov,nickdiego
BUG=1066670

Change-Id: I5a2020c10f158f98697074fb9fec19c13ff6e644
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2168927
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#763465}

ui/base/x/x11_workspace_handler.cc

@@ -7,6 +7,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/events/x/x11_window_event_manager.h"
+#include "ui/gfx/x/connection.h"
 #include "ui/gfx/x/x11_atom_cache.h"
 #include "ui/gfx/x/xproto.h"
 
@@ -14,9 +15,10 @@ namespace ui {
 
 namespace {
 
-x11::Future<x11::XProto::GetPropertyReply> GetWorkspace(XDisplay* display) {
-  return x11::XProto{display}.GetProperty({
-      .window = XDefaultRootWindow(display),
+x11::Future<x11::XProto::GetPropertyReply> GetWorkspace() {
+  auto* connection = x11::Connection::Get();
+  return connection->GetProperty({
+      .window = XDefaultRootWindow(connection->display()),
       .property = gfx::GetAtom("_NET_CURRENT_DESKTOP"),
       .type = gfx::GetAtom("CARDINAL"),
       .long_length = 1,
@@ -44,7 +46,7 @@ X11WorkspaceHandler::~X11WorkspaceHandler() {
 
 std::string X11WorkspaceHandler::GetCurrentWorkspace() {
   if (workspace_.empty())
-    OnWorkspaceResponse(GetWorkspace(xdisplay_).Sync());
+    OnWorkspaceResponse(GetWorkspace().Sync());
   return workspace_;
 }
 
@@ -56,7 +58,7 @@ bool X11WorkspaceHandler::DispatchXEvent(XEvent* event) {
   switch (event->type) {
     case PropertyNotify: {
       if (event->xproperty.atom == gfx::GetAtom("_NET_CURRENT_DESKTOP")) {
-        GetWorkspace(xdisplay_).OnResponse(
+        GetWorkspace().OnResponse(
             base::BindOnce(&X11WorkspaceHandler::OnWorkspaceResponse,
                            weak_factory_.GetWeakPtr()));
       }

ui/gfx/x/BUILD.gn

@@ -93,6 +93,10 @@ component("xprotos") {
               "request_queue.cc",
               "xproto_util.h",
               "xproto_util.cc",
+              "connection.h",
+              "connection.cc",
+              "x11_switches.cc",
+              "x11_switches.h",
             ]
   configs += [
     ":x11_private_config",
@@ -112,8 +116,6 @@ jumbo_component("x") {
     "x11_error_tracker.h",
     "x11_path.cc",
     "x11_path.h",
-    "x11_switches.cc",
-    "x11_switches.h",
     "x11_types.cc",
     "x11_types.h",
   ]

ui/gfx/x/connection.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 "ui/gfx/x/connection.h"
+
+#include "base/command_line.h"
+#include "ui/gfx/x/x11_switches.h"
+
+namespace x11 {
+
+namespace {
+
+XDisplay* OpenNewXDisplay() {
+  if (!XInitThreads())
+    return nullptr;
+  std::string display_str =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kX11Display);
+  return XOpenDisplay(display_str.empty() ? nullptr : display_str.c_str());
+}
+
+}  // namespace
+
+Connection* Connection::Get() {
+  static Connection* instance = new Connection(OpenNewXDisplay());
+  return instance;
+}
+
+Connection::Connection(XDisplay* display) : XProto(display) {}
+
+}  // namespace x11

ui/gfx/x/connection.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_GFX_X_CONNECTION_H_
+#define UI_GFX_X_CONNECTION_H_
+
+#include "base/component_export.h"
+#include "ui/gfx/x/xproto.h"
+
+namespace x11 {
+
+// Represents a socket to the X11 server.
+class COMPONENT_EXPORT(X11) Connection : public XProto {
+ public:
+  // Gets or creates the singeton connection.
+  static Connection* Get();
+
+  Connection(const Connection&) = delete;
+  Connection(Connection&&) = delete;
+
+ private:
+  explicit Connection(XDisplay* display);
+};
+
+}  // namespace x11
+
+#endif  // UI_GFX_X_CONNECTION_H_

ui/gfx/x/gen_xproto.py

@@ -30,6 +30,8 @@
 #  public:
 #   explicit XProto(XDisplay* display);
 #
+#   XDisplay* display() { return display_; }
+#
 #   struct RGB {
 #     uint16_t red{};
 #     uint16_t green{};
@@ -682,6 +684,8 @@ class GenXproto:
             self.write('public:')
             self.write('explicit %s(XDisplay* display);' % name)
             self.write()
+            self.write('XDisplay* display() { return display_; }')
+            self.write()
             for (name, item) in self.module.all:
                 self.declare_type(item, name)
             self.write('private:')

ui/gfx/x/x11_switches.cc

@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "build/build_config.h"
 #include "ui/gfx/x/x11_switches.h"
 
 namespace switches {

ui/gfx/x/x11_switches.h

@@ -5,14 +5,13 @@
 #ifndef UI_GFX_X_X11_SWITCHES_H_
 #define UI_GFX_X_X11_SWITCHES_H_
 
-#include "build/build_config.h"
-#include "ui/gfx/gfx_export.h"
+#include "base/component_export.h"
 
 namespace switches {
 
-GFX_EXPORT extern const char kX11Display[];
+COMPONENT_EXPORT(X11) extern const char kX11Display[];
 
-GFX_EXPORT extern const char kNoXshm[];
+COMPONENT_EXPORT(X11) extern const char kNoXshm[];
 
 }  // namespace switches
 

ui/gfx/x/x11_types.cc

@@ -10,26 +10,13 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "build/build_config.h"
+#include "ui/gfx/x/connection.h"
 #include "ui/gfx/x/x11_switches.h"
 
 namespace gfx {
 
-namespace {
-
-XDisplay* OpenNewXDisplay() {
-  if (!XInitThreads())
-    return nullptr;
-  std::string display_str =
-      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-          switches::kX11Display);
-  return XOpenDisplay(display_str.empty() ? nullptr : display_str.c_str());
-}
-
-}  // namespace
-
 XDisplay* GetXDisplay() {
-  static XDisplay* display = OpenNewXDisplay();
-  return display;
+  return x11::Connection::Get()->display();
 }
 
 XDisplay* CloneXDisplay(XDisplay* display) {
@@ -90,10 +77,10 @@ void PutARGBImage(XDisplay* display,
 
   image.width = data_width;
   image.height = data_height;
-  image.format = ZPixmap;
-  image.byte_order = LSBFirst;
+  image.format = static_cast<int>(x11::XProto::ImageFormat::ZPixmap);
+  image.byte_order = static_cast<int>(x11::XProto::ImageOrder::LSBFirst);
   image.bitmap_unit = 8;
-  image.bitmap_bit_order = LSBFirst;
+  image.bitmap_bit_order = static_cast<int>(x11::XProto::ImageOrder::LSBFirst);
   image.depth = depth;
   image.bits_per_pixel = pixmap_bpp;
   image.bytes_per_line = data_width * pixmap_bpp / 8;

ui/gfx/x/x11_types.h

@@ -17,10 +17,10 @@ typedef unsigned long VisualID;
 typedef struct _XcursorImage XcursorImage;
 typedef union _XEvent XEvent;
 typedef struct _XImage XImage;
-typedef struct _XGC *GC;
+typedef struct _XGC* GC;
 typedef struct _XDisplay XDisplay;
 typedef struct _XRegion XRegion;
-typedef struct __GLXFBConfigRec *GLXFBConfig;
+typedef struct __GLXFBConfigRec* GLXFBConfig;
 typedef XID GLXWindow;
 typedef XID GLXDrawable;
 
@@ -38,9 +38,7 @@ struct XObjectDeleter {
 template <class T, class D = XObjectDeleter<void, int, XFree>>
 using XScopedPtr = std::unique_ptr<T, D>;
 
-// TODO(oshima|evan): This assume there is one display and doesn't work
-// under multiple displays/monitor environment. Remove this and change the
-// chrome codebase to get the display from window.
+// Get the XDisplay singleton.  Prefer x11::Connection::Get() instead.
 GFX_EXPORT XDisplay* GetXDisplay();
 
 // Given a connection to an X server, opens a new parallel connection to the

ui/gl/gl_surface_egl_x11.cc

@@ -6,6 +6,7 @@
 
 #include "base/threading/thread_task_runner_handle.h"
 #include "ui/base/x/x11_display_util.h"
+#include "ui/gfx/x/connection.h"
 #include "ui/gfx/x/x11.h"
 #include "ui/gfx/x/xproto.h"
 #include "ui/gl/egl_util.h"
@@ -55,8 +56,7 @@ bool NativeViewGLSurfaceEGLX11::Initialize(GLSurfaceFormat format) {
   // Query all child windows and store them. ANGLE creates a child window when
   // eglCreateWidnowSurface is called on X11 and expose events from this window
   // need to be received by this class.
-  x11::XProto conn{GetXNativeDisplay()};
-  if (auto reply = conn.QueryTree({window_}).Sync())
+  if (auto reply = x11::Connection::Get()->QueryTree({window_}).Sync())
     children_ = std::move(reply->children);
 
   if (ui::X11EventSource::HasInstance()) {