Revert "Snap for GestureFling on main thread."

This reverts commit 80efb86. Reason for revert: The new test fast/scroll-snap/animate-fling-to-snap-points.html is very flaky: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests%20(with%20patch)&tests=fast%2Fscroll-snap%2Fanimate-fling-to-snap-points.html https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&tests=fast%2Fscroll-snap%2Fanimate-fling-to-snap-points.html Original change's description: > Snap for GestureFling on main thread. > > This patch implements snapping for gesture fling on main thread. > The ScrollManager would implement SnapFlingClient to collect snap > information, schedule animation, and execute the scroll as instructed > by SnapFlingController. > > This patch also updates the interface of > SnapCoordinator::GetPositionForPoint() for it to return an Optional > FloatPoint, clearly indicating whether there exists a snap point for > the current scroll. > > Bug: 778259 > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I196e8c58c238cfad2a96e3f6a1c218f2c4f77a3c > Reviewed-on: https://chromium-review.googlesource.com/1083144 > Reviewed-by: Rick Byers <rbyers@chromium.org> > Reviewed-by: David Bokan <bokan@chromium.org> > Reviewed-by: Majid Valipour <majidvp@chromium.org> > Commit-Queue: Sandra Sun <sunyunjia@chromium.org> > Cr-Commit-Position: refs/heads/master@{#572263} TBR=rbyers@chromium.org,bokan@chromium.org,majidvp@chromium.org,sunyunjia@chromium.org Change-Id: Ie5add53034eb8558bd7df1c4efc17388065cbee5 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 778259 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1124910 Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#572345}
Denger-Network · Jul 3, 2018 · d352e8e · d352e8e
1 parent b4a829e
commit d352e8e
Show file tree

Hide file tree

Showing 23 changed files with 95 additions and 475 deletions.
diff --git a/cc/input/snap_fling_controller.h b/cc/input/snap_fling_controller.h
@@ -24,8 +24,8 @@ class SnapFlingCurve;
 class SnapFlingClient {
  public:
   virtual bool GetSnapFlingInfo(const gfx::Vector2dF& natural_displacement,
-                                gfx::Vector2dF* out_initial_offset,
-                                gfx::Vector2dF* out_target_offset) const = 0;
+                                gfx::Vector2dF* initial_offset,
+                                gfx::Vector2dF* target_offset) const = 0;
   virtual gfx::Vector2dF ScrollByForSnapFling(const gfx::Vector2dF& delta) = 0;
   virtual void ScrollEndForSnapFling() = 0;
   virtual void RequestAnimationForSnapFling() = 0;

diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
@@ -4354,8 +4354,8 @@ gfx::ScrollOffset LayerTreeHostImpl::GetVisualScrollOffset(
 
 bool LayerTreeHostImpl::GetSnapFlingInfo(
     const gfx::Vector2dF& natural_displacement_in_viewport,
-    gfx::Vector2dF* out_initial_offset,
-    gfx::Vector2dF* out_target_offset) const {
+    gfx::Vector2dF* initial_offset,
+    gfx::Vector2dF* target_offset) const {
   const ScrollNode* scroll_node = CurrentlyScrollingNode();
   if (!scroll_node || !scroll_node->snap_container_data.has_value())
     return false;
@@ -4365,7 +4365,7 @@ bool LayerTreeHostImpl::GetSnapFlingInfo(
   gfx::Vector2dF natural_displacement_in_content =
       gfx::ScaleVector2d(natural_displacement_in_viewport, 1.f / scale_factor);
 
-  *out_initial_offset =
+  *initial_offset =
       ScrollOffsetToVector2dF(GetVisualScrollOffset(*scroll_node));
 
   bool did_scroll_x = did_scroll_x_for_scroll_gesture_ ||
@@ -4374,15 +4374,15 @@ bool LayerTreeHostImpl::GetSnapFlingInfo(
                       natural_displacement_in_content.y() != 0;
 
   gfx::ScrollOffset snap_offset;
-  if (!data.FindSnapPosition(gfx::ScrollOffset(*out_initial_offset +
-                                               natural_displacement_in_content),
-                             did_scroll_x, did_scroll_y, &snap_offset)) {
+  if (!data.FindSnapPosition(
+          gfx::ScrollOffset(*initial_offset + natural_displacement_in_content),
+          did_scroll_x, did_scroll_y, &snap_offset)) {
     return false;
   }
 
-  *out_target_offset = ScrollOffsetToVector2dF(snap_offset);
-  out_target_offset->Scale(scale_factor);
-  out_initial_offset->Scale(scale_factor);
+  *target_offset = ScrollOffsetToVector2dF(snap_offset);
+  target_offset->Scale(scale_factor);
+  initial_offset->Scale(scale_factor);
   return true;
 }
 

diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h
@@ -615,8 +615,8 @@ class CC_EXPORT LayerTreeHostImpl
   gfx::ScrollOffset GetVisualScrollOffset(const ScrollNode& scroll_node) const;
 
   bool GetSnapFlingInfo(const gfx::Vector2dF& natural_displacement_in_viewport,
-                        gfx::Vector2dF* out_initial_offset,
-                        gfx::Vector2dF* out_target_offset) const override;
+                        gfx::Vector2dF* initial_offset,
+                        gfx::Vector2dF* target_offset) const override;
 
   // Returns the amount of delta that can be applied to scroll_node, taking
   // page scale into account.

diff --git a/third_party/WebKit/LayoutTests/fast/scroll-snap/animate-fling-to-snap-points.html b/third_party/WebKit/LayoutTests/fast/scroll-snap/animate-fling-to-snap-points.html
diff --git a/third_party/WebKit/LayoutTests/resources/gesture-util.js b/third_party/WebKit/LayoutTests/resources/gesture-util.js
@@ -27,30 +27,6 @@ function waitFor(condition) {
   });
 }
 
-function waitForAnimationEnd(getValue, max_frame, max_unchanged_frame) {
-  const MAX_FRAME = max_frame;
-  const MAX_UNCHANGED_FRAME = max_unchanged_frame;
-  var last_changed_frame = 0;
-  var last_position = getValue();
-  return new Promise((resolve, reject) => {
-    function tick(frames) {
-    // We requestAnimationFrame either for MAX_FRAME or until
-    // MAX_UNCHANGED_FRAME with no change have been observed.
-      if (frames >= MAX_FRAME || frames - last_changed_frame > MAX_UNCHANGED_FRAME) {
-        resolve();
-      } else {
-        current_value = getValue();
-        if (last_position != current_value) {
-          last_changed_frame = frames;
-          last_position = current_value;
-        }
-        requestAnimationFrame(tick.bind(this, frames + 1));
-      }
-    }
-    tick(0);
-  })
-}
-
 // Enums for gesture_source_type parameters in gpuBenchmarking synthetic
 // gesture methods. Must match C++ side enums in synthetic_gesture_params.h
 const GestureSourceType = {
@@ -84,21 +60,6 @@ function smoothScroll(pixels_to_scroll, start_x, start_y, gesture_source_type, d
   });
 }
 
