Removed main-thread one dimensional scrolling paths.

third_party/WebKit/Source/core/frame/RootFrameViewport.cpp

Index: third_party/WebKit/Source/core/frame/RootFrameViewport.cpp
diff --git a/third_party/WebKit/Source/core/frame/RootFrameViewport.cpp b/third_party/WebKit/Source/core/frame/RootFrameViewport.cpp
index 2446d420779e8101175f849d7ddfe8f001489690..9ad1acb71a76376bd47f56c333fa1780230dda32 100644
--- a/third_party/WebKit/Source/core/frame/RootFrameViewport.cpp
+++ b/third_party/WebKit/Source/core/frame/RootFrameViewport.cpp
@@ -232,50 +232,54 @@ GraphicsLayer* RootFrameViewport::layerForVerticalScrollbar() const
     return layoutViewport().layerForVerticalScrollbar();
 }
 
-ScrollResultOneDimensional RootFrameViewport::userScroll(ScrollDirectionPhysical direction, ScrollGranularity granularity, float delta)
+ScrollResult RootFrameViewport::userScroll(ScrollGranularity granularity, const FloatSize& delta)
 {
-    updateScrollAnimator();
+    // TODO(bokan/ymalik): This method specialization can likely go away as a
+    // result of the smooth scrolling work.

[tdresser, 2016/02/29 14:53:43]

I may just be lacking context, but it isn't clear to me what this comment is
referring to.

[bokan, 2016/03/01 05:56:24]

I think it should be clearer now.

 
-    ScrollbarOrientation orientation = scrollbarOrientationFromDirection(direction);
+    updateScrollAnimator();
 
-    if (layoutViewport().userInputScrollable(orientation) && visualViewport().userInputScrollable(orientation)) {
-        // Distribute the scroll between the visual and layout viewport.
-        float step = scrollStep(granularity, orientation);
+    // Distribute the scroll between the visual and layout viewport.
 
-        if (direction == ScrollUp || direction == ScrollLeft)
-            delta = -delta;
+    float stepX = scrollStep(granularity, HorizontalScrollbar);
+    float stepY = scrollStep(granularity, VerticalScrollbar);
 
-        // This is the total amount we need to scroll. Instead of passing step
-        // to the scroll animator and letting it compute the total delta, we
-        // give it the delta it should scroll. This way we can apply the
-        // unused delta from the visual viewport to the layout viewport.
-        delta *= step;
+    FloatSize pixelDelta(delta);
+    pixelDelta.scale(stepX, stepY);
 
-        cancelProgrammaticScrollAnimation();
+    if (!layoutViewport().userInputScrollable(HorizontalScrollbar))
+        pixelDelta.setWidth(0);

[tdresser, 2016/02/29 14:53:43]

Can we avoid calling scrollStep in cases where we're going to ignore its output?

[bokan, 2016/03/01 05:56:24]

Actually, we'll still want to call scrollStep so we can return the
unusedScrollDelta in pixels, though I realized I don't do that here or in
ScrollableArea::userScroll. I've updated both.

+    if (!layoutViewport().userInputScrollable(VerticalScrollbar))
+        pixelDelta.setHeight(0);
 
-        float visualUsedDelta = visualViewport().scrollAnimator().computeDeltaToConsume(orientation, delta);
-        ScrollResultOneDimensional visualResult = visualViewport().scrollAnimator().userScroll(
-            orientation, granularity, visualUsedDelta);
+    // If the layoutViewport isn't scrollable, just let the visual viewport
+    // handle the scroll since it's always userInputScrollable.
+    if (pixelDelta.isZero())
+        return visualViewport().userScroll(granularity, delta);

[tdresser, 2016/02/29 14:53:43]

If one axis didn't scroll, but the other did, this will cause a change in
behavior, won't it? Shouldn't we allow scrolling the layout viewport on one
axis, but the visual viewport on the other?

[bokan, 2016/03/01 05:56:24]

I'm not sure I understand. If pixelDelta.isZero() is true then neither axis is
scrollable and we just let the visual viewport handle it (visual viewport is
always scrollable so I removed the checks). Otherwise we drop down and do the
distribution logic which I think should handle the case you mention.

I've actually rewritten this method to correctly handle the return type. It's a
little complicated (no way around it I think - the viewport distribution
requires a bit of bookkeeping for the return value) but I've added plenty of
comments and hopefully made it more clear.

 
-        // Scroll the layout viewport if all of the scroll was not applied to the
-        // visual viewport.
-        if (visualUsedDelta == delta)
-            return visualResult;
+    cancelProgrammaticScrollAnimation();
 
-        ScrollResultOneDimensional layoutResult = layoutViewport().scrollAnimator().userScroll(
-            orientation, granularity, delta - visualUsedDelta);
+    // TODO(bokan): Why do we call userScroll on the animators directly and
+    // not through the ScrollableAreas?
 
-        return ScrollResultOneDimensional(visualResult.didScroll || layoutResult.didScroll,
-            layoutResult.unusedScrollDelta);
-    }
+    // Precompute the amount of possible scrolling since, when animated,
+    // ScrollAnimator::userScroll will consume the total scroll delta even
+    // beyond the scrolling bounds.
+    FloatSize visualUsedDelta = visualViewport().scrollAnimator().computeDeltaToConsume(pixelDelta);
+    ScrollResult visualResult = visualViewport().scrollAnimator().userScroll(granularity, visualUsedDelta);
 
-    if (visualViewport().userInputScrollable(orientation))
-        return visualViewport().userScroll(direction, granularity, delta);
+    // Scroll the layout viewport if all of the scroll was not applied to the
+    // visual viewport.
+    if (visualUsedDelta == pixelDelta)
+        return visualResult;
 
-    if (layoutViewport().userInputScrollable(orientation))
-        return layoutViewport().userScroll(direction, granularity, delta);
+    ScrollResult layoutResult = layoutViewport().scrollAnimator().userScroll(granularity, pixelDelta - visualUsedDelta);

[tdresser, 2016/02/29 14:53:43]

I'm a bit confused about how this works. If I understand correctly:
1. If the layout viewport is user scrollable, scroll it.
2. If the layout viewport didn't scroll, try to scroll the visual viewport.
3. If the visual viewport didn't consume all the scroll, scroll the layout
viewport.

Why would we ever do anything in step #3? Is it for cases where the layout
viewport isn't user scrollable?

[bokan, 2016/03/01 05:56:24]

It's actually:

1. Try to scroll the visual viewport
2. If there's still unused delta remaining and the layout viewport is scrollable
in that direction, scroll it.

Were you looking at the old version? Admittedly, the various scrollability
checks and cases make it seem a little confusing...Take a look at the updated
version. Hopefully we can remove this as the TODO at the top says and use the
much simpler ScrollableArea::userScroll (it does the distribution once the
offset is actually applied).

 
-    return ScrollResultOneDimensional(false, delta);
+    return ScrollResult(
+        visualResult.didScrollX || layoutResult.didScrollX,
+        visualResult.didScrollY || layoutResult.didScrollY,
+        layoutResult.unusedScrollDeltaX,
+        layoutResult.unusedScrollDeltaY);
 }
 
 bool RootFrameViewport::scrollAnimatorEnabled() const