Fix client bounds on snapping

Before this change, when window is transitioning from maximized to
snapped in a clamshell mode, the new bounds sent to the client included
the caption height. But actually it shouldn't.
The same issue is also observed for a snapped window in a tablet mode is
transitioning to in a clamshell mode.

This change fixes these bounds to have a correct client bounds.

Bug: b:171283459
Test: ClientControlledShellSurfaceTest
Change-Id: I9b566d17dd6e11c478b26c336b77e43cb49ca2e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2494290
Commit-Queue: Hiroki Sato <hirokisato@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822594}

ash/frame/non_client_frame_view_ash.cc

@@ -377,6 +377,10 @@ int NonClientFrameViewAsh::NonClientTopBorderHeight() const {
   return header_view_->GetPreferredHeight();
 }
 
+int NonClientFrameViewAsh::NonClientTopBorderPreferredHeight() const {
+  return header_view_->GetPreferredHeight();
+}
+
 const views::View* NonClientFrameViewAsh::GetAvatarIconViewForTest() const {
   return header_view_->avatar_icon();
 }

ash/frame/non_client_frame_view_ash.h

@@ -99,6 +99,10 @@ class ASH_EXPORT NonClientFrameViewAsh : public views::NonClientFrameView {
   // Height from top of window to top of client area.
   int NonClientTopBorderHeight() const;
 
+  // Expected height from top of window to top of client area when non client
+  // view is visible.
+  int NonClientTopBorderPreferredHeight() const;
+
   const views::View* GetAvatarIconViewForTest() const;
 
   SkColor GetActiveFrameColorForTest() const;

components/exo/client_controlled_shell_surface.cc

@@ -35,6 +35,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/traced_value.h"
+#include "chromeos/ui/base/tablet_state.h"
 #include "chromeos/ui/base/window_pin_type.h"
 #include "chromeos/ui/base/window_properties.h"
 #include "chromeos/ui/base/window_state_type.h"
@@ -53,6 +54,7 @@
 #include "ui/compositor/compositor_lock.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
+#include "ui/display/tablet_state.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/views/widget/widget.h"
@@ -663,21 +665,10 @@ void ClientControlledShellSurface::OnBoundsChangeEvent(
 
   // Sends the client bounds, which matches the geometry
   // when frame is enabled.
-  ash::NonClientFrameViewAsh* frame_view = GetFrameView();
+  const gfx::Rect client_bounds = GetClientBoundsForWindowBoundsAndWindowState(
+      window_bounds, requested_state);
 
-  // The client's geometry uses fullscreen in client controlled,
-  // (but the surface is placed under the frame), so just use
-  // the window bounds instead for maximixed state.
-  // Snapped window states in tablet mode do not include the caption height.
-  const bool becoming_snapped =
-      requested_state == chromeos::WindowStateType::kLeftSnapped ||
-      requested_state == chromeos::WindowStateType::kRightSnapped;
-  const bool is_tablet_mode = WMHelper::GetInstance()->InTabletMode();
-  gfx::Rect client_bounds =
-      widget_->IsMaximized() || (becoming_snapped && is_tablet_mode)
-          ? window_bounds
-          : frame_view->GetClientBoundsForWindowBounds(window_bounds);
-  gfx::Size current_size = frame_view->GetBoundsForClientView().size();
+  gfx::Size current_size = GetFrameView()->GetBoundsForClientView().size();
   bool is_resize = client_bounds.size() != current_size &&
                    !widget_->IsMaximized() && !widget_->IsFullscreen();
 
@@ -1413,6 +1404,39 @@ float ClientControlledShellSurface::GetClientToDpPendingScale() const {
   return use_default_scale_cancellation_ ? 1.f : 1.f / pending_scale_;
 }
 
+gfx::Rect
+ClientControlledShellSurface::GetClientBoundsForWindowBoundsAndWindowState(
+    const gfx::Rect& window_bounds,
+    chromeos::WindowStateType window_state) const {
+  // The client's geometry uses fullscreen in client controlled,
+  // (but the surface is placed under the frame), so just use
+  // the window bounds instead for maximixed state.
+  // Snapped window states in tablet mode do not include the caption height.
+  const bool is_snapped =
+      window_state == chromeos::WindowStateType::kLeftSnapped ||
+      window_state == chromeos::WindowStateType::kRightSnapped;
+  const bool is_maximized =
+      window_state == chromeos::WindowStateType::kMaximized;
+  const display::TabletState tablet_state =
+      chromeos::TabletState::Get()->state();
+  const bool is_tablet_mode = WMHelper::GetInstance()->InTabletMode();
+
+  if (is_maximized || (is_snapped && is_tablet_mode))
+    return window_bounds;
+
+  gfx::Rect client_bounds =
+      GetFrameView()->GetClientBoundsForWindowBounds(window_bounds);
+
+  if (is_snapped && tablet_state == display::TabletState::kExitingTabletMode) {
+    // Until the next commit, the frame view is in immersive mode, and the above
+    // GetClientBoundsForWindowBounds doesn't return bounds taking the caption
+    // height into account.
+    client_bounds.Inset(0, GetFrameView()->NonClientTopBorderPreferredHeight(),
+                        0, 0);
+  }
+  return client_bounds;
+}
+
 // static
 void ClientControlledShellSurface::
     SetClientControlledStateDelegateFactoryForTest(

components/exo/client_controlled_shell_surface.h

@@ -317,6 +317,10 @@ class ClientControlledShellSurface : public ShellSurfaceBase,
   void EnsurePendingScale();
   float GetClientToDpPendingScale() const;
 
+  gfx::Rect GetClientBoundsForWindowBoundsAndWindowState(
+      const gfx::Rect& window_bounds,
+      chromeos::WindowStateType window_state) const;
+
   GeometryChangedCallback geometry_changed_callback_;
 
   int top_inset_height_ = 0;

components/exo/client_controlled_shell_surface_unittest.cc

@@ -2613,4 +2613,72 @@ TEST_F(ClientControlledShellSurfaceScaleTest,
   EXPECT_GE(bounds_change_count(), 1);
 }
 
+TEST_F(ClientControlledShellSurfaceTest, SnappedClientBounds) {
+  UpdateDisplay("800x600");
+
+  gfx::Size buffer_size(256, 256);
+  std::unique_ptr<Buffer> buffer(
+      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
+
+  std::unique_ptr<Surface> surface(new Surface);
+  auto shell_surface =
+      exo_test_helper()->CreateClientControlledShellSurface(surface.get());
+
+  // Clear insets so that it won't affects the bounds.
+  shell_surface->SetSystemUiVisibility(true);
+  int64_t display_id = display::Screen::GetScreen()->GetPrimaryDisplay().id();
+  ash::Shell::Get()->display_manager()->UpdateWorkAreaOfDisplay(
+      display_id, gfx::Insets(0, 0, 0, 0));
+
+  gfx::Rect requested_bounds;
+  shell_surface->set_bounds_changed_callback(base::BindRepeating(
+      [](gfx::Rect* dst, WindowStateType current_state,
+         WindowStateType requested_state, int64_t display_id,
+         const gfx::Rect& bounds, bool is_resize,
+         int bounds_change) { *dst = bounds; },
+      base::Unretained(&requested_bounds)));
+
+  surface->Attach(buffer.get());
+  surface->Commit();
+  views::Widget* widget = shell_surface->GetWidget();
+  aura::Window* window = widget->GetNativeWindow();
+
+  // Normal state -> Snap.
+  shell_surface->SetGeometry(gfx::Rect(50, 100, 200, 300));
+  surface->SetFrame(SurfaceFrameType::NORMAL);
+  surface->Commit();
+  EXPECT_EQ(gfx::Rect(50, 68, 200, 332), widget->GetWindowBoundsInScreen());
+
+  ash::WMEvent event(ash::WM_EVENT_SNAP_LEFT);
+  ash::WindowState::Get(window)->OnWMEvent(&event);
+  EXPECT_EQ(gfx::Rect(0, 32, 400, 568), requested_bounds);
+
+  // Maximized -> Snap.
+  shell_surface->SetMaximized();
+  shell_surface->SetGeometry(gfx::Rect(0, 0, 800, 568));
+  surface->Commit();
+  EXPECT_TRUE(widget->IsMaximized());
+
+  ash::WindowState::Get(window)->OnWMEvent(&event);
+  EXPECT_EQ(gfx::Rect(0, 32, 400, 568), requested_bounds);
+  shell_surface->SetSnappedToLeft();
+  shell_surface->SetGeometry(gfx::Rect(0, 0, 400, 568));
+  surface->Commit();
+
+  // Clamshell mode -> tablet mode. The bounds start from top-left corner.
+  EnableTabletMode(true);
+  EXPECT_EQ(gfx::Rect(0, 0, 396, 564), requested_bounds);
+  shell_surface->SetGeometry(gfx::Rect(0, 0, 396, 568));
+  surface->SetFrame(SurfaceFrameType::AUTOHIDE);
+  surface->Commit();
+
+  // Tablet mode -> clamshell mode. Top caption height should be reserved.
+  EnableTabletMode(false);
+  EXPECT_EQ(gfx::Rect(0, 32, 396, 568), requested_bounds);
+
+  // Clean up state.
+  shell_surface->SetSnappedToLeft();
+  surface->Commit();
+}
+
 }  // namespace exo