Fix mouse wheel over-scrolls when display is scaled and scroll is paginated

third_party/WebKit/Source/web/WebViewImpl.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 731 matching lines @@
 
       m_client->didHandleGestureEvent(event, eventCancelled);
       return eventResult;
     default:
       break;
   }
 
   WebGestureEvent scaledEvent =
       TransformWebGestureEvent(mainFrameImpl()->frameView(), event);
 
+  if (event.sourceDevice == WebGestureDeviceTouchpad &&
+      event.type == WebInputEvent::GestureScrollUpdate) {
+    float scaleFactor = client()->GetOriginalDeviceScaleFactor();

[chengx, 2017/01/06 23:46:36]

aelias 2017/01/05 00:55:45
CC impl thread also has the ability to handle scroll update events directly
without ever making it to this method, so it doesn't seem that doing it here can
be correct.  I'd suggest adjusting the value on the browser process, is there
any reason not to do it there?

Secondly, this adjustment would apply to all kinds of scroll updates, not just
wheel page scrolling.  So A) please whether DPI affects non-paginated wheel
scroll distance as well (either it's an unnoticed bug and you'd want to fix
both, or you'd want to make your fix specific to paginated wheel scroll).  And
B) make sure that your change doesn't regress touchscreen scrolling.

=====================

chengx:
I have dig into both the compositor thread and the main thread, which turns
out to be fairly tricky. For mouse wheel event, only the non-paginated ones
are handled in compositor thread, while the paginated ones are handled
here. Since this bug is related to paginated wheel scroll, I think this is 
the right place to make the change.

A) As mentioned above, since non-paginated wheel scroll is handled in
compositor thread, this CL won't affect it at all. This addresses your 
concern A. 
B) The mouse wheel event is used to generate a web gesture event, which is 
handled here. The sourceDevice property of this event is set to 
"WebGestureDeviceTouchpad". I have added a "if" condition related to this in the
new patch, so touchscreen scrolling won't be affected. I believe mouse wheel
event is equivalent to scroll event from the touchpad, so it is unnecessary to
differentiate these two as mentioned in your concern B.

==========

aelias@:
But I'm proposing you make the change in the browser process (in the environs of
RenderWidgetHostImpl, or above), which is code that should be gone through
irrespective of whether it's handled on the main or impl side in the renderer
process.  So you shouldn't have to make the change twice.

I believe it should be the case that non-paginated ones are also handled on the
main thread if a Javascript wheel listener is registered.  Secondly, I would be
curious to learn whether there actually is a bug with non-paginated ones
already, since it's a bit suspicious.

+    scaledEvent.data.scrollUpdate.deltaX /= scaleFactor;
+    scaledEvent.data.scrollUpdate.deltaY /= scaleFactor;

[chengx, 2017/01/06 23:46:36]

Bret:
Use client()->convertViewportToWindow() instead of doing the math here, which
should simplify this CL.

=========

chengx:
convertViewportToWindow() takes WebRect as parameter, while WebFloatRec is
needed here. Either I need to make a new function to put all the math here, or I
will go with the current implementation.

=========

Bret:
I think it would be better to add a version that uses a float rect. The existing
function can delegate to the new one and call gfx::ToEnclosedRect on the result.

+  }
+
   // Special handling for double tap and scroll events as we don't want to
   // hit test for them.
   switch (event.type) {
     case WebInputEvent::GestureDoubleTap:
       if (m_webSettings->doubleTapToZoomEnabled() &&
           minimumPageScaleFactor() != maximumPageScaleFactor()) {
         m_client->cancelScheduledContentIntents();
         animateDoubleTapZoom(
             flooredIntPoint(scaledEvent.positionInRootFrame()));
       }
@@ skipping 3434 matching lines @@
   if (focusedFrame->localFrameRoot() != mainFrameImpl()->frame())
     return nullptr;
   return focusedFrame;
 }
 
 LocalFrame* WebViewImpl::focusedLocalFrameAvailableForIme() const {
   return m_imeAcceptEvents ? focusedLocalFrameInWidget() : nullptr;
 }
 
 }  // namespace blink