overview: Fix grid from shifting past bounds when rotating with new layout.

It's possible to get to this state by scrolling a bit, then rotating a
couple times. Not entirely reproducible (depends on num windows and how
much scrolling and display size), but added a test case which repros
100% without this fix.

Test: Added test
Bug: 1013224
Change-Id: I15a4bcdcda83ccac4346727342108b0c295fd151
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1853130
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706155}

ash/wm/overview/overview_grid.cc

@@ -1631,23 +1631,22 @@ std::vector<gfx::RectF> OverviewGrid::GetWindowRectsForTabletModeLayout(
   // Windows occupy vertically centered area with additional vertical insets.
   total_bounds.Inset(GetGridInsets(total_bounds));
 
-  // When the dragged item becomes an |ignored_item|, move the other windows
-  // accordingly. |window_position| matches the positions of the windows'
-  // indexes from |window_list_|. However, if a window turns out to be an
-  // ignored item, |window_position| remains where the item was as to then
-  // reposition the other window's bounds in place of that item.
-
-  // This function may be called as the result of closing an overview item. If
-  // the closed item is the last item in the list, adjust |scroll_offset_| so
-  // that the grid is right aligned.
-  float right_most = 0.f;
-  for (const auto& window : window_list_) {
-    if (window->animating_to_close() || ignored_items.contains(window.get()))
+  // |scroll_offset_min_| may be changed on positioning (either by closing
+  // windows or display changes). Recalculate it and clamp |scroll_offset_|, so
+  // that the items are always aligned left or right.
+  float rightmost_window_right = 0;
+  for (const auto& item : window_list_) {
+    if (item->animating_to_close() || ignored_items.contains(item.get()))
       continue;
-    right_most = std::max(right_most, window->target_bounds().right());
+    rightmost_window_right =
+        std::max(rightmost_window_right, item->target_bounds().right());
   }
-  if (right_most != 0.f && right_most < total_bounds.right())
-    scroll_offset_ = -(right_most - scroll_offset_ - total_bounds.right());
+
+  // |rightmost_window_right| may have been modified by an earlier scroll.
+  // |scroll_offset_| is added to adjust for that.
+  rightmost_window_right -= scroll_offset_;
+  scroll_offset_min_ = total_bounds.right() - rightmost_window_right;
+  scroll_offset_ = base::ClampToRange(scroll_offset_, scroll_offset_min_, 0.f);
 
   // Map which contains up to |kTabletLayoutRow| entries with information on the
   // last items right bound per row. Used so we can place the next item directly
@@ -1656,7 +1655,12 @@ std::vector<gfx::RectF> OverviewGrid::GetWindowRectsForTabletModeLayout(
   base::flat_map<float, float> right_edge_map;
 
   // Since the number of rows is limited, windows are laid out column-wise so
-  // that the most recently used windows are displayed first.
+  // that the most recently used windows are displayed first. When the dragged
+  // item becomes an |ignored_item|, move the other windows accordingly.
+  // |window_position| matches the positions of the windows' indexes from
+  // |window_list_|. However, if a window turns out to be an ignored item,
+  // |window_position| remains where the item was as to then reposition the
+  // other window's bounds in place of that item.
   const int height = total_bounds.height() / kTabletLayoutRow;
   int window_position = 0;
   std::vector<gfx::RectF> rects;

ash/wm/overview/overview_session_unittest.cc

@@ -3121,6 +3121,58 @@ TEST_P(OverviewSessionNewLayoutTest, CheckWindowActivateOnTap) {
       0, user_action_tester.GetActionCount(kActiveWindowChangedFromOverview));
 }
 
+TEST_P(OverviewSessionNewLayoutTest, LayoutValidAfterRotation) {
+  UpdateDisplay("1366x768");
+  display::test::ScopedSetInternalDisplayId set_internal(
+      Shell::Get()->display_manager(),
+      display::Screen::GetScreen()->GetPrimaryDisplay().id());
+  auto windows = CreateTestWindows(7);
+
+  // Helper to determine whether a grid layout is valid. It is considered valid
+  // if the left edge of the first item is close enough to the left edge of the
+  // grid bounds and if the right edge of the last item is close enough to the
+  // right edge of the grid bounds. Either of these being false would mean there
+  // is a large padding which shouldn't be there.
+  auto layout_valid = [&windows, this](int expected_padding) {
+    OverviewItem* first_item = GetOverviewItemForWindow(windows.front().get());
+    OverviewItem* last_item = GetOverviewItemForWindow(windows.back().get());
+
+    const gfx::Rect first_bounds =
+        gfx::ToEnclosedRect(first_item->target_bounds());
+    const gfx::Rect last_bounds =
+        gfx::ToEnclosedRect(last_item->target_bounds());
+
+    const gfx::Rect grid_bounds = GetGridBounds();
+    const bool first_bounds_valid =
+        first_bounds.x() <= (grid_bounds.x() + expected_padding);
+    const bool last_bounds_valid =
+        last_bounds.right() >= (grid_bounds.right() - expected_padding);
+    return first_bounds_valid && last_bounds_valid;
+  };
+
+  // Enter overview and scroll to the edge of the grid. The layout should remain
+  // valid.
+  ToggleOverview();
+  ASSERT_TRUE(InOverviewSession());
+  // The expected padding should be the x position of the first item, before the
+  // grid gets shifted.
+  const int expected_padding =
+      GetOverviewItemForWindow(windows.front().get())->target_bounds().x();
+  GenerateScrollSequence(gfx::Point(1300, 10), gfx::Point(100, 10));
+  EXPECT_TRUE(layout_valid(expected_padding));
+
+  // Tests that the layout is still valid after a couple rotations.
+  ScreenOrientationControllerTestApi test_api(
+      Shell::Get()->screen_orientation_controller());
+  test_api.SetDisplayRotation(display::Display::ROTATE_90,
+                              display::Display::RotationSource::ACTIVE);
+  EXPECT_TRUE(layout_valid(expected_padding));
+
+  test_api.SetDisplayRotation(display::Display::ROTATE_180,
+                              display::Display::RotationSource::ACTIVE);
+  EXPECT_TRUE(layout_valid(expected_padding));
+}
+
 // Tests that windows snap through long press and drag to left or right side of
 // the screen.
 TEST_P(OverviewSessionNewLayoutTest, DragOverviewWindowToSnap) {