Arrange compositor scrolling into scroll customization format

Arrange compositor scrolling into scroll customization format (WIP)

This splits compositor driven scrolling into applyScroll and
distributeScroll methods. Future patches will allow JavaScript on
a CompositorWorker to override these methods.

BUG=505851
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/d9e20147dd5649be4c9b742d3e6984034853715c
Cr-Commit-Position: refs/heads/master@{#341323}

[tdresser, 2015-07-09 13:49:00]

tdresser@chromium.org changed reviewers:
+ vollick@chromium.org

[tdresser, 2015-07-09 13:49:01]

PTAL

There's no functional change here, this just reorganizes things to look more
like the scroll customization API.

[Ian Vollick, 2015-07-09 14:03:55]

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

https://codereview.chromium.org/1215183004/diff/80001/cc/input/scroll_state.h...
cc/input/scroll_state.h:88: std::list<LayerImpl*> scroll_chain_;
I'm a little nervous about the LayerImpl pointers because we're  eventually like
to unify scrolling and this is certainly compositor-thread specific, and because
we're moving away from layer trees.

How would you feel about introducing a ScrollNode interface (implemented by
LayerImpl) and hanging onto pointers to those?

https://codereview.chromium.org/1215183004/diff/80001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/1215183004/diff/80001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:2443: static LayerImpl*
nextLayerInScrollOrder(LayerImpl* layer) {
This feels like it would be a function in the ScrollNode interface.

https://codereview.chromium.org/1215183004/diff/80001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:2677: const float kEpsilon = 0.1f;
Why this number? Should it not be one device pixel?

[tdresser, 2015-07-09 16:07:46]

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

https://codereview.chromium.org/1215183004/diff/80001/cc/input/scroll_state.h...
cc/input/scroll_state.h:88: std::list<LayerImpl*> scroll_chain_;
On 2015/07/09 14:03:55, vollick wrote:
> I'm a little nervous about the LayerImpl pointers because we're  eventually
like
> to unify scrolling and this is certainly compositor-thread specific, and
because
> we're moving away from layer trees.
> 
> How would you feel about introducing a ScrollNode interface (implemented by
> LayerImpl) and hanging onto pointers to those?

We'll absolutely want to introduce a layer of abstraction here at some point. In
my experience abstracting things before it's necessary causes more trouble than
it's worth, but I'm fine introducing that abstraction now if you think it's
superior to introducing it later.

https://codereview.chromium.org/1215183004/diff/80001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/1215183004/diff/80001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:2677: const float kEpsilon = 0.1f;
On 2015/07/09 14:03:55, vollick wrote:
> Why this number? Should it not be one device pixel?

I'm trying to avoid introducing any functional changes.

This comes directly from:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/layer_tre...

I agree that this seems odd though, I'm happy to file a bug for this if you
think it's worth changing.

[Ian Vollick, 2015-07-09 16:46:52]

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

https://codereview.chromium.org/1215183004/diff/80001/cc/input/scroll_state.h...
cc/input/scroll_state.h:88: std::list<LayerImpl*> scroll_chain_;
On 2015/07/09 16:07:45, tdresser wrote:
> On 2015/07/09 14:03:55, vollick wrote:
> > I'm a little nervous about the LayerImpl pointers because we're  eventually
> like
> > to unify scrolling and this is certainly compositor-thread specific, and
> because
> > we're moving away from layer trees.
> > 
> > How would you feel about introducing a ScrollNode interface (implemented by
> > LayerImpl) and hanging onto pointers to those?
> 
> We'll absolutely want to introduce a layer of abstraction here at some point.
In
> my experience abstracting things before it's necessary causes more trouble
than
> it's worth, but I'm fine introducing that abstraction now if you think it's
> superior to introducing it later.
> 

Please put a TODO here, then, and link to the scroll unification bug.

https://codereview.chromium.org/1215183004/diff/80001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/1215183004/diff/80001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:2677: const float kEpsilon = 0.1f;
On 2015/07/09 16:07:45, tdresser wrote:
> On 2015/07/09 14:03:55, vollick wrote:
> > Why this number? Should it not be one device pixel?
> 
> I'm trying to avoid introducing any functional changes.
> 
> This comes directly from:
>
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/layer_tre...
> 
> I agree that this seems odd though, I'm happy to file a bug for this if you
> think it's worth changing.

Yes, please file a bug for this, add a TODO and explain the source of this
magic.

[tdresser, 2015-07-16 14:04:50]

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

https://codereview.chromium.org/1215183004/diff/80001/cc/input/scroll_state.h...
cc/input/scroll_state.h:88: std::list<LayerImpl*> scroll_chain_;
On 2015/07/09 16:46:51, vollick wrote:
> On 2015/07/09 16:07:45, tdresser wrote:
> > On 2015/07/09 14:03:55, vollick wrote:
> > > I'm a little nervous about the LayerImpl pointers because we're 
eventually
> > like
> > > to unify scrolling and this is certainly compositor-thread specific, and
> > because
> > > we're moving away from layer trees.
> > > 
> > > How would you feel about introducing a ScrollNode interface (implemented
by
> > > LayerImpl) and hanging onto pointers to those?
> > 
> > We'll absolutely want to introduce a layer of abstraction here at some
point.
> In
> > my experience abstracting things before it's necessary causes more trouble
> than
> > it's worth, but I'm fine introducing that abstraction now if you think it's
> > superior to introducing it later.
> > 
> 
> Please put a TODO here, then, and link to the scroll unification bug.

Done.

https://codereview.chromium.org/1215183004/diff/80001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/1215183004/diff/80001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:2677: const float kEpsilon = 0.1f;
On 2015/07/09 16:46:51, vollick wrote:
> On 2015/07/09 16:07:45, tdresser wrote:
> > On 2015/07/09 14:03:55, vollick wrote:
> > > Why this number? Should it not be one device pixel?
> > 
> > I'm trying to avoid introducing any functional changes.
> > 
> > This comes directly from:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/layer_tre...
> > 
> > I agree that this seems odd though, I'm happy to file a bug for this if you
> > think it's worth changing.
> 
> Yes, please file a bug for this, add a TODO and explain the source of this
> magic.

Done.

[Ian Vollick, 2015-07-30 19:02:25]

https://codereview.chromium.org/1215183004/diff/120001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/1215183004/diff/120001/cc/layers/layer_impl.c...
cc/layers/layer_impl.cc:212: scroll_state->delta_consumed_for_scroll_sequence()
&&
Hmm, what if we shouldn't propagate, and the true scrolling layer (that isn't
us) decides not to consume any scroll delta. Won't this cause us to ApplyScroll
incorrectly here? Or by not consuming any scrolling is it relinquishing the
right to be the scrolling layer?

[tdresser, 2015-07-30 20:34:20]

https://codereview.chromium.org/1215183004/diff/120001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/1215183004/diff/120001/cc/layers/layer_impl.c...
cc/layers/layer_impl.cc:212: scroll_state->delta_consumed_for_scroll_sequence()
&&
On 2015/07/30 19:02:25, vollick (ooo until July 27) wrote:
> Hmm, what if we shouldn't propagate, and the true scrolling layer (that isn't
> us) decides not to consume any scroll delta. Won't this cause us to
ApplyScroll
> incorrectly here? Or by not consuming any scrolling is it relinquishing the
> right to be the scrolling layer?

