[X11] Keep WindowCache alive for a time interval

GetWindowAtScreenPoint() is called under certain conditions when Chrome
has mouse capture (eg. when a menu is showing, but not when the mouse
is clicked in web UI).  Creating a WindowCache on the stack is not
practical since it's difficult to separate the cases where a
WindowCache will be needed.  Also extra logic would have to be added to
ensure 2 WindowCache's are not active at once.  Therefore, this CL
switches to a simple timing based solution where a WindowCache will be
kept alive for 3 seconds after a call to GetWindowAtScreenPoint().

R=sky

Bug: 739898
Change-Id: I29053cd2ad5cdc4ff3e8cc6d4ef3979222976b59
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3565227
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988192}

ui/base/x/x11_whole_screen_move_loop.cc

@@ -26,7 +26,6 @@
 #include "ui/events/x/x11_event_translation.h"
 #include "ui/gfx/x/connection.h"
 #include "ui/gfx/x/keysyms/keysyms.h"
-#include "ui/gfx/x/window_cache.h"
 #include "ui/gfx/x/x11_window_event_manager.h"
 #include "ui/gfx/x/xproto.h"
 
@@ -152,10 +151,6 @@ bool X11WholeScreenMoveLoop::RunMoveLoop(
   auto* connection = x11::Connection::Get();
   CreateDragInputWindow(connection);
 
-  // Keep a window cache alive for the duration of the drag so that the drop
-  // target under the drag window can be quickly determined.
-  x11::WindowCache cache(connection, connection->default_root(), true);
-
   // Only grab mouse capture of |grab_input_window_| if |can_grab_pointer| is
   // true aka the source that initiated the move loop doesn't have explicit
   // grab.

ui/gfx/x/window_cache.cc

@@ -11,6 +11,8 @@
 #include "base/notreached.h"
 #include "base/ranges/algorithm.h"
 #include "base/run_loop.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "base/time/time.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/vector2d.h"
@@ -22,19 +24,23 @@
 
 namespace x11 {
 
+const base::TimeDelta kDestroyTimerInterval = base::Seconds(3);
+
 Window GetWindowAtPoint(const gfx::Point& point_px,
                         const base::flat_set<Window>* ignore) {
   auto* connection = Connection::Get();
   Window root = connection->default_root();
 
-  if (auto* instance = WindowCache::instance()) {
-    instance->WaitUntilReady();
-    return instance->GetWindowAtPoint(point_px, root, ignore);
+  if (!WindowCache::instance()) {
+    auto instance =
+        std::make_unique<WindowCache>(connection, connection->default_root());
+    auto* cache = instance.get();
+    cache->BeginDestroyTimer(std::move(instance));
   }
 
-  WindowCache cache(connection, connection->default_root(), false);
-  cache.WaitUntilReady();
-  return cache.GetWindowAtPoint(point_px, root, ignore);
+  auto* instance = WindowCache::instance();
+  instance->WaitUntilReady();
+  return instance->GetWindowAtPoint(point_px, root, ignore);
 }
 
 ScopedShapeEventSelector::ScopedShapeEventSelector(Connection* connection,
@@ -56,24 +62,21 @@ WindowCache::WindowInfo::~WindowInfo() = default;
 // static
 WindowCache* WindowCache::instance_ = nullptr;
 
-WindowCache::WindowCache(Connection* connection, Window root, bool track_events)
+WindowCache::WindowCache(Connection* connection, Window root)
     : connection_(connection),
       root_(root),
-      track_events_(track_events),
       gtk_frame_extents_(GetAtom("_GTK_FRAME_EXTENTS")) {
   DCHECK(!instance_) << "Only one WindowCache should be active at a time";
   instance_ = this;
 
   connection_->AddEventObserver(this);
 
-  if (track_events_) {
-    // We select for SubstructureNotify events on all windows (to receive
-    // CreateNotify events), which will cause events to be sent for all child
-    // windows.  This means we need to additionally select for StructureNotify
-    // changes for the root window.
-    root_events_ = std::make_unique<XScopedEventSelector>(
-        root_, EventMask::StructureNotify);
-  }
+  // We select for SubstructureNotify events on all windows (to receive
+  // CreateNotify events), which will cause events to be sent for all child
+  // windows.  This means we need to additionally select for StructureNotify
+  // changes for the root window.
+  root_events_ =
+      std::make_unique<XScopedEventSelector>(root_, EventMask::StructureNotify);
   AddWindow(root_, Window::None);
 }
 
@@ -103,6 +106,16 @@ void WindowCache::WaitUntilReady() {
     last_processed_event_ = events[event - 1].sequence();
 }
 
+void WindowCache::BeginDestroyTimer(std::unique_ptr<WindowCache> self) {
+  DCHECK_EQ(this, self.get());
+  delete_when_destroy_timer_fires_ = false;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(&WindowCache::OnDestroyTimerExpired,
+                     base::Unretained(this), std::move(self)),
+      kDestroyTimerInterval);
+}
+
 void WindowCache::SyncForTest() {
   do {
     // Perform a blocking sync to prevent spinning while waiting for replies.
@@ -114,6 +127,7 @@ void WindowCache::SyncForTest() {
 Window WindowCache::GetWindowAtPoint(gfx::Point point_px,
                                      Window window,
                                      const base::flat_set<Window>* ignore) {
+  delete_when_destroy_timer_fires_ = true;
   if (ignore && ignore->contains(window))
     return Window::None;
   auto* info = GetInfo(window);
@@ -251,12 +265,10 @@ void WindowCache::AddWindow(Window window, Window parent) {
     return;
   WindowInfo& info = windows_[window];
   info.parent = parent;
-  if (track_events_) {
-    // Events must be selected before getting the initial window info to
-    // prevent race conditions.
-    info.events = std::make_unique<XScopedEventSelector>(
-        window, EventMask::SubstructureNotify | EventMask::PropertyChange);
-  }
+  // Events must be selected before getting the initial window info to
+  // prevent race conditions.
+  info.events = std::make_unique<XScopedEventSelector>(
+      window, EventMask::SubstructureNotify | EventMask::PropertyChange);
 
   AddRequest(connection_->GetWindowAttributes(window),
              &WindowCache::OnGetWindowAttributesResponse, window);
@@ -270,10 +282,8 @@ void WindowCache::AddWindow(Window window, Window parent) {
 
   auto& shape = connection_->shape();
   if (shape.present()) {
-    if (track_events_) {
-      info.shape_events =
-          std::make_unique<ScopedShapeEventSelector>(connection_, window);
-    }
+    info.shape_events =
+        std::make_unique<ScopedShapeEventSelector>(connection_, window);
 
     for (auto kind : {Shape::Sk::Bounding, Shape::Sk::Input}) {
       AddRequest(shape.GetRectangles(window, kind),
@@ -381,4 +391,11 @@ void WindowCache::OnGetRectanglesResponse(
   }
 }
 
+void WindowCache::OnDestroyTimerExpired(std::unique_ptr<WindowCache> self) {
+  if (!delete_when_destroy_timer_fires_)
+    return;  // destroy `this`
+
+  BeginDestroyTimer(std::move(self));
+}
+
 }  // namespace x11

ui/gfx/x/window_cache.h

@@ -77,7 +77,7 @@ class COMPONENT_EXPORT(X11) WindowCache : public EventObserver {
   // If `track_events` is true, the WindowCache will keep the cache state synced
   // with the server's state over time. It may be set to false if the cache is
   // short-lived, if only a single GetWindowAtPoint call is made.
-  WindowCache(Connection* connection, Window root, bool track_events);
+  WindowCache(Connection* connection, Window root);
   WindowCache(const WindowCache&) = delete;
   WindowCache& operator=(const WindowCache&) = delete;
   ~WindowCache() override;
@@ -91,6 +91,10 @@ class COMPONENT_EXPORT(X11) WindowCache : public EventObserver {
   // Blocks until all outstanding requests are processed.
   void WaitUntilReady();
 
+  // Destroys |self| if no calls to GetWindowAtPoint() are made within
+  // a time window.
+  void BeginDestroyTimer(std::unique_ptr<WindowCache> self);
+
   void SyncForTest();
 
   const std::unordered_map<Window, WindowInfo>& windows() const {
@@ -142,11 +146,12 @@ class COMPONENT_EXPORT(X11) WindowCache : public EventObserver {
                                Shape::Sk kind,
                                Shape::GetRectanglesResponse response);
 
+  void OnDestroyTimerExpired(std::unique_ptr<WindowCache> self);
+
   static WindowCache* instance_;
 
   Connection* const connection_;
   const Window root_;
-  const bool track_events_;
   const Atom gtk_frame_extents_;
   std::unique_ptr<XScopedEventSelector> root_events_;
 
@@ -158,6 +163,9 @@ class COMPONENT_EXPORT(X11) WindowCache : public EventObserver {
   // processed in order.
   absl::optional<uint32_t> last_processed_event_;
 
+  // True iff GetWindowAtPoint() was called since the last timer interval.
+  bool delete_when_destroy_timer_fires_ = false;
+
   // Although only one instance of WindowCache may be created at a time, the
   // instance will be created and destroyed as needed, so WeakPtrs are still
   // necessary.

ui/gfx/x/window_cache_unittest.cc

@@ -20,7 +20,7 @@ class WindowCacheTest : public testing::Test {
  protected:
   void ResetCache() {
     cache_.reset();
-    cache_ = std::make_unique<WindowCache>(connection_, root_, true);
+    cache_ = std::make_unique<WindowCache>(connection_, root_);
     cache_->SyncForTest();
   }