overview: Fix flinging and slow scrolling for grid scrolling.

Slow scrolling was not working properly because of an optimization I
tried to add to not process scroll events that barely change the grid
but it was dropping too many events resulting in small scrolls not
eventually scrolling the grid at all.

Fix a bug with fling and add a test for it. Fling was not working, due
to a bug where the grid wouldn't shift until the API returned no offset,
in which it would not shift anyways.

Test: Added test
Bug: 978112
Change-Id: Ib9544706975aa7adfa5163c07eb835c1b06708e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1802957
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697754}

ash/wm/overview/overview_grid.cc

@@ -1386,6 +1386,7 @@ void OverviewGrid::StartScroll() {
   // |scroll_offset_| is added to adjust for that.
   rightmost_window_right -= scroll_offset_;
   scroll_offset_min_ = total_bounds.right() - rightmost_window_right;
+  scroll_current_delta_ = 0.f;
 
   presentation_time_recorder_ = CreatePresentationTimeHistogramRecorder(
       const_cast<ui::Compositor*>(root_window()->layer()->GetCompositor()),
@@ -1393,18 +1394,24 @@ void OverviewGrid::StartScroll() {
 }
 
 bool OverviewGrid::UpdateScrollOffset(float delta) {
+  scroll_current_delta_ += delta;
   float new_scroll_offset = scroll_offset_;
-  new_scroll_offset += delta;
+  new_scroll_offset += scroll_current_delta_;
   new_scroll_offset =
       base::ClampToRange(new_scroll_offset, scroll_offset_min_, 0.f);
+
+  // For flings, we want to return false if we hit one of the edges, which is
+  // when |new_scroll_offset| is exactly 0.f or |scroll_offset_min_|.
+  const bool in_range =
+      new_scroll_offset < 0.f && new_scroll_offset > scroll_offset_min_;
   if (new_scroll_offset == scroll_offset_)
-    return false;
+    return in_range;
 
   // Do not process scrolls that haven't moved much, unless we are at the
   // edges.
   if (std::abs(scroll_offset_ - new_scroll_offset) < kMinimumScrollDistanceDp &&
-      scroll_offset_ > scroll_offset_min_ && scroll_offset_ < 0) {
-    return false;
+      in_range) {
+    return true;
   }
 
   // Update the bounds of the items which are currently visible on screen.
@@ -1419,11 +1426,12 @@ bool OverviewGrid::UpdateScrollOffset(float delta) {
     }
   }
 
+  scroll_current_delta_ = 0.f;
   scroll_offset_ = new_scroll_offset;
 
   DCHECK(presentation_time_recorder_);
   presentation_time_recorder_->RequestNext();
-  return true;
+  return in_range;
 }
 
 void OverviewGrid::EndScroll() {

ash/wm/overview/overview_grid.h

@@ -285,7 +285,7 @@ class ASH_EXPORT OverviewGrid : public aura::WindowObserver,
 
   // |delta| is used for updating |scroll_offset_| with new scroll values so
   // that windows in tablet overview mode get positioned accordingly. Returns
-  // true if the grid was scrolled.
+  // true if the grid was moved to the edge.
   bool UpdateScrollOffset(float delta);
 
   void EndScroll();
@@ -317,6 +317,8 @@ class ASH_EXPORT OverviewGrid : public aura::WindowObserver,
 
   views::Widget* drop_target_widget() { return drop_target_widget_.get(); }
 
+  float scroll_offset() const { return scroll_offset_; }
+
   OverviewGridEventHandler* grid_event_handler() {
     return grid_event_handler_.get();
   }
@@ -452,6 +454,10 @@ class ASH_EXPORT OverviewGrid : public aura::WindowObserver,
   // are visible in tablet overview mode.
   float scroll_offset_min_ = 0;
 
+  // Sum of the deltas passed by |UpdateScrollOffset|, this is so we can ignore
+  // deltas that are too small for performance reasons.
+  float scroll_current_delta_ = 0.f;
+
   // Cached values of the item bounds so that they do not have to be calculated
   // on each scroll update.
   std::vector<gfx::RectF> items_scrolling_bounds_;

ash/wm/overview/overview_grid_event_handler.cc

@@ -77,6 +77,7 @@ void OverviewGridEventHandler::OnGestureEvent(ui::GestureEvent* event) {
     case ui::ET_GESTURE_SCROLL_UPDATE: {
       if (!ShouldUseTabletModeGridLayout())
         return;
+
       grid_->UpdateScrollOffset(event->details().scroll_x());
       event->SetHandled();
       break;
@@ -84,6 +85,7 @@ void OverviewGridEventHandler::OnGestureEvent(ui::GestureEvent* event) {
     case ui::ET_GESTURE_SCROLL_END: {
       if (!ShouldUseTabletModeGridLayout())
         return;
+
       grid_->EndScroll();
       event->SetHandled();
       break;
@@ -97,18 +99,19 @@ void OverviewGridEventHandler::OnAnimationStep(base::TimeTicks timestamp) {
   // Updates |grid_| based on |offset| when |observed_compositor_| begins a new
   // frame.
   DCHECK(observed_compositor_);
-  gfx::Vector2dF offset;
 
   // As a fling progresses, the velocity degenerates, and the difference in
   // offset is passed into |grid_| as an updated scroll value. Stop flinging if
   // the API for fling says to finish, or we reach one of the edges of the
-  // overview grid.
+  // overview grid. Update the grid even if the API says to stop flinging as it
+  // still produces a usable |offset|, but end the fling afterwards.
+  gfx::Vector2dF offset;
   bool continue_fling =
       fling_curve_->ComputeScrollOffset(timestamp, &offset, &fling_velocity_);
-  if (!continue_fling) {
-    continue_fling = grid_->UpdateScrollOffset(
-        fling_last_offset_ ? offset.x() - fling_last_offset_->x() : offset.x());
-  }
+  continue_fling = grid_->UpdateScrollOffset(
+                       fling_last_offset_ ? offset.x() - fling_last_offset_->x()
+                                          : offset.x()) &&
+                   continue_fling;
   fling_last_offset_ = base::make_optional(offset);
 
   if (!continue_fling)
@@ -143,6 +146,7 @@ void OverviewGridEventHandler::HandleFlingScroll(ui::GestureEvent* event) {
 void OverviewGridEventHandler::EndFling() {
   if (!observed_compositor_)
     return;
+
   observed_compositor_->RemoveAnimationObserver(this);
   observed_compositor_ = nullptr;
   fling_curve_.reset();

ash/wm/overview/overview_grid_event_handler.h

@@ -38,6 +38,8 @@ class OverviewGridEventHandler : public ui::EventHandler,
   void OnMouseEvent(ui::MouseEvent* event) override;
   void OnGestureEvent(ui::GestureEvent* event) override;
 
+  bool IsFlingInProgressForTesting() const { return !!fling_curve_; }
+
  private:
   // ui::CompositorAnimationObserver:
   void OnAnimationStep(base::TimeTicks timestamp) override;

ash/wm/overview/overview_session.cc

@@ -70,8 +70,8 @@ constexpr SkColor kNoItemsIndicatorTextColor = SK_ColorWHITE;
 
 // Values for scrolling the grid by using the keyboard.
 // TODO(sammiequon): See if we can use the same values used for web scrolling.
-constexpr int kKeyboardPressScrollingDp = 25;
-constexpr int kKeyboardHoldScrollingDp = 5;
+constexpr int kKeyboardPressScrollingDp = 75;
+constexpr int kKeyboardHoldScrollingDp = 15;
 
 // Returns the bounds for the overview window grid according to the split view
 // state. If split view mode is active, the overview window should open on the

ash/wm/overview/overview_session_unittest.cc

@@ -33,6 +33,7 @@
 #include "ash/wm/overview/overview_constants.h"
 #include "ash/wm/overview/overview_controller.h"
 #include "ash/wm/overview/overview_grid.h"
+#include "ash/wm/overview/overview_grid_event_handler.h"
 #include "ash/wm/overview/overview_highlight_controller.h"
 #include "ash/wm/overview/overview_item.h"
 #include "ash/wm/overview/overview_test_util.h"
@@ -48,6 +49,7 @@
 #include "ash/wm/splitview/split_view_utils.h"
 #include "ash/wm/tablet_mode/tablet_mode_browser_window_drag_delegate.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
+#include "ash/wm/tablet_mode/tablet_mode_controller_test_api.h"
 #include "ash/wm/tablet_mode/tablet_mode_window_drag_controller.h"
 #include "ash/wm/window_preview_view.h"
 #include "ash/wm/window_state.h"
@@ -69,6 +71,7 @@
 #include "ui/base/hit_test.h"
 #include "ui/compositor/layer_animation_sequence.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
+#include "ui/compositor/test/draw_waiter_for_test.h"
 #include "ui/display/display_layout.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/test/display_manager_test_api.h"
@@ -2819,9 +2822,6 @@ class OverviewSessionNewLayoutTest : public OverviewSessionTest {
     return windows;
   }
 
-  // TODO(sammiequon): Investigate simulating fling event for testing inertial
-  // scrolling.
-
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
 
@@ -2989,6 +2989,74 @@ TEST_F(OverviewSessionNewLayoutTest, HorizontalScrollingOnOverviewItem) {
   EXPECT_LT(leftmost_window->target_bounds(), left_bounds);
 }
 
+// A unique test class for testing flings in overview as those rely on observing
+// compositior animations which require a mock time task environment.
+class OverviewSessionNewLayoutFlingTest : public AshTestBase {
+ public:
+  OverviewSessionNewLayoutFlingTest()
+      : AshTestBase(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
+  ~OverviewSessionNewLayoutFlingTest() override = default;
+
+  // AshTestBase:
+  void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(features::kNewOverviewLayout);
+    AshTestBase::SetUp();
+
+    // Overview flinging is only available in tablet mode.
+    base::RunLoop().RunUntilIdle();
+    TabletModeControllerTestApi().EnterTabletMode();
+    base::RunLoop().RunUntilIdle();
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+  DISALLOW_COPY_AND_ASSIGN(OverviewSessionNewLayoutFlingTest);
+};
+
+TEST_F(OverviewSessionNewLayoutFlingTest, BasicFling) {
+  std::vector<std::unique_ptr<aura::Window>> windows(16);
+  for (int i = 15; i >= 0; --i)
+    windows[i] = CreateTestWindow();
+
+  ToggleOverview();
+  OverviewGrid* grid = GetOverviewSession()->grid_list()[0].get();
+  OverviewGridEventHandler* grid_event_handler = grid->grid_event_handler();
+
+  OverviewItem* item = GetOverviewItemForWindow(windows[2].get());
+  const gfx::Point item_center =
+      gfx::ToRoundedPoint(item->target_bounds().CenterPoint());
+
+  // Create a scroll sequence which results in a fling.
+  const gfx::Vector2d shift(-200, 0);
+  GetEventGenerator()->GestureScrollSequence(
+      item_center, item_center + shift, base::TimeDelta::FromMilliseconds(10),
+      10);
+
+  ui::DrawWaiterForTest::WaitForCompositingStarted(
+      windows[0]->GetRootWindow()->layer()->GetCompositor());
+  ASSERT_TRUE(grid_event_handler->IsFlingInProgressForTesting());
+
+  // Test that the scroll offset decreases as we advance the clock. Check the
+  // scroll offset instead of the item bounds as there is an optimization which
+  // does not update the item bounds of invisible elements. On some iterations,
+  // there may not be enough time passed to decay the velocity so the scroll
+  // offset will not change, but the overall change should be substantial.
+  constexpr int kMaxLoops = 10;
+  const float initial_scroll_offset = grid->scroll_offset();
+  float previous_scroll_offset = initial_scroll_offset;
+  for (int i = 0;
+       i < kMaxLoops && grid_event_handler->IsFlingInProgressForTesting();
+       ++i) {
+    task_environment_->FastForwardBy(base::TimeDelta::FromMilliseconds(50));
+
+    float scroll_offset = grid->scroll_offset();
+    EXPECT_LE(scroll_offset, previous_scroll_offset);
+    previous_scroll_offset = scroll_offset;
+  }
+
+  EXPECT_LT(grid->scroll_offset(), initial_scroll_offset - 100.f);
+}
+
 // Tests that a vertical scroll sequence will close the window it is scrolled
 // on.
 TEST_F(OverviewSessionNewLayoutTest, VerticalScrollingOnOverviewItem) {