Find root scroll layer at tree activation

Find root scroll layer at tree activation

This finds the root scroll layer when a LayerTreeImpl is activated since we can
only interact with the active tree. Doing this allows adding more checks for
proper use and cuts down on the number of functions that have to be called in a
specific order.

I've also tightened up the TopControlsManagerClient interface, since previously
the interface defined "virtual LayerTreeImpl* activeTree()" but LayerTreeHostImpl
also defined a non-virtual "const LayerTreeImpl* activeTree() const" so depending
on the constness of a pointer callers would get one or the other. Turns out the
top controls system doesn't need the active tree, it just needs the root scroll
layer. It actually doesn't even need the full layer, it just needs to know if
there is a layer and if so what the y offset is.

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

[jamesr, 2013-01-18 21:45:19]

https://codereview.chromium.org/12025031/diff/1/cc/top_controls_manager_client.h
File cc/top_controls_manager_client.h (right):

https://codereview.chromium.org/12025031/diff/1/cc/top_controls_manager_clien...
cc/top_controls_manager_client.h:16: virtual LayerImpl* rootScrollLayer() const
= 0;
hmmmm - this const-ness isn't ideal. will see if I can either make this
non-const or make the return pointer const

[enne (OOO), 2013-01-18 21:47:29]

https://codereview.chromium.org/12025031/diff/1/cc/top_controls_manager_client.h
File cc/top_controls_manager_client.h (right):

https://codereview.chromium.org/12025031/diff/1/cc/top_controls_manager_clien...
cc/top_controls_manager_client.h:16: virtual LayerImpl* rootScrollLayer() const
= 0;
On 2013/01/18 21:45:19, jamesr wrote:
> hmmmm - this const-ness isn't ideal. will see if I can either make this
> non-const or make the return pointer const

Did you not like the idea of just exposing the scroll value and the boolean
existence of this layer?

[jamesr, 2013-01-18 21:48:14]

On 2013/01/18 21:47:29, enne wrote:
>
https://codereview.chromium.org/12025031/diff/1/cc/top_controls_manager_client.h
> File cc/top_controls_manager_client.h (right):
> 
>
https://codereview.chromium.org/12025031/diff/1/cc/top_controls_manager_clien...
> cc/top_controls_manager_client.h:16: virtual LayerImpl* rootScrollLayer()
const
> = 0;
> On 2013/01/18 21:45:19, jamesr wrote:
> > hmmmm - this const-ness isn't ideal. will see if I can either make this
> > non-const or make the return pointer const
> 
> Did you not like the idea of just exposing the scroll value and the boolean
> existence of this layer?

I do like it, just didn't get to it.  That might be easier than trying to
rationalize the constness of TopControlsManagerClient

[enne (OOO), 2013-01-18 21:57:01]

https://codereview.chromium.org/12025031/diff/1/cc/layer_tree_host.cc
File cc/layer_tree_host.cc (left):

https://codereview.chromium.org/12025031/diff/1/cc/layer_tree_host.cc#oldcode284
cc/layer_tree_host.cc:284: syncTree->FindRootScrollLayer();
Ooh, this is nice to see.

[jamesr, 2013-01-18 21:58:27]

TopControlsManagerClient changes weren't so bad, PTAL

[Ted C, 2013-01-18 22:00:37]

lgtm for top controls changes

[enne (OOO), 2013-01-18 22:01:07]

lgtm

[commit-bot: I haz the power, 2013-01-18 22:02:28]

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

[jamesr, 2013-01-18 23:53:12]

Hmmm....this appears to break the non-css transform pinch zoom path

[commit-bot: I haz the power, 2013-01-19 02:23:46]

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

[commit-bot: I haz the power, 2013-01-19 04:58:03]

Change committed as 177847

[aelias_OOO_until_Jul13, 2013-01-21 01:12:14]

https://chromiumcodereview.appspot.com/12025031/diff/3003/cc/layer_tree_impl.cc
File cc/layer_tree_impl.cc (right):

https://chromiumcodereview.appspot.com/12025031/diff/3003/cc/layer_tree_impl....
cc/layer_tree_impl.cc:193: return root_scroll_layer_->bounds();
FYI, looks like this broke scrolling.  I can revert it back in
https://codereview.chromium.org/11958004/ which was changing this method anyway.

[enne (OOO), 2013-01-21 01:15:51]

Does the version you're testing on include
http://trac.webkit.org/changeset/139385?

[aelias_OOO_until_Jul13, 2013-01-21 01:22:50]

Ah, I was wondering how that change made sense, thanks for pointing me to that.

The version I'm testing on should have included it.  I'll try and figure out why
I saw a problem instead of reverting, then.

[aelias_OOO_until_Jul13, 2013-01-21 01:53:11]

OK, filed http://crbug.com/171188 and created to
https://codereview.chromium.org/12034003/ to revert this part.