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);
 
     // 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.

[bokan, 2015/06/25 17:50:31]

Nit: Can you elaborate on this comment so that future generations don't have to
do the digging we did? e.g. "The delta in PlatformWheelEvent is negative when
scrolling the wheel towards the user so negate it to get the scroll delta that
should be applied to the page"

Also, a note above that the unusedScrollDelta in viewScrollResult is negative
(and incorrect) is also probably warranted.

[MuVen, 2015/06/26 09:55:44]

Done.

     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();
-
-    DoubleSize unusedLocationDelta(locationDelta - usedLocationDelta);
-    bool didScrollX = viewScrollResult.didScrollX || unusedLocationDelta.width();
-    bool didScrollY = viewScrollResult.didScrollY || unusedLocationDelta.height();
-    return ScrollResult(didScrollX, didScrollY, -unusedLocationDelta.width(), -unusedLocationDelta.height());
+    bool didScrollX = viewScrollResult.didScrollX || usedLocationDelta.x();
+    bool didScrollY = viewScrollResult.didScrollY || usedLocationDelta.y();
+    return ScrollResult(didScrollX, didScrollY, viewScrollResult.unusedScrollDeltaX - usedLocationDelta.x(), viewScrollResult.unusedScrollDeltaY - usedLocationDelta.y());

[bokan, 2015/06/25 17:50:31]

Is this working as expected? AFAICT, the unusedScrollDelta will be negative for
a downwards scroll but usedLocationDelta will be positive, so you'll get a
larger negative overscroll...

[MuVen, 2015/06/26 09:55:44]

You are right on this. i was negating the unusedDelta in the render_widget(which
i have noticed, on debugging, it was a mistake sorry:(). Changed as you have
suggested. Negating here itself. Will check the bug 504389 to ensure unusedDelta
state remains same everywhere. Thanks.

 }
 
 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
     // the pinch viewport with the scrollbar-excluded frameView content rect. However, we don't
     // use visibleContentRect directly since it floors the scroll position. Instead, we use
     // FrameView::scrollPositionDouble and construct a LayoutRect from that (the FrameView size
     // is always integer sized.
 
@@ skipping 182 matching lines @@
 }
 
 void RootFrameViewport::updateCompositorScrollAnimations()
 {
     ScrollableArea::updateCompositorScrollAnimations();
     layoutViewport().updateCompositorScrollAnimations();
 }
 
 
 } // namespace blink