Reland "[XProto] Remove all remaining non-event usages of XRandR"

This is a reland of 0af7d7aa

Original change's description:
> [XProto] Remove all remaining non-event usages of XRandR
>
> BUG=1066670
> R=msisov
> CC=sky
>
> Change-Id: Iecd593c0c77878a80bf84e9e3c504fd90815db6c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2220686
> Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
> Reviewed-by: Maksim Sisov <msisov@igalia.com>
> Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#773876}

Bug: 1066670
R=msisov
TBR=jamiewalch

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

ui/base/x/x11_display_manager.cc

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "ui/base/x/x11_display_util.h"
+#include "ui/gfx/x/randr.h"
 #include "ui/gfx/x/x11.h"
 #include "ui/gfx/x/x11_atom_cache.h"
 
@@ -22,21 +23,22 @@ constexpr int kMinXrandrVersion = 103;  // Need at least xrandr version 1.3
 
 XDisplayManager::XDisplayManager(Delegate* delegate)
     : delegate_(delegate),
-      xdisplay_(gfx::GetXDisplay()),
-      x_root_window_(DefaultRootWindow(xdisplay_)),
-      xrandr_version_(GetXrandrVersion(xdisplay_)),
+      connection_(x11::Connection::Get()),
+      x_root_window_(connection_->default_screen()->root),
+      xrandr_version_(GetXrandrVersion()),
       workspace_handler_(this) {}
 
 XDisplayManager::~XDisplayManager() = default;
 
 void XDisplayManager::Init() {
   if (IsXrandrAvailable()) {
-    int error_base_ignored = 0;
-    XRRQueryExtension(xdisplay_, &xrandr_event_base_, &error_base_ignored);
+    auto* randr = connection_->randr();
+    xrandr_event_base_ = randr->first_event();
 
-    XRRSelectInput(xdisplay_, x_root_window_,
-                   RRScreenChangeNotifyMask | RROutputChangeNotifyMask |
-                       RRCrtcChangeNotifyMask);
+    randr->SelectInput(
+        {x_root_window_, x11::RandR::NotifyMask::ScreenChange |
+                             x11::RandR::NotifyMask::OutputChange |
+                             x11::RandR::NotifyMask::CrtcChange});
   }
   FetchDisplayList();
 }
@@ -60,10 +62,11 @@ void XDisplayManager::RemoveObserver(display::DisplayObserver* observer) {
 }
 
 bool XDisplayManager::CanProcessEvent(const XEvent& xev) {
-  return xev.type - xrandr_event_base_ == RRScreenChangeNotify ||
-         xev.type - xrandr_event_base_ == RRNotify ||
+  return xev.type - xrandr_event_base_ ==
+             x11::RandR::ScreenChangeNotifyEvent::opcode ||
+         xev.type - xrandr_event_base_ == x11::RandR::NotifyEvent::opcode ||
          (xev.type == PropertyNotify &&
-          xev.xproperty.window == x_root_window_ &&
+          static_cast<x11::Window>(xev.xproperty.window) == x_root_window_ &&
           xev.xproperty.atom ==
               static_cast<uint32_t>(gfx::GetAtom("_NET_WORKAREA")));
 }
