Move viewport actions into an ApplyScroll callback.

third_party/WebKit/Source/core/input/EventHandler.cpp

Index: third_party/WebKit/Source/core/input/EventHandler.cpp
diff --git a/third_party/WebKit/Source/core/input/EventHandler.cpp b/third_party/WebKit/Source/core/input/EventHandler.cpp
index ae2dd983b48968685bf13af7b6a02b9a18abed67..2eb8959a0f002f41bbeb54f386b962f5508b6134 100644
--- a/third_party/WebKit/Source/core/input/EventHandler.cpp
+++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -130,6 +130,45 @@ const AtomicString& touchEventNameForTouchPointState(PlatformTouchPoint::TouchSt
     }
 }
 
+void computeScrollChainForOne(Node& node, std::deque<int>& scrollChain)

[tdresser, 2016/04/05 19:31:12]

Let's be a bit more verbose, and go with "computeScrollChainForSingleNode", or
something like that.

Should we add a TODO that this method should cease existing once we've migrated
everything over to the scroll customization path?

[bokan, 2016/04/06 15:21:01]

Both done.

+{
+    scrollChain.clear();
+
+    ASSERT(node.layoutObject());
+    Element* element = toElement(&node);
+
+    scrollChain.push_front(DOMNodeIds::idForNode(element));
+}
+
+void recomputeScrollChain(const LocalFrame& frame, const Node& startNode,
+    std::deque<int>& scrollChain)
+{
+    scrollChain.clear();
+
+    ASSERT(startNode.layoutObject());
+    LayoutBox* curBox = startNode.layoutObject()->enclosingBox();
+
+    // Scrolling propagates along the containing block chain.
+    while (curBox && !curBox->isLayoutView()) {
+        Node* curNode = curBox->node();
+        // FIXME: this should reject more elements, as part of crbug.com/410974.
+        if (curNode && curNode->isElementNode()) {
+            Element* curElement = toElement(curNode);
+            if (curElement == frame.document()->scrollingElement())
+                break;
+            scrollChain.push_front(DOMNodeIds::idForNode(curElement));
+        }
+        curBox = curBox->containingBlock();
+    }
+    // TODO(tdresser): this should sometimes be excluded, as part of crbug.com/410974.
+    // We need to ensure that the scrollingElement is always part of
+    // the scroll chain. In quirks mode, when the scrollingElement is
+    // the body, some elements may use the documentElement as their
+    // containingBlock, so we ensure the scrollingElement is added
+    // here.
+    scrollChain.push_front(DOMNodeIds::idForNode(frame.document()->scrollingElement()));
+}
+
 } // namespace
 
 using namespace HTMLNames;
@@ -211,35 +250,6 @@ static inline bool shouldRefetchEventTarget(const MouseEventWithHitTestResults&
     return targetNode->isShadowRoot() && isHTMLInputElement(*toShadowRoot(targetNode)->host());
 }
 
-void recomputeScrollChain(const LocalFrame& frame, const Node& startNode,
-    std::deque<int>& scrollChain)
-{
-    scrollChain.clear();
-
-    ASSERT(startNode.layoutObject());
-    LayoutBox* curBox = startNode.layoutObject()->enclosingBox();
-
-    // Scrolling propagates along the containing block chain.
-    while (curBox && !curBox->isLayoutView()) {
-        Node* curNode = curBox->node();
-        // FIXME: this should reject more elements, as part of crbug.com/410974.
-        if (curNode && curNode->isElementNode()) {
-            Element* curElement = toElement(curNode);
-            if (curElement == frame.document()->scrollingElement())
-                break;
-            scrollChain.push_front(DOMNodeIds::idForNode(curElement));
-        }
-        curBox = curBox->containingBlock();
-    }
-    // TODO(tdresser): this should sometimes be excluded, as part of crbug.com/410974.
-    // We need to ensure that the scrollingElement is always part of
-    // the scroll chain. In quirks mode, when the scrollingElement is
-    // the body, some elements may use the documentElement as their
-    // containingBlock, so we ensure the scrollingElement is added
-    // here.
-    scrollChain.push_front(DOMNodeIds::idForNode(frame.document()->scrollingElement()));
-}
-
 EventHandler::EventHandler(LocalFrame* frame)
     : m_frame(frame)
     , m_mousePressed(false)
@@ -602,7 +612,82 @@ void EventHandler::stopAutoscroll()
         controller->stopAutoscroll();
 }
 
