snap-group: Get the closest fixed snap ratio converting to tablet

There are fixed snap ratios in tablet mode, whereas the divider can be at arbitrary positions after resizing the snap group in clamshell mode. When transiting to tablet mode, the arbitrary position needs to be converted to its closest fixed snap ratio. This cl implements and verifies the behavior described above.

Demo: http://b/294444834#comment3
Fixed: b/294444834
Test: Added unit test + manual
Change-Id: I88c519c84338036144f3112e063114213f0e0b95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4796542
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Michele Fan <michelefan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1188603}

ash/wm/snap_group/snap_group_controller.cc

@@ -214,8 +214,6 @@ void SnapGroupController::RestoreSnapGroups() {
   // TODO(b/288335850): Currently `SplitViewController` only supports two
   // windows, the group at the end will overwrite any split view operations.
   // This will be addressed in multiple snap groups feature.
-  // TODO(b/288333989): The order in `snap_groups_` doesn't reflect the mru
-  // order yet, which will be addressed in b/288333989.
   // TODO(b/288334530): Iterate through all the displays and restore the snap
   // groups based on the mru order.
   for (const auto& snap_group : snap_groups_) {

ash/wm/snap_group/snap_group_unittest.cc

@@ -9,6 +9,7 @@
 #include "ash/constants/ash_features.h"
 #include "ash/public/cpp/test/shell_test_api.h"
 #include "ash/resources/vector_icons/vector_icons.h"
+#include "ash/screen_util.h"
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/style/ash_color_id.h"
@@ -1360,6 +1361,77 @@ TEST_F(SnapGroupEntryPointArm1Test, ClamshellTabletTransitionWithOneSnapGroup) {
   EXPECT_TRUE(split_view_divider());
 }
 
+// Tests that when converting to tablet mode with split view divider at an
+// arbitrary location, the bounds of the two windows and the divider will be
+// updated such that the snap ratio of the layout is one of the fixed snap
+// ratios.
+TEST_F(SnapGroupEntryPointArm1Test,
+       ClamshellTabletTransitionGetClosestFixedRatio) {
+  UpdateDisplay("900x600");
+  std::unique_ptr<aura::Window> window1(CreateTestWindowInShellWithId(0));
+  std::unique_ptr<aura::Window> window2(CreateTestWindowInShellWithId(1));
+  SnapTwoTestWindowsInArm1(window1.get(), window2.get(), /*horizontal=*/true);
+  ASSERT_TRUE(split_view_divider());
+  EXPECT_EQ(*WindowState::Get(window1.get())->snap_ratio(),
+            chromeos::kDefaultSnapRatio);
+
+  // Build test cases to be used for divider dragging, with expected fixed ratio
+  // and corresponding pixels shown in the ASCII diagram below:
+  //   ┌────────────────┬────────────────┐
+  //   │                │                │
+  //   │                │                │
+  //   │                │                │
+  //   │                │                │
+  //   │                │                │
+  //   │                │                │
+  //   │                │                │
+  //   └─────────┬──────┼───────┬────────┘
+  //             │      │       │
+  // ratio:     1/3    1/2     2/3
+  // pixel:     300    450     600      900
+
+  struct {
+    int distance_delta;
+    float expected_snap_ratio;
+  } kTestCases[]{{/*distance_delta=*/-200, chromeos::kOneThirdSnapRatio},
+                 {/*distance_delta=*/400, chromeos::kTwoThirdSnapRatio},
+                 {/*distance_delta=*/-180, chromeos::kDefaultSnapRatio}};
+
+  auto* event_generator = GetEventGenerator();
+  const gfx::Rect work_area_bounds_in_screen =
+      screen_util::GetDisplayWorkAreaBoundsInScreenForActiveDeskContainer(
+          split_view_controller()->root_window()->GetChildById(
+              desks_util::GetActiveDeskContainerId()));
+  for (const auto test_case : kTestCases) {
+    event_generator->set_current_screen_location(
+        split_view_divider_bounds_in_screen().CenterPoint());
+    event_generator->DragMouseBy(test_case.distance_delta, 0);
+    split_view_controller()->EndResizeWithDivider(
+        event_generator->current_screen_location());
+    SwitchToTabletMode();
+    const auto current_divider_position =
+        split_view_divider()
+            ->GetDividerBoundsInScreen(/*is_dragging=*/false)
+            .x();
+
+    // We need to take into consideration of the variation introduced by the
+    // divider shorter side length when calculating using snap ratio, i.e.
+    // `kSplitviewDividerShortSideLength / 2`.
+    const auto expected_divider_position = std::round(
+        work_area_bounds_in_screen.width() * test_case.expected_snap_ratio -
+        kSplitviewDividerShortSideLength / 2);
+
+    // Verifies that the bounds of the windows and divider are updated correctly
+    // such that snap ratio in the new window layout is expected.
+    EXPECT_NEAR(current_divider_position, expected_divider_position,
+                /*abs_error=*/1);
+    EXPECT_NEAR(float(window1->GetBoundsInScreen().width()) /
+                    work_area_bounds_in_screen.width(),
+                test_case.expected_snap_ratio, /*abs_error=*/1);
+    ExitTabletMode();
+  }
+}
+
 // Tests that the swap window source histogram is recorded correctly.
 // TODO(michelefan): move this test to the snap group histogram test fixture
 // when implementing the histograms for the feature.

ash/wm/splitview/split_view_controller.cc

@@ -302,8 +302,6 @@ class SplitViewController::DividerSnapAnimation
     : public gfx::SlideAnimation,
       public gfx::AnimationDelegate {
  public:
-  // Before you change the value of `duration`, read the comment on
-  // kIsWindowMovedTimeoutMs in tablet_mode_window_drag_delegate.cc.
   DividerSnapAnimation(SplitViewController* split_view_controller,
                        int starting_position,
                        int ending_position,
@@ -1399,7 +1397,7 @@ bool SplitViewController::IsDividerAnimating() const {
 
 void SplitViewController::StartResizeWithDivider(
     const gfx::Point& location_in_screen) {
-  DCHECK(InSplitViewMode());
+  CHECK(InSplitViewMode());
 
   // `is_resizing_with_divider_` may be true here, because you can start
   // dragging the divider with a pointing device while already dragging it by
@@ -2198,10 +2196,9 @@ void SplitViewController::OnTabletModeStarting() {
 
 void SplitViewController::OnTabletModeStarted() {
   is_previous_layout_right_side_up_ = IsCurrentScreenOrientationPrimary();
-  // If splitview is active when tablet mode is starting, do the clamshell mode
-  // splitview to tablet mode splitview transition by adding the split view
-  // divider bar and also adjust the |divider_position_| so that it's on one of
-  // the three fixed positions.
+  // If splitview is active when tablet mode is starting, create the split view
+  // divider if not exists and adjust the `divider_position_` to be one
+  // of the fixed positions.
   if (InSplitViewMode()) {
     divider_position_ = GetClosestFixedDividerPosition();
     if (!split_view_divider_) {
@@ -2216,9 +2213,6 @@ void SplitViewController::OnTabletModeStarted() {
 void SplitViewController::OnTabletModeEnding() {
   split_view_type_ = SplitViewType::kClamshellType;
 
-  // There is no divider in clamshell split view unless the feature flag
-  // `kSnapGroup` is enabled and the feature param `kAutomaticallyLockGroup` is
-  // true.
   const bool is_divider_animating = IsDividerAnimating();
   if (is_resizing_with_divider_ || is_divider_animating) {
     is_resizing_with_divider_ = false;
@@ -2229,7 +2223,12 @@ void SplitViewController::OnTabletModeEnding() {
     EndResizeWithDividerImpl();
   }
 
-  if (state_ != State::kBothSnapped) {
+  // There is no divider in clamshell split view unless the two snapped windows
+  // belong to a snap group.
+  auto* snap_group_controller = SnapGroupController::Get();
+  if (state_ != State::kBothSnapped || !snap_group_controller ||
+      !snap_group_controller->AreWindowsInSnapGroup(primary_window_,
+                                                    secondary_window_)) {
     split_view_divider_.reset();
   }
 }
@@ -2666,7 +2665,7 @@ int SplitViewController::GetClosestFixedDividerPosition() {
   // the endpoints.
   int divider_end_position = GetDividerEndPosition();
   divider_closest_ratio_ = FindClosestPositionRatio(
-      divider_position_ + kSplitviewDividerShortSideLength / 2,
+      float(divider_position_ + kSplitviewDividerShortSideLength / 2) /
       divider_end_position);
   int fixed_position = divider_end_position * divider_closest_ratio_;
   if (divider_closest_ratio_ > 0.f && divider_closest_ratio_ < 1.f) {
@@ -2891,9 +2890,7 @@ void SplitViewController::OnSnappedWindowDetached(aura::Window* window,
   }
 }
 
-float SplitViewController::FindClosestPositionRatio(float distance,
-                                                    float length) {
-  const float current_ratio = distance / length;
+float SplitViewController::FindClosestPositionRatio(float current_ratio) {
   float closest_ratio = 0.f;
   std::vector<float> position_ratios(
       kFixedPositionRatios,

ash/wm/splitview/split_view_controller.h

@@ -27,9 +27,12 @@
 #include "ui/aura/window_observer.h"
 #include "ui/display/display.h"
 #include "ui/display/display_observer.h"
-#include "ui/gfx/geometry/point.h"
 #include "ui/wm/public/activation_change_observer.h"
 
+namespace gfx {
+class Point;
+}  // namespace gfx
+
 namespace ui {
 class Layer;
 class PresentationTimeRecorder;
@@ -566,8 +569,10 @@ class ASH_EXPORT SplitViewController : public aura::WindowObserver,
   void OnSnappedWindowDetached(aura::Window* window,
                                WindowDetachedReason reason);
 
-  // Returns the closest position ratio based on |distance| and |length|.
-  float FindClosestPositionRatio(float distance, float length);
+  // Returns the closest ratio to the `current_ratio`. `current_ratio` is the
+  // the ratio between current divider position and the farthest position
+  // divider is allowed to end at.
+  float FindClosestPositionRatio(float current_ratio);
 
   // Gets the divider optional position ratios. The divider can always be
   // moved to the positions in `kFixedPositionRatios`. Whether the divider can