@@ -71,12 +74,12 @@ bool XDisplayManager::CanProcessEvent(const XEvent& xev) {
 bool XDisplayManager::ProcessEvent(XEvent* xev) {
   DCHECK(xev);
   int ev_type = xev->type - xrandr_event_base_;
-  if (ev_type == RRScreenChangeNotify) {
+  if (ev_type == x11::RandR::ScreenChangeNotifyEvent::opcode) {
     // Pass the event through to xlib.
     XRRUpdateConfiguration(xev);
     return true;
   }
-  if (ev_type == RRNotify ||
+  if (ev_type == x11::RandR::NotifyEvent::opcode ||
       (xev->type == PropertyNotify &&
        xev->xproperty.atom ==
            static_cast<uint32_t>(gfx::GetAtom("_NET_WORKAREA")))) {
@@ -125,13 +128,9 @@ void XDisplayManager::DispatchDelayedDisplayListUpdate() {
 }
 
 gfx::Point XDisplayManager::GetCursorLocation() const {
-  XID root, child;
-  int root_x, root_y, win_x, win_y;
-  unsigned int mask;
-  XQueryPointer(xdisplay_, x_root_window_, &root, &child, &root_x, &root_y,
-                &win_x, &win_y, &mask);
-
-  return gfx::Point(root_x, root_y);
+  if (auto response = connection_->QueryPointer({x_root_window_}).Sync())
+    return {response->root_x, response->root_y};
+  return {};
 }
 
 std::string XDisplayManager::GetCurrentWorkspace() {

ui/base/x/x11_display_manager.h

@@ -76,8 +76,8 @@ class COMPONENT_EXPORT(UI_BASE_X) XDisplayManager
   std::vector<display::Display> displays_;
   display::DisplayChangeNotifier change_notifier_;
 
-  XDisplay* const xdisplay_;
-  XID x_root_window_;
+  x11::Connection* const connection_;
+  x11::Window x_root_window_;
   int64_t primary_display_index_ = 0;
 
   // XRandR version. MAJOR * 100 + MINOR. Zero if no xrandr is present.

ui/base/x/x11_display_util.cc

@@ -28,6 +28,8 @@ namespace {
 
 constexpr int kMinVersionXrandr = 103;  // Need at least xrandr version 1.3.
 
+constexpr const char kRandrEdidProperty[] = "EDID";
+
 std::map<x11::RandR::Output, int> GetMonitors(int version,
                                               x11::RandR* randr,
                                               x11::Window window) {
@@ -138,7 +140,7 @@ std::vector<uint8_t> GetEDIDProperty(x11::RandR* randr,
                                      x11::RandR::Output output) {
   auto future = randr->GetOutputProperty({
       .output = output,
-      .property = gfx::GetAtom(RR_PROPERTY_RANDR_EDID),
+      .property = gfx::GetAtom(kRandrEdidProperty),
       .long_length = 128,
   });
   auto response = future.Sync();
@@ -150,16 +152,19 @@ std::vector<uint8_t> GetEDIDProperty(x11::RandR* randr,
 
 }  // namespace
 
-int GetXrandrVersion(XDisplay* xdisplay) {
-  int xrandr_version = 0;
-  // We only support 1.3+. There were library changes before this and we should
-  // use the new interface instead of the 1.2 one.
-  int randr_version_major = 0;
-  int randr_version_minor = 0;
-  if (XRRQueryVersion(xdisplay, &randr_version_major, &randr_version_minor)) {
-    xrandr_version = randr_version_major * 100 + randr_version_minor;
-  }
-  return xrandr_version;
+int GetXrandrVersion() {
+  auto impl = []() -> int {
+    auto* randr = x11::Connection::Get()->randr();
+    if (!randr)
+      return 0;
+    auto future = randr->QueryVersion(
+        {x11::RandR::major_version, x11::RandR::minor_version});
+    if (auto response = future.Sync())
+      return response->major_version * 100 + response->minor_version;
+    return 0;
+  };
+  static int version = impl();
+  return version;
 }
 
 std::vector<display::Display> GetFallbackDisplayList(float scale) {

ui/base/x/x11_display_util.h

@@ -16,7 +16,7 @@ namespace ui {
 // Return the version for xrandr. It multiplies the major number by 100 and
 // adds the minor like MAJOR * 100 + MINOR. It returns zero if no xrandr is
 // present.
-COMPONENT_EXPORT(UI_BASE_X) int GetXrandrVersion(XDisplay* xdisplay);
+COMPONENT_EXPORT(UI_BASE_X) int GetXrandrVersion();
 
 // Builds a list of displays for fallback.
 COMPONENT_EXPORT(UI_BASE_X)

ui/gfx/x/connection.cc

@@ -5,6 +5,7 @@
 #include "ui/gfx/x/connection.h"
 
 #include <X11/Xlib-xcb.h>
+#include <X11/Xlib.h>
 #include <xcb/xcb.h>
 
 #include <algorithm>
@@ -51,25 +52,25 @@ XDisplay* OpenNewXDisplay() {
 }  // namespace
 
 Connection* Connection::Get() {
-  static Connection* instance = new Connection(OpenNewXDisplay());
+  static Connection* instance = new Connection;
   return instance;
 }
 
-Connection::Connection(XDisplay* display) : XProto(this), display_(display) {
+Connection::Connection() : XProto(this), display_(OpenNewXDisplay()) {
   if (!display_)
     return;
 
-  setup_ = std::make_unique<x11::Setup>(x11::Read<x11::Setup>(
+  setup_ = std::make_unique<Setup>(Read<Setup>(
       reinterpret_cast<const uint8_t*>(xcb_get_setup(XcbConnection()))));
-  default_screen_ = &setup_->roots[DefaultScreen(display)];
+  default_screen_ = &setup_->roots[DefaultScreen(display_)];
   default_root_depth_ = &*std::find_if(
       default_screen_->allowed_depths.begin(),
-      default_screen_->allowed_depths.end(), [&](const x11::Depth& depth) {
+      default_screen_->allowed_depths.end(), [&](const Depth& depth) {
         return depth.depth == default_screen_->root_depth;
       });
   defualt_root_visual_ = &*std::find_if(
       default_root_depth_->visuals.begin(), default_root_depth_->visuals.end(),
-      [&](const x11::VisualType visual) {
+      [&](const VisualType visual) {
         return visual.visual_id == default_screen_->root_visual;
       });
 
@@ -80,7 +81,10 @@ Connection::Connection(XDisplay* display) : XProto(this), display_(display) {
   }
 }
 
-Connection::~Connection() = default;
+Connection::~Connection() {
+  if (display_)
+    XCloseDisplay(display_);
+}
 
 xcb_connection_t* Connection::XcbConnection() {
   if (!display())

ui/gfx/x/connection.h

@@ -29,6 +29,9 @@ class COMPONENT_EXPORT(X11) Connection : public XProto,
   // Gets or creates the singeton connection.
   static Connection* Get();
 
+  explicit Connection();
+  ~Connection();
+
   Connection(const Connection&) = delete;
   Connection(Connection&&) = delete;
 
@@ -39,12 +42,10 @@ class COMPONENT_EXPORT(X11) Connection : public XProto,
     return extended_max_request_length_;
   }
 
-  const x11::Setup* setup() const { return setup_.get(); }
-  const x11::Screen* default_screen() const { return default_screen_; }
-  const x11::Depth* default_root_depth() const { return default_root_depth_; }
-  const x11::VisualType* default_root_visual() const {
-    return defualt_root_visual_;
-  }
+  const Setup* setup() const { return setup_.get(); }
+  const Screen* default_screen() const { return default_screen_; }
+  const Depth* default_root_depth() const { return default_root_depth_; }
+  const VisualType* default_root_visual() const { return defualt_root_visual_; }
 
   void Dispatch(Delegate* delegate);
 
@@ -60,19 +61,16 @@ class COMPONENT_EXPORT(X11) Connection : public XProto,
     FutureBase::ResponseCallback callback;
   };
 
-  explicit Connection(XDisplay* display);
-  ~Connection();
-
   void AddRequest(unsigned int sequence, FutureBase::ResponseCallback callback);
 
   XDisplay* const display_;
 
   uint32_t extended_max_request_length_ = 0;
 
-  std::unique_ptr<x11::Setup> setup_;
-  const x11::Screen* default_screen_ = nullptr;
-  const x11::Depth* default_root_depth_ = nullptr;
-  const x11::VisualType* defualt_root_visual_ = nullptr;
+  std::unique_ptr<Setup> setup_;
+  const Screen* default_screen_ = nullptr;
+  const Depth* default_root_depth_ = nullptr;
+  const VisualType* defualt_root_visual_ = nullptr;
 
   std::queue<Request> requests_;
 };

ui/gfx/x/gen_xproto.py

@@ -1100,6 +1100,7 @@ class GenXproto(FileWriter):
         self.write('class Connection;')
         self.write()
 
+        self.namespace = ['x11']
         if not self.module.namespace.is_ext:
             for (name, item) in self.module.all:
                 self.declare_type(item, name)
@@ -1110,8 +1111,19 @@ class GenXproto(FileWriter):
             self.namespace = ['x11', self.class_name]
             self.write('public:')
             if self.module.namespace.is_ext:
-                self.write(name +
-                           '(Connection* connection, uint8_t major_opcode);')
+                self.write('static constexpr unsigned major_version = %s;' %
+                           self.module.namespace.major_version)
+                self.write('static constexpr unsigned minor_version = %s;' %
+                           self.module.namespace.minor_version)
+                self.write()
+                self.write(name + '(')
+                self.write('    Connection* connection, uint8_t major_opcode,')
+                self.write('    uint8_t first_event, uint8_t first_error);')
+                self.write()
+                with Indent(self, 'uint8_t first_event() const {', '}'):
+                    self.write('return first_event_;')
+                with Indent(self, 'uint8_t first_error() const {', '}'):
+                    self.write('return first_error_;')
             else:
                 self.write('explicit %s(Connection* connection);' % name)
             self.write()
@@ -1121,13 +1133,14 @@ class GenXproto(FileWriter):
             for (name, item) in self.module.all:
                 if self.module.namespace.is_ext:
                     self.declare_type(item, name)
-                else:
-                    if isinstance(item, self.xcbgen.xtypes.Request):
-                        self.declare_request(item)
+                elif isinstance(item, self.xcbgen.xtypes.Request):
+                    self.declare_request(item)
             self.write('private:')
             self.write('x11::Connection* const connection_;')
             if self.module.namespace.is_ext:
-                self.write('uint8_t major_opcode_;')
+                self.write('const uint8_t major_opcode_;')
+                self.write('const uint8_t first_event_;')
+                self.write('const uint8_t first_error_;')
 
         self.write()
         self.write('}  // namespace x11')
@@ -1158,9 +1171,11 @@ class GenXproto(FileWriter):
         self.write()
         ctor = '%s::%s' % (self.class_name, self.class_name)
         if self.module.namespace.is_ext:
-            self.write(ctor + '(x11::Connection* connection, uint8_t opcode)')
-            self.write(
-                '    : connection_(connection), major_opcode_(opcode) {}')
+            self.write(ctor + '(x11::Connection* connection, uint8_t opcode,')
+            self.write('    uint8_t first_event, uint8_t first_error)')
+            self.write('    : connection_(connection), major_opcode_(opcode),')
+            self.write('      first_event_(first_event),')
+            self.write('      first_error_(first_error) {}')
         else:
             self.write(ctor +
                        '(Connection* connection) : connection_(connection) {}')
@@ -1221,7 +1236,7 @@ class GenExtensionManager(FileWriter):
                            (extension.class_name, name, name))
             self.write()
             self.write('protected:')
-            self.write('void Init(XProto* xproto);')
+            self.write('void Init(Connection* conn);')
             self.write()
             self.write('private:')
             for extension in self.extensions:
@@ -1237,28 +1252,32 @@ class GenExtensionManager(FileWriter):
                          'w')
         self.write('#include "ui/gfx/x/extension_manager.h"')
         self.write()
+        self.write('#include "ui/gfx/x/connection.h"')
         for genproto in self.genprotos:
             self.write('#include "ui/gfx/x/%s.h"' % genproto.proto)
         self.write()
         self.write('namespace x11 {')
         self.write()
         init = 'void ExtensionManager::Init'
-        with Indent(self, init + '(XProto* xproto) {', '}'):
-            self.write('Connection* connection = xproto->connection();')
-            self.write('if (!connection)')
+        with Indent(self, init + '(Connection* conn) {', '}'):
+            self.write('if (!conn)')
             self.write('  return;')
+            self.write()
             for extension in self.extensions:
                 self.write(
-                    'auto %s_future = xproto->QueryExtension({"%s"});' %
+                    'auto %s_future = conn->QueryExtension({"%s"});' %
                     (extension.proto, extension.module.namespace.ext_xname))
             self.write()
             for extension in self.extensions:
                 name = extension.proto
                 self.write('auto {0}_reply = {0}_future.Sync();'.format(name))
-                self.write('if ({0}_reply && {0}_reply->present)'.format(name))
-                args = 'connection, %s_reply->major_opcode' % name
-                self.write('  %s_ = std::make_unique<%s>(%s);' %
-                           (name, extension.class_name, args))
+                cond = 'if ({0}_reply && {0}_reply->present) {{'.format(name)
+                with Indent(self, cond, '}'):
+                    self.write('auto* reply = %s_reply.reply.get();' % name)
+                    self.write('  %s_ = std::make_unique<%s>(' %
+                               (name, extension.class_name))
+                    self.write('    conn, reply->major_opcode,')
+                    self.write('    reply->first_event, reply->first_error);')
         self.write()
         self.write('ExtensionManager::ExtensionManager() = default;')
         self.write('ExtensionManager::~ExtensionManager() = default;')