Fix the issue that scrollable shelf shows incorrectly after rotation

Bug: 1000764
Change-Id: I70ad53d49a5f4f266fe94f58cf2b4b2d35b78f18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1843775Reviewed-by: Manu Cornet <manucornet@chromium.org>
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703901}

ash/shelf/scrollable_shelf_view.cc

@@ -373,6 +373,10 @@ views::View* ScrollableShelfView::GetShelfContainerViewForTest() {
   return shelf_container_view_;
 }
 
+bool ScrollableShelfView::ShouldAdjustForTest() const {
+  return CalculateAdjustedOffset();
+}
+
 int ScrollableShelfView::CalculateScrollUpperBound() const {
   if (layout_strategy_ == kNotShowArrowButtons)
     return 0;
@@ -645,15 +649,16 @@ void ScrollableShelfView::Layout() {
   shelf_container_view_->TranslateShelfView(total_offset);
 
   UpdateTappableIconIndices();
+
+  // Ensures that the app icons are shown correctly when the scrollable shelf
+  // is idle (not under scrolling or during animation process).
+  if (!during_scrolling_animation_ && scroll_status_ == kNotInScroll)
+    AdjustOffset();
 }
 
 void ScrollableShelfView::ChildPreferredSizeChanged(views::View* child) {
   // Add/remove a shelf icon may change the layout strategy.
   Layout();
-
-  // Ensures that the app icons are shown correctly when the layout strategy
-  // changes.
-  AdjustOffsetAfterScrolling();
 }
 
 void ScrollableShelfView::OnMouseEvent(ui::MouseEvent* event) {
@@ -891,7 +896,7 @@ bool ScrollableShelfView::ProcessGestureEvent(const ui::GestureEvent& event) {
   }
 
   if (event.type() == ui::ET_GESTURE_END) {
-    AdjustOffsetAfterScrolling();
+    AdjustOffset();
     return true;
   }
 
@@ -1207,22 +1212,13 @@ bool ScrollableShelfView::ShouldHandleScroll(const gfx::Vector2dF& offset,
   return abs(main_axis_offset) > threshold;
 }
 
-void ScrollableShelfView::AdjustOffsetAfterScrolling() {
+void ScrollableShelfView::AdjustOffset() {
   // The type of scrolling offset is float to ensure that ScrollableShelfView
-  // is responsive to slow gesture scrolling. However, when scrolling ends, the
-  // scrolling offset should be floored.
+  // is responsive to slow gesture scrolling. However, after offset adjustment,
+  // the scrolling offset should be floored.
   scroll_offset_ = gfx::ToFlooredVector2d(scroll_offset_);
 
-  // Returns early when it does not need to adjust the shelf view's location.
-  const int scroll_distance = GetShelf()->IsHorizontalAlignment()
-                                  ? scroll_offset_.x()
-                                  : scroll_offset_.y();
-  if (scroll_distance >= CalculateScrollUpperBound())
-    return;
-
-  const int residue = GetActualScrollOffset() % GetUnit();
-  int offset =
-      residue > GetGestureDragThreshold() ? GetUnit() - residue : -residue;
+  const int offset = CalculateAdjustedOffset();
 
   // Returns early when it does not need to adjust the shelf view's location.
   if (!offset)
@@ -1234,4 +1230,19 @@ void ScrollableShelfView::AdjustOffsetAfterScrolling() {
     ScrollByYOffset(offset, /*animate=*/true);
 }
 
+int ScrollableShelfView::CalculateAdjustedOffset() const {
+  // Returns early when it does not need to adjust the shelf view's location.
+  const int scroll_distance = GetShelf()->IsHorizontalAlignment()
+                                  ? scroll_offset_.x()
+                                  : scroll_offset_.y();
+  if (scroll_distance >= CalculateScrollUpperBound())
+    return 0;
+
+  const int remainder = GetActualScrollOffset() % GetUnit();
+  int offset = remainder > GetGestureDragThreshold() ? GetUnit() - remainder
+                                                     : -remainder;
+
+  return offset;
+}
+
 }  // namespace ash

ash/shelf/scrollable_shelf_view.h

@@ -69,6 +69,7 @@ class ASH_EXPORT ScrollableShelfView : public views::AccessiblePaneView,
   gfx::Rect GetHotseatBackgroundBounds();
 
   views::View* GetShelfContainerViewForTest();
+  bool ShouldAdjustForTest() const;
 
   ShelfView* shelf_view() { return shelf_view_; }
   ShelfContainerView* shelf_container_view() { return shelf_container_view_; }
@@ -249,8 +250,12 @@ class ASH_EXPORT ScrollableShelfView : public views::AccessiblePaneView,
   bool ShouldHandleScroll(const gfx::Vector2dF& offset,
                           bool is_gesture_fling) const;
 
-  // Ensures that the app icons are shown correctly when scrolling ends.
-  void AdjustOffsetAfterScrolling();
+  // Ensures that the app icons are shown correctly.
+  void AdjustOffset();
+
+  // Returns the offset by which the shelf view should be translated to ensure
+  // the correct UI.
+  int CalculateAdjustedOffset() const;
 
   LayoutStrategy layout_strategy_ = kNotShowArrowButtons;
 

ash/shelf/scrollable_shelf_view_unittest.cc

@@ -77,9 +77,10 @@ class ScrollableShelfViewTest : public AshTestBase {
   DISALLOW_COPY_AND_ASSIGN(ScrollableShelfViewTest);
 };
 
-// Verifies that the display rotation should not break the scrollable shelf's
-// UI behavior (https://crbug.com/1000764).
-TEST_F(ScrollableShelfViewTest, CorrectUIAfterDisplayRotation) {
+// Verifies that the display rotation from the short side to the long side
+// should not break the scrollable shelf's UI
+// behavior(https://crbug.com/1000764).
+TEST_F(ScrollableShelfViewTest, CorrectUIAfterDisplayRotationShortToLong) {
   // Changes the display setting in order that the display's height is greater
   // than the width.
   UpdateDisplay("600x800");
@@ -117,6 +118,7 @@ TEST_F(ScrollableShelfViewTest, CorrectUIAfterDisplayRotation) {
   // After rotation, checks the following things:
   // (1) The scrollable shelf has the correct layout strategy.
   // (2) The last app icon has the correct bounds.
+  // (3) The scrollable shelf does not need further adjustment.
   EXPECT_EQ(ScrollableShelfView::kShowLeftArrowButton,
             scrollable_shelf_view_->layout_strategy_for_test());
   views::ViewModel* view_model = shelf_view_->view_model();
@@ -126,6 +128,42 @@ TEST_F(ScrollableShelfViewTest, CorrectUIAfterDisplayRotation) {
   const gfx::Rect shelf_container_bounds =
       scrollable_shelf_view_->shelf_container_view()->GetBoundsInScreen();
   EXPECT_EQ(icon_bounds.right(), shelf_container_bounds.right());
+  EXPECT_FALSE(scrollable_shelf_view_->ShouldAdjustForTest());
+}
+
+// Verifies that the display rotation from the long side to the short side
+// should not break the scrollable shelf's UI behavior
+// (https://crbug.com/1000764).
+TEST_F(ScrollableShelfViewTest, CorrectUIAfterDisplayRotationLongToShort) {
+  // Changes the display setting in order that the display's width is greater
+  // than the height.
+  UpdateDisplay("600x300");
+
+  display::Display display = GetPrimaryDisplay();
+  AddAppShortcutUntilOverflow();
+
+  // Presses the right arrow until reaching the last page of shelf icons.
+  const views::View* right_arrow = scrollable_shelf_view_->right_arrow();
+  const gfx::Point center_point =
+      right_arrow->GetBoundsInScreen().CenterPoint();
+  while (right_arrow->GetVisible()) {
+    GetEventGenerator()->MoveMouseTo(center_point);
+    GetEventGenerator()->PressLeftButton();
+    GetEventGenerator()->ReleaseLeftButton();
+  }
+  ASSERT_EQ(ScrollableShelfView::kShowLeftArrowButton,
+            scrollable_shelf_view_->layout_strategy_for_test());
+
+  // Rotates the display by 90 degrees. In order to reproduce the bug,
+  // both arrow buttons should show after rotation.
+  display::DisplayManager* display_manager = Shell::Get()->display_manager();
+  display_manager->SetDisplayRotation(display.id(), display::Display::ROTATE_90,
+                                      display::Display::RotationSource::ACTIVE);
+  ASSERT_EQ(ScrollableShelfView::kShowButtons,
+            scrollable_shelf_view_->layout_strategy_for_test());
+
+  // Verifies that the scrollable shelf does not need further adjustment.
+  EXPECT_FALSE(scrollable_shelf_view_->ShouldAdjustForTest());
 }
 
 // When hovering mouse on a shelf icon, the tooltip only shows for the visible