delta_consumed_for_scroll_sequence persists for the duration of the scroll.

The only case where we shouldn't propagate is within a touchscreen fling. In
this case, we want to scroll the very first scroller that scrolls, not
necessarily the innermost scroller.

So essentially yes, but not consuming any scroll it's relinquishing the right to
be the scrolling layer.

[tdresser, 2015-07-30 20:42:16]

https://codereview.chromium.org/1215183004/diff/120001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/1215183004/diff/120001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_impl.cc:2737: active_tree_->CurrentlyScrollingLayer());
Just use CurrentlyScrollingLayer.

[Ian Vollick, 2015-07-30 20:44:27]

On 2015/07/30 20:42:16, tdresser wrote:
>
https://codereview.chromium.org/1215183004/diff/120001/cc/trees/layer_tree_ho...
> File cc/trees/layer_tree_host_impl.cc (right):
> 
>
https://codereview.chromium.org/1215183004/diff/120001/cc/trees/layer_tree_ho...
> cc/trees/layer_tree_host_impl.cc:2737:
active_tree_->CurrentlyScrollingLayer());
> Just use CurrentlyScrollingLayer.

Thanks. lgtm.

[tdresser, 2015-07-31 12:11:16]

https://codereview.chromium.org/1215183004/diff/120001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/1215183004/diff/120001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_impl.cc:2737: active_tree_->CurrentlyScrollingLayer());
On 2015/07/30 20:42:16, tdresser wrote:
> Just use CurrentlyScrollingLayer.

Done.

[tdresser, 2015-07-31 12:11:20]

The CQ bit was checked by tdresser@chromium.org

[tdresser, 2015-07-31 12:11:20]

The patchset sent to the CQ was uploaded after l-g-t-m from vollick@chromium.org
Link to the patchset: https://codereview.chromium.org/1215183004/#ps140001
(title: "Fix nit.")

[commit-bot: I haz the power, 2015-07-31 12:11:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1215183004/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1215183004/140001

[commit-bot: I haz the power, 2015-07-31 13:15:17]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-07-31 13:15:46]

Patchset 8 (id:??) landed as
https://crrev.com/d9e20147dd5649be4c9b742d3e6984034853715c
Cr-Commit-Position: refs/heads/master@{#341323}