Change bounds in PWD::BoundsChange to origin_changed

The pixel origin may or may not make sense depending on the platform
and may result in bogus value.

Change it to two bools with 1 bit that indicates of the platform window has moved. A delegate can use the platform window's size if it needs to know the new size. WindowTreeHost caches the old size, so no need to send size_changed.

Also removed the origin argument in WindowTreeHostObserver::OnHostMovedInPixels. If an implementation needs
it, it can get the origin from WindowTreeHost passed to the observer.

Bug: 1306688
Test: WaylandWindowTest.WindowMovedResized to test origin change scenario. No functional change.

Change-Id: Ia44daf8f5016fdbcf977b47412db4f8d59139cd0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3759025
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Peter McNeeley <petermcneeley@google.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Peter McNeeley <petermcneeley@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1028415}

ash/host/ash_window_tree_host_unified.cc

@@ -117,9 +117,9 @@ void AshWindowTreeHostUnified::OnCursorVisibilityChangedNative(bool show) {
     host->AsWindowTreeHost()->OnCursorVisibilityChanged(show);
 }
 
-void AshWindowTreeHostUnified::OnBoundsChanged(const BoundsChange& bounds) {
+void AshWindowTreeHostUnified::OnBoundsChanged(const BoundsChange& change) {
   if (platform_window())
-    OnHostResizedInPixels(bounds.bounds.size());
+    OnHostResizedInPixels(platform_window()->GetBoundsInPixels().size());
 }
 
 void AshWindowTreeHostUnified::OnWindowDestroying(aura::Window* window) {

components/viz/demo/demo_main.cc

@@ -159,7 +159,7 @@ class DemoWindow : public ui::PlatformWindowDelegate {
 
   // ui::PlatformWindowDelegate:
   void OnBoundsChanged(const BoundsChange& bounds) override {
-    host_->Resize(bounds.bounds.size());
+    host_->Resize(platform_window_->GetBoundsInPixels().size());
   }
 
   void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget) override {

content/browser/renderer_host/render_widget_host_view_aura.cc

@@ -2111,11 +2111,8 @@ void RenderWidgetHostViewAura::OnWindowFocused(aura::Window* gained_focus,
 ////////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostViewAura, aura::WindowTreeHostObserver implementation:
 
-void RenderWidgetHostViewAura::OnHostMovedInPixels(
-    aura::WindowTreeHost* host,
-    const gfx::Point& new_origin_in_pixels) {
-  TRACE_EVENT1("ui", "RenderWidgetHostViewAura::OnHostMovedInPixels",
-               "new_origin_in_pixels", new_origin_in_pixels.ToString());
+void RenderWidgetHostViewAura::OnHostMovedInPixels(aura::WindowTreeHost* host) {
+  TRACE_EVENT0("ui", "RenderWidgetHostViewAura::OnHostMovedInPixels");
 
   UpdateScreenInfo();
 }

content/browser/renderer_host/render_widget_host_view_aura.h

@@ -334,8 +334,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
                        aura::Window* lost_focus) override;
 
   // Overridden from aura::WindowTreeHostObserver:
-  void OnHostMovedInPixels(aura::WindowTreeHost* host,
-                           const gfx::Point& new_origin_in_pixels) override;
+  void OnHostMovedInPixels(aura::WindowTreeHost* host) override;
 
   // RenderFrameMetadataProvider::Observer implementation.
   void OnRenderFrameMetadataChangedBeforeActivation(

content/browser/web_contents/web_contents_view_aura.cc

@@ -600,8 +600,7 @@ class WebContentsViewAura::WindowObserver
     pending_window_changes_.reset();
   }
 
-  void OnHostMovedInPixels(aura::WindowTreeHost* host,
-                           const gfx::Point& new_origin_in_pixels) override {
+  void OnHostMovedInPixels(aura::WindowTreeHost* host) override {
     if (!ShouldNotifyOfBoundsChanges())
       return;
 

headless/lib/browser/headless_window_tree_host.cc

@@ -61,7 +61,7 @@ void HeadlessWindowTreeHost::SetBoundsInPixels(const gfx::Rect& bounds) {
   bool size_changed = bounds_.size() != bounds.size();
   bounds_ = bounds;
   if (origin_changed)
-    OnHostMovedInPixels(bounds.origin());
+    OnHostMovedInPixels();
   if (size_changed)
     OnHostResizedInPixels(bounds.size());
 }

ui/aura/window_tree_host.cc

@@ -660,13 +660,11 @@ void WindowTreeHost::OnAcceleratedWidgetAvailable() {
   }
 }
 
-void WindowTreeHost::OnHostMovedInPixels(
-    const gfx::Point& new_location_in_pixels) {
-  TRACE_EVENT1("ui", "WindowTreeHost::OnHostMovedInPixels", "origin",
-               new_location_in_pixels.ToString());
+void WindowTreeHost::OnHostMovedInPixels() {
+  TRACE_EVENT0("ui", "WindowTreeHost::OnHostMovedInPixels");
 
   for (WindowTreeHostObserver& observer : observers_)
-    observer.OnHostMovedInPixels(this, new_location_in_pixels);
+    observer.OnHostMovedInPixels(this);
 }
 
 void WindowTreeHost::OnHostResizedInPixels(

ui/aura/window_tree_host.h

@@ -342,7 +342,7 @@ class AURA_EXPORT WindowTreeHost : public ui::ImeKeyEventDispatcher,
   // Returns the location of the RootWindow on native screen.
   virtual gfx::Point GetLocationOnScreenInPixels() const = 0;
 
-  void OnHostMovedInPixels(const gfx::Point& new_location_in_pixels);
+  void OnHostMovedInPixels();
   void OnHostResizedInPixels(const gfx::Size& new_size_in_pixels);
   void OnHostWorkspaceChanged();
   void OnHostDisplayChanged();

ui/aura/window_tree_host_observer.h

@@ -12,10 +12,6 @@
 
 class SkRegion;
 
-namespace gfx {
-class Point;
-}
-
 namespace aura {
 class WindowTreeHost;
 
@@ -25,8 +21,7 @@ class AURA_EXPORT WindowTreeHostObserver {
   virtual void OnHostResized(WindowTreeHost* host) {}
 
   // Called when the host is moved on screen.
-  virtual void OnHostMovedInPixels(WindowTreeHost* host,
-                                   const gfx::Point& new_origin_in_pixels) {}
+  virtual void OnHostMovedInPixels(WindowTreeHost* host) {}
 
   // Called when the host is moved to a different workspace.
   virtual void OnHostWorkspaceChanged(WindowTreeHost* host) {}

ui/aura/window_tree_host_platform.cc

@@ -52,7 +52,7 @@ WindowTreeHostPlatform::WindowTreeHostPlatform(
     ui::PlatformWindowInitProperties properties,
     std::unique_ptr<Window> window)
     : WindowTreeHost(std::move(window)) {
-  bounds_in_pixels_ = properties.bounds;
+  size_in_pixels_ = properties.bounds.size();
   CreateCompositor(false, false, properties.enable_compositing_based_throttling,
                    properties.compositor_memory_limit_mb);
   CreateAndSetPlatformWindow(std::move(properties));
@@ -65,10 +65,10 @@ WindowTreeHostPlatform::WindowTreeHostPlatform(std::unique_ptr<Window> window)
 
 void WindowTreeHostPlatform::CreateAndSetPlatformWindow(
     ui::PlatformWindowInitProperties properties) {
-  // Cache initial bounds used to create |platform_window_| so that it does not
+  // Cache initial size used to create |platform_window_| so that it does not
   // end up propagating unneeded bounds change event when it is first notified
   // through OnBoundsChanged, which may lead to unneeded re-layouts, etc.
-  bounds_in_pixels_ = properties.bounds;
+  size_in_pixels_ = properties.bounds.size();
 #if defined(USE_OZONE)
   platform_window_ = ui::OzonePlatform::GetInstance()->CreatePlatformWindow(
       this, std::move(properties));
@@ -197,18 +197,18 @@ void WindowTreeHostPlatform::OnBoundsChanged(const BoundsChange& change) {
   }
   float current_scale = compositor()->device_scale_factor();
   float new_scale = ui::GetScaleFactorForNativeView(window());
-  gfx::Rect old_bounds = bounds_in_pixels_;
   auto weak_ref = GetWeakPtr();
-  bounds_in_pixels_ = change.bounds;
-  if (bounds_in_pixels_.origin() != old_bounds.origin()) {
-    OnHostMovedInPixels(bounds_in_pixels_.origin());
+  auto new_size = GetBoundsInPixels().size();
+  bool size_changed = size_in_pixels_ != new_size;
+  size_in_pixels_ = new_size;
+  if (change.origin_changed) {
+    OnHostMovedInPixels();
     // Changing the bounds may destroy this.
     if (!weak_ref)
       return;
   }
-  if (bounds_in_pixels_.size() != old_bounds.size() ||
-      current_scale != new_scale) {
-    OnHostResizedInPixels(bounds_in_pixels_.size());
+  if (size_changed || current_scale != new_scale) {
+    OnHostResizedInPixels(new_size);
     // Changing the size may destroy this.
     if (!weak_ref)
       return;

ui/aura/window_tree_host_platform.h

@@ -97,7 +97,8 @@ class AURA_EXPORT WindowTreeHostPlatform : public WindowTreeHost,
   gfx::AcceleratedWidget widget_;
   std::unique_ptr<ui::PlatformWindow> platform_window_;
   gfx::NativeCursor current_cursor_;
-  gfx::Rect bounds_in_pixels_;
+  // TODO: use compositor's size.
+  gfx::Size size_in_pixels_;
 
   std::unique_ptr<ui::KeyboardHook> keyboard_hook_;
 

ui/aura/window_tree_host_platform_unittest.cc

@@ -135,8 +135,7 @@ class DeleteHostWindowTreeHostObserver : public WindowTreeHostObserver {
   TestWindowTreeHost* host() { return host_.get(); }
 
   // WindowTreeHostObserver:
-  void OnHostMovedInPixels(WindowTreeHost* host,
-                           const gfx::Point& new_origin_in_pixels) override {
+  void OnHostMovedInPixels(WindowTreeHost* host) override {
     host_->RemoveObserver(this);
     host_.reset();
   }

ui/ozone/platform/drm/host/drm_window_host.cc

@@ -79,8 +79,9 @@ bool DrmWindowHost::IsVisible() const {
 void DrmWindowHost::PrepareForShutdown() {}
 
 void DrmWindowHost::SetBoundsInPixels(const gfx::Rect& bounds) {
+  bool origin_changed = bounds_.origin() != bounds.origin();
   bounds_ = bounds;
-  delegate_->OnBoundsChanged(bounds);
+  delegate_->OnBoundsChanged({origin_changed});
   SendBoundsChange();
 }
 

ui/ozone/platform/flatland/flatland_window.cc

@@ -328,11 +328,11 @@ void FlatlandWindow::UpdateSize() {
   const uint32_t width = view_properties_->width;
   const uint32_t height = view_properties_->height;
 
+  const gfx::Point old_origin = bounds_.origin();
   bounds_ = gfx::Rect(ceilf(width * device_pixel_ratio_),
                       ceilf(height * device_pixel_ratio_));
 
-  PlatformWindowDelegate::BoundsChange bounds;
-  bounds.bounds = bounds_;
+  PlatformWindowDelegate::BoundsChange bounds(old_origin != bounds_.origin());
   // TODO(fxbug.dev/93998): Calculate insets and update.
   window_delegate_->OnBoundsChanged(bounds);
 }

ui/ozone/platform/scenic/scenic_window.cc

@@ -395,6 +395,7 @@ void ScenicWindow::UpdateSize() {
   const float height = view_properties_->bounding_box.max.y -
                        view_properties_->bounding_box.min.y;
 
+  const gfx::Point old_origin = bounds_.origin();
   bounds_ = gfx::Rect(ceilf(width * device_pixel_ratio_),
                       ceilf(height * device_pixel_ratio_));
 
@@ -418,8 +419,7 @@ void ScenicWindow::UpdateSize() {
   // separately and we need to make sure our sizes change is committed.
   safe_presenter_.QueuePresent();
 
-  PlatformWindowDelegate::BoundsChange bounds;
-  bounds.bounds = bounds_;
+  PlatformWindowDelegate::BoundsChange bounds(old_origin != bounds_.origin());
   bounds.system_ui_overlap =
       ConvertInsets(device_pixel_ratio_, *view_properties_);
   delegate_->OnBoundsChanged(bounds);

ui/ozone/platform/wayland/host/wayland_window.cc

@@ -301,11 +301,12 @@ void WaylandWindow::SetBoundsInPixels(const gfx::Rect& bounds_px) {
   gfx::Rect adjusted_bounds_px = AdjustBoundsToConstraintsPx(bounds_px);
   if (bounds_px_ == adjusted_bounds_px)
     return;
+  bool origin_changed = bounds_px_.origin() != adjusted_bounds_px.origin();
   bounds_px_ = adjusted_bounds_px;
 
   if (update_visual_size_immediately_for_testing_)
     UpdateVisualSize(bounds_px.size(), window_scale());
-  delegate_->OnBoundsChanged(bounds_px_);
+  delegate_->OnBoundsChanged({origin_changed});
 }
 
 gfx::Rect WaylandWindow::GetBoundsInPixels() const {

ui/ozone/platform/wayland/host/wayland_window_drag_controller_unittest.cc

@@ -64,6 +64,7 @@ class WaylandWindowDragControllerTest : public WaylandDragDropTest {
   }
 
   MockWaylandPlatformWindowDelegate& delegate() { return delegate_; }
+  WaylandWindow* window() { return window_.get(); }
 
  protected:
   using State = WaylandWindowDragController::State;
@@ -215,10 +216,11 @@ TEST_P(WaylandWindowDragControllerTest, DragInsideWindowAndDrop) {
   });
 
   EXPECT_CALL(delegate_, OnBoundsChanged(_))
-      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& bounds) {
+      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& change) {
         EXPECT_EQ(State::kDetached, drag_controller()->state());
         EXPECT_EQ(kDragging, test_step);
-        EXPECT_EQ(gfx::Point(20, 20), bounds.bounds.origin());
+        EXPECT_EQ(gfx::Point(20, 20), window_->GetBoundsInDIP().origin());
+        EXPECT_TRUE(change.origin_changed);
 
         SendDndDrop();
         test_step = kDropping;
@@ -292,10 +294,11 @@ TEST_P(WaylandWindowDragControllerTest, DragInsideWindowAndDrop_TOUCH) {
   });
 
   EXPECT_CALL(delegate_, OnBoundsChanged(_))
-      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& bounds) {
+      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& change) {
         EXPECT_EQ(State::kDetached, drag_controller()->state());
         EXPECT_EQ(kDragging, test_step);
-        EXPECT_EQ(gfx::Point(20, 20), bounds.bounds.origin());
+        EXPECT_EQ(gfx::Point(20, 20), window_->GetBoundsInDIP().origin());
+        EXPECT_TRUE(change.origin_changed);
 
         SendDndDrop();
         test_step = kDropping;
@@ -448,10 +451,11 @@ TEST_P(WaylandWindowDragControllerTest, DragExitWindowAndDrop) {
   });
 
   EXPECT_CALL(delegate_, OnBoundsChanged(_))
-      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& bounds) {
+      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& change) {
         EXPECT_EQ(State::kDetached, drag_controller()->state());
         EXPECT_EQ(kDragging, test_step);
-        EXPECT_EQ(gfx::Point(20, 20), bounds.bounds.origin());
+        EXPECT_EQ(gfx::Point(20, 20), window_->GetBoundsInDIP().origin());
+        EXPECT_TRUE(change.origin_changed);
 
         SendDndLeave();
         SendDndDrop();
@@ -557,10 +561,11 @@ TEST_P(WaylandWindowDragControllerTest, DragToOtherWindowSnapDragDrop) {
   });
 
   EXPECT_CALL(delegate_, OnBoundsChanged(_))
-      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& bounds) {
+      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& change) {
         EXPECT_EQ(State::kDetached, drag_controller()->state());
         EXPECT_EQ(kDragging, test_step);
-        EXPECT_EQ(gfx::Point(50, 50), bounds.bounds.origin());
+        EXPECT_EQ(gfx::Point(50, 50), window_->GetBoundsInDIP().origin());
+        EXPECT_TRUE(change.origin_changed);
 
         // Exit |source_window| and enter the |target_window|.
         SendDndLeave();
@@ -687,10 +692,11 @@ TEST_P(WaylandWindowDragControllerTest, DragToOtherWindowSnapDragDrop_TOUCH) {
   } test_step = kStarted;
 
   EXPECT_CALL(delegate_, OnBoundsChanged(_))
-      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& bounds) {
+      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& change) {
         EXPECT_EQ(State::kDetached, drag_controller()->state());
         EXPECT_EQ(kDragging, test_step);
-        EXPECT_EQ(gfx::Point(50, 50), bounds.bounds.origin());
+        EXPECT_EQ(gfx::Point(50, 50), window_->GetBoundsInDIP().origin());
+        EXPECT_TRUE(change.origin_changed);
 
         // Exit |source_window| and enter the |target_window|.
         SendDndLeave();
@@ -858,18 +864,23 @@ TEST_P(WaylandWindowDragControllerTest, DragExitAttached_TOUCH) {
   Sync();
 }
 
+using BoundsChange = PlatformWindowDelegate::BoundsChange;
+
 TEST_P(WaylandWindowDragControllerTest, RestoreDuringWindowDragSession) {
   const gfx::Rect original_bounds = window_->GetBoundsInPixels();
   wl::ScopedWlArray states({XDG_TOPLEVEL_STATE_ACTIVATED});
 
   // Maximize and check restored bounds is correctly set.
   constexpr gfx::Rect kMaximizedBounds{1024, 768};
-  EXPECT_CALL(delegate_, OnBoundsChanged(testing::Eq(kMaximizedBounds)));
+  EXPECT_CALL(delegate_, OnBoundsChanged(testing::Eq(BoundsChange(false))));
   window_->Maximize();
   states.AddStateToWlArray(XDG_TOPLEVEL_STATE_MAXIMIZED);
   SendConfigureEvent(surface_->xdg_surface(), kMaximizedBounds.size(), 1,
                      states.get());
   Sync();
+
+  EXPECT_EQ(kMaximizedBounds, window_->GetBoundsInDIP());
+
   auto restored_bounds = window_->GetRestoredBoundsInDIP();
   EXPECT_EQ(original_bounds, restored_bounds);
 
@@ -970,10 +981,12 @@ TEST_P(WaylandWindowDragControllerTest, IgnorePointerEventsUntilDrop) {
   });
 
   EXPECT_CALL(delegate_, OnBoundsChanged(_))
-      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& bounds) {
+      .WillOnce([&](const PlatformWindowDelegate::BoundsChange& change) {
         EXPECT_EQ(State::kDetached, drag_controller()->state());
         EXPECT_EQ(kDragging, test_step);
-        EXPECT_EQ(gfx::Point(100, 100), bounds.bounds.origin());
+        EXPECT_TRUE(change.origin_changed);
+        EXPECT_EQ(gfx::Point(100, 100), window_->GetBoundsInDIP().origin());
+        EXPECT_TRUE(change.origin_changed);
 
         // Send a few wl_pointer::motion events skipping sync and dispatch
         // checks, which will be done at |kDropping| test step handling.
@@ -1034,7 +1047,9 @@ TEST_P(WaylandWindowDragControllerTest, MotionEventsSkippedWhileReattaching) {
     self->SendDndMotion({30, 30});
     EXPECT_CALL(self->delegate(), OnBoundsChanged(_))
         .WillOnce([&](const PlatformWindowDelegate::BoundsChange& change) {
-          EXPECT_EQ(gfx::Point(30, 30), change.bounds.origin());
+          EXPECT_EQ(gfx::Point(30, 30),
+                    self->window()->GetBoundsInDIP().origin());
+          EXPECT_TRUE(change.origin_changed);
         });
     self->Sync();