Fix the issue of hotseat's end padding

There should be a 4-pixel padding between hotseat's end and edging
app icon. The padding is implemented via shelf view's
|app_icons_layout_offset_|. However, update on
|app_icons_layout_offset_| does not change the shelf view's layout
directly. As a result, the padding may be missing. This CL fixes such
an issue by refreshing the shelf view's layout right after value update.

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

ash/shelf/scrollable_shelf_view.cc

@@ -84,12 +84,6 @@ bool IsInTabletMode() {
   return true;
 }
 
-// Returns the padding between the app icon and the end of the ScrollableShelf.
-int GetAppIconEndPadding() {
-  return (chromeos::switches::ShouldShowShelfHotseat() && IsInTabletMode()) ? 4
-                                                                            : 0;
-}
-
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -445,6 +439,12 @@ views::View* ScrollableShelfView::GetShelfContainerViewForTest() {
   return shelf_container_view_;
 }
 
+// static
+int ScrollableShelfView::GetAppIconEndPadding() {
+  return (chromeos::switches::ShouldShowShelfHotseat() && IsInTabletMode()) ? 4
+                                                                            : 0;
+}
+
 bool ScrollableShelfView::ShouldAdjustForTest() const {
   return CalculateAdjustedOffset();
 }
@@ -639,8 +639,7 @@ 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());
+  shelf_view_->SetAppIconsLayoutOffset(before_padding + GetAppIconEndPadding());
 
   // Adjust the bounds when not showing in the horizontal
   // alignment.tShelf()->IsHorizontalAlignment()) {

ash/shelf/scrollable_shelf_view.h

@@ -102,8 +102,8 @@ class ASH_EXPORT ScrollableShelfView : public views::AccessiblePaneView,
 
   const gfx::Rect& visible_space() const { return visible_space_; }
 
-  // Size of the arrow button.
-  static int GetArrowButtonSize();
+  // Returns the padding between the app icon and the end of the ScrollableShelf
+  static int GetAppIconEndPadding();
 
   // Padding at the two ends of the shelf.
   static constexpr int kEndPadding = 4;

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"
@@ -66,8 +67,9 @@ class ScrollableShelfViewTest : public AshTestBase {
   ~ScrollableShelfViewTest() override = default;
 
   void SetUp() override {
-    scoped_feature_list_.InitWithFeatures(
-        {chromeos::features::kShelfScrollable}, {});
+    scoped_feature_list_.InitWithFeatures({chromeos::features::kShelfScrollable,
+                                           chromeos::features::kShelfHotseat},
+                                          {});
 
     AshTestBase::SetUp();
     scrollable_shelf_view_ = GetPrimaryShelf()
@@ -229,6 +231,36 @@ TEST_F(ScrollableShelfViewTest, CorrectUIAfterDisplayRotationLongToShort) {
   EXPECT_FALSE(scrollable_shelf_view_->ShouldAdjustForTest());
 }
 
+// Verifies that there is padding between the edging app icon and the end
+// of hotseat background in tablet mode (https://crbug.com/1017979).
+TEST_F(ScrollableShelfViewTest, CorrectEdgePaddingInTabletMode) {
+  for (int i = 0; i < 3; i++)
+    AddAppShortcut();
+  ASSERT_EQ(ScrollableShelfView::kNotShowArrowButtons,
+            scrollable_shelf_view_->layout_strategy_for_test());
+
+  Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
+  ASSERT_TRUE(Shell::Get()->tablet_mode_controller()->InTabletMode());
+
+  gfx::Rect hotseat_background_in_screen =
+      scrollable_shelf_view_->GetHotseatBackgroundBounds();
+  views::View::ConvertRectToScreen(scrollable_shelf_view_,
+                                   &hotseat_background_in_screen);
+
+  const int end_padding = ScrollableShelfView::GetAppIconEndPadding();
+  ASSERT_GT(end_padding, 0);
+
+  views::ViewModel* view_model = shelf_view_->view_model();
+  views::View* first_icon =
+      view_model->view_at(scrollable_shelf_view_->first_tappable_app_index());
+  EXPECT_EQ(first_icon->GetBoundsInScreen().x(),
+            hotseat_background_in_screen.x() + end_padding);
+  views::View* last_icon =
+      view_model->view_at(scrollable_shelf_view_->last_tappable_app_index());
+  EXPECT_EQ(last_icon->GetBoundsInScreen().right(),
+            hotseat_background_in_screen.right() - end_padding);
+}
+
 // When hovering mouse on a shelf icon, the tooltip only shows for the visible
 // icon (see https://crbug.com/997807).
 TEST_F(ScrollableShelfViewTest, NotShowTooltipForHiddenIcons) {

ash/shelf/shelf_view.cc

@@ -462,6 +462,14 @@ ShelfAppButton* ShelfView::GetShelfAppButton(const ShelfID& id) {
   return static_cast<ShelfAppButton*>(view);
 }
 
+void ShelfView::SetAppIconsLayoutOffset(int app_icons_layout_offset) {
+  if (app_icons_layout_offset_ == app_icons_layout_offset)
+    return;
+
+  app_icons_layout_offset_ = app_icons_layout_offset;
+  LayoutToIdealBounds();
+}
+
 bool ShelfView::ShouldHideTooltip(const gfx::Point& cursor_location) const {
   // There are thin gaps between launcher buttons but the tooltip shouldn't hide
   // in the gaps, but the tooltip should hide if the mouse moved totally outside

ash/shelf/shelf_view.h

@@ -297,6 +297,8 @@ class ASH_EXPORT ShelfView : public views::AccessiblePaneView,
   // Returns the ShelfAppButton associated with |id|.
   ShelfAppButton* GetShelfAppButton(const ShelfID& id);
 
+  void SetAppIconsLayoutOffset(int app_icons_layout_offset);
+
   // Return the view model for test purposes.
   const views::ViewModel* view_model_for_test() const {
     return view_model_.get();
@@ -325,10 +327,6 @@ class ASH_EXPORT ShelfView : public views::AccessiblePaneView,
     default_last_focusable_child_ = default_last_focusable_child;
   }
 
-  void set_app_icons_layout_offset(int app_icons_layout_offset) {
-    app_icons_layout_offset_ = app_icons_layout_offset;
-  }
-
   const ShelfAppButton* drag_view() const { return drag_view_; }
 
   // Returns true when this ShelfView is used for Overflow Bubble.