-function swipe(pixels_to_scroll, start_x, start_y, direction, speed_in_pixels_s) {
-  return new Promise((resolve, reject) => {
-    if (chrome && chrome.gpuBenchmarking) {
-      chrome.gpuBenchmarking.swipe(direction,
-                                   pixels_to_scroll,
-                                   resolve,
-                                   start_x,
-                                   start_y,
-                                   speed_in_pixels_s);
-    } else {
-      reject('This test requires chrome.gpuBenchmarking');
-    }
-  })
-}
-
 function pinchBy(scale, centerX, centerY, speed_in_pixels_s, gesture_source_type) {
   return new Promise((resolve, reject) => {
     if (chrome && chrome.gpuBenchmarking) {
@@ -196,7 +157,3 @@ function mouseDragAndDrop(start_x, start_y, end_x, end_y) {
     }
   });
 }
-
-function approx_equals(actual, expected, epsilon) {
-  return actual >= expected - epsilon && actual <= expected + epsilon;
-}
diff --git a/third_party/blink/public/platform/web_input_event.h b/third_party/blink/public/platform/web_input_event.h
@@ -346,17 +346,6 @@ class WebInputEvent {
     return type_ == other.type_;
   }
 
-  bool IsGestureScroll() const {
-    switch (type_) {
-      case Type::kGestureScrollBegin:
-      case Type::kGestureScrollUpdate:
-      case Type::kGestureScrollEnd:
-        return true;
-      default:
-        return false;
-    }
-  }
-
   // Returns true if the WebInputEvent |type| is a pinch gesture event.
   static bool IsPinchGestureEventType(WebInputEvent::Type type) {
     return kGesturePinchTypeFirst <= type && type <= kGesturePinchTypeLast;

diff --git a/third_party/blink/renderer/core/dom/element.cc b/third_party/blink/renderer/core/dom/element.cc
@@ -971,11 +971,9 @@ void Element::setScrollLeft(double new_left) {
 
     FloatPoint end_point(new_left * box->Style()->EffectiveZoom(),
                          box->ScrollTop().ToFloat());
-    if (RuntimeEnabledFeatures::CSSScrollSnapPointsEnabled()) {
-      end_point = GetDocument()
-                      .GetSnapCoordinator()
-                      ->GetSnapPositionForPoint(*box, end_point, true, false)
-                      .value_or(end_point);
+    if (SnapCoordinator* coordinator = GetDocument().GetSnapCoordinator()) {
+      end_point =
+          coordinator->GetSnapPositionForPoint(*box, end_point, true, false);
     }
     box->SetScrollLeft(LayoutUnit::FromFloatRound(end_point.X()));
   }
@@ -1002,11 +1000,9 @@ void Element::setScrollTop(double new_top) {
 
     FloatPoint end_point(box->ScrollLeft().ToFloat(),
                          new_top * box->Style()->EffectiveZoom());
-    if (RuntimeEnabledFeatures::CSSScrollSnapPointsEnabled()) {
-      end_point = GetDocument()
-                      .GetSnapCoordinator()
-                      ->GetSnapPositionForPoint(*box, end_point, false, true)
-                      .value_or(end_point);
+    if (SnapCoordinator* coordinator = GetDocument().GetSnapCoordinator()) {
+      end_point =
+          coordinator->GetSnapPositionForPoint(*box, end_point, false, true);
     }
     box->SetScrollTop(LayoutUnit::FromFloatRound(end_point.Y()));
   }
@@ -1123,14 +1119,10 @@ void Element::ScrollLayoutBoxBy(const ScrollToOptions& scroll_to_options) {
         top * box->Style()->EffectiveZoom() + current_scaled_top;
 
     FloatPoint new_scaled_position(new_scaled_left, new_scaled_top);
-    if (RuntimeEnabledFeatures::CSSScrollSnapPointsEnabled()) {
-      new_scaled_position =
-          GetDocument()
-              .GetSnapCoordinator()
-              ->GetSnapPositionForPoint(*box, new_scaled_position,
-                                        scroll_to_options.hasLeft(),
-                                        scroll_to_options.hasTop())
-              .value_or(new_scaled_position);
+    if (SnapCoordinator* coordinator = GetDocument().GetSnapCoordinator()) {
+      new_scaled_position = coordinator->GetSnapPositionForPoint(
+          *box, new_scaled_position, scroll_to_options.hasLeft(),
+          scroll_to_options.hasTop());
     }
     box->ScrollToPosition(new_scaled_position, scroll_behavior);
   }
@@ -1155,14 +1147,10 @@ void Element::ScrollLayoutBoxTo(const ScrollToOptions& scroll_to_options) {
           box->Style()->EffectiveZoom();
 
     FloatPoint new_scaled_position(scaled_left, scaled_top);
-    if (RuntimeEnabledFeatures::CSSScrollSnapPointsEnabled()) {
-      new_scaled_position =
-          GetDocument()
-              .GetSnapCoordinator()
-              ->GetSnapPositionForPoint(*box, new_scaled_position,
-                                        scroll_to_options.hasLeft(),
-                                        scroll_to_options.hasTop())
-              .value_or(new_scaled_position);
+    if (SnapCoordinator* coordinator = GetDocument().GetSnapCoordinator()) {
+      new_scaled_position = coordinator->GetSnapPositionForPoint(
+          *box, new_scaled_position, scroll_to_options.hasLeft(),
+          scroll_to_options.hasTop());
     }
     box->ScrollToPosition(new_scaled_position, scroll_behavior);
   }
@@ -1201,14 +1189,10 @@ void Element::ScrollFrameBy(const ScrollToOptions& scroll_to_options) {
 
   FloatPoint new_scaled_position = viewport->ScrollOffsetToPosition(
       ScrollOffset(new_scaled_left, new_scaled_top));
-  if (RuntimeEnabledFeatures::CSSScrollSnapPointsEnabled()) {
-    new_scaled_position =
-        GetDocument()
-            .GetSnapCoordinator()
-            ->GetSnapPositionForPoint(
-                *GetDocument().GetLayoutView(), new_scaled_position,
-                scroll_to_options.hasLeft(), scroll_to_options.hasTop())
-            .value_or(new_scaled_position);
+  if (SnapCoordinator* coordinator = GetDocument().GetSnapCoordinator()) {
+    new_scaled_position = coordinator->GetSnapPositionForPoint(
+        *GetDocument().GetLayoutView(), new_scaled_position,
+        scroll_to_options.hasLeft(), scroll_to_options.hasTop());
   }
   viewport->SetScrollOffset(
       viewport->ScrollPositionToOffset(new_scaled_position),
@@ -1246,14 +1230,10 @@ void Element::ScrollFrameTo(const ScrollToOptions& scroll_to_options) {
 
   FloatPoint new_scaled_position =
       viewport->ScrollOffsetToPosition(ScrollOffset(scaled_left, scaled_top));
-  if (RuntimeEnabledFeatures::CSSScrollSnapPointsEnabled()) {
-    new_scaled_position =
-        GetDocument()
-            .GetSnapCoordinator()
-            ->GetSnapPositionForPoint(
-                *GetDocument().GetLayoutView(), new_scaled_position,
-                scroll_to_options.hasLeft(), scroll_to_options.hasTop())
-            .value_or(new_scaled_position);
+  if (SnapCoordinator* coordinator = GetDocument().GetSnapCoordinator()) {
+    new_scaled_position = coordinator->GetSnapPositionForPoint(
+        *GetDocument().GetLayoutView(), new_scaled_position,
+        scroll_to_options.hasLeft(), scroll_to_options.hasTop());
   }
   viewport->SetScrollOffset(
       viewport->ScrollPositionToOffset(new_scaled_position),

diff --git a/third_party/blink/renderer/core/frame/local_dom_window.cc b/third_party/blink/renderer/core/frame/local_dom_window.cc
@@ -1139,14 +1139,10 @@ void LocalDOMWindow::scrollBy(const ScrollToOptions& scroll_to_options) const {
                             y * GetFrame()->PageZoomFactor());
   FloatPoint new_scaled_position =
       viewport->ScrollOffsetToPosition(scaled_delta + current_offset);
-  if (RuntimeEnabledFeatures::CSSScrollSnapPointsEnabled()) {
-    new_scaled_position =
-        document()
-            ->GetSnapCoordinator()
-            ->GetSnapPositionForPoint(
-                *document()->GetLayoutView(), new_scaled_position,
-                scroll_to_options.hasLeft(), scroll_to_options.hasTop())
-            .value_or(new_scaled_position);
+  if (SnapCoordinator* coordinator = document()->GetSnapCoordinator()) {
+    new_scaled_position = coordinator->GetSnapPositionForPoint(
+        *document()->GetLayoutView(), new_scaled_position,
+        scroll_to_options.hasLeft(), scroll_to_options.hasTop());
   }
 
   ScrollBehavior scroll_behavior = kScrollBehaviorAuto;
@@ -1214,14 +1210,10 @@ void LocalDOMWindow::scrollTo(const ScrollToOptions& scroll_to_options) const {
 
   FloatPoint new_scaled_position =
       viewport->ScrollOffsetToPosition(ScrollOffset(scaled_x, scaled_y));
-  if (RuntimeEnabledFeatures::CSSScrollSnapPointsEnabled()) {
-    new_scaled_position =
-        document()
-            ->GetSnapCoordinator()
-            ->GetSnapPositionForPoint(
-                *document()->GetLayoutView(), new_scaled_position,
-                scroll_to_options.hasLeft(), scroll_to_options.hasTop())
-            .value_or(new_scaled_position);
+  if (SnapCoordinator* coordinator = document()->GetSnapCoordinator()) {
+    new_scaled_position = coordinator->GetSnapPositionForPoint(
+        *document()->GetLayoutView(), new_scaled_position,
+        scroll_to_options.hasLeft(), scroll_to_options.hasTop());
   }
 
   ScrollBehavior scroll_behavior = kScrollBehaviorAuto;

diff --git a/third_party/blink/renderer/core/frame/local_frame.cc b/third_party/blink/renderer/core/frame/local_frame.cc
@@ -1431,8 +1431,4 @@ void LocalFrame::ResumeSubresourceLoading() {
   pause_handle_bindings_.CloseAllBindings();
 }
 
-void LocalFrame::AnimateSnapFling(base::TimeTicks monotonic_time) {
-  GetEventHandler().AnimateSnapFling(monotonic_time);
-}
-
 }  // namespace blink
diff --git a/third_party/blink/renderer/core/frame/local_frame.h b/third_party/blink/renderer/core/frame/local_frame.h
@@ -359,8 +359,6 @@ class CORE_EXPORT LocalFrame final : public Frame,
 
   void ResumeSubresourceLoading();
 
-  void AnimateSnapFling(base::TimeTicks monotonic_time);
-
  private:
   friend class FrameNavigationDisabler;
 

diff --git a/third_party/blink/renderer/core/input/event_handler.cc b/third_party/blink/renderer/core/input/event_handler.cc
@@ -1215,10 +1215,6 @@ void EventHandler::ClearDragState() {
   should_only_fire_drag_over_event_ = false;
 }
 
-void EventHandler::AnimateSnapFling(base::TimeTicks monotonic_time) {
-  scroll_manager_->AnimateSnapFling(monotonic_time);
-}
-
 void EventHandler::SetCapturingMouseEventsNode(Node* n) {
   capturing_mouse_events_node_ = n;
   event_handler_will_reset_capturing_mouse_events_node_ = false;

diff --git a/third_party/blink/renderer/core/input/event_handler.h b/third_party/blink/renderer/core/input/event_handler.h
@@ -272,8 +272,6 @@ class CORE_EXPORT EventHandler final
     return *event_handler_registry_;
   }
 
-  void AnimateSnapFling(base::TimeTicks monotonic_time);
-
  private:
   enum NoCursorChangeType { kNoCursorChange };