[X11] Refactor bounds handling to use WM synchronization

* Replace the direct use of the bounds_in_pixels_ with a new
  sync-based mechanism that tracks the last set bounds (stored in
  last_set_bounds_px_) and relies on a WmSync object plus a
  GeometryCache to obtain current geometry via GetBoundsInPixels().
* Update SetBoundsInPixels() and fullscreen logic to call
  SetBoundsWithWmSync(), so that window bounds are only updated once the
  WM has synchronized the change. This replaces ad-hoc direct
  assignments with a coordinated update via OnWmSynced() and
  OnBoundsChanged().
* Modify OnConfigureEvent() to initialize and later use the new sync
  tracking (via last_configure_size_ and last_swapped_size_), and add a
  helper MaybeUpdateSyncCounter() to flush pending counter updates when
  the configured size matches the swapped size.
* Update the X11Extension interface (and its usage in
  DesktopWindowTreeHostLinux) so that OnCompleteSwapAfterResize() now
  accepts a new size parameter.

Bug: 40528928, 381224161
Change-Id: I1a25874fc47c6573ec77427b38532f53e9802684
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6289229
Reviewed-by: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Elly FJ <ellyjones@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Nick Yamane <nickdiego@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1429329}

chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.cc

@@ -487,7 +487,7 @@ Browser* TabDragControllerTest::CreateAnotherBrowserAndResize() {
   gfx::Rect browser_rect(work_area.origin() + gfx::Vector2d(50, 50), size);
 
   if (test::PlatformSupportsScreenCoordinates()) {
-    browser()->window()->SetBounds(browser_rect);
+    ui_test_utils::SetAndWaitForBounds(*browser(), browser_rect);
     browser_rect.set_x(browser_rect.right());
   } else {
     test::ResizeUsingMouseEmulation(browser(), browser_rect);
@@ -509,7 +509,7 @@ Browser* TabDragControllerTest::CreateAnotherBrowserAndResize() {
   Browser* browser2 = CreateBrowser(browser()->profile());
   ResetIDs(browser2->tab_strip_model(), 100);
   if (test::PlatformSupportsScreenCoordinates()) {
-    browser2->window()->SetBounds(browser_rect);
+    ui_test_utils::SetAndWaitForBounds(*browser2, browser_rect);
   } else {
     test::ResizeUsingMouseEmulation(browser2, browser_rect);
   }

ui/gfx/x/connection.cc

@@ -412,6 +412,17 @@ int Connection::GetFd() {
   return Ready() ? xcb_get_file_descriptor(XcbConnection()) : -1;
 }
 
+bool Connection::CanSyncWithWm() const {
+  // For some WMs, we don't need to experimentally sync with them to determine
+  // sync support, so we can use WmSync right away. For now, only check for
+  // Openbox since that's what is used in tests. The list may be expanded as
+  // nearly all WMs should work with WmSync.
+  if (GetWmName() == "Openbox") {
+    return true;
+  }
+  return synced_with_wm_;
+}
+
 const std::string& Connection::DisplayString() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return display_string_;

ui/gfx/x/connection.h

@@ -255,9 +255,10 @@ class COMPONENT_EXPORT(X11) Connection final : public XProto,
 
   WindowEventManager& window_event_manager() { return window_event_manager_; }
 
-  // Indicates if the connection was able to successfully sync with the
+  // Indicates if the connection is able to sync with the WM, either because the
-  // window manager.
+  // WM is on an allowlist or the connection successfully synced with the WM to
-  bool synced_with_wm() const { return synced_with_wm_; }
+  // test support experimentally.
+  bool CanSyncWithWm() const;
 
   // Returns the underlying socket's FD if the connection is valid, or -1
   // otherwise.

ui/gfx/x/geometry_cache.cc

@@ -52,11 +52,17 @@ gfx::Rect GeometryCache::GetBoundsPx() {
 }
 
 void GeometryCache::OnQueryTreeResponse(QueryTreeResponse response) {
+  if (have_parent_) {
+    return;
+  }
   OnParentChanged(response ? response->parent : Window::None,
                   geometry_.origin());
 }
 
 void GeometryCache::OnGetGeometryResponse(GetGeometryResponse response) {
+  if (have_geometry_) {
+    return;
+  }
   OnGeometryChanged(response ? gfx::Rect(response->x, response->y,
                                          response->width, response->height)
                              : gfx::Rect());

ui/gfx/x/wm_sync.cc

@@ -27,7 +27,7 @@ Window GetWindowForEvent(const Event& event) {
 }  // namespace
 
 WmSync::WmSync(Connection* connection, base::OnceClosure on_synced)
-    : WmSync(connection, std::move(on_synced), connection->synced_with_wm()) {}
+    : WmSync(connection, std::move(on_synced), connection->CanSyncWithWm()) {}
 
 WmSync::WmSync(Connection* connection,
                base::OnceClosure on_synced,

ui/ozone/platform/x11/x11_window.cc

@@ -45,8 +45,10 @@
 #include "ui/gfx/geometry/transform.h"
 #include "ui/gfx/image/image_skia_rep.h"
 #include "ui/gfx/x/atom_cache.h"
+#include "ui/gfx/x/geometry_cache.h"
 #include "ui/gfx/x/visual_manager.h"
 #include "ui/gfx/x/window_event_manager.h"
+#include "ui/gfx/x/wm_sync.h"
 #include "ui/gfx/x/x11_path.h"
 #include "ui/gfx/x/xproto.h"
 #include "ui/ozone/platform/x11/hit_test_x11.h"
@@ -494,9 +496,9 @@ void X11Window::SetBoundsInPixels(const gfx::Rect& bounds) {
                                  AdjustSizeForDisplay(bounds.size()));
 
   const bool size_changed =
-      bounds_in_pixels_.size() != new_bounds_in_pixels.size();
+      GetBoundsInPixels().size() != new_bounds_in_pixels.size();
   const bool origin_changed =
-      bounds_in_pixels_.origin() != new_bounds_in_pixels.origin();
+      GetBoundsInPixels().origin() != new_bounds_in_pixels.origin();
 
   // Assume that the resize will go through as requested, which should be the
   // case if we're running without a window manager.  If there's a window
@@ -537,7 +539,6 @@ void X11Window::SetBoundsInPixels(const gfx::Rect& bounds) {
   }
 
   if (origin_changed || size_changed) {
-    bounds_change_in_flight_ = true;
     connection_->ConfigureWindow(req);
   }
 
@@ -546,7 +547,7 @@ void X11Window::SetBoundsInPixels(const gfx::Rect& bounds) {
   // manager, it can modify or ignore the request, but (per ICCCM) we'll get a
   // (possibly synthetic) ConfigureNotify about the actual size and correct
   // |bounds_in_pixels_| later.
-  bounds_in_pixels_ = new_bounds_in_pixels;
+  SetBoundsWithWmSync(new_bounds_in_pixels);
   ResetWindowRegion();
 
   // Even if the pixel bounds didn't change this call to the delegate should
@@ -556,7 +557,8 @@ void X11Window::SetBoundsInPixels(const gfx::Rect& bounds) {
 }
 
 gfx::Rect X11Window::GetBoundsInPixels() const {
-  return bounds_in_pixels_;
+  return bounds_wm_sync_ || !geometry_cache_ ? last_set_bounds_px_
+                                             : geometry_cache_->GetBoundsPx();
 }
 
 void X11Window::SetBoundsInDIP(const gfx::Rect& bounds_in_dip) {
@@ -565,7 +567,7 @@ void X11Window::SetBoundsInDIP(const gfx::Rect& bounds_in_dip) {
 }
 
 gfx::Rect X11Window::GetBoundsInDIP() const {
-  return platform_window_delegate_->ConvertRectToDIP(bounds_in_pixels_);
+  return platform_window_delegate_->ConvertRectToDIP(GetBoundsInPixels());
 }
 
 void X11Window::SetTitle(const std::u16string& title) {
@@ -673,23 +675,11 @@ void X11Window::SetFullscreen(bool fullscreen, int64_t target_display_id) {
 
   UpdateDecorationInsets();
 
-  // Do not go through SetBounds as long as it adjusts bounds and sets them to X
+  // Pretend the bounds changed immediately, and wait for a WM sync to use the
-  // Server. Instead, we just store the bounds and notify the client that the
+  // server's bounds.
-  // window occupies the entire screen.
+  bool origin_changed = GetBoundsInPixels().origin() != new_bounds_px.origin();
-  bool origin_changed = bounds_in_pixels_.origin() != new_bounds_px.origin();
+  SetBoundsWithWmSync(new_bounds_px);
-  bounds_in_pixels_ = new_bounds_px;
 
-  // If there is a restore and/or bounds change in flight, then set a flag to
-  // ignore the next one or two configure events (hopefully) coming from those
-  // requests. This prevents any in-flight restore requests from changing the
-  // bounds in a way that conflicts with the `bounds_in_pixels_` setting above.
-  // This is not perfect, and if there is some other in-flight bounds change for
-  // some reason, or if the ordering of events from the WM behaves differently,
-  // this will not prevent the issue.  See: http://crbug.com/1227451
-  ignore_next_configures_ = restore_in_flight_ ? 1 : 0;
-  if (bounds_change_in_flight_) {
-    ignore_next_configures_++;
-  }
   // This must be the final call in this function, as `this` may be deleted
   // during the observation of this event.
   platform_window_delegate_->OnBoundsChanged({origin_changed});
@@ -736,11 +726,9 @@ void X11Window::Minimize() {
 
 void X11Window::Restore() {
   if (IsMinimized()) {
-    restore_in_flight_ = true;
     SetWMSpecState(false, x11::GetAtom("_NET_WM_STATE_HIDDEN"),
                    x11::Atom::None);
   } else if (IsMaximized()) {
-    restore_in_flight_ = true;
     should_maximize_after_map_ = false;
     SetWMSpecState(false, x11::GetAtom("_NET_WM_STATE_MAXIMIZED_VERT"),
                    x11::GetAtom("_NET_WM_STATE_MAXIMIZED_HORZ"));
@@ -852,8 +840,8 @@ void X11Window::SetCursor(scoped_refptr<PlatformCursor> cursor) {
 void X11Window::MoveCursorTo(const gfx::Point& location_px) {
   connection_->WarpPointer(x11::WarpPointerRequest{
       .dst_window = x_root_window_,
-      .dst_x = static_cast<int16_t>(bounds_in_pixels_.x() + location_px.x()),
+      .dst_x = static_cast<int16_t>(GetBoundsInPixels().x() + location_px.x()),
-      .dst_y = static_cast<int16_t>(bounds_in_pixels_.y() + location_px.y()),
+      .dst_y = static_cast<int16_t>(GetBoundsInPixels().y() + location_px.y()),
   });
   // The cached cursor location is no longer valid.
   X11EventSource::GetInstance()->ClearLastCursorLocation();
@@ -866,7 +854,7 @@ void X11Window::ConfineCursorToBounds(const gfx::Rect& bounds) {
     return;
   }
 
-  gfx::Rect barrier = bounds + bounds_in_pixels_.OffsetFromOrigin();
+  gfx::Rect barrier = bounds + GetBoundsInPixels().OffsetFromOrigin();
 
   auto make_barrier = [&](uint16_t x1, uint16_t y1, uint16_t x2, uint16_t y2,
                           x11::XFixes::BarrierDirections directions) {
@@ -1165,6 +1153,7 @@ void X11Window::NotifyStartupComplete(const std::string& startup_id) {
     event.type = net_startup_info;
   }
 
+  geometry_cache_.reset();
   connection_->DestroyWindow(window);
   connection_->Flush();
 }
@@ -1214,11 +1203,10 @@ bool X11Window::IsWmTiling() const {
   return ui::IsWmTiling(ui::GuessWindowManager());
 }
 
-void X11Window::OnCompleteSwapAfterResize() {
+void X11Window::OnCompleteSwapAfterResize(const gfx::Size& new_size) {
-  if (configure_counter_value_ && have_configure_) {
+  last_swapped_size_ = new_size;
-    connection_->sync().SetCounter(update_counter_, *configure_counter_value_);
+  if (configure_counter_value_) {
-    configure_counter_value_.reset();
+    MaybeUpdateSyncCounter();
-    have_configure_ = false;
   }
 }
 
@@ -1260,6 +1248,10 @@ void X11Window::SetX11ExtensionDelegate(X11ExtensionDelegate* delegate) {
   x11_extension_delegate_ = delegate;
 }
 
+bool X11Window::IsWmSyncActiveForTest() {
+  return bounds_wm_sync_.get();
+}
+
 bool X11Window::HandleAsAtkEvent(const x11::KeyEvent& key_event,
                                  bool send_event,
                                  bool transient) {
@@ -1414,10 +1406,6 @@ void X11Window::OnXWindowStateChanged() {
     new_state = PlatformWindowState::kMaximized;
   }
 
-  if (restore_in_flight_ && !IsMaximized()) {
-    restore_in_flight_ = false;
-  }
-
   // fullscreen state is set syschronously at ToggleFullscreen() and must be
   // kept and propagated to the client only when explicitly requested by upper
   // layers, as it means we are in "browser fullscreen mode" (where
@@ -1445,16 +1433,8 @@ void X11Window::OnXWindowStateChanged() {
     return;
   }
 
-  if (restored_bounds_in_pixels_.IsEmpty()) {
+  if (!restored_bounds_in_pixels_.IsEmpty() && !IsMaximized() &&
-    if (IsMaximized()) {
+      !IsFullscreen()) {
-      // The request that we become maximized originated from a different
-      // process. |bounds_in_pixels_| already contains our maximized bounds. Do
-      // a best effort attempt to get restored bounds by setting it to our
-      // previously set bounds (and if we get this wrong, we aren't any worse
-      // off since we'd otherwise be returning our maximized bounds).
-      restored_bounds_in_pixels_ = previous_bounds_in_pixels_;
-    }
-  } else if (!IsMaximized() && !IsFullscreen()) {
     // If we have restored bounds, but WM_STATE no longer claims to be
     // maximized or fullscreen, we should clear our restored bounds.
     restored_bounds_in_pixels_ = gfx::Rect();
@@ -1719,7 +1699,7 @@ scoped_refptr<X11Cursor> X11Window::GetLastCursor() {
 }
 
 gfx::Size X11Window::GetSize() {
-  return bounds_in_pixels_.size();
+  return GetBoundsInPixels().size();
 }
 
 void X11Window::QuitDragLoop() {
@@ -1846,12 +1826,12 @@ void X11Window::CreateXWindow(const PlatformWindowInitProperties& properties) {
   // same as the parent depth.
   req.border_pixel = 0;
 
-  bounds_in_pixels_ = SanitizeBounds(bounds);
+  last_set_bounds_px_ = SanitizeBounds(bounds);
   req.parent = x_root_window_;
-  req.x = bounds_in_pixels_.x();
+  req.x = last_set_bounds_px_.x();
-  req.y = bounds_in_pixels_.y();
+  req.y = last_set_bounds_px_.y();
-  req.width = bounds_in_pixels_.width();
+  req.width = last_set_bounds_px_.width();
-  req.height = bounds_in_pixels_.height();
+  req.height = last_set_bounds_px_.height();
   req.depth = depth;
   req.c_class = x11::WindowClass::InputOutput;
   req.visual = visual_id;
@@ -1859,6 +1839,10 @@ void X11Window::CreateXWindow(const PlatformWindowInitProperties& properties) {
   xwindow_ = connection_->GenerateId<x11::Window>();
   req.wid = xwindow_;
   connection_->CreateWindow(req);
+  // Unretained is safe since we own `geometry_cache_`.
+  geometry_cache_ = std::make_unique<x11::GeometryCache>(
+      &*connection_, xwindow_,
+      base::BindRepeating(&X11Window::OnBoundsChanged, base::Unretained(this)));
 }
 
 void X11Window::CloseXWindow() {
@@ -1875,6 +1859,7 @@ void X11Window::CloseXWindow() {
                             security_surfaces.end());
   }
 
+  geometry_cache_.reset();
   connection_->DestroyWindow({xwindow_});
   xwindow_ = x11::Window::None;
 
@@ -1891,8 +1876,8 @@ void X11Window::Map(bool inactive) {
   memset(&size_hints, 0, sizeof(size_hints));
   connection_->GetWmNormalHints(xwindow_, &size_hints);
   size_hints.flags |= x11::SIZE_HINT_P_POSITION;
-  size_hints.x = bounds_in_pixels_.x();
+  size_hints.x = GetBoundsInPixels().x();
-  size_hints.y = bounds_in_pixels_.y();
+  size_hints.y = GetBoundsInPixels().y();
   // Set STATIC_GRAVITY so that the window position is not affected by the
   // frame width when running with window manager.
   size_hints.flags |= x11::SIZE_HINT_P_WIN_GRAVITY;
@@ -1964,7 +1949,7 @@ bool X11Window::IsFullscreen() const {
 }
 
 gfx::Rect X11Window::GetOuterBounds() const {
-  gfx::Rect outer_bounds(bounds_in_pixels_);
+  gfx::Rect outer_bounds(GetBoundsInPixels());
   outer_bounds.Inset(-native_window_frame_borders_in_pixels_);
   return outer_bounds;
 }
@@ -2272,23 +2257,6 @@ void X11Window::HandleEvent(const x11::Event& xev) {
     return;
   }
 
-  // We can lose track of the window's position when the window is reparented.
-  // When the parent window is moved, we won't get an event, so the window's
-  // position relative to the root window will get out-of-sync.  We can re-sync
-  // when getting pointer events (EnterNotify, LeaveNotify, ButtonPress,
-  // ButtonRelease, MotionNotify) which include the pointer location both
-  // relative to this window and relative to the root window, so we can
-  // calculate this window's position from that information.
-  gfx::Point window_point = EventLocationFromXEvent(xev);
-  gfx::Point root_point = EventSystemLocationFromXEvent(xev);
-  if (!window_point.IsOrigin() && !root_point.IsOrigin()) {
-    gfx::Point window_origin = gfx::Point() + (root_point - window_point);
-    if (bounds_in_pixels_.origin() != window_origin) {
-      bounds_in_pixels_.set_origin(window_origin);
-      NotifyBoundsChanged(/*origin changed=*/true);
-    }
-  }
-
   // May want to factor CheckXEventForConsistency(xev); into a common location
   // since it is called here.
   if (auto* crossing = xev.As<x11::CrossingEvent>()) {
@@ -2303,7 +2271,7 @@ void X11Window::HandleEvent(const x11::Event& xev) {
     OnFocusEvent(focus->opcode == x11::FocusEvent::In, focus->mode,
                  focus->detail);
   } else if (auto* configure = xev.As<x11::ConfigureNotifyEvent>()) {
-    OnConfigureEvent(*configure, xev.send_event());
+    OnConfigureEvent(*configure);
   } else if (auto* crossing_input = xev.As<x11::Input::CrossingEvent>()) {
     TouchFactory* factory = TouchFactory::GetInstance();
     if (factory->ShouldProcessCrossingEvent(*crossing_input)) {
@@ -2347,7 +2315,6 @@ void X11Window::HandleEvent(const x11::Event& xev) {
                                    x11::EventMask::SubstructureRedirect);
       } else if (protocol == x11::GetAtom("_NET_WM_SYNC_REQUEST")) {
         configure_counter_value_.reset();
-        have_configure_ = false;
         const int32_t hi = static_cast<int32_t>(client->data.data32[3]);
         const uint32_t lo = client->data.data32[2];
         // The spec says the WM should never send a counter value of 0, so
@@ -2405,57 +2372,12 @@ void X11Window::OnWindowMapped() {
   }
 }
 
-void X11Window::OnConfigureEvent(const x11::ConfigureNotifyEvent& configure,
+void X11Window::OnConfigureEvent(const x11::ConfigureNotifyEvent& configure) {
-                                 bool send_event) {
   DCHECK_EQ(xwindow_, configure.window);
 
-  if (configure_counter_value_) {
+  if (configure_counter_value_ && !last_configure_size_) {
-    have_configure_ = true;
+    last_configure_size_ = gfx::Size(configure.width, configure.height);
-  }
+    MaybeUpdateSyncCounter();
-
-  // During a Restore() -> ToggleFullscreen() or Restore() -> SetBounds() ->
-  // ToggleFullscreen() sequence, ignore the configure events from the Restore
-  // and SetBounds requests, if we're waiting on fullscreen.  After
-  // OnXWindowStateChanged unsets this flag, there will be a configuration event
-  // that will set the bounds to the final fullscreen bounds.
-  if (ignore_next_configures_ > 0) {
-    ignore_next_configures_--;
-    return;
-  }
-
-  // Note: This OnConfigureEvent might not necessarily correspond to a previous
-  // SetBounds request. Due to limitations in X11 there isn't a way to
-  // match events to its original request. For now, we assume that the next
-  // OnConfigureEvent event after a SetBounds (ConfigureWindow) request is from
-  // that request. This would break in some scenarios (for example calling
-  // SetBounds more than once quickly). See crbug.com/1227451.
-  bounds_change_in_flight_ = false;
-
-  // It's possible that the X window may be resized by some other means than
-  // from within aura (e.g. the X window manager can change the size). Make
-  // sure the root window size is maintained properly.
-  int translated_x_in_pixels = configure.x;
-  int translated_y_in_pixels = configure.y;
-  if (!send_event && !configure.override_redirect) {
-    auto future =
-        connection_->TranslateCoordinates({xwindow_, x_root_window_, 0, 0});
-    if (auto coords = future.Sync()) {
-      translated_x_in_pixels = coords->dst_x;
-      translated_y_in_pixels = coords->dst_y;
-    }
-  }
-  gfx::Rect new_bounds_px(translated_x_in_pixels, translated_y_in_pixels,
-                          configure.width, configure.height);
-  const bool size_changed = bounds_in_pixels_.size() != new_bounds_px.size();
-  const bool origin_changed =
-      bounds_in_pixels_.origin() != new_bounds_px.origin();
-  previous_bounds_in_pixels_ = bounds_in_pixels_;
-  bounds_in_pixels_ = new_bounds_px;
-
-  if (size_changed) {
-    DispatchResize(origin_changed);
-  } else if (origin_changed) {
-    NotifyBoundsChanged(/*origin changed=*/true);
   }
 }
 
@@ -2537,6 +2459,7 @@ void X11Window::OnFrameExtentsUpdated() {
 }
 
 void X11Window::DispatchResize(bool origin_changed) {
+  last_swapped_size_.reset();
   if (configure_counter_value_) {
     // WM handles resize throttling. No need to delay resize events.
     DelayedResize(origin_changed);
@@ -2656,4 +2579,39 @@ bool X11Window::InitializeAsStatusIcon() {
   return !future.Sync().error;
 }
 
+void X11Window::SetBoundsWithWmSync(const gfx::Rect& bounds_px) {
+  last_set_bounds_px_ = bounds_px;
+  // Unretained is safe since we own `bounds_wm_sync_`.
+  bounds_wm_sync_ = std::make_unique<x11::WmSync>(
+      &*connection_,
+      base::BindOnce(&X11Window::OnWmSynced, base::Unretained(this)));
+}
+
+void X11Window::OnWmSynced() {
+  bounds_wm_sync_.reset();
+  OnBoundsChanged(last_set_bounds_px_, GetBoundsInPixels());
+}
+
+void X11Window::OnBoundsChanged(const std::optional<gfx::Rect>& old_bounds_px,
+                                const gfx::Rect& new_bounds_px) {
+  const bool size_changed = !old_bounds_px.has_value() ||
+                            old_bounds_px->size() != new_bounds_px.size();
+  const bool origin_changed = !old_bounds_px.has_value() ||
+                              old_bounds_px->origin() != new_bounds_px.origin();
+
+  if (size_changed) {
+    DispatchResize(origin_changed);
+  } else if (origin_changed) {
+    NotifyBoundsChanged(/*origin changed=*/true);
+  }
+}
+
+void X11Window::MaybeUpdateSyncCounter() {
+  if (last_configure_size_ == last_swapped_size_) {
+    connection_->sync().SetCounter(update_counter_, *configure_counter_value_);
+    configure_counter_value_.reset();
+    last_configure_size_.reset();
+  }
+}
+
 }  // namespace ui

ui/ozone/platform/x11/x11_window.h

@@ -36,6 +36,11 @@
 
 class SkPath;
 
+namespace x11 {
+class GeometryCache;
+class WmSync;
+}  // namespace x11
+
 namespace ui {
 
 class PlatformWindowDelegate;
@@ -134,12 +139,13 @@ class X11Window : public PlatformWindow,
   // X11Extension:
   bool IsSyncExtensionAvailable() const override;
   bool IsWmTiling() const override;
-  void OnCompleteSwapAfterResize() override;
+  void OnCompleteSwapAfterResize(const gfx::Size& new_size) override;
   gfx::Rect GetXRootWindowOuterBounds() const override;
   void LowerXWindow() override;
   void SetOverrideRedirect(bool override_redirect) override;
   bool CanResetOverrideRedirect() const override;
   void SetX11ExtensionDelegate(X11ExtensionDelegate* delegate) override;
+  bool IsWmSyncActiveForTest() override;
 
   // x11::EventObserver:
   void OnEvent(const x11::Event& event) override;
@@ -283,8 +289,7 @@ class X11Window : public PlatformWindow,
   // Called when |xwindow_|'s _NET_FRAME_EXTENTS property is updated.
   void OnFrameExtentsUpdated();
 
-  void OnConfigureEvent(const x11::ConfigureNotifyEvent& event,
+  void OnConfigureEvent(const x11::ConfigureNotifyEvent& event);
-                        bool send_event);
 
   void OnWorkspaceUpdated();
 
@@ -323,6 +328,15 @@ class X11Window : public PlatformWindow,
   // Initializes as a status icon window.
   bool InitializeAsStatusIcon();
 
+  void SetBoundsWithWmSync(const gfx::Rect& bounds_px);
+
+  void OnWmSynced();
+
+  void OnBoundsChanged(const std::optional<gfx::Rect>& old_bounds_px,
+                       const gfx::Rect& new_bounds_px);
+
+  void MaybeUpdateSyncCounter();
+
   // Stores current state of this window.
   PlatformWindowState state_ = PlatformWindowState::kUnknown;
 
@@ -393,8 +407,12 @@ class X11Window : public PlatformWindow,
   // Whether the window is mapped with respect to the X server.
   bool window_mapped_in_server_ = false;
 
-  // The bounds of |xwindow_|.
+  // The bounds of `xwindow_`.  If `bounds_wm_sync_` is active, then
-  gfx::Rect bounds_in_pixels_;
+  // `last_set_bounds_px_` should be treated as the current bounds.  Otherwise,
+  // the bounds from `geometry_cache_` should be used.
+  gfx::Rect last_set_bounds_px_;
+  std::unique_ptr<x11::WmSync> bounds_wm_sync_;
+  std::unique_ptr<x11::GeometryCache> geometry_cache_;
 
   x11::VisualId visual_id_{};
 
@@ -450,15 +468,6 @@ class X11Window : public PlatformWindow,
   bool had_pointer_grab_ = false;
   bool had_window_focus_ = false;
 
-  // Whenever the bounds are set, we keep the previous set of bounds around so
-  // we can have a better chance of getting the real
-  // |restored_bounds_in_pixels_|. Window managers tend to send a Configure
-  // message with the maximized bounds, and then set the window maximized
-  // property. (We don't rely on this for when we request that the window be
-  // maximized, only when we detect that some other process has requested that
-  // we become the maximized window.)
-  gfx::Rect previous_bounds_in_pixels_;
-
   // True if a Maximize() call should be done after mapping the window.
   bool should_maximize_after_map_ = false;
 
@@ -490,22 +499,10 @@ class X11Window : public PlatformWindow,
   gfx::Insets native_window_frame_borders_in_pixels_;
 
   // Used for synchronizing between `xwindow_` and the WM during resizing.
-  std::optional<x11::Sync::Int64> configure_counter_value_;
-  bool have_configure_ = false;
   x11::Sync::Counter update_counter_{};
-
+  std::optional<x11::Sync::Int64> configure_counter_value_;
-  // Used for ignoring bounds changes during the fullscreening process.  For
+  std::optional<gfx::Size> last_configure_size_;
-  // cross-display fullscreening, there is a Restore() (called by BrowserView)
+  std::optional<gfx::Size> last_swapped_size_;
-  // that may cause configuration bounds updates that make this window appear to
-  // temporarily be on a different screen than its destination screen.  This
-  // restore only happens if the window is maximized. The integer represents how
-  // many events to ignore.
-  int ignore_next_configures_ = 0;
-  // True between Restore() and the next OnXWindowStateChanged().
-  bool restore_in_flight_ = false;
-  // True between SetBoundsInPixels (when the bounds actually change) and the
-  // next OnConfigureEvent.
-  bool bounds_change_in_flight_ = false;
 
   base::CancelableOnceClosure delayed_resize_task_;
 

ui/platform_window/extensions/x11_extension.h

@@ -27,7 +27,7 @@ class COMPONENT_EXPORT(PLATFORM_WINDOW) X11Extension {
   virtual bool IsWmTiling() const = 0;
 
   // Handles CompleteSwapAfterResize event coming from the compositor observer.
-  virtual void OnCompleteSwapAfterResize() = 0;
+  virtual void OnCompleteSwapAfterResize(const gfx::Size& new_size) = 0;
 
   // Returns the current bounds in terms of the X11 Root Window including the
   // borders provided by the window manager (if any).
@@ -46,6 +46,9 @@ class COMPONENT_EXPORT(PLATFORM_WINDOW) X11Extension {
 
   virtual void SetX11ExtensionDelegate(X11ExtensionDelegate* delegate) = 0;
 
+  // Indicates whether a WmSync is pending. Should only be used for testing.
+  virtual bool IsWmSyncActiveForTest() = 0;
+
  protected:
   virtual ~X11Extension();
 

ui/views/test/widget_test_api.cc

@@ -16,6 +16,7 @@
 #include "ui/aura/window_tree_host_platform.h"
 #include "ui/ozone/public/ozone_platform.h"
 #include "ui/platform_window/extensions/wayland_extension.h"
+#include "ui/platform_window/extensions/x11_extension.h"
 
 #endif  // BUILDFLAG(IS_LINUX)
 
@@ -48,11 +49,11 @@ AsyncWidgetRequestWaiter::~AsyncWidgetRequestWaiter() {
 void AsyncWidgetRequestWaiter::Wait() {
   CHECK(!waited_) << "`Wait` may only be called once.";
 #if BUILDFLAG(IS_LINUX)
+  auto* host = aura::WindowTreeHostPlatform::GetHostForWindow(
+      widget_->GetNativeWindow());
   if (ui::OzonePlatform::GetPlatformNameForTest() == "wayland") {
     // Wait for a Wayland roundtrip to ensure all side effects have been
     // processed.
-    auto* host = aura::WindowTreeHostPlatform::GetHostForWindow(
-        widget_->GetNativeWindow());
     auto* wayland_extension = ui::GetWaylandExtension(*host->platform_window());
     wayland_extension->RoundTripQueue();
 
@@ -72,6 +73,15 @@ void AsyncWidgetRequestWaiter::Wait() {
     // to be processed on the server side.
     wayland_extension->RoundTripQueue();
     wayland_extension->SetLatchImmediately(true);
+  } else if (ui::OzonePlatform::GetPlatformNameForTest() == "x11") {
+    // Setting the window bounds on X11 is asynchronous, so the platform window
+    // pretends the bounds change completed successfully until it can sync with
+    // the WM. This may cause inconsistent state with respect to other caches
+    // such as x11::WindowCache. Wait for the WM sync to complete to ensure
+    // consistency.
+    auto* x11_extension = ui::GetX11Extension(*host->platform_window());
+    CHECK(base::test::RunUntil(
+        [&]() { return !x11_extension->IsWmSyncActiveForTest(); }));
   } else {
     NOTIMPLEMENTED_LOG_ONCE();
   }

ui/views/widget/desktop_aura/desktop_window_tree_host_linux.cc

@@ -348,7 +348,7 @@ DesktopWindowTreeHostLinux::GetKeyboardLayoutMap() {
 void DesktopWindowTreeHostLinux::OnCompleteSwapWithNewSize(
     const gfx::Size& size) {
   if (GetX11Extension()) {
-    GetX11Extension()->OnCompleteSwapAfterResize();
+    GetX11Extension()->OnCompleteSwapAfterResize(size);
   }
 }