exo: Consider default scale cancellation in all bounds-related methods

This CL fixes SetBoundsSize and SetBoundsOrigin methods to take “default scale cancellation” into account.
If “default scale cancellation” is disabled, we expect a client to send bounds in pixels instead of dp, so these SetBounds* method should be sensitive to the flag.

Also, this CL adds an unittest for SetBounds methods in the case of default scale cancellation-enabled and -disabled.

BUG=b:167997508
TEST=SetBoundsWithAndWithoutDefaultScaleCancellation

Change-Id: Icfcaf445372c5eae0425226f802b27e7d7372bf4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2398055
Commit-Queue: Toshiki Kikuchi <toshikikikuchi@chromium.org>
Reviewed-by: Mitsuru Oshima (Slow: gardener) <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806035}

components/exo/client_controlled_shell_surface.cc

@@ -52,6 +52,8 @@
 #include "ui/compositor/compositor_lock.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
+#include "ui/gfx/geometry/point.h"
+#include "ui/gfx/geometry/size.h"
 #include "ui/views/widget/widget.h"
 #include "ui/wm/core/coordinate_conversion.h"
 #include "ui/wm/core/window_util.h"
@@ -321,8 +323,7 @@ ClientControlledShellSurface::ClientControlledShellSurface(
     bool default_scale_cancellation)
     : ShellSurfaceBase(surface, gfx::Point(), true, can_minimize, container),
       current_pin_(ash::WindowPinType::kNone),
-      use_default_scale_cancellation_(default_scale_cancellation) {
-}
+      use_default_scale_cancellation_(default_scale_cancellation) {}
 
 ClientControlledShellSurface::~ClientControlledShellSurface() {
   // Reset the window delegate here so that we won't try to do any dragging
@@ -346,31 +347,11 @@ void ClientControlledShellSurface::SetBounds(int64_t display_id,
     return;
   }
 
-  // Handle the case where we receive bounds from the client before the initial
-  // scale has been set.
-  if (pending_scale_ == 0.0) {
-    DCHECK(!use_default_scale_cancellation_);
-    display::Display display;
-    if (display::Screen::GetScreen()->GetDisplayWithDisplayId(display_id,
-                                                              &display)) {
-      SetScale(display.device_scale_factor());
-      CommitPendingScale();
-    }
-  }
-
-  float client_to_dp_scale = 1.f;
-  // When the client is scale-aware, we expect that it will resize windows when
-  // reacting to scale changes. Since we do not commit the scale until the
-  // buffer size changes, any bounds sent after a scale change and before the
-  // scale commit will result in mismatched sizes between widget and the buffer.
-  // To work around this, we use pending_scale_ to calculate bounds in DP
-  // instead of GetClientToDpScale().
-  if (!use_default_scale_cancellation_)
-    client_to_dp_scale = 1.f / pending_scale_;
-
-  gfx::Rect bounds_dp = gfx::ScaleToRoundedRect(bounds, client_to_dp_scale);
-
   SetDisplay(display_id);
+  EnsurePendingScale();
+
+  const gfx::Rect bounds_dp =
+      gfx::ScaleToRoundedRect(bounds, GetClientToDpPendingScale());
   SetGeometry(bounds_dp);
 }
 
@@ -378,7 +359,10 @@ void ClientControlledShellSurface::SetBoundsOrigin(const gfx::Point& origin) {
   TRACE_EVENT1("exo", "ClientControlledShellSurface::SetBoundsOrigin", "origin",
                origin.ToString());
 
-  pending_geometry_.set_origin(origin);
+  EnsurePendingScale();
+  const gfx::Point origin_dp =
+      gfx::ScaleToRoundedPoint(origin, GetClientToDpPendingScale());
+  pending_geometry_.set_origin(origin_dp);
 }
 
 void ClientControlledShellSurface::SetBoundsSize(const gfx::Size& size) {
@@ -390,8 +374,12 @@ void ClientControlledShellSurface::SetBoundsSize(const gfx::Size& size) {
     return;
   }
 
-  pending_geometry_.set_size(size);
+  EnsurePendingScale();
+  const gfx::Size size_dp =
+      gfx::ScaleToRoundedSize(size, GetClientToDpPendingScale());
+  pending_geometry_.set_size(size_dp);
 }
+
 void ClientControlledShellSurface::SetMaximized() {
   TRACE_EVENT0("exo", "ClientControlledShellSurface::SetMaximized");
   pending_window_state_ = ash::WindowStateType::kMaximized;
@@ -1394,6 +1382,30 @@ const ash::NonClientFrameViewAsh* ClientControlledShellSurface::GetFrameView()
       widget_->non_client_view()->frame_view());
 }
 
+void ClientControlledShellSurface::EnsurePendingScale() {
+  // Handle the case where we receive bounds from the client before the initial
+  // scale has been set.
+  if (pending_scale_ == 0.0) {
+    DCHECK(!use_default_scale_cancellation_);
+    display::Display display;
+    if (display::Screen::GetScreen()->GetDisplayWithDisplayId(
+            pending_display_id_, &display)) {
+      SetScale(display.device_scale_factor());
+      CommitPendingScale();
+    }
+  }
+}
+
+float ClientControlledShellSurface::GetClientToDpPendingScale() const {
+  // When the client is scale-aware, we expect that it will resize windows when
+  // reacting to scale changes. Since we do not commit the scale until the
+  // buffer size changes, any bounds sent after a scale change and before the
+  // scale commit will result in mismatched sizes between widget and the buffer.
+  // To work around this, we use pending_scale_ to calculate bounds in DP
+  // instead of GetClientToDpScale().
+  return use_default_scale_cancellation_ ? 1.f : 1.f / pending_scale_;
+}
+
 // static
 void ClientControlledShellSurface::
     SetClientControlledStateDelegateFactoryForTest(

components/exo/client_controlled_shell_surface.h

@@ -311,6 +311,9 @@ class ClientControlledShellSurface : public ShellSurfaceBase,
   ash::NonClientFrameViewAsh* GetFrameView();
   const ash::NonClientFrameViewAsh* GetFrameView() const;
 
+  void EnsurePendingScale();
+  float GetClientToDpPendingScale() const;
+
   GeometryChangedCallback geometry_changed_callback_;
 
   int top_inset_height_ = 0;

components/exo/client_controlled_shell_surface_unittest.cc

@@ -1820,6 +1820,58 @@ TEST_F(ClientControlledShellSurfaceTest, SetBoundsReparentsToDisplay) {
   EXPECT_EQ(secondary_display.id(), display.id());
 }
 
+// Test if the surface bounds is correctly set when default scale cancellation
+// is enabled or disabled.
+TEST_F(ClientControlledShellSurfaceTest,
+       SetBoundsWithAndWithoutDefaultScaleCancellation) {
+  UpdateDisplay("800x600*2");
+
+  const auto primary_display_id =
+      display::Screen::GetScreen()->GetPrimaryDisplay().id();
+
+  const gfx::Size buffer_size(64, 64);
+  std::unique_ptr<Buffer> buffer(
+      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
+
+  const gfx::Rect bounds(64, 64, 128, 128);
+  const gfx::Rect bounds_dp = gfx::ScaleToRoundedRect(bounds, 1.f / 2.f);
+
+  for (const auto default_scale_cancellation : {true, false}) {
+    const auto bounds_to_set = default_scale_cancellation ? bounds_dp : bounds;
+
+    {
+      // Set display id, bounds origin, bounds size at the same time via
+      // SetBounds method.
+      std::unique_ptr<Surface> surface(new Surface);
+      auto shell_surface(exo_test_helper()->CreateClientControlledShellSurface(
+          surface.get(), /*is_modal=*/false, default_scale_cancellation));
+
+      shell_surface->SetBounds(primary_display_id, bounds_to_set);
+      surface->Attach(buffer.get());
+      surface->Commit();
+
+      EXPECT_EQ(bounds_dp,
+                shell_surface->GetWidget()->GetWindowBoundsInScreen());
+    }
+
+    {
+      // Set display id, bounds origin, bounds size separately.
+      std::unique_ptr<Surface> surface(new Surface);
+      auto shell_surface(exo_test_helper()->CreateClientControlledShellSurface(
+          surface.get(), /*is_modal=*/false, default_scale_cancellation));
+
+      shell_surface->SetDisplay(primary_display_id);
+      shell_surface->SetBoundsOrigin(bounds_to_set.origin());
+      shell_surface->SetBoundsSize(bounds_to_set.size());
+      surface->Attach(buffer.get());
+      surface->Commit();
+
+      EXPECT_EQ(bounds_dp,
+                shell_surface->GetWidget()->GetWindowBoundsInScreen());
+    }
+  }
+}
+
 // Set orientation lock to a window.
 TEST_F(ClientControlledShellSurfaceTest, SetOrientationLock) {
   display::test::DisplayManagerTestApi(ash::Shell::Get()->display_manager())