Report accurate Overscroll on handleWheel.

Source/core/frame/RootFrameViewport.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/frame/RootFrameViewport.h"
 
 #include "core/frame/FrameView.h"
 #include "core/layout/ScrollAlignment.h"
 #include "platform/geometry/DoubleRect.h"
@@ skipping 78 matching lines @@
 
 ScrollBehavior RootFrameViewport::scrollBehaviorStyle() const
 {
     return layoutViewport().scrollBehaviorStyle();
 }
 
 ScrollResult RootFrameViewport::handleWheel(const PlatformWheelEvent& event)
 {
     updateScrollAnimator();
 
-    ScrollResult viewScrollResult;
-    if (layoutViewport().isScrollable())
-        viewScrollResult = layoutViewport().handleWheel(event);
+    ScrollResult viewScrollResult = layoutViewport().handleWheel(event);

[MuVen, 2015/06/25 12:55:51]

bokan@, handleWheel should be triggered, irrespective of isScrollable() to
calculate unUsedDelta. wdyt ? Example: 

https://css-tricks.com/examples/WebKitScrollbars/ in this case as the
layoutViewport is not scrollable(reason: NotScrollableNoOverflow), due to which
unusedDelta is not calculated. 

[bokan, 2015/06/25 14:54:35]

Sounds fine to me - it's actually an improvement - as long as all the tests
pass.

[MuVen, 2015/06/25 17:05:36]

Done.

 
     // The visual viewport will only accept pixel scrolls.
     if (!event.canScroll() || event.granularity() == ScrollByPageWheelEvent)
         return viewScrollResult;
 
     // Move the location by the negative of the remaining scroll delta.
     DoublePoint oldOffset = visualViewport().scrollPositionDouble();
     DoublePoint locationDelta;
     if (viewScrollResult.didScroll()) {
         locationDelta = -DoublePoint(viewScrollResult.unusedScrollDeltaX, viewScrollResult.unusedScrollDeltaY);
     } else {
         if (event.railsMode() != PlatformEvent::RailsModeVertical)
             locationDelta.setX(-event.deltaX());
         if (event.railsMode() != PlatformEvent::RailsModeHorizontal)
             locationDelta.setY(-event.deltaY());
     }
 
     DoublePoint targetPosition = visualViewport().adjustScrollPositionWithinRange(
         visualViewport().scrollPositionDouble() + toDoubleSize(locationDelta));
     visualViewport().setScrollPosition(targetPosition, UserScroll);
 
     DoublePoint usedLocationDelta(visualViewport().scrollPositionDouble() - oldOffset);
     if (!viewScrollResult.didScroll() && usedLocationDelta == DoublePoint::zero())
-        return ScrollResult();
+        return ScrollResult(false, false, viewScrollResult.unusedScrollDeltaX - usedLocationDelta.x(), viewScrollResult.unusedScrollDeltaY - usedLocationDelta.y());

[bokan, 2015/06/25 14:54:35]

This still looks wrong to me, I think you have the negatives mixed up -
admittedly they're very confusing here.

Digging through the wheel handing code, it seems scrolling down on a page will
generate PlatformWheelEvents with a negative delta.
ScrollAnimator::handleWheeEvent negates the delta, calls userScroll, then
negates the result so the ScrollResult.unusedScrollDelta is the unused *event*
delta. i.e. calling layoutViewport().handleWheel with WheelEvent{0, -10}, say it
scrolls the page down 2px, viewScrollResult.unusedScrollDeltaY == -8. This is
why we negate the locationDelta above. So this should be
-viewScrollResult.unusedScrollDelta - usedLocationDelta.

Unfortunately, it looks like the ScrollResult returned by this method has
different semantics than the base class version in ScrollableArea. That version
returns the unused portion of the PlatformWheelEvent (i.e. negative amount
remaining). This method will return the amount of *applied* delta that was
unused (i.e. positive amount remaining). We should definitely fix that (I think
it makes sense to make it always return the amount of unused applied delta) but
lets do that in a separate patch. Please leave a TODO and/or file a bug.

P.S. When testing your changes, make sure you're using
--disable-threaded-scrolling, otherwise you'll be scrolling on the compositor
and not testing these changes.

[MuVen, 2015/06/25 17:05:36]

Done. filed bug @ crbug.com/504389.

 
     DoubleSize unusedLocationDelta(locationDelta - usedLocationDelta);
     bool didScrollX = viewScrollResult.didScrollX || unusedLocationDelta.width();
     bool didScrollY = viewScrollResult.didScrollY || unusedLocationDelta.height();
     return ScrollResult(didScrollX, didScrollY, -unusedLocationDelta.width(), -unusedLocationDelta.height());
 }
 
 LayoutRect RootFrameViewport::scrollIntoView(const LayoutRect& rectInContent, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
 {
     // We want to move the rect into the viewport that excludes the scrollbars so we intersect
@@ skipping 187 matching lines @@
 }
 
 void RootFrameViewport::updateCompositorScrollAnimations()
 {
     ScrollableArea::updateCompositorScrollAnimations();
     layoutViewport().updateCompositorScrollAnimations();
 }
 
 
 } // namespace blink