-ScrollResult EventHandler::physicalScroll(ScrollGranularity granularity, const FloatSize& delta, Node* startNode, Node** stopNode, bool* consumed)
+ScrollResult EventHandler::scrollBox(LayoutBox* box,
+    ScrollGranularity granularity, const FloatSize& delta,
+    const FloatPoint& position, const FloatSize& velocity,
+    bool* wasRootScroller)
+{
+    ASSERT(box);
+    Node* node = box->node();
+    Document* document = m_frame->document();
+    Element* scrollingElement = document->scrollingElement();
+
+    bool isRootFrame = !document->ownerElement();
+
+    // TODO(bokan): If the applyScroll is installed on the body, we can still
+    // hit the HTML element for scrolling in which case it'll bubble up to the
+    // document node and try to scroll the LayoutView directly. Make sure we
+    // never scroll the LayoutView like this by manually resetting the scroll to
+    // happen on the scrolling element. This can also happen in QuirksMode when
+    // the body is scrollable and scrollingElement == nullptr.
+    if (node && node->isDocumentNode() && isRootFrame) {
+        node = scrollingElement
+            ? scrollingElement
+            : document->documentElement();
+    }
+
+    // If there's no ApplyScroll callback on the element, scroll as usuall in
+    // the non-scroll-customization case.
+    if (!node || !node->isElementNode() || !toElement(node)->getApplyScroll()) {
+        ASSERT(!isRootFrame
+            || node != scrollingElement
+            || (!scrollingElement && node != document->documentElement()));
+        *wasRootScroller = false;
+        return box->scroll(granularity, delta);
+    }
+
+    ASSERT(isRootFrame);
+
+    // If there is an ApplyScroll callback, its because we placed one on the
+    // root scroller to control top controls and overscroll. Invoke a scroll
+    // using parts of the scroll customization framework on just this element.
+    computeScrollChainForOne(*node, m_currentScrollChain);
+
+    OwnPtr<ScrollStateData> scrollStateData = adoptPtr(new ScrollStateData());
+    scrollStateData->delta_x = delta.width();
+    scrollStateData->delta_y = delta.height();
+    // TODO(bokan): This is abusing start_position. How should we store current

[bokan, 2016/04/04 21:44:38]

I obviously shouldn't be doing this but I'd like your input on how to pass the
current position through. Should we add another field?

[tdresser, 2016/04/05 19:31:12]

Yeah, add another field.

[bokan, 2016/04/06 15:21:01]

Ok, I'll add another field in a separate CL before landing this one (since it'll
add lots of boilerplate to this already large CL).

[bokan, 2016/04/07 01:27:15]

On second look, I think start_position has the semantics I'm looking for and I
got confused by the name. I sent you an email to clarify but I think this is
doing the same thing as CC does so I removed the TODO.

+    // position?
+    scrollStateData->start_position_x = position.x();
+    scrollStateData->start_position_y = position.y();
+    // TODO(bokan): This also feels wrong.

[bokan, 2016/04/04 21:44:38]

Ditto here. delta_garnularity is a double so I think you mean to pass the number
of pixels per unit of delta. I cast the enum here but that's a hack to make
everything work. Thoughts?

[tdresser, 2016/04/05 19:31:12]

Eventually granularity will be web exposed, so there will be spec debate here. I
don't feel particularly strongly about what we assign granularity to here for
now, we can likely just leave a TODO.

Though it wouldn't be too much effort to add a method which mapped from
granularity to something at least plausible.

[bokan, 2016/04/06 15:21:01]

