[synthesized scroll] Fix waiting for synthesized scrolls.

When wheel-events are dispatched to the renderer to perform a scroll, it
can start a compositor-driven animation to perform the scroll. The
animation can continue even after the event-dispatch completes. As a
result, for a synthesized scroll, when the request for
WaitForInputProcessed() returns to the browser, not all of the desired
scrolls may have taken effect. To fix this, introduce a notification API
in LayerTreeHost for when any ongoing scroll animation ends. Use this
API to ensure all scroll-animations drive by the compositor are complete
before returning from a WaitForInputProcessed() request.

BUG=1085891

Change-Id: Ie4d67df69786cd30da7d830fa3f952967f1399be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2213401Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771837}

cc/trees/layer_tree_host.cc

@@ -94,6 +94,9 @@ LayerTreeHost::InitParams::InitParams(InitParams&&) = default;
 LayerTreeHost::InitParams& LayerTreeHost::InitParams::operator=(InitParams&&) =
     default;
 
+LayerTreeHost::ScrollAnimationState::ScrollAnimationState() = default;
+LayerTreeHost::ScrollAnimationState::~ScrollAnimationState() = default;
+
 std::unique_ptr<LayerTreeHost> LayerTreeHost::CreateThreaded(
     scoped_refptr<base::SingleThreadTaskRunner> impl_task_runner,
     InitParams params) {
@@ -846,6 +849,24 @@ void LayerTreeHost::ApplyViewportChanges(const ScrollAndScaleSet& info) {
   if (info.inner_viewport_scroll.element_id)
     inner_viewport_scroll_delta = info.inner_viewport_scroll.scroll_delta;
 
+  // When a new scroll-animation starts, it is necessary to check
+  // |info.manipulation_info| to make sure the scroll-animation was started by
+  // an input event.
+  // If there is already an ongoing scroll-animation, then it is necessary to
+  // only look at |info.ongoing_scroll_animation| (since it is possible for the
+  // scroll-animation to continue even if no event was handled).
+  bool new_ongoing_scroll =
+      scroll_animation_.in_progress
+          ? info.ongoing_scroll_animation
+          : (info.ongoing_scroll_animation && info.manipulation_info);
+  if (scroll_animation_.in_progress && !new_ongoing_scroll) {
+    scroll_animation_.in_progress = false;
+    if (!scroll_animation_.end_notification.is_null())
+      std::move(scroll_animation_.end_notification).Run();
+  } else {
+    scroll_animation_.in_progress = new_ongoing_scroll;
+  }
+
   if (inner_viewport_scroll_delta.IsZero() && info.page_scale_delta == 1.f &&
       info.elastic_overscroll_delta.IsZero() && !info.top_controls_delta &&
       !info.bottom_controls_delta &&
@@ -1065,6 +1086,15 @@ void LayerTreeHost::RequestPresentationTimeForNextFrame(
   pending_presentation_time_callbacks_.push_back(std::move(callback));
 }
 
+void LayerTreeHost::RequestScrollAnimationEndNotification(
+    base::OnceClosure callback) {
+  DCHECK(scroll_animation_.end_notification.is_null());
+  if (scroll_animation_.in_progress)
+    scroll_animation_.end_notification = std::move(callback);
+  else
+    std::move(callback).Run();
+}
+
 void LayerTreeHost::SetRootLayer(scoped_refptr<Layer> root_layer) {
   if (root_layer_.get() == root_layer.get())
     return;

cc/trees/layer_tree_host.h

@@ -320,6 +320,10 @@ class CC_EXPORT LayerTreeHost : public MutatorHostClient {
       base::OnceCallback<void(const gfx::PresentationFeedback&)>;
   void RequestPresentationTimeForNextFrame(PresentationTimeCallback callback);
 
+  // Registers a callback that is run when any ongoing scroll-animation ends. If
+  // there are no ongoing animations, then the callback is run immediately.
+  void RequestScrollAnimationEndNotification(base::OnceClosure callback);
+
   // Layer tree accessors and modifiers ------------------------
 
   // Sets or gets the root of the Layer tree. Children of the root Layer are
@@ -461,6 +465,10 @@ class CC_EXPORT LayerTreeHost : public MutatorHostClient {
     return raster_color_space_;
   }
 
+  bool HasCompositorDrivenScrollAnimationForTesting() const {
+    return scroll_animation_.in_progress;
+  }
+
   // This layer tree may be embedded in a hierarchy that has page scale
   // factor controlled at the top level. We represent that scale here as
   // 'external_page_scale_factor', a value that affects raster scale in the
@@ -898,6 +906,17 @@ class CC_EXPORT LayerTreeHost : public MutatorHostClient {
   // added here.
   std::vector<PresentationTimeCallback> pending_presentation_time_callbacks_;
 
+  struct ScrollAnimationState {
+    ScrollAnimationState();
+    ~ScrollAnimationState();
+
+    // Tracks whether there is an ongoing compositor-driven scroll animation.
+    bool in_progress = false;
+
+    // Callback to run when the scroll-animation ends.
+    base::OnceClosure end_notification;
+  } scroll_animation_;
+
   // Latency information for work done in ProxyMain::BeginMainFrame. The
   // unique_ptr is allocated in RequestMainFrameUpdate, and passed to Blink's
   // LocalFrameView that fills in the fields. This object adds the timing for

cc/trees/layer_tree_host_impl.cc

@@ -5280,6 +5280,9 @@ std::unique_ptr<ScrollAndScaleSet> LayerTreeHostImpl::ProcessScrollDeltas() {
   scroll_info->overscroll_delta = overscroll_delta_for_main_thread_;
   overscroll_delta_for_main_thread_ = gfx::Vector2dF();
 
+  scroll_info->ongoing_scroll_animation =
+      !!mutator_host_->ImplOnlyScrollAnimatingElement();
+
   // Use the |last_latched_scroller_| rather than the |CurrentlyScrollingNode|
   // since the latter may be cleared by a GSE before we've committed these
   // values to the main thread.

cc/trees/layer_tree_host_unittest_scroll.cc

@@ -10,6 +10,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "build/build_config.h"
 #include "cc/animation/animation_host.h"
 #include "cc/base/completion_event.h"
 #include "cc/input/main_thread_scrolling_reason.h"
@@ -1058,6 +1059,116 @@ class LayerTreeHostScrollTestImplOnlyScroll : public LayerTreeHostScrollTest {
 // This tests scrolling on the impl side which is only possible with a thread.
 MULTI_THREAD_TEST_F(LayerTreeHostScrollTestImplOnlyScroll);
 
+// TODO(crbug.com/574283): Mac currently doesn't support smooth scrolling wheel
+// events.
+#if !defined(OS_MACOSX)
+// This test simulates scrolling on the impl thread such that it starts a scroll
+// animation. It ensures that RequestScrollAnimationEndNotification() correctly
+// notifies the callback after the animation ends.
+class SmoothScrollAnimationEndNotification : public LayerTreeHostScrollTest {
+ public:
+  SmoothScrollAnimationEndNotification() = default;
+
+  void InitializeSettings(LayerTreeSettings* settings) override {
+    LayerTreeHostScrollTest::InitializeSettings(settings);
+    settings->enable_smooth_scroll = true;
+  }
+
+  void SetupTree() override {
+    LayerTreeHostScrollTest::SetupTree();
+
+    Layer* root_layer = layer_tree_host()->root_layer();
+    Layer* root_scroll_layer =
+        layer_tree_host()->OuterViewportScrollLayerForTesting();
+
+    child_layer_ = Layer::Create();
+    child_layer_->SetElementId(
+        LayerIdToElementIdForTesting(child_layer_->id()));
+    child_layer_->SetBounds(gfx::Size(110, 110));
+
+    child_layer_->SetIsDrawable(true);
+    child_layer_->SetHitTestable(true);
+    child_layer_->SetElementId(
+        LayerIdToElementIdForTesting(child_layer_->id()));
+    child_layer_->SetBounds(root_scroll_layer->bounds());
+    root_layer->AddChild(child_layer_);
+
+    CopyProperties(root_scroll_layer, child_layer_.get());
+    CreateTransformNode(child_layer_.get());
+    CreateScrollNode(child_layer_.get(), root_layer->bounds());
+  }
+
+  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
+
+  void WillCommit() override {
+    // Keep the test committing (otherwise the early out for no update
+    // will stall the test).
+    if (layer_tree_host()->SourceFrameNumber() < 2) {
+      layer_tree_host()->SetNeedsCommit();
+    }
+  }
+
+  void DidActivateTreeOnThread(LayerTreeHostImpl* host_impl) override {
+    if (host_impl->active_tree()->source_frame_number() < 0)
+      return;
+
+    if (host_impl->active_tree()->source_frame_number() == 0) {
+      const gfx::Point scroll_point(10, 10);
+      const gfx::Vector2dF scroll_amount(350, -350);
+      auto scroll_state = BeginState(scroll_point);
+      scroll_state->data()->delta_granularity =
+          ui::ScrollGranularity::kScrollByPixel;
+      InputHandler::ScrollStatus status = host_impl->ScrollBegin(
+          scroll_state.get(), ui::ScrollInputType::kWheel);
+      EXPECT_EQ(InputHandler::SCROLL_ON_IMPL_THREAD, status.thread);
+      scroll_state = UpdateState(scroll_point, scroll_amount);
+      scroll_state->data()->delta_granularity =
+          ui::ScrollGranularity::kScrollByPixel;
+      host_impl->ScrollUpdate(scroll_state.get());
+
+      EXPECT_TRUE(
+          !!host_impl->mutator_host()->ImplOnlyScrollAnimatingElement());
+    } else if (!scroll_end_requested_) {
+      host_impl->ScrollEnd(false);
+      scroll_end_requested_ = true;
+    }
+    PostSetNeedsCommitToMainThread();
+  }
+
+  void UpdateLayerTreeHost() override {
+    if (scroll_animation_started_)
+      return;
+
+    if (layer_tree_host()->HasCompositorDrivenScrollAnimationForTesting()) {
+      scroll_animation_started_ = true;
+      layer_tree_host()->RequestScrollAnimationEndNotification(
+          base::BindOnce(&SmoothScrollAnimationEndNotification::OnScrollEnd,
+                         base::Unretained(this)));
+    }
+  }
+
+  void AfterTest() override {
+    EXPECT_TRUE(scroll_end_requested_);
+    EXPECT_TRUE(scroll_animation_started_);
+    EXPECT_TRUE(scroll_animation_ended_);
+  }
+
+ private:
+  void OnScrollEnd() {
+    scroll_animation_ended_ = true;
+    EndTest();
+  }
+
+  scoped_refptr<Layer> child_layer_;
+
+  bool scroll_end_requested_ = false;
+  bool scroll_animation_started_ = false;
+  bool scroll_animation_ended_ = false;
+};
+
+MULTI_THREAD_TEST_F(SmoothScrollAnimationEndNotification);
+#endif  // !defined(OS_MACOSX)
+
 void DoGestureScroll(LayerTreeHostImpl* host_impl,
                      const scoped_refptr<Layer>& scroller,
                      gfx::Vector2dF offset) {

cc/trees/scroll_and_scale_set.h

@@ -97,6 +97,10 @@ struct CC_EXPORT ScrollAndScaleSet {
   // ended.
   bool scroll_gesture_did_end;
 
+  // Tracks whether there is an ongoing compositor-driven animation for a
+  // scroll.
+  bool ongoing_scroll_animation = false;
+
   // Tracks different methods of scrolling (e.g. wheel, touch, precision
   // touchpad, etc.).
   ManipulationInfo manipulation_info;

content/renderer/input/widget_input_handler_manager.cc

@@ -471,15 +471,19 @@ static void WaitForInputProcessedFromMain(
   auto redraw_complete_callback = base::BindOnce(
       &WidgetInputHandlerManager::InputWasProcessed, manager->AsWeakPtr());
 
-  // We consider all observable effects of an input gesture to be processed
+  // Since wheel-events can kick off animations, we can not consider
+  // all observable effects of an input gesture to be processed
   // when the CompositorFrame caused by that input has been produced, send, and
-  // displayed. RequestPresentation will force a a commit and redraw and
-  // callback when the CompositorFrame has been displayed in the display
-  // service. Some examples of non-trivial effects that require waiting that
-  // long: committing NonFastScrollRegions to the compositor, sending
-  // touch-action rects to the browser, and sending updated surface information
-  // to the display compositor for up-to-date OOPIF hit-testing.
-  render_widget->RequestPresentation(std::move(redraw_complete_callback));
+  // displayed. Therefore, explicitly request the presentation *after* any
+  // ongoing scroll-animation ends. After the scroll-animation ends (if any),
+  // the call will force a commit and redraw and callback when the
+  // CompositorFrame has been displayed in the display service. Some examples of
+  // non-trivial effects that require waiting that long: committing
+  // NonFastScrollRegions to the compositor, sending touch-action rects to the
+  // browser, and sending updated surface information to the display compositor
+  // for up-to-date OOPIF hit-testing.
+  render_widget->RequestPresentationAfterScrollAnimationEnd(
+      std::move(redraw_complete_callback));
 }
 
 void WidgetInputHandlerManager::WaitForInputProcessed(
@@ -490,9 +494,9 @@ void WidgetInputHandlerManager::WaitForInputProcessed(
   input_processed_callback_ = std::move(callback);
 
   // We mustn't touch render_widget_ from the impl thread so post all the setup
-  // to the main thread.
-  main_thread_task_runner_->PostTask(
-      FROM_HERE,
+  // to the main thread. Make sure the callback runs after all the queued events
+  // are dispatched.
+  input_event_queue_->QueueClosure(
       base::BindOnce(&WaitForInputProcessedFromMain, render_widget_));
 }
 

content/renderer/render_widget.cc

@@ -1038,6 +1038,13 @@ void RenderWidget::RequestPresentation(PresentationTimeCallback callback) {
   layer_tree_host_->SetNeedsCommitWithForcedRedraw();
 }
 
+void RenderWidget::RequestPresentationAfterScrollAnimationEnd(
+    PresentationTimeCallback callback) {
+  layer_tree_host_->RequestScrollAnimationEndNotification(
+      base::BindOnce(&RenderWidget::RequestPresentation,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
 void RenderWidget::DidPresentForceDrawFrame(
     int snapshot_id,
     const gfx::PresentationFeedback& feedback) {

content/renderer/render_widget.h

@@ -561,6 +561,11 @@ class CONTENT_EXPORT RenderWidget
       base::OnceCallback<void(const gfx::PresentationFeedback&)>;
   virtual void RequestPresentation(PresentationTimeCallback callback);
 
+  // Forces a redraw after any ongoing scroll-animation ends, and invokes the
+  // callback once the frame is displayed to the user.
+  void RequestPresentationAfterScrollAnimationEnd(
+      PresentationTimeCallback callback);
+
   base::WeakPtr<RenderWidget> AsWeakPtr();
 
  protected: