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

This reverts commit 0af7d7aa.

Reason for revert: Causing failures on multiple tests in
linux-trusty-rel: https://ci.chromium.org/p/chromium/builders/ci/linux-trusty-rel/12178

Example stack trace:

Received signal 11 SEGV_MAPERR 000000000008
#0 0x56517bc8acf9 base::debug::CollectStackTrace()
#1 0x56517bbf6703 base::debug::StackTrace::StackTrace()
#2 0x56517bc8a895 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#3 0x7f12826f7330 (/lib/x86_64-linux-gnu/libpthread-2.19.so+0x1032f)
#4 0x56517c963d2e x11::RandR::QueryVersion()
#5 0x56517ca4e18f ui::GetXrandrVersion()
#6 0x56517ca4d101 ui::XDisplayManager::XDisplayManager()
#7 0x56517d4af33b views::DesktopScreenX11::DesktopScreenX11()

From:

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8878654764661294768/+/steps/extensions_browsertests_on_Ubuntu-14.04/0/logs/Deterministic_failure:_BluetoothSocketApiTest.Listen__status_CRASH_/0

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}

TBR=jamiewalch@chromium.org,thomasanderson@chromium.org,msisov@igalia.com

Change-Id: Id94555856b3e06dcafc0f4c78a7a737718ddd37d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1066670
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225998Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773995}

ui/base/x/x11_display_manager.cc

@@ -9,7 +9,6 @@
 #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"
 
@@ -23,22 +22,21 @@ constexpr int kMinXrandrVersion = 103;  // Need at least xrandr version 1.3
 
 XDisplayManager::XDisplayManager(Delegate* delegate)
     : delegate_(delegate),
-      connection_(x11::Connection::Get()),
-      x_root_window_(connection_->default_screen()->root),
-      xrandr_version_(GetXrandrVersion()),
+      xdisplay_(gfx::GetXDisplay()),
+      x_root_window_(DefaultRootWindow(xdisplay_)),
+      xrandr_version_(GetXrandrVersion(xdisplay_)),
       workspace_handler_(this) {}
 
 XDisplayManager::~XDisplayManager() = default;
 
 void XDisplayManager::Init() {
   if (IsXrandrAvailable()) {
-    auto* randr = connection_->randr();
-    xrandr_event_base_ = randr->first_event();
+    int error_base_ignored = 0;
+    XRRQueryExtension(xdisplay_, &xrandr_event_base_, &error_base_ignored);
 
-    randr->SelectInput(
-        {x_root_window_, x11::RandR::NotifyMask::ScreenChange |
-                             x11::RandR::NotifyMask::OutputChange |
-                             x11::RandR::NotifyMask::CrtcChange});
+    XRRSelectInput(xdisplay_, x_root_window_,
+                   RRScreenChangeNotifyMask | RROutputChangeNotifyMask |
+                       RRCrtcChangeNotifyMask);
   }
   FetchDisplayList();
 }
@@ -62,11 +60,10 @@ void XDisplayManager::RemoveObserver(display::DisplayObserver* observer) {
 }
 
 bool XDisplayManager::CanProcessEvent(const XEvent& xev) {
-  return xev.type - xrandr_event_base_ ==
-             x11::RandR::ScreenChangeNotifyEvent::opcode ||
-         xev.type - xrandr_event_base_ == x11::RandR::NotifyEvent::opcode ||
+  return xev.type - xrandr_event_base_ == RRScreenChangeNotify ||
+         xev.type - xrandr_event_base_ == RRNotify ||
          (xev.type == PropertyNotify &&
-          static_cast<x11::Window>(xev.xproperty.window) == x_root_window_ &&
+          xev.xproperty.window == x_root_window_ &&
           xev.xproperty.atom ==
               static_cast<uint32_t>(gfx::GetAtom("_NET_WORKAREA")));
 }
@@ -74,12 +71,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 == x11::RandR::ScreenChangeNotifyEvent::opcode) {
+  if (ev_type == RRScreenChangeNotify) {
     // Pass the event through to xlib.
     XRRUpdateConfiguration(xev);
     return true;
   }
-  if (ev_type == x11::RandR::NotifyEvent::opcode ||
+  if (ev_type == RRNotify ||
       (xev->type == PropertyNotify &&
        xev->xproperty.atom ==
            static_cast<uint32_t>(gfx::GetAtom("_NET_WORKAREA")))) {
@@ -128,9 +125,13 @@ void XDisplayManager::DispatchDelayedDisplayListUpdate() {
 }
 
 gfx::Point XDisplayManager::GetCursorLocation() const {
-  if (auto response = connection_->QueryPointer({x_root_window_}).Sync())
-    return {response->root_x, response->root_y};
-  return {};
+  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);
 }
 
 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_;
 
-  x11::Connection* const connection_;
-  x11::Window x_root_window_;
+  XDisplay* const xdisplay_;
+  XID 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,8 +28,6 @@ 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) {
@@ -140,7 +138,7 @@ std::vector<uint8_t> GetEDIDProperty(x11::RandR* randr,
                                      x11::RandR::Output output) {
   auto future = randr->GetOutputProperty({
       .output = output,
-      .property = gfx::GetAtom(kRandrEdidProperty),
+      .property = gfx::GetAtom(RR_PROPERTY_RANDR_EDID),
       .long_length = 128,
   });
   auto response = future.Sync();
@@ -152,17 +150,16 @@ std::vector<uint8_t> GetEDIDProperty(x11::RandR* randr,
 
 }  // namespace
 
-int GetXrandrVersion() {
-  auto impl = []() -> int {
-    auto* randr = x11::Connection::Get()->randr();
-    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;
+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;
 }
 
 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();
+COMPONENT_EXPORT(UI_BASE_X) int GetXrandrVersion(XDisplay* xdisplay);
 
 // Builds a list of displays for fallback.
 COMPONENT_EXPORT(UI_BASE_X)

ui/gfx/x/connection.cc

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

ui/gfx/x/connection.h

@@ -29,9 +29,6 @@ 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;
 
@@ -42,10 +39,12 @@ class COMPONENT_EXPORT(X11) Connection : public XProto,
     return extended_max_request_length_;
   }
 
-  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_; }
+  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_;
+  }
 
   void Dispatch(Delegate* delegate);
 
@@ -61,16 +60,19 @@ 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<Setup> setup_;
-  const Screen* default_screen_ = nullptr;
-  const Depth* default_root_depth_ = nullptr;
-  const VisualType* defualt_root_visual_ = nullptr;
+  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::queue<Request> requests_;
 };

ui/gfx/x/gen_xproto.py

@@ -1100,7 +1100,6 @@ 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)
@@ -1111,19 +1110,8 @@ class GenXproto(FileWriter):
             self.namespace = ['x11', self.class_name]
             self.write('public:')
             if self.module.namespace.is_ext:
-                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_;')
+                self.write(name +
+                           '(Connection* connection, uint8_t major_opcode);')
             else:
                 self.write('explicit %s(Connection* connection);' % name)
             self.write()
@@ -1133,14 +1121,13 @@ class GenXproto(FileWriter):
             for (name, item) in self.module.all:
                 if self.module.namespace.is_ext:
                     self.declare_type(item, name)
-                elif isinstance(item, self.xcbgen.xtypes.Request):
-                    self.declare_request(item)
+                else:
+                    if 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('const uint8_t major_opcode_;')
-                self.write('const uint8_t first_event_;')
-                self.write('const uint8_t first_error_;')
+                self.write('uint8_t major_opcode_;')
 
         self.write()
         self.write('}  // namespace x11')
@@ -1171,11 +1158,9 @@ 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('    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) {}')
+            self.write(ctor + '(x11::Connection* connection, uint8_t opcode)')
+            self.write(
+                '    : connection_(connection), major_opcode_(opcode) {}')
         else:
             self.write(ctor +
                        '(Connection* connection) : connection_(connection) {}')
@@ -1236,7 +1221,7 @@ class GenExtensionManager(FileWriter):
                            (extension.class_name, name, name))
             self.write()
             self.write('protected:')
-            self.write('void Init(Connection* conn);')
+            self.write('void Init(XProto* xproto);')
             self.write()
             self.write('private:')
             for extension in self.extensions:
@@ -1252,32 +1237,28 @@ 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 + '(Connection* conn) {', '}'):
-            self.write('if (!conn)')
+        with Indent(self, init + '(XProto* xproto) {', '}'):
+            self.write('Connection* connection = xproto->connection();')
+            self.write('if (!connection)')
             self.write('  return;')
-            self.write()
             for extension in self.extensions:
                 self.write(
-                    'auto %s_future = conn->QueryExtension({"%s"});' %
+                    'auto %s_future = xproto->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))
-                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('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))
         self.write()
         self.write('ExtensionManager::ExtensionManager() = default;')
         self.write('ExtensionManager::~ExtensionManager() = default;')