ozone/x11: Handle XRandR events in X11ScreenOzone

This CL adds event handling for XRandR |XEvent|s, re-fetching and updating
display list dynamically and communicating |DisplayObserver|s about the
changes.

No unit tests are added due to the current dependency on XRandR/Xlib API.

Bug: 891175
Test: None
Change-Id: I96c5fe83fc4f1792503b34b0676b778b601d42d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1709491
Commit-Queue: Nick Yamane <nickdiego@igalia.com>
Reviewed-by: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680591}

ui/ozone/platform/x11/ozone_platform_x11.cc

@@ -96,7 +96,9 @@ class OzonePlatformX11 : public OzonePlatform {
 
   std::unique_ptr<PlatformScreen> CreateScreen() override {
     DCHECK(window_manager_);
-    return std::make_unique<X11ScreenOzone>(window_manager_.get());
+    auto screen = std::make_unique<X11ScreenOzone>(window_manager_.get());
+    screen->Init();
+    return screen;
   }
 
   PlatformClipboard* GetPlatformClipboard() override {

ui/ozone/platform/x11/x11_screen_ozone.cc

@@ -4,16 +4,18 @@
 
 #include "ui/ozone/platform/x11/x11_screen_ozone.h"
 
+#include "base/bind.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "ui/base/x/x11_display_util.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/display/display_finder.h"
 #include "ui/display/util/display_util.h"
 #include "ui/display/util/x11/edid_parser_x11.h"
-#include "ui/events/platform/platform_event_source.h"
 #include "ui/events/platform/x11/x11_event_source.h"
 #include "ui/gfx/font_render_params.h"
 #include "ui/gfx/geometry/dip_util.h"
 #include "ui/gfx/x/x11.h"
+#include "ui/gfx/x/x11_atom_cache.h"
 #include "ui/ozone/platform/x11/x11_window_manager_ozone.h"
 #include "ui/ozone/platform/x11/x11_window_ozone.h"
 
@@ -23,6 +25,8 @@ namespace {
 
 constexpr int kMinVersionXrandr = 103;  // Need at least xrandr version 1.3.
 
+constexpr auto kDisplayListUpdateDelay = base::TimeDelta::FromMilliseconds(250);
+
 float GetDeviceScaleFactor() {
   float device_scale_factor = 1.0f;
   // TODO(crbug.com/891175): Implement PlatformScreen for X11
@@ -105,32 +109,44 @@ bool LocalProcessWindowFinder::MatchWindow(X11WindowOzone* window) const {
 
 }  // namespace
 
-X11ScreenOzone::X11ScreenOzone(X11WindowManagerOzone* wm, bool fetch)
-    : window_manager_(wm),
+X11ScreenOzone::X11ScreenOzone(X11WindowManagerOzone* window_manager)
+    : window_manager_(window_manager),
       xdisplay_(gfx::GetXDisplay()),
       x_root_window_(DefaultRootWindow(xdisplay_)),
       xrandr_version_(GetXrandrVersion(xdisplay_)) {
   DCHECK(window_manager_);
-
-  // TODO(nickdiego): Factor this out from ctor
-  if (fetch)
-    FetchDisplayList();
 }
 
 X11ScreenOzone::~X11ScreenOzone() {
   if (xrandr_version_ >= kMinVersionXrandr &&
-      PlatformEventSource::GetInstance()) {
-    PlatformEventSource::GetInstance()->RemovePlatformEventDispatcher(this);
+      X11EventSourceLibevent::GetInstance()) {
+    X11EventSourceLibevent::GetInstance()->RemoveXEventDispatcher(this);
+  }
+}
+
+void X11ScreenOzone::Init() {
+  // Need at least xrandr version 1.3.
+  if (xrandr_version_ >= kMinVersionXrandr) {
+    int error_base_ignored = 0;
+    XRRQueryExtension(xdisplay_, &xrandr_event_base_, &error_base_ignored);
+
+    DCHECK(X11EventSourceLibevent::GetInstance());
+    X11EventSourceLibevent::GetInstance()->AddXEventDispatcher(this);
+
+    XRRSelectInput(xdisplay_, x_root_window_,
+                   RRScreenChangeNotifyMask | RROutputChangeNotifyMask |
+                       RRCrtcChangeNotifyMask);
   }
+  FetchDisplayList();
 }
 
 const std::vector<display::Display>& X11ScreenOzone::GetAllDisplays() const {
-  return display_list_.displays();
+  return displays_;
 }
 
 display::Display X11ScreenOzone::GetPrimaryDisplay() const {
-  auto iter = display_list_.GetPrimaryDisplayIterator();
-  if (iter == display_list_.displays().end())
+  auto iter = displays_.begin();
+  if (iter == displays_.end())
     return display::Display::GetDefaultDisplay();
   return *iter;
 }
@@ -174,74 +190,72 @@ display::Display X11ScreenOzone::GetDisplayMatching(
     const gfx::Rect& match_rect) const {
   const display::Display* matching_display =
       display::FindDisplayWithBiggestIntersection(
-          display_list_.displays(),
-          gfx::ConvertRectToDIP(GetDeviceScaleFactor(), match_rect));
+          displays_, gfx::ConvertRectToDIP(GetDeviceScaleFactor(), match_rect));
   return matching_display ? *matching_display : GetPrimaryDisplay();
 }
 
 void X11ScreenOzone::AddObserver(display::DisplayObserver* observer) {
-  display_list_.AddObserver(observer);
+  change_notifier_.AddObserver(observer);
 }
 
 void X11ScreenOzone::RemoveObserver(display::DisplayObserver* observer) {
-  display_list_.RemoveObserver(observer);
+  change_notifier_.RemoveObserver(observer);
 }
 
-bool X11ScreenOzone::CanDispatchEvent(const ui::PlatformEvent& event) {
-  // TODO(crbug.com/891175): Implement PlatformScreen for X11
-  NOTIMPLEMENTED_LOG_ONCE();
+// TODO(nickdiego): Factor event dispatching and display fetching so that it
+// can be shared between ozone and non-ozone code paths.
+bool X11ScreenOzone::DispatchXEvent(XEvent* xev) {
+  DCHECK(xev);
+  int ev_type = xev->type - xrandr_event_base_;
+  if (ev_type == RRScreenChangeNotify) {
+    // Pass the event through to xlib.
+    XRRUpdateConfiguration(xev);
+    return true;
+  }
+  if (ev_type == RRNotify ||
+      (xev->type == PropertyNotify &&
+       xev->xproperty.atom == gfx::GetAtom("_NET_WORKAREA"))) {
+    RestartDelayedUpdateDisplayListTask();
+    return true;
+  }
   return false;
 }
 
-uint32_t X11ScreenOzone::DispatchEvent(const ui::PlatformEvent& event) {
-  // TODO(crbug.com/891175): Implement PlatformScreen for X11
-  NOTIMPLEMENTED_LOG_ONCE();
-  return ui::POST_DISPATCH_NONE;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // X11ScreenOzone, private:
 
-void X11ScreenOzone::AddDisplay(const display::Display& display,
-                                bool is_primary) {
-  display_list_.AddDisplay(
-      display, is_primary ? display::DisplayList::Type::PRIMARY
-                          : display::DisplayList::Type::NOT_PRIMARY);
-
-  if (is_primary) {
-    gfx::SetFontRenderParamsDeviceScaleFactor(
-        GetPrimaryDisplay().device_scale_factor());
-  }
-}
-
-void X11ScreenOzone::RemoveDisplay(const display::Display& display) {
-  display_list_.RemoveDisplay(display.id());
+void X11ScreenOzone::SetDisplayList(std::vector<display::Display> displays) {
+  displays_ = std::move(displays);
+  gfx::SetFontRenderParamsDeviceScaleFactor(
+      GetPrimaryDisplay().device_scale_factor());
 }
 
 // Talks to xrandr to get the information of the outputs for a screen and
 // updates display::Display list. The minimum required version of xrandr is
 // 1.3.
 void X11ScreenOzone::FetchDisplayList() {
-  float scale = GetDeviceScaleFactor();
   std::vector<display::Display> displays;
-  // Need at least xrandr version 1.3.
+  float scale = GetDeviceScaleFactor();
   if (xrandr_version_ >= kMinVersionXrandr) {
-    int error_base_ignored = 0;
-    XRRQueryExtension(xdisplay_, &xrandr_event_base_, &error_base_ignored);
-
-    if (PlatformEventSource::GetInstance())
-      PlatformEventSource::GetInstance()->AddPlatformEventDispatcher(this);
-    XRRSelectInput(xdisplay_, x_root_window_,
-                   RRScreenChangeNotifyMask | RROutputChangeNotifyMask |
-                       RRCrtcChangeNotifyMask);
-
     displays = BuildDisplaysFromXRandRInfo(xrandr_version_, scale,
                                            &primary_display_index_);
   } else {
     displays = GetFallbackDisplayList(scale);
   }
-  for (auto& display : displays)
-    AddDisplay(display, display.id() == primary_display_index_);
+  SetDisplayList(std::move(displays));
+}
+
+void X11ScreenOzone::UpdateDisplayList() {
+  std::vector<display::Display> old_displays = displays_;
+  FetchDisplayList();
+  change_notifier_.NotifyDisplaysChanged(old_displays, displays_);
+}
+
+void X11ScreenOzone::RestartDelayedUpdateDisplayListTask() {
+  delayed_update_task_.Reset(base::BindOnce(&X11ScreenOzone::UpdateDisplayList,
+                                            base::Unretained(this)));
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, delayed_update_task_.callback(), kDisplayListUpdateDelay);
 }
 
 gfx::Point X11ScreenOzone::GetCursorLocation() const {

ui/ozone/platform/x11/x11_screen_ozone.h

@@ -8,10 +8,12 @@
 #include <memory>
 #include <vector>
 
+#include "base/cancelable_callback.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
-#include "ui/display/display_list.h"
-#include "ui/events/platform/platform_event_dispatcher.h"
+#include "ui/display/display.h"
+#include "ui/display/display_change_notifier.h"
+#include "ui/events/platform/x11/x11_event_source_libevent.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/x/x11_types.h"
 #include "ui/ozone/public/platform_screen.h"
@@ -21,12 +23,15 @@ namespace ui {
 class X11WindowManagerOzone;
 
 // A PlatformScreen implementation for X11.
-class X11ScreenOzone : public PlatformScreen, public PlatformEventDispatcher {
+class X11ScreenOzone : public PlatformScreen, public XEventDispatcher {
  public:
-  explicit X11ScreenOzone(X11WindowManagerOzone* wm, bool fetch = true);
+  explicit X11ScreenOzone(X11WindowManagerOzone* window_manager);
   ~X11ScreenOzone() override;
 
-  // PlatformScreen:
+  // Fetch display list through Xlib/XRandR
+  void Init();
+
+  // Overridden from ui::PlatformScreen:
   const std::vector<display::Display>& GetAllDisplays() const override;
   display::Display GetPrimaryDisplay() const override;
   display::Display GetDisplayForAcceleratedWidget(
@@ -41,22 +46,22 @@ class X11ScreenOzone : public PlatformScreen, public PlatformEventDispatcher {
   void AddObserver(display::DisplayObserver* observer) override;
   void RemoveObserver(display::DisplayObserver* observer) override;
 
-  // PlatformEventDispatcher:
-  bool CanDispatchEvent(const ui::PlatformEvent& event) override;
-  uint32_t DispatchEvent(const ui::PlatformEvent& event) override;
+  // Overridden from ui::XEventDispatcher:
+  bool DispatchXEvent(XEvent* event) override;
 
  private:
   friend class X11ScreenOzoneTest;
 
-  void AddDisplay(const display::Display& display, bool is_primary);
-  void RemoveDisplay(const display::Display& display);
+  void SetDisplayList(std::vector<display::Display> displays);
   void FetchDisplayList();
+  void UpdateDisplayList();
+  void RestartDelayedUpdateDisplayListTask();
   gfx::Point GetCursorLocation() const;
 
-  X11WindowManagerOzone* const window_manager_;
-  display::DisplayList display_list_;
+  std::vector<display::Display> displays_;
+  display::DisplayChangeNotifier change_notifier_;
 
-  base::ObserverList<display::DisplayObserver> observers_;
+  X11WindowManagerOzone* const window_manager_;
 
   XDisplay* const xdisplay_;
   XID x_root_window_;
@@ -69,6 +74,10 @@ class X11ScreenOzone : public PlatformScreen, public PlatformEventDispatcher {
   // decoding events regarding output add/remove.
   int xrandr_event_base_ = 0;
 
+  // The task to delay fetching display info. We delay it so that we can
+  // coalesce events.
+  base::CancelableOnceClosure delayed_update_task_;
+
   DISALLOW_COPY_AND_ASSIGN(X11ScreenOzone);
 };
 

ui/ozone/platform/x11/x11_screen_ozone_unittest.cc

@@ -7,8 +7,10 @@
 #include <memory>
 
 #include "base/test/scoped_task_environment.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/display/display.h"
+#include "ui/display/display_observer.h"
 #include "ui/events/platform/x11/x11_event_source_libevent.h"
 #include "ui/ozone/platform/x11/x11_window_manager_ozone.h"
 #include "ui/ozone/platform/x11/x11_window_ozone.h"
@@ -34,6 +36,14 @@ int64_t NextDisplayId() {
   return next_id++;
 }
 
+struct MockDisplayObserver : public display::DisplayObserver {
+  MockDisplayObserver() = default;
+  ~MockDisplayObserver() override = default;
+
+  MOCK_METHOD1(OnDisplayAdded, void(const display::Display& new_display));
+  MOCK_METHOD1(OnDisplayRemoved, void(const display::Display& old_display));
+};
+
 }  // namespace
 
 class X11ScreenOzoneTest : public testing::Test {
@@ -49,8 +59,9 @@ class X11ScreenOzoneTest : public testing::Test {
     window_manager_ = std::make_unique<X11WindowManagerOzone>();
     primary_display_ = std::make_unique<display::Display>(
         NextDisplayId(), kPrimaryDisplayBounds);
-    screen_.reset(new X11ScreenOzone(window_manager_.get(), false));
-    screen_->AddDisplay(*primary_display_, true);
+    screen_.reset(new X11ScreenOzone(window_manager_.get()));
+    screen_->SetDisplayList({*primary_display_});
+    screen_->AddObserver(&display_observer_);
   }
 
  protected:
@@ -62,11 +73,23 @@ class X11ScreenOzoneTest : public testing::Test {
   }
 
   void AddDisplayForTest(const display::Display& display) {
-    screen_->AddDisplay(display, false);
+    std::vector<display::Display> new_displays(screen_->displays_);
+    new_displays.push_back(display);
+    UpdateDisplayListForTest(std::move(new_displays));
   }
 
-  void RemoveDisplayForTest(const display::Display& display) {
-    screen_->RemoveDisplay(display);
+  void RemoveDisplayForTest(const display::Display& display_to_remove) {
+    std::vector<display::Display> new_displays(screen_->displays_.size() - 1);
+    std::remove_copy(screen_->displays_.begin(), screen_->displays_.end(),
+                     new_displays.begin(), display_to_remove);
+    UpdateDisplayListForTest(std::move(new_displays));
+  }
+
+  void UpdateDisplayListForTest(std::vector<display::Display> displays) {
+    std::vector<display::Display> old_displays = std::move(screen_->displays_);
+    screen_->SetDisplayList(std::move(displays));
+    screen_->change_notifier_.NotifyDisplaysChanged(old_displays,
+                                                    screen_->displays_);
   }
 
   std::unique_ptr<X11WindowOzone> CreatePlatformWindow(
@@ -80,6 +103,8 @@ class X11ScreenOzoneTest : public testing::Test {
                                             window_manager_.get());
   }
 
+  MockDisplayObserver display_observer_;
+
  private:
   std::unique_ptr<X11WindowManagerOzone> window_manager_;
   std::unique_ptr<display::Display> primary_display_;
@@ -95,6 +120,8 @@ class X11ScreenOzoneTest : public testing::Test {
 TEST_F(X11ScreenOzoneTest, AddRemoveListDisplays) {
   // Initially only primary display is expected to be in place
   EXPECT_EQ(1u, screen()->GetAllDisplays().size());
+  EXPECT_CALL(display_observer_, OnDisplayAdded(_)).Times(2);
+  EXPECT_CALL(display_observer_, OnDisplayRemoved(_)).Times(3);
 
   auto display_2 = CreateDisplay(gfx::Rect(800, 0, 1280, 720));
   AddDisplayForTest(*display_2);