snap-group: Update snap ratio logic to accommodate for split divider

This CL updates the snap ratio logic calculation and takes split
view divider into consideration if exits, i.e. half of the divider
shorter side length needs to be added to the window bounds width/
height(depending on the orientation) when divided by the work
area width/height.

Fixed: b/272384615
Test: Modified unit tests
Change-Id: I1e392e2751cab12d390c0b5598a88c3250aa786a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4344070
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Michele Fan <michelefan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1118281}

ash/wm/snap_group/snap_group_unittest.cc

@@ -262,6 +262,8 @@ class SnapGroupEntryPointArm1Test : public SnapGroupTest {
 
     // The split view divider will show on two windows snapped.
     EXPECT_TRUE(split_view_divider());
+    EXPECT_EQ(0.5f, *WindowState::Get(window1)->snap_ratio());
+    EXPECT_EQ(0.5f, *WindowState::Get(window2)->snap_ratio());
   }
 
  private:
@@ -317,13 +319,15 @@ TEST_F(SnapGroupEntryPointArm1Test,
 
   const gfx::Point hover_location =
       split_view_divider_bounds_in_screen().CenterPoint();
-  const int distance_delta = work_area_bounds.width() / 6;
+  const int distance_delta = work_area_bounds.width() / 4;
   event_generator->MoveMouseTo(hover_location);
   event_generator->PressLeftButton();
   event_generator->MoveMouseTo(hover_location.x() - distance_delta,
                                hover_location.y());
   event_generator->ReleaseLeftButton();
   EXPECT_TRUE(split_view_controller()->InSplitViewMode());
+  EXPECT_EQ(0.25f, WindowState::Get(w1.get())->snap_ratio());
+  EXPECT_EQ(0.75f, WindowState::Get(w2.get())->snap_ratio());
 }
 
 // Tests that when snapping a snapped window to the same snapped state, the

ash/wm/splitview/split_view_controller_unittest.cc

@@ -3232,34 +3232,39 @@ TEST_F(SplitViewControllerTest, SnapBetweenDifferentRatios) {
   WMEvent snap_secondary_default(WM_EVENT_SNAP_SECONDARY);
   WindowState::Get(window2.get())->OnWMEvent(&snap_secondary_default);
 
-  // Test that the divider position and both window bounds are at half the
-  // work area width.
+  // Test that both window bounds are at half the work area width and that the
+  // divider is positioned at half of the work area width minus the
+  // `divider_delta`.
   const gfx::Rect work_area_bounds =
       display::Screen::GetScreen()->GetPrimaryDisplay().work_area();
-  gfx::Rect divider_bounds = split_view_divider()->GetDividerBoundsInScreen(
-      /*is_dragging=*/false);
-  ASSERT_NEAR(divider_bounds.x(), work_area_bounds.width() * 0.5f,
-              divider_bounds.width());
-  ASSERT_NEAR(work_area_bounds.width() * 0.5f, window1->bounds().width(),
-              divider_bounds.width());
-  ASSERT_NEAR(work_area_bounds.width() * 0.5f, window2->bounds().x(),
-              divider_bounds.width());
+  int divider_origin_x = split_view_divider()
+                             ->GetDividerBoundsInScreen(
+                                 /*is_dragging=*/false)
+                             .x();
+  const int divider_delta = kSplitviewDividerShortSideLength / 2;
+  EXPECT_EQ(divider_origin_x, work_area_bounds.width() * 0.5f - divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.5f,
+            window1->bounds().width() + divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.5f,
+            window2->bounds().width() + divider_delta);
 
   // Snap `window1`, still in primary position, but with two thirds snap ratio.
   WMEvent snap_primary_two_third(WM_EVENT_SNAP_PRIMARY,
                                  chromeos::kTwoThirdSnapRatio);
   WindowState::Get(window1.get())->OnWMEvent(&snap_primary_two_third);
 
-  // Test that the divider position and both window bounds have updated to two
-  // thirds the work area width.
-  divider_bounds = split_view_divider()->GetDividerBoundsInScreen(
-      /*is_dragging=*/false);
-  ASSERT_NEAR(divider_bounds.x(), work_area_bounds.width() * 0.67f,
-              divider_bounds.width());
-  ASSERT_NEAR(work_area_bounds.width() * 0.67f, window1->bounds().width(),
-              divider_bounds.width());
-  ASSERT_NEAR(work_area_bounds.width() * 0.67f, window2->bounds().x(),
-              divider_bounds.width());
+  // Test that the window bounds have updated to two thirds and one third of the
+  // work area width respectively. The the divider is positioned at two thirds
+  // of the work area width minus the `divider_delta`.
+  divider_origin_x = split_view_divider()
+                         ->GetDividerBoundsInScreen(
+                             /*is_dragging=*/false)
+                         .x();
+  EXPECT_EQ(divider_origin_x, work_area_bounds.width() * 0.67f - divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.67f,
+            window1->bounds().width() + divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.33f,
+            window2->bounds().width() + divider_delta);
 }
 
 // Tests that swap partial windows keeps the window sizes.