sgtm, TODO added.
The problem with mapping granularity to pixel deltas here is that it'll depend
on the scrollable area it ends up scrolling. e.g. If we set page granularity,
the amount to scroll will increase as the area is larger. I suppose we could do
it here (it would require exposing bits of ScrollableArea I'd rather not) since
we know the node we're going to scrolling but this won't work in the general
ScrollCustomization case.

+    scrollStateData->delta_granularity = granularity;
+    scrollStateData->velocity_x = velocity.width();
+    scrollStateData->velocity_y = velocity.height();
+    scrollStateData->should_propagate = false;
+    scrollStateData->is_in_inertial_phase = false;
+    scrollStateData->from_user_input = true;
+    scrollStateData->delta_consumed_for_scroll_sequence = false;
+    ScrollState* scrollState =
+        ScrollState::create(scrollStateData.release());
+
+    customizedScroll(*node, *scrollState);
+
+    ScrollResult result(
+        scrollState->deltaX() != delta.width(),
+        scrollState->deltaY() != delta.height(),
+        scrollState->deltaX(),
+        scrollState->deltaY());
+
+    *wasRootScroller = true;
+    m_currentScrollChain.clear();
+
+    return result;
+}
+
+ScrollResult EventHandler::physicalScroll(ScrollGranularity granularity,
+    const FloatSize& delta, const FloatPoint& position,
+    const FloatSize& velocity, Node* startNode, Node** stopNode, bool* consumed)
 {
     if (consumed)
         *consumed = false;
@@ -622,7 +707,15 @@ ScrollResult EventHandler::physicalScroll(ScrollGranularity granularity, const F
         // chain past it.
         bool shouldStopChaining =
             stopNode && *stopNode && curBox->node() == *stopNode;
-        result = curBox->scroll(granularity, delta);
+        bool wasRootScroller = false;
+
+        result = scrollBox(
+            curBox,
+            granularity,
+            delta,
+            position,
+            velocity,
+            &wasRootScroller);
 
         if (result.didScroll() && stopNode)
             *stopNode = curBox->node();
@@ -632,6 +725,10 @@ ScrollResult EventHandler::physicalScroll(ScrollGranularity granularity, const F
             if (consumed)
                 *consumed = true;
             return result;
+        } else if (wasRootScroller) {
+            // Don't try to chain past the root scroller, even if there's
+            // eligible ancestors.
+            break;
         }
 
         curBox = curBox->containingBlock();
@@ -1887,13 +1984,21 @@ void EventHandler::defaultWheelEventHandler(Node* startNode, WheelEvent* wheelEv
 
     // FIXME: enable scroll customization in this case. See crbug.com/410974.
     bool consumed = false;
-    ScrollResult result = physicalScroll(granularity, delta, startNode, &node, &consumed);
+
+    // TODO(bokan): Use a negative position to designate wheel scrolling so
+    // that the handler can check settings()->reportWheelOverscroll. This is
+    // hacky, there should be a more explicit way to do this.

[bokan, 2016/04/04 21:44:38]

Wheel events shouldn't report overscroll unless this setting is on. OTOH, I
think wheel gestures provides overscroll without checking this flag so I think
we could maybe just remove that check and then we wouldn't need a way to
designate this as a wheel scroll. WDYT?

[tdresser, 2016/04/05 19:31:12]

There have been a bunch of bugs recently with wheel overscroll. dtapuska@, do
you know if we should be respecting this flag?

[dtapuska, 2016/04/05 19:58:58]

The setting is kind of useless; my guess is that it was to reduce the IPCs sent.
handleOverscroll was always sent on gestures; which causes an IPC up to the
render_host. Only Android and MacOSX do overscroll handling.

The reportWheelOverscroll is pretty much always set on Mac OSX, there is an
option that users can adjust but it was more useful when we supported older Mac
OS versions that didn't do rubberbanding. It was never set for Android;
ultimately I do believe this code can be removed.

[bokan, 2016/04/06 15:21:01]

Ok, I've removed the setting from Blink in a separate CL
(https://codereview.chromium.org/1861343002). It can still be disabled on Mac by
setting a value for the NSScrollViewRubberbanding system setting so drop the
overscroll based on that flag before sending it to the browser process on Mac. 

+    physicalScroll(
+        granularity,
+        delta,
+        FloatPoint(-1, -1),
+        FloatSize(),
+        startNode,
+        &node,
+        &consumed);
 
     if (consumed)
         wheelEvent->setDefaultHandled();
-
-    if (m_frame->isMainFrame() && m_frame->settings() && m_frame->settings()->reportWheelOverscroll())
-        handleOverscroll(result);
 }
 
 WebInputEventResult EventHandler::handleGestureShowPress()
@@ -2381,6 +2486,20 @@ void EventHandler::handleOverscroll(const ScrollResult& scrollResult, const Floa
     }
 }
 
+bool EventHandler::isRootScroller(const Node& node) const
+{
+    // The root scroller is the one Element on the page designated to perform
+    // "viewport actions" like top controls movement and overscroll glow.
+
+    if (!node.isElementNode() || node.document().ownerElement())
+        return false;
+
+    Element* scrollingElement = node.document().scrollingElement();
+    return scrollingElement
+        ? toElement(&node) == node.document().scrollingElement()
+        : toElement(&node) == node.document().documentElement();
+}
+
 WebInputEventResult EventHandler::handleGestureScrollUpdate(const PlatformGestureEvent& gestureEvent)
 {
     ASSERT(gestureEvent.type() == PlatformEvent::GestureScrollUpdate);
@@ -2425,6 +2544,7 @@ WebInputEventResult EventHandler::handleGestureScrollUpdate(const PlatformGestur
             OwnPtr<ScrollStateData> scrollStateData = adoptPtr(new ScrollStateData());
             scrollStateData->delta_x = delta.width();
             scrollStateData->delta_y = delta.height();
+            scrollStateData->delta_granularity = ScrollByPrecisePixel;
             scrollStateData->velocity_x = velocity.width();
             scrollStateData->velocity_y = velocity.height();
             scrollStateData->should_propagate = !gestureEvent.preventPropagation();
@@ -2454,17 +2574,20 @@ WebInputEventResult EventHandler::handleGestureScrollUpdate(const PlatformGestur
                 stopNode = m_previousGestureScrolledNode.get();
 
             bool consumed = false;
-            ScrollResult result = physicalScroll(granularity, delta, node, &stopNode, &consumed);
+            ScrollResult result = physicalScroll(
+                granularity,
+                delta,
+                FloatPoint(gestureEvent.position()),
+                velocity,
+                node,
+                &stopNode,
+                &consumed);
 
             if (gestureEvent.preventPropagation())
                 m_previousGestureScrolledNode = stopNode;
 
-            if (m_frame->isMainFrame() && (!stopNode || stopNode->layoutObject() == m_frame->view()->layoutView())) {
-                FloatPoint position = FloatPoint(gestureEvent.position().x(), gestureEvent.position().y());
-                handleOverscroll(result, position, velocity);
-            } else {
+            if (!stopNode || !isRootScroller(*stopNode))
                 resetOverscroll(result.didScrollX, result.didScrollY);
-            }
 
             if (consumed)
                 return WebInputEventResult::HandledSystem;