shelf: No fade animation when removing shelf item with 0 opacity

When removing an item from a shelf view, the animation to update item
bounds depends on view visibility:
*   if view is visible, the view first fades out, then the rest of the
    items' bounds are updated
*   if view is not visible, the view is removed, and the rest of the
    items' bounds animate immediately.

Though, whether the view is visible is determined only by using
View::GetVisible(), while the view may be in visible state, with zero
opacity (e.g. during drag and drop from launcher). This case should be
handled the same way as views whose visibility is set to false.
(Otherwise, removing dragged launcher item from shelf apparently happens
in two stages - first items right of the last dragged item spot move
left to fill out the void, and then all items animate right to center
the hotseat).

BUG=1042493

Change-Id: Ic81c0ab24797714f1223d8c8e21282066360dea7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026348Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736155}

ash/shelf/shelf_view.cc

@@ -2315,7 +2315,7 @@ void ShelfView::ShelfItemRemoved(int model_index, const ShelfItem& old_item) {
   if (view.get() == shelf_->tooltip()->GetCurrentAnchorView())
     shelf_->tooltip()->Close();
 
-  if (view->GetVisible()) {
+  if (view->GetVisible() && view->layer()->opacity() > 0.0f) {
     if (chromeos::switches::ShouldShowScrollableShelf())
       UpdateVisibleIndices();
 

ash/shelf/shelf_view_unittest.cc

@@ -269,23 +269,34 @@ TEST_F(ShelfObserverIconTest, AddRemoveWithMultipleDisplays) {
 
   Shelf* second_shelf = Shelf::ForWindow(Shell::GetAllRootWindows()[1]);
   TestShelfObserver second_observer(second_shelf);
+  ShelfViewTestAPI second_shelf_test_api(
+      second_shelf->GetShelfViewForTesting());
 
   ShelfItem item;
   item.id = ShelfID("foo");
   item.type = TYPE_APP;
   EXPECT_FALSE(observer()->icon_positions_changed());
   EXPECT_FALSE(second_observer.icon_positions_changed());
+
+  // Add item and wait for all animations to finish.
   const int shelf_item_index = ShelfModel::Get()->Add(item);
   shelf_view_test()->RunMessageLoopUntilAnimationsDone();
+  second_shelf_test_api.RunMessageLoopUntilAnimationsDone();
+
   EXPECT_TRUE(observer()->icon_positions_changed());
   EXPECT_TRUE(second_observer.icon_positions_changed());
+
+  // Reset observer so they can track the next set of animations.
   observer()->Reset();
+  ASSERT_FALSE(observer()->icon_positions_changed());
   second_observer.Reset();
+  ASSERT_FALSE(second_observer.icon_positions_changed());
 
-  EXPECT_FALSE(observer()->icon_positions_changed());
-  EXPECT_FALSE(second_observer.icon_positions_changed());
+  // Remove the item, and wait for all the animations to complete.
   ShelfModel::Get()->RemoveItemAt(shelf_item_index);
   shelf_view_test()->RunMessageLoopUntilAnimationsDone();
+  second_shelf_test_api.RunMessageLoopUntilAnimationsDone();
+
   EXPECT_TRUE(observer()->icon_positions_changed());
   EXPECT_TRUE(second_observer.icon_positions_changed());