@@ -3277,14 +3282,16 @@ TEST_F(SplitViewControllerTest, SwapPartialWindows) {
   WindowState::Get(window2.get())->OnWMEvent(&snap_secondary_one_third);
   const gfx::Rect work_area_bounds =
       display::Screen::GetScreen()->GetPrimaryDisplay().work_area();
-  gfx::Rect divider_bounds = split_view_divider()->GetDividerBoundsInScreen(
-      /*is_dragging=*/false);
-  ASSERT_NEAR(divider_bounds.x(), work_area_bounds.width() * 0.67f,
-              divider_bounds.width());
-  ASSERT_NEAR(work_area_bounds.width() * 0.67f, window1->bounds().width(),
-              divider_bounds.width());
-  ASSERT_NEAR(work_area_bounds.width() * 0.33f, window2->bounds().width(),
-              divider_bounds.width());
+  int divider_origin_x = split_view_divider()
+                             ->GetDividerBoundsInScreen(
+                                 /*is_dragging=*/false)
+                             .x();
+  const int divider_delta = kSplitviewDividerShortSideLength / 2;
+  EXPECT_EQ(divider_origin_x, work_area_bounds.width() * 0.67f - divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.67f,
+            window1->bounds().width() + divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.33f,
+            window2->bounds().width() + divider_delta);
 
   // Verify that after swapping windows, the window widths remain the same, and
   // the divider is now at 1/3 of the work area.
@@ -3293,14 +3300,15 @@ TEST_F(SplitViewControllerTest, SwapPartialWindows) {
             chromeos::WindowStateType::kSecondarySnapped);
   EXPECT_EQ(WindowState::Get(window2.get())->GetStateType(),
             chromeos::WindowStateType::kPrimarySnapped);
-  divider_bounds = split_view_divider()->GetDividerBoundsInScreen(
-      /*is_dragging=*/false);
-  ASSERT_NEAR(divider_bounds.x(), work_area_bounds.width() * 0.33f,
-              divider_bounds.width());
-  ASSERT_NEAR(work_area_bounds.width() * 0.67f, window1->bounds().width(),
-              divider_bounds.width());
-  ASSERT_NEAR(work_area_bounds.width() * 0.33f, window2->bounds().width(),
-              divider_bounds.width());
+  divider_origin_x = split_view_divider()
+                         ->GetDividerBoundsInScreen(
+                             /*is_dragging=*/false)
+                         .x();
+  EXPECT_EQ(divider_origin_x, work_area_bounds.width() * 0.33f - divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.67f,
+            window1->bounds().width() + divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.33f,
+            window2->bounds().width() + divider_delta);
 }
 
 // Tests that, if two windows are snapped and one window has min size, trying to
@@ -3338,10 +3346,11 @@ TEST_F(SplitViewControllerTest, SnapWindowWithMinSizeOpensOverview) {
   wm::ActivateWindow(window2.get());
   EXPECT_EQ(split_view_controller()->state(),
             SplitViewController::State::kBothSnapped);
-  ASSERT_NEAR(work_area_bounds.width() * 0.5f, window1->bounds().width(),
-              kSplitviewDividerShortSideLength / 2);
-  ASSERT_NEAR(work_area_bounds.width() * 0.5f, window2->bounds().width(),
-              kSplitviewDividerShortSideLength / 2);
+  const int divider_delta = kSplitviewDividerShortSideLength / 2;
+  EXPECT_EQ(work_area_bounds.width() * 0.5f,
+            window1->bounds().width() + divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.5f,
+            window2->bounds().width() + divider_delta);
 }
 
 // Tests that auto-snap for partial windows works correctly.
