Add initial CC support for scroll-blocks-on

Add initial CC support for scroll-blocks-on

When 'scroll-blocks-on: scroll-event' is applied anywhere in the scroll ancestor tree where is a scroll event handler, force the scroll to be executed on the main thread.  Note that this is still experimental.  We'll be at least adding a timeout mechanism to this to mitigate the risk of improper usage.

When 'scroll-blocks-on: wheel-event' is NOT applied (it's on by default), ignore the existence of wheel event handlers (allowing scrolling to run freely on impl).

When 'scroll-blocks-on: start-touch' is NOT applied (it's on by default), suppress sending touch events to blink.  Eventually we'll make this smarter - sending async touch events that don't block scroll.

Depends on blink change https://src.chromium.org/viewvc/blink?view=rev&revision=188230

BUG=347272
Committed: https://crrev.com/18779d82a7f4a3ca040f247c8454c16b7cfc235b
Cr-Commit-Position: refs/heads/master@{#314765}

[Rick Byers, 2015-02-03 05:53:01]

rbyers@chromium.org changed reviewers:
+ aelias@chromium.org, jdduke@chromium.org

[Rick Byers, 2015-02-03 08:11:28]

aelias@ PTAL - at least at cc/
jdduke please review at least content/renderer/input (happy for feedback on all
of it of course).

As discussed this feature is still pretty far from shipping (see open questions
in design doc, etc.
https://docs.google.com/document/d/1aOQRw76C0enLBd0mCG_-IM6bso7DxXwvqTiRWgNdT...).
 But with this CL it's now usable and should be enough to unblock some other
efforts (scroll customization).

Thanks,
   Rick

[jdduke (slow), 2015-02-03 16:43:35]

content/renderer/input lgtm with just a couple points.

https://codereview.chromium.org/784463002/diff/80001/cc/input/input_handler.h
File cc/input/input_handler.h (right):

https://codereview.chromium.org/784463002/diff/80001/cc/input/input_handler.h...
cc/input/input_handler.h:145: virtual bool HaveWheelEventHandlersAt(const
gfx::Point& viewport_point) = 0;
Now that "blocks on" is the phrasing used for both wheel events and touch
events, is there a reason to have different names for the two queries
(HaveWheelEventHandlersAt vs DoTouchEventsBlockScrollAt)? I guess the semantics
are slightly different, but I wonder if there's a consistent naming scheme here
that communicates just the necessary bits, e.g.,

|ShouldForwardTouchToMain|
|ShouldForwardWheelToMain|

or something like that? Not a huge deal though.

https://codereview.chromium.org/784463002/diff/80001/cc/layers/layer.cc
File cc/layers/layer.cc (right):

https://codereview.chromium.org/784463002/diff/80001/cc/layers/layer.cc#newco...
cc/layers/layer.cc:74: scroll_blocks_on_(ScrollBlocksOnNone),
I wonder if it's worth a static assert for ScrollBlocksOn values taking 3 bits
or less?

https://codereview.chromium.org/784463002/diff/80001/content/renderer/input/i...
File content/renderer/input/input_handler_proxy.cc (right):

https://codereview.chromium.org/784463002/diff/80001/content/renderer/input/i...
content/renderer/input/input_handler_proxy.cc:565: // main asynchronously
(crbug.com/347272).
Might be nice to have a more specific bug for this behavior, but not necessary.

[aelias_OOO_until_Jul13, 2015-02-03 22:22:28]

lgtm

More of a comment on the Blink-side CL
https://codereview.chromium.org/784453003, but I'm a bit unhappy with how this
promotes to a compositing layer purely to avoid pain in terms of rect tracking. 
That has a cost and I'd probably have asked for this to be implemented
differently, except that slimming paint is likely to come along and solve the
problem by the time this is used in the wild anyway.  Are you planning to work
with slimming paint team on formulating these in terms of display-list
annotations?

[Rick Byers, 2015-02-04 00:01:51]

On 2015/02/03 22:22:28, aelias wrote:
> lgtm
> 
> More of a comment on the Blink-side CL
> https://codereview.chromium.org/784453003, but I'm a bit unhappy with how this
> promotes to a compositing layer purely to avoid pain in terms of rect
tracking. 
> That has a cost and I'd probably have asked for this to be implemented
> differently, except that slimming paint is likely to come along and solve the
> problem by the time this is used in the wild anyway.  Are you planning to work
> with slimming paint team on formulating these in terms of display-list
> annotations?

Yes, exactly!  vollick@ and I had a long discussion about this.  I started from
the assumption that we'd want to do rect-tracking (despite my deep loathing for
the complexity and brittleness of our rect-tracking techniques today).  Ian
convinced me it wasn't worth the cost.  Chris, Ian and I already have a plan to
eliminate rect-tracking with slimming paint (eg. touch rects will be replaced by
a touch handler annotation on a display list entry), we'll definitely use the
same pattern here.

Also, in practice we hope it'll be unlikely that people will want to apply
scroll-blocks-on to an element that's not already it's own composted layer (in
high-dpi at least).  But this is definitely a risk.  We've got a bunch of
optimization opportunities if we find this is a problem in the short term (prior
to slimming paint).  Really we only need a new lawyer when an element adds new
bits that none of it's ancestors have.

[Rick Byers, 2015-02-05 04:35:54]

Thanks Jared.

https://codereview.chromium.org/784463002/diff/80001/cc/input/input_handler.h
File cc/input/input_handler.h (right):

https://codereview.chromium.org/784463002/diff/80001/cc/input/input_handler.h...
cc/input/input_handler.h:145: virtual bool HaveWheelEventHandlersAt(const
gfx::Point& viewport_point) = 0;
On 2015/02/03 16:43:34, jdduke wrote:
> Now that "blocks on" is the phrasing used for both wheel events and touch
> events, is there a reason to have different names for the two queries
> (HaveWheelEventHandlersAt vs DoTouchEventsBlockScrollAt)? I guess the
semantics
> are slightly different, but I wonder if there's a consistent naming scheme
here
> that communicates just the necessary bits, e.g.,
> 
> |ShouldForwardTouchToMain|
> |ShouldForwardWheelToMain|
> 
> or something like that? Not a huge deal though.

Yeah I considered changing this but the semantics are different.  In particular
HaveWheelEventHandlersAt returns ONLY if there are wheel event handlers
(scroll-blocks-on handling is handled outside).  DoTouchEventBlockScrollAt looks
at both touch handler rects and the scroll-blocks-on mode.  The reason for the
difference is just the different places these are used -
HaveWheelEventHandlersAt is part of the scrolling system (which already looks at
scroll-blocks-on), but DoTouchEventsBlockScroll is used more in isolation to
decide when to forward touch events to blink.  So I think the asymmetry reflects
a real difference - it's now misleading to think of these as the same thing.

But I only spent a few minutes looking for ways to refactor things to be more
symmetric.  I'm open to suggestions.

https://codereview.chromium.org/784463002/diff/80001/cc/layers/layer.cc
File cc/layers/layer.cc (right):

https://codereview.chromium.org/784463002/diff/80001/cc/layers/layer.cc#newco...
cc/layers/layer.cc:74: scroll_blocks_on_(ScrollBlocksOnNone),
On 2015/02/03 16:43:35, jdduke wrote:
> I wonder if it's worth a static assert for ScrollBlocksOn values taking 3 bits
> or less? 

Sure, done.  I also updated my COMPILE_ASSERTs to match the static_assert
pattern I see elsewhere in cc/.

https://codereview.chromium.org/784463002/diff/80001/content/renderer/input/i...
File content/renderer/input/input_handler_proxy.cc (right):

https://codereview.chromium.org/784463002/diff/80001/content/renderer/input/i...
content/renderer/input/input_handler_proxy.cc:565: // main asynchronously
(crbug.com/347272).
On 2015/02/03 16:43:35, jdduke wrote:
> Might be nice to have a more specific bug for this behavior, but not
necessary.

Yeah, good idea.  At some point we might want to weigh the cost/benefit tradeoff
of doing that - good to have one bug to track the debate.  Done.

[Rick Byers, 2015-02-05 04:36:16]

New patchsets have been uploaded after l-g-t-m from aelias@chromium.org

[Rick Byers, 2015-02-05 04:36:35]

The CQ bit was checked by rbyers@chromium.org

[commit-bot: I haz the power, 2015-02-05 04:37:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/784463002/100001

[commit-bot: I haz the power, 2015-02-05 06:22:36]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-02-05 06:24:15]

Patchset 6 (id:??) landed as
https://crrev.com/18779d82a7f4a3ca040f247c8454c16b7cfc235b
Cr-Commit-Position: refs/heads/master@{#314765}