Ensure to update shelf view's layout after switching to tablet mode

Move the logic of adding the extra four-pixel padding for tablet mode
from ScrollableShelfView::Layout to ScrollableShelfContainerView's
layout. It helps to update the shelf view's layout after switching to
the tablet mode.

Bug: 1012354
Change-Id: I975954faf687c144166e8a86ae9fa89959e931f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1850796Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712673}

ash/shelf/scrollable_shelf_view.cc

@@ -251,6 +251,12 @@ void ScrollableShelfContainerView::Layout() {
   const gfx::Rect local_bounds = GetLocalBounds();
   gfx::Rect shelf_view_bounds =
       local_bounds.Contains(ideal_bounds) ? local_bounds : ideal_bounds;
+
+  if (shelf_view_->shelf()->IsHorizontalAlignment())
+    shelf_view_bounds.set_x(GetAppIconEndPadding());
+  else
+    shelf_view_bounds.set_y(GetAppIconEndPadding());
+
   shelf_view_->SetBoundsRect(shelf_view_bounds);
 }
 
@@ -640,9 +646,9 @@ void ScrollableShelfView::Layout() {
   }
 
   // Paddings are within the shelf view. It makes sure that |shelf_view_|'s
-  // bounds are never changed.
-  shelf_view_->set_app_icons_layout_offset(before_padding +
-                                           GetAppIconEndPadding());
+  // bounds are not changed by adding/removing the shelf icon under the same
+  // layout strategy.
+  shelf_view_->set_app_icons_layout_offset(before_padding);
 
   // Adjust the bounds when not showing in the horizontal
   // alignment.tShelf()->IsHorizontalAlignment()) {

ash/shelf/scrollable_shelf_view_unittest.cc

@@ -10,6 +10,7 @@
 #include "ash/shelf/shelf_view_test_api.h"
 #include "ash/shelf/shelf_widget.h"
 #include "ash/test/ash_test_base.h"
+#include "ash/wm/tablet_mode/tablet_mode_controller.h"
 #include "base/test/scoped_feature_list.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "ui/display/manager/display_manager.h"
@@ -412,4 +413,58 @@ TEST_F(ScrollableShelfViewTest, DragIconToNewPage) {
   EXPECT_EQ(0, view_model->GetIndexOfView(dragged_view));
 }
 
+// Verifies that the scrollable shelf in oveflow mode has the correct layout
+// after switching to tablet mode (https://crbug.com/1017979).
+TEST_F(ScrollableShelfViewTest, CorrectUIAfterSwitchingToTablet) {
+  // Add enough app shortcuts to ensure that at least three pages of icons show.
+  for (int i = 0; i < 25; i++)
+    AddAppShortcut();
+  ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
+            scrollable_shelf_view_->layout_strategy_for_test());
+  GetEventGenerator()->GestureTapAt(
+      scrollable_shelf_view_->right_arrow()->GetBoundsInScreen().CenterPoint());
+  ASSERT_EQ(ScrollableShelfView::kShowButtons,
+            scrollable_shelf_view_->layout_strategy_for_test());
+
+  Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
+  base::RunLoop().RunUntilIdle();
+
+  views::ViewModel* view_model = shelf_view_->view_model();
+  views::View* first_tappable_view =
+      view_model->view_at(scrollable_shelf_view_->first_tappable_app_index());
+
+  // Verifies that the gap between the left arrow button and the first tappable
+  // icon is expected.
+  const gfx::Rect left_arrow_bounds =
+      scrollable_shelf_view_->left_arrow()->GetBoundsInScreen();
+  EXPECT_EQ(left_arrow_bounds.right() + 2,
+            first_tappable_view->GetBoundsInScreen().x());
+}
+
+// Verifies that the scrollable shelf without overflow has the correct layout in
+// tablet mode.
+TEST_F(ScrollableShelfViewTest, CorrectUIInTabletWithoutOverflow) {
+  Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
+
+  for (int i = 0; i < 3; i++)
+    AddAppShortcut();
+  ASSERT_EQ(ScrollableShelfView::kNotShowArrowButtons,
+            scrollable_shelf_view_->layout_strategy_for_test());
+
+  gfx::Rect hotseat_background =
+      scrollable_shelf_view_->GetHotseatBackgroundBounds();
+  views::View::ConvertRectToScreen(scrollable_shelf_view_, &hotseat_background);
+
+  views::ViewModel* view_model = shelf_view_->view_model();
+  const gfx::Rect first_tappable_view_bounds =
+      view_model->view_at(scrollable_shelf_view_->first_tappable_app_index())
+          ->GetBoundsInScreen();
+  const gfx::Rect last_tappable_view_bounds =
+      view_model->view_at(scrollable_shelf_view_->last_tappable_app_index())
+          ->GetBoundsInScreen();
+
+  EXPECT_EQ(hotseat_background.x() + 4, first_tappable_view_bounds.x());
+  EXPECT_EQ(hotseat_background.right() - 4, last_tappable_view_bounds.right());
+}
+
 }  // namespace ash