overview: Nudging part 2.

Finishes up a few left behind works from the first cl.

1. Handles window items that are still alive but only for animation
purposes by ignoring them in PositionWindows. Previously had some
indexing errors and the windows would bounce around if swipe was used
too quickly.

2. Handles one more case where nudge should not occur - when an item
from the previous row drops down to the nudging row, which looks funny.

3. Use the nudging function described in the bug instead of just linear
interpolation.

Test: added tests
Bug: 843273
Change-Id: I9d2088083a71779fc78f72309b5674f35bc162c7
Reviewed-on: https://chromium-review.googlesource.com/1100389
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567462}

ash/wm/overview/overview_window_drag_controller.cc

@@ -98,9 +98,9 @@ void OverviewWindowDragController::Drag(const gfx::Point& location_in_screen) {
     float val = std::abs(static_cast<float>(location_in_screen.y()) -
                          initial_event_location_.y()) /
                 kDragToCloseDistanceThresholdDp;
-    val = base::ClampToRange(val, 0.f, 1.f);
     window_selector_->GetGridWithRootWindow(item_->root_window())
         ->UpdateNudge(item_, val);
+    val = base::ClampToRange(val, 0.f, 1.f);
     float opacity = original_opacity_;
     if (opacity > kItemMinOpacity) {
       opacity = original_opacity_ - val * (original_opacity_ - kItemMinOpacity);
@@ -135,13 +135,13 @@ void OverviewWindowDragController::CompleteDrag(
     // If we are in drag to close mode close the window if it has been dragged
     // enough, otherwise reposition it and set its opacity back to its original
     // value.
+    window_selector_->GetGridWithRootWindow(item_->root_window())->EndNudge();
     if (std::abs((location_in_screen - initial_event_location_).y()) >
         kDragToCloseDistanceThresholdDp) {
       item_->AnimateAndCloseWindow(
           (location_in_screen - initial_event_location_).y() < 0);
     } else {
       item_->SetOpacity(original_opacity_);
-      window_selector_->GetGridWithRootWindow(item_->root_window())->EndNudge();
       window_selector_->PositionWindows(/*animate=*/true);
     }
   } else {

ash/wm/overview/window_grid.cc

@@ -33,6 +33,7 @@
 #include "ash/wm/overview/window_selector_item.h"
 #include "ash/wm/window_state.h"
 #include "base/i18n/string_search.h"
+#include "base/numerics/ranges.h"
 #include "base/strings/string_number_conversions.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/aura/client/aura_constants.h"
@@ -366,8 +367,10 @@ void WindowGrid::PositionWindows(bool animate,
   // position |ignored_item| if it is not nullptr and matches a item in
   // |window_list_|.
   for (size_t i = 0; i < window_list_.size(); ++i) {
-    if (ignored_item != nullptr && window_list_[i].get() == ignored_item)
+    if (window_list_[i]->animating_to_close() ||
+        (ignored_item != nullptr && window_list_[i].get() == ignored_item)) {
       continue;
+    }
 
     const bool should_animate = window_list_[i]->ShouldAnimateWhenEntering();
     window_list_[i]->SetBounds(
@@ -652,6 +655,10 @@ void WindowGrid::OnWindowDestroying(aura::Window* window) {
   auto iter = GetWindowSelectorItemIterContainingWindow(window);
   DCHECK(iter != window_list_.end());
 
+  // Windows that are animating to a close state already call PositionWindows,
+  // no need to call it twice.
+  const bool needs_repositioning = !((*iter)->animating_to_close());
+
   size_t removed_index = iter - window_list_.begin();
   window_list_.erase(iter);
 
@@ -673,7 +680,8 @@ void WindowGrid::OnWindowDestroying(aura::Window* window) {
       SelectedWindow()->SendAccessibleSelectionEvent();
   }
 
-  PositionWindows(true);
+  if (needs_repositioning)
+    PositionWindows(true);
 }
 
 void WindowGrid::OnWindowBoundsChanged(aura::Window* window,
@@ -866,9 +874,20 @@ void WindowGrid::StartNudge(WindowSelectorItem* item) {
     return;
   }
 
-  // TODO(sammiequon): We also want to do nothing if the an item from the
-  // previous row will drop onto the current row, this will cause the items to
-  // shift to the right, which looks weird.
+  // Do nothing if the last item from the previous row will drop onto the
+  // current row, this will cause the items in the current row to shift to the
+  // right while the previous item stays in the previous row, which looks weird.
+  if (src_rows[index] > 1) {
+    // Find the last item from the previous row.
+    size_t previous_row_last_index = index;
+    while (src_rows[previous_row_last_index] == src_rows[index]) {
+      --previous_row_last_index;
+    }
+
+    // Early return if the last item in the previous row changes rows.
+    if (src_rows[previous_row_last_index] != dst_rows[previous_row_last_index])
+      return;
+  }
 
   // Helper to check whether the item at |item_index| will be nudged.
   auto should_nudge = [&src_rows, &dst_rows, &index](size_t item_index) {
@@ -918,20 +937,14 @@ void WindowGrid::StartNudge(WindowSelectorItem* item) {
 }
 
 void WindowGrid::UpdateNudge(WindowSelectorItem* item, double value) {
-  DCHECK_GE(value, 0.0);
-  DCHECK_LE(value, 1.0);
-
   for (const auto& data : nudge_data_) {
-    // TODO(sammiequon): This can happen if a new drag has started while
-    // a previous window is still animating out. Find a more graceful way to
-    // handle this.
-    if (data.index > window_list_.size())
-      continue;
+    DCHECK_LT(data.index, window_list_.size());
 
     WindowSelectorItem* nudged_item = window_list_[data.index].get();
-    // TODO(sammiequon): Use a better looking formula than linear
-    // interpolation.
-    gfx::Rect bounds = gfx::Tween::RectValueBetween(value, data.src, data.dst);
+    double nudge_param = value * value / 30.0;
+    nudge_param = base::ClampToRange(nudge_param, 0.0, 1.0);
+    gfx::Rect bounds =
+        gfx::Tween::RectValueBetween(nudge_param, data.src, data.dst);
     nudged_item->SetBounds(bounds, OVERVIEW_ANIMATION_NONE);
   }
 }
@@ -1233,7 +1246,8 @@ bool WindowGrid::FitWindowRectsInBounds(const gfx::Rect& bounds,
   const gfx::Size item_size(0, height);
   size_t i = 0;
   for (const auto& window : window_list_) {
-    if (ignored_item && ignored_item == window.get()) {
+    if (window->animating_to_close() ||
+        (ignored_item && ignored_item == window.get())) {
       // Increment the index anyways. PositionWindows will handle skipping this
       // entry.
       ++i;
@@ -1273,8 +1287,10 @@ bool WindowGrid::FitWindowRectsInBounds(const gfx::Rect& bounds,
         // If the |ignored_item| is the last item, update |out_max_bottom|
         // before breaking the loop, but no need to add the height, as the last
         // item does not contribute to the grid bounds.
-        if (ignored_item && ignored_item == window_list_.back().get())
+        if (window_list_.back()->animating_to_close() ||
+            (ignored_item && ignored_item == window_list_.back().get())) {
           *out_max_bottom = top;
+        }
         break;
       }
       left = bounds.x();

ash/wm/overview/window_selector_item.cc

@@ -819,7 +819,8 @@ void WindowSelectorItem::SendAccessibleSelectionEvent() {
 void WindowSelectorItem::AnimateAndCloseWindow(bool up) {
   base::RecordAction(base::UserMetricsAction("WindowSelector_SwipeToClose"));
 
-  window_selector_->PositionWindows(/*animate=*/true, /*ignored_item=*/this);
+  animating_to_close_ = true;
+  window_selector_->PositionWindows(/*animate=*/true);
   caption_container_view_->listener_button()->ResetListener();
   close_button_->ResetListener();
 

ash/wm/overview/window_selector_item.h

@@ -242,6 +242,9 @@ class ASH_EXPORT WindowSelectorItem : public views::ButtonListener,
     should_restack_on_animation_end_ = val;
   }
 
+  bool animating_to_close() const { return animating_to_close_; }
+  void set_animating_to_close(bool val) { animating_to_close_ = val; }
+
   float GetCloseButtonOpacityForTesting();
   float GetTitlebarOpacityForTesting();
   gfx::Rect GetShadowBoundsForTesting();
@@ -388,6 +391,11 @@ class ASH_EXPORT WindowSelectorItem : public views::ButtonListener,
   // widgets.
   bool should_restack_on_animation_end_ = false;
 
+  // True if the windows are still alive so they can have a closing animation.
+  // These windows should not be used in calculations for
+  // WindowGrid::PositionWindows.
+  bool animating_to_close_ = false;
+
   // The shadow around the overview window. Shadows the original window, not
   // |item_widget_|. Done here instead of on the original window because of the
   // rounded edges mask applied on entering overview window.

ash/wm/overview/window_selector_unittest.cc

@@ -3075,6 +3075,18 @@ TEST_F(WindowSelectorTest, PositionWindows) {
   EXPECT_EQ(bounds1, item1->target_bounds());
   EXPECT_NE(bounds2, item2->target_bounds());
   EXPECT_NE(bounds3, item3->target_bounds());
+
+  // Return the windows to their original bounds.
+  window_selector()->PositionWindows(/*animate=*/false);
+
+  // Verify that items that are animating before closing are ignored by
+  // PositionWindows.
+  item1->set_animating_to_close(true);
+  item2->set_animating_to_close(true);
+  window_selector()->PositionWindows(/*animate=*/false);
+  EXPECT_EQ(bounds1, item1->target_bounds());
+  EXPECT_EQ(bounds2, item2->target_bounds());
+  EXPECT_NE(bounds3, item3->target_bounds());
 }
 
 class SplitViewWindowSelectorTest : public WindowSelectorTest {
@@ -3554,6 +3566,65 @@ TEST_F(SplitViewWindowSelectorTest, NoNudgingWhenNumRowsChange) {
   EXPECT_EQ(item4_bounds, item4->target_bounds());
 }
 
+// Tests that no nudging occurs when the item to be deleted results in an item
+// from the previous row to drop down to the current row, thus causing the items
+// to the right of the item to be shifted right, which is visually unacceptable.
+TEST_F(SplitViewWindowSelectorTest, NoNudgingWhenLastItemOnPreviousRowDrops) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(features::kOverviewSwipeToClose);
+
+  // Set up five equal windows, which would split into two rows in overview
+  // mode. Removing one window would cause the rows to rearrange, with the third
+  // item dropping down from the first row to the second row. Create the windows
+  // backward so the the window indexs match the order seen in overview, as
+  // overview windows are ordered by MRU.
+  const int kWindows = 5;
+  std::unique_ptr<aura::Window> windows[kWindows];
+  for (int i = kWindows - 1; i >= 0; --i)
+    windows[i] = CreateTestWindow();
+
+  ToggleOverview();
+  ASSERT_TRUE(window_selector_controller()->IsSelecting());
+
+  WindowSelectorItem* items[kWindows];
+  gfx::Rect item_bounds[kWindows];
+  for (int i = 0; i < kWindows; ++i) {
+    items[i] = GetWindowItemForWindow(0, windows[i].get());
+    item_bounds[i] = items[i]->target_bounds();
+  }
+
+  // Drag the forth item past the drag to swipe threshold. None of the other
+  // window bounds should change, as none of them should be nudged, because
+  // deleting the fourth item will cause the third item to drop down from the
+  // first row to the second.
+  window_selector()->InitiateDrag(items[3], item_bounds[3].CenterPoint());
+  window_selector()->Drag(items[3],
+                          gfx::Point(item_bounds[3].CenterPoint().x(),
+                                     item_bounds[3].CenterPoint().y() + 160));
+  EXPECT_EQ(item_bounds[0], items[0]->target_bounds());
+  EXPECT_EQ(item_bounds[1], items[1]->target_bounds());
+  EXPECT_EQ(item_bounds[2], items[2]->target_bounds());
+  EXPECT_EQ(item_bounds[4], items[4]->target_bounds());
+
+  // Drag the fourth item back to its start drag location and release, so that
+  // it does not get deleted.
+  window_selector()->Drag(items[3], item_bounds[3].CenterPoint());
+  window_selector()->CompleteDrag(items[3], item_bounds[3].CenterPoint());
+
+  // Drag the first item past the drag to swipe threshold. The second and third
+  // items should nudge as expected as there is no item dropping down to their
+  // row. The fourth and fifth items should not nudge as they are in a different
+  // row than the first item.
+  window_selector()->InitiateDrag(items[0], item_bounds[0].CenterPoint());
+  window_selector()->Drag(items[0],
+                          gfx::Point(item_bounds[0].CenterPoint().x(),
+                                     item_bounds[0].CenterPoint().y() + 160));
+  EXPECT_NE(item_bounds[1], items[1]->target_bounds());
+  EXPECT_NE(item_bounds[2], items[2]->target_bounds());
+  EXPECT_EQ(item_bounds[3], items[3]->target_bounds());
+  EXPECT_EQ(item_bounds[4], items[4]->target_bounds());
+}
+
 // Verify the window grid size changes as expected when dragging items around in
 // overview mode when split view is enabled.
 TEST_F(SplitViewWindowSelectorTest, WindowGridSizeWhileDraggingWithSplitView) {