Fix scrollable shelf animation of adding/removing shelf item

Mainly two modifications in this CL:
(1) When no arrow button shows, ensures that the shelf view's bounds
are not changed by adding/removing a shelf item. It helps the correct
bounds animation for shelf items.

(2) Only notify scrollable shelf of shelf view's changed preferred size
when adding/removing shelf items.

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

ash/shelf/scrollable_shelf_view.cc

@@ -538,7 +538,6 @@ void ScrollableShelfView::Layout() {
     left_arrow_bounds.Inset(kArrowButtonEndPadding, 0, kDistanceToArrowButton,
                             0);
     left_arrow_bounds.ClampToCenteredSize(arrow_button_size);
-    shelf_container_bounds.Inset(kArrowButtonGroupWidth, 0, 0, 0);
   }
 
   if (layout_strategy_ == kShowRightArrowButton ||
@@ -551,15 +550,31 @@ void ScrollableShelfView::Layout() {
     right_arrow_bounds.Inset(kDistanceToArrowButton, 0, kArrowButtonEndPadding,
                              0);
     right_arrow_bounds.ClampToCenteredSize(arrow_button_size);
-    shelf_container_bounds.Inset(0, 0, kArrowButtonGroupWidth, 0);
+  }
+
+  // The layout offset before/after |shelf_container_view_|.
+  int shelf_container_before_offset = 0;
+  int shelf_container_after_offset = 0;
+
+  if (layout_strategy_ == kNotShowArrowButtons) {
+    // Paddings are within the shelf view. It makes sure that |shelf_view_|'s
+    // bounds are not changed under this layout strategy. It facilitates
+    // |shelf_view_| to create bounds animations when adding/removing app icons.
+    shelf_view_->set_app_icons_layout_offset(before_padding);
+  } else {
+    shelf_container_before_offset = before_padding;
+    shelf_container_after_offset = after_padding;
+    shelf_view_->set_app_icons_layout_offset(0);
   }
 
   shelf_container_bounds.Inset(
-      before_padding +
-          (left_arrow_bounds.IsEmpty() ? GetAppIconEndPadding() : 0),
+      shelf_container_before_offset + (left_arrow_bounds.IsEmpty()
+                                           ? GetAppIconEndPadding()
+                                           : kArrowButtonGroupWidth),
       0,
-      after_padding +
-          (right_arrow_bounds.IsEmpty() ? GetAppIconEndPadding() : 0),
+      shelf_container_after_offset + (right_arrow_bounds.IsEmpty()
+                                          ? GetAppIconEndPadding()
+                                          : kArrowButtonGroupWidth),
       0);
 
   // Adjust the bounds when not showing in the horizontal

ash/shelf/shelf_container_view.cc

@@ -36,7 +36,13 @@ void ShelfContainerView::Layout() {
   // CalculateIdealSize. Because ShelfView::CalculatePreferredSize relies on the
   // bounds of app icon. Meanwhile, the icon's bounds may be updated by
   // animation.
-  shelf_view_->SetBoundsRect(gfx::Rect(CalculateIdealSize()));
+  const gfx::Rect ideal_bounds = gfx::Rect(CalculateIdealSize());
+
+  const gfx::Rect local_bounds = GetLocalBounds();
+  if (local_bounds.Contains(ideal_bounds))
+    shelf_view_->SetBoundsRect(local_bounds);
+  else
+    shelf_view_->SetBoundsRect(ideal_bounds);
 }
 
 const char* ShelfContainerView::GetClassName() const {

ash/shelf/shelf_view.cc

@@ -1034,8 +1034,8 @@ void ShelfView::CalculateIdealBounds() {
                          separator_index < last_visible_index() &&
                          !virtual_keyboard_visible);
 
-  int x = 0;
-  int y = 0;
+  int x = shelf()->PrimaryAxisValue(app_icons_layout_offset_, 0);
+  int y = shelf()->PrimaryAxisValue(0, app_icons_layout_offset_);
 
   // When scrollable shelf is enabled, the padding is handled in
   // ScrollableShelfView.
@@ -1951,6 +1951,11 @@ std::pair<int, int> ShelfView::GetDragRange(int index) {
 }
 
 void ShelfView::OnFadeOutAnimationEnded() {
+  // Call PreferredSizeChanged() to notify container to re-layout at the end
+  // of removal animation.
+  if (chromeos::switches::ShouldShowScrollableShelf())
+    PreferredSizeChanged();
+
   AnimateToIdealBounds();
   StartFadeInLastVisibleItem();
 }
@@ -2127,9 +2132,14 @@ void ShelfView::ShelfItemAdded(int model_index) {
 
   // When the scrollable shelf is enabled, |last_visible_index_| is always the
   // index to the last shelf item.
-  if (chromeos::switches::ShouldShowScrollableShelf())
+  if (chromeos::switches::ShouldShowScrollableShelf()) {
     UpdateVisibleIndice();
 
+    // Call PreferredSizeChanged() to notify container to re-layout before
+    // starting the animation of the shelf item addition.
+    PreferredSizeChanged();
+  }
+
   // Give the button its ideal bounds. That way if we end up animating the
   // button before this animation completes it doesn't appear at some random
   // spot (because it was in the middle of animating from 0,0 0x0 to its
@@ -2415,6 +2425,10 @@ void ShelfView::OnMenuClosed(views::View* source) {
 
 void ShelfView::OnBoundsAnimatorProgressed(views::BoundsAnimator* animator) {
   shelf_->NotifyShelfIconPositionsChanged();
+
+  // Do not call PreferredSizeChanged() so that container does not re-layout
+  // during the bounds animation.
+  if (!chromeos::switches::ShouldShowScrollableShelf())
     PreferredSizeChanged();
 }
 

ash/shelf/shelf_view.h

@@ -319,6 +319,10 @@ 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.
@@ -715,6 +719,11 @@ class ASH_EXPORT ShelfView : public views::AccessiblePaneView,
   // first) when focused.
   bool default_last_focusable_child_ = false;
 
+  // Indicates the starting position of shelf items on the main axis. (Main
+  // axis is x-axis when the shelf is horizontally aligned; otherwise, it
+  // becomes y-axis)
+  int app_icons_layout_offset_ = 0;
+
   base::WeakPtrFactory<ShelfView> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ShelfView);