Implement windows mousewheel scroll by page functionality

Implement windows mousewheel scroll by page functionality

Windows has a system setting to enable vertical scrolling by page.  This implements support for that feature in the compositor's input handling.  "By page" means scrolling by a fraction of the visible content height.

BUG=180655
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190082

[jamesr, 2013-03-22 23:57:26]

This patch works in manual testing but I haven't written tests yet.  It needs a
WebKit component as well to call the new functionality.  This side will have to
land and roll in before the WebKit component.

[jamesr, 2013-03-23 00:25:45]

PTAL.  This has some comments explaining what this thing is and a super basic
sanity test of the math.

[enne (OOO), 2013-03-23 00:25:51]

https://codereview.chromium.org/12582009/diff/8001/cc/trees/layer_tree_host_i...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/12582009/diff/8001/cc/trees/layer_tree_host_i...
cc/trees/layer_tree_host_impl.cc:1587: //
http://msdn.microsoft.com/en-us/library/windows/desktop/ms645601(v=vs.85).asp...
80 col.  ;)

https://codereview.chromium.org/12582009/diff/8001/cc/trees/layer_tree_host_i...
File cc/trees/layer_tree_host_impl.h (right):

https://codereview.chromium.org/12582009/diff/8001/cc/trees/layer_tree_host_i...
cc/trees/layer_tree_host_impl.h:96: WebKit::WebScrollbar::ScrollDirection
direction) OVERRIDE;
I know we already depend on a lot of WebScrollbar-related things in cc, but do
we have to add more dependencies?

[jamesr, 2013-03-23 00:28:21]

On 2013/03/23 00:25:51, enne wrote:
>
https://codereview.chromium.org/12582009/diff/8001/cc/trees/layer_tree_host_i...
> File cc/trees/layer_tree_host_impl.cc (right):
> 
>
https://codereview.chromium.org/12582009/diff/8001/cc/trees/layer_tree_host_i...
> cc/trees/layer_tree_host_impl.cc:1587: //
>
http://msdn.microsoft.com/en-us/library/windows/desktop/ms645601%28v=vs.85%29...
> 80 col.  ;)

Pretty sure you're joking, but I checked:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Line_Length

and this is explicitly called out as OK:

Exception: if a comment line contains an example command or a literal URL longer
than 80 characters, that line may be longer than 80 characters for ease of cut
and paste.

> 
>
https://codereview.chromium.org/12582009/diff/8001/cc/trees/layer_tree_host_i...
> File cc/trees/layer_tree_host_impl.h (right):
> 
>
https://codereview.chromium.org/12582009/diff/8001/cc/trees/layer_tree_host_i...
> cc/trees/layer_tree_host_impl.h:96: WebKit::WebScrollbar::ScrollDirection
> direction) OVERRIDE;
> I know we already depend on a lot of WebScrollbar-related things in cc, but do
> we have to add more dependencies?


At some point we'll have to make a wrapper for WebKit::WebScrollbar in cc or
figure out a different factoring.  Whatever that looks like (wrap wrapper or new
design) it'll probably have some notion of a ScrollDirection.  So I think this
is OK for now.

[enne (OOO), 2013-03-23 00:30:53]

lgtm

[jamesr, 2013-03-23 00:57:21]

D'oh, something in the math is a bit off (this doesn't scroll by exactly the
same amount as the webcore logic on windows).  Unticking to figure out...

[jamesr, 2013-03-23 01:02:22]

I got the wrong magic number for ScrollbarTheme::maxOverlapBetweenPages.  I got
40 from here:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

but that's mac, on windows we use this value:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

which has a value of std::numeric_limits<int>::max(), so that entire part of the
equation is just useless.  The actual scroll amount is max(height * 0.875, 1).

Will update magic number, unit test, and then put back in CQ

[jamesr, 2013-03-23 01:04:21]

On 2013/03/23 01:02:22, jamesr wrote:
> I got the wrong magic number for ScrollbarTheme::maxOverlapBetweenPages.  I
got
> 40 from here:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> but that's mac, on windows we use this value:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> which has a value of std::numeric_limits<int>::max(), so that entire part of
the
> equation is just useless.  The actual scroll amount is max(height * 0.875, 1).
> 
> Will update magic number, unit test, and then put back in CQ

Manually confirmed this gets the same offset as the old path on windows for both
long and short scrollable documents.

[Vangelis Kokkevis, 2013-03-23 01:49:14]

https://codereview.chromium.org/12582009/diff/18002/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/12582009/diff/18002/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:1603: float height =
layer_impl->vertical_scrollbar_layer()->bounds().height();
If you're scrolling an overflow-scroll element, do you want to scroll by the
entire height of the element (which presumably is the height of its scrollbar)
or just its visible portion?

https://codereview.chromium.org/12582009/diff/18002/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:1615: if (!applied_delta.IsZero()) {
scrollBy checks against a small threshold ( move_threshold_squared) to avoid
getting stuck . Seems like a good idea to do so here too.

https://codereview.chromium.org/12582009/diff/18002/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:1622: 
Do we care about bubbling up the remainder of the unused scroll delta?

[jamesr, 2013-03-23 02:25:09]

On 2013/03/23 01:49:14, Vangelis Kokkevis wrote:
>
https://codereview.chromium.org/12582009/diff/18002/cc/trees/layer_tree_host_...
> File cc/trees/layer_tree_host_impl.cc (right):
> 
>
https://codereview.chromium.org/12582009/diff/18002/cc/trees/layer_tree_host_...
> cc/trees/layer_tree_host_impl.cc:1603: float height =
> layer_impl->vertical_scrollbar_layer()->bounds().height();
> If you're scrolling an overflow-scroll element, do you want to scroll by the
> entire height of the element (which presumably is the height of its scrollbar)
> or just its visible portion?

The height of the scrollbar is the height of the visible scrollable portion. 
This is the same as the WebKit logic.

> 
>
https://codereview.chromium.org/12582009/diff/18002/cc/trees/layer_tree_host_...
> cc/trees/layer_tree_host_impl.cc:1615: if (!applied_delta.IsZero()) {
> scrollBy checks against a small threshold ( move_threshold_squared) to avoid
> getting stuck . Seems like a good idea to do so here too.

This scroll delta is floored to 1 css pixel, so I don't think we can get stuck.
> 
>
https://codereview.chromium.org/12582009/diff/18002/cc/trees/layer_tree_host_...
> cc/trees/layer_tree_host_impl.cc:1622: 
> Do we care about bubbling up the remainder of the unused scroll delta?

No, wheel event scrolls don't bubble up and this path is only used for wheels.

[commit-bot: I haz the power, 2013-03-23 12:48:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/12582009/18002

[commit-bot: I haz the power, 2013-03-23 15:04:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/12582009/18002

[commit-bot: I haz the power, 2013-03-23 21:36:17]

Change committed as 190082