@@ -3361,10 +3370,11 @@ TEST_F(SplitViewControllerTest, AutoSnapPartialWindows) {
           window1.get());
   EXPECT_EQ(split_view_controller()->state(),
             SplitViewController::State::kBothSnapped);
-  ASSERT_NEAR(work_area_bounds.width() * 0.67f, window1->bounds().width(),
-              kSplitviewDividerShortSideLength / 2);
-  ASSERT_NEAR(work_area_bounds.width() * 0.33f, window2->bounds().width(),
-              kSplitviewDividerShortSideLength / 2);
+  const int divider_delta = kSplitviewDividerShortSideLength / 2;
+  EXPECT_EQ(work_area_bounds.width() * 0.67f,
+            window1->bounds().width() + divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.33f,
+            window2->bounds().width() + divider_delta);
   EndSplitView();
 
   // 2. Test with min size. Set `window2` min length so that it can't fit in 1/3
@@ -3379,10 +3389,10 @@ TEST_F(SplitViewControllerTest, AutoSnapPartialWindows) {
   wm::ActivateWindow(window2.get());
   EXPECT_EQ(split_view_controller()->state(),
             SplitViewController::State::kBothSnapped);
-  ASSERT_NEAR(work_area_bounds.width() * 0.5f, window1->bounds().width(),
-              kSplitviewDividerShortSideLength / 2);
-  ASSERT_NEAR(work_area_bounds.width() * 0.5f, window2->bounds().width(),
-              kSplitviewDividerShortSideLength / 2);
+  EXPECT_EQ(work_area_bounds.width() * 0.5f,
+            window1->bounds().width() + divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.5f,
+            window2->bounds().width() + divider_delta);
 }
 
 TEST_F(SplitViewControllerTest, WMSnapEventDeviceOrientationMetricsInTablet) {

ash/wm/tablet_mode/tablet_mode_multitask_menu_event_handler_unittest.cc

@@ -356,13 +356,9 @@ TEST_F(TabletModeMultitaskMenuEventHandlerTest, HalfButtonFunctionality) {
             WindowState::Get(window.get())->GetStateType());
   const gfx::Rect work_area_bounds =
       display::Screen::GetScreen()->GetPrimaryDisplay().work_area();
-  const gfx::Rect divider_bounds =
-      SplitViewController::Get(Shell::GetPrimaryRootWindow())
-          ->split_view_divider()
-          ->GetDividerBoundsInScreen(
-              /*is_dragging*/ false);
-  ASSERT_NEAR(work_area_bounds.width() * 0.5f,
-              window->GetBoundsInScreen().width(), divider_bounds.width());
+  EXPECT_EQ(work_area_bounds.width() * 0.5f,
+            window->GetBoundsInScreen().width() +
+                kSplitviewDividerShortSideLength / 2);
 
   // Verify that the multitask menu has been closed.
   ASSERT_FALSE(GetMultitaskMenu());
@@ -387,13 +383,9 @@ TEST_F(TabletModeMultitaskMenuEventHandlerTest, PartialButtonFunctionality) {
             WindowState::Get(window.get())->GetStateType());
   const gfx::Rect work_area_bounds =
       display::Screen::GetScreen()->GetPrimaryDisplay().work_area();
-  const gfx::Rect divider_bounds =
-      SplitViewController::Get(Shell::GetPrimaryRootWindow())
-          ->split_view_divider()
-          ->GetDividerBoundsInScreen(
-              /*is_dragging*/ false);
-  ASSERT_NEAR(work_area_bounds.width() * 0.67f, window->bounds().width(),
-              divider_bounds.width());
+  const int divider_delta = kSplitviewDividerShortSideLength / 2;
+  EXPECT_EQ(work_area_bounds.width() * 0.67f,
+            window->bounds().width() + divider_delta);
   ASSERT_FALSE(GetMultitaskMenu());
   histogram_tester_.ExpectBucketCount(
       chromeos::GetActionTypeHistogramName(),
@@ -403,8 +395,8 @@ TEST_F(TabletModeMultitaskMenuEventHandlerTest, PartialButtonFunctionality) {
   PressPartialSecondary(*window);
   ASSERT_EQ(chromeos::WindowStateType::kSecondarySnapped,
             WindowState::Get(window.get())->GetStateType());
-  ASSERT_NEAR(work_area_bounds.width() * 0.33f, window->bounds().width(),
-              divider_bounds.width());
+  EXPECT_EQ(work_area_bounds.width() * 0.33f,
+            window->bounds().width() + divider_delta);
   ASSERT_FALSE(GetMultitaskMenu());
   histogram_tester_.ExpectBucketCount(
       chromeos::GetActionTypeHistogramName(),
@@ -427,8 +419,8 @@ TEST_F(TabletModeMultitaskMenuEventHandlerTest, AdjustedMenuBounds) {
   // Test that the menu fits on the 1/3 window on the right.
   const gfx::Rect work_area =
       display::Screen::GetScreen()->GetPrimaryDisplay().work_area();
-  ASSERT_NEAR(work_area.width() * 0.33f, window2->bounds().width(),
-              kSplitviewDividerShortSideLength);
+  EXPECT_EQ(work_area.width() * 0.33f,
+            window2->bounds().width() + kSplitviewDividerShortSideLength / 2);
   ShowMultitaskMenu(*window2);
   ASSERT_TRUE(GetMultitaskMenu());
   EXPECT_EQ(work_area.right(),

ash/wm/tablet_mode/tablet_mode_window_manager_unittest.cc

@@ -1775,30 +1775,32 @@ TEST_F(TabletModeWindowManagerTest,
 // Tests partial split clamshell <-> tablet transition.
 TEST_F(TabletModeWindowManagerTest, PartialClamshellTabletTransitionTest) {
   // 1. Create a window and snap to primary 2/3.
-  auto window = CreateTestWindow();
+  auto window1 = CreateTestWindow();
   OverviewController* overview_controller = Shell::Get()->overview_controller();
   const WMEvent snap_primary_two_third(WM_EVENT_SNAP_PRIMARY,
                                        chromeos::kTwoThirdSnapRatio);
-  WindowState::Get(window.get())->OnWMEvent(&snap_primary_two_third);
-  // Enter tablet mode and verify that overview is open and the window and
+  WindowState::Get(window1.get())->OnWMEvent(&snap_primary_two_third);
+  // Enter tablet mode and verify that overview opens and the window and
   // divider are at 2/3.
   CreateTabletModeWindowManager();
   EXPECT_TRUE(overview_controller->InOverviewSession());
-  EXPECT_TRUE(split_view_controller()->IsWindowInSplitView(window.get()));
+  EXPECT_TRUE(split_view_controller()->IsWindowInSplitView(window1.get()));
   const gfx::Rect work_area_bounds =
       display::Screen::GetScreen()->GetPrimaryDisplay().work_area();
-  gfx::Rect divider_bounds =
-      split_view_controller()->split_view_divider()->GetDividerBoundsInScreen(
-          /*is_dragging=*/false);
-  ASSERT_NEAR(work_area_bounds.width() * 0.67f, window->bounds().width(),
-              divider_bounds.width());
-  ASSERT_NEAR(work_area_bounds.width() * 0.67f, divider_bounds.x(),
-              divider_bounds.width());
+  int divider_origin_x = split_view_controller()
+                             ->split_view_divider()
+                             ->GetDividerBoundsInScreen(
+                                 /*is_dragging=*/false)
+                             .x();
+  const int divider_delta = kSplitviewDividerShortSideLength / 2;
+  EXPECT_EQ(work_area_bounds.width() * 0.67f,
+            window1->bounds().width() + divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.67f, divider_origin_x + divider_delta);
   // Exit tablet mode and verify the window stays in the same position.
   DestroyTabletModeWindowManager();
-  EXPECT_EQ(work_area_bounds.width() * 0.67f, window->bounds().width());
+  EXPECT_EQ(work_area_bounds.width() * 0.67f, window1->bounds().width());
 
-  // 2. Create another window and snap to secondary 1/3.
+  // 2. Create another window and snap to secondary at 1/3.
   auto window2 = CreateTestWindow();
   const WMEvent snap_secondary_one_third(WM_EVENT_SNAP_SECONDARY,
                                          chromeos::kOneThirdSnapRatio);
@@ -1807,24 +1809,25 @@ TEST_F(TabletModeWindowManagerTest, PartialClamshellTabletTransitionTest) {
   // Enter tablet mode and verify the windows are in splitview and the window
   // bounds and divider are at 2/3.
   CreateTabletModeWindowManager();
-  EXPECT_TRUE(split_view_controller()->IsWindowInSplitView(window.get()));
+  EXPECT_TRUE(split_view_controller()->IsWindowInSplitView(window1.get()));
   EXPECT_TRUE(split_view_controller()->IsWindowInSplitView(window2.get()));
-  divider_bounds =
-      split_view_controller()->split_view_divider()->GetDividerBoundsInScreen(
-          /*is_dragging=*/false);
-  ASSERT_NEAR(work_area_bounds.width() * 0.67f, window->bounds().width(),
-              divider_bounds.width() / 2);
-  ASSERT_NEAR(work_area_bounds.width() * 0.33f, window2->bounds().width(),
-              divider_bounds.width() / 2);
-  ASSERT_NEAR(work_area_bounds.width() * 0.67f, divider_bounds.x(),
-              divider_bounds.width() / 2);
+  divider_origin_x = split_view_controller()
+                         ->split_view_divider()
+                         ->GetDividerBoundsInScreen(
+                             /*is_dragging=*/false)
+                         .x();
+
+  EXPECT_EQ(work_area_bounds.width() * 0.67f,
+            window1->bounds().width() + divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.33f,
+            window2->bounds().width() + divider_delta);
+  EXPECT_EQ(work_area_bounds.width() * 0.67f - divider_delta, divider_origin_x);
+
   // Exit tablet mode and verify the windows are still at 2/3, with allowance
   // for the divider width since it is only there in tablet mode.
   DestroyTabletModeWindowManager();
-  EXPECT_NEAR(work_area_bounds.width() * 0.67f, window->bounds().width(),
-              divider_bounds.width() / 2);
-  EXPECT_NEAR(work_area_bounds.width() * 0.33f, window2->bounds().width(),
-              divider_bounds.width() / 2);
+  EXPECT_EQ(work_area_bounds.width() * 0.67f, window1->bounds().width());
+  EXPECT_EQ(work_area_bounds.width() * 0.33f, window2->bounds().width());
 }
 
 // Test that when switching from clamshell mode to tablet mode, if overview mode

ash/wm/window_state.cc

@@ -26,6 +26,8 @@
 #include "ash/wm/desks/persistent_desks_bar/persistent_desks_bar_controller.h"
 #include "ash/wm/float/float_controller.h"
 #include "ash/wm/pip/pip_positioner.h"
+#include "ash/wm/splitview/split_view_constants.h"
+#include "ash/wm/splitview/split_view_controller.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
 #include "ash/wm/window_animations.h"
 #include "ash/wm/window_positioning_utils.h"
@@ -232,14 +234,27 @@ WMEventType WMEventTypeFromShowState(ui::WindowShowState requested_show_state) {
   return WM_EVENT_NORMAL;
 }
 
+// Returns true if the split view divider exits which should be taken into
+// consideration when calculating the snap ratio.
+bool ShouldConsiderDivider(aura::Window* window) {
+  SplitViewController* split_view_controller =
+      SplitViewController::Get(window->GetRootWindow());
+  return split_view_controller->InSplitViewMode() &&
+         split_view_controller->split_view_divider();
+}
+
 float GetCurrentSnapRatio(aura::Window* window) {
   gfx::Rect maximized_bounds =
       screen_util::GetMaximizedWindowBoundsInParent(window);
+  const int divider_delta =
+      ShouldConsiderDivider(window) ? kSplitviewDividerShortSideLength / 2 : 0;
   if (SplitViewController::IsLayoutHorizontal(window)) {
-    return static_cast<float>(window->GetTargetBounds().width()) /
+    return static_cast<float>(window->GetTargetBounds().width() +
+                              divider_delta) /
            static_cast<float>(maximized_bounds.width());
   }
-  return static_cast<float>(window->GetTargetBounds().height()) /
+  return static_cast<float>(window->GetTargetBounds().height() +
+                            divider_delta) /
          static_cast<float>(maximized_bounds.height());
 }