cc: Block commit on activate by setting a flag on LayerTreeHost.

cc: Block commit on activate by setting a flag on LayerTreeHost.

Currently the ThreadProxy recursively asks all layers in the tree if
they should block the commit. This is problematic as when you remove
a layer from a the tree, it may want to block the commit to get back
resources from its active-tree impl-layer.

Instead, have layers call SetNextCommitWaitsForActivation() when they
want the next commit to block on activate. This way we only block
commits that matter, not every commit when there's a texture layer
present. And we can allow a layer to block the commit when it is
leaving the tree.

Tests:
TextureLayerNoMailboxIsActivatedDuringCommit
TextureLayerMailboxIsActivatedDuringCommit
DelegatedFrameIsActivatedDuringCommit

R=enne, piman
BUG=277953
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=220418

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

[danakj, 2013-08-29 01:58:18]

https://codereview.chromium.org/23530003/diff/1/cc/trees/layer_tree_host.h
File cc/trees/layer_tree_host.h (right):

https://codereview.chromium.org/23530003/diff/1/cc/trees/layer_tree_host.h#ne...
cc/trees/layer_tree_host.h:270: commit_waits_for_activation_ = true;
It occured to me, maybe this should just set a bit on the proxy, so the proxy
doesn't have to read it off the LTH later, and it can reset it once it's done
with it. Less magical maybe?

I'll do it this way tomorrow if no one cries foul.

[danakj, 2013-08-29 01:59:16]

https://codereview.chromium.org/23530003/diff/1/cc/layers/texture_layer_unitt...
File cc/layers/texture_layer_unittest.cc (right):

https://codereview.chromium.org/23530003/diff/1/cc/layers/texture_layer_unitt...
cc/layers/texture_layer_unittest.cc:754: SetMailbox('2');
@piman I am actually not sure if using SetTextureMailbox is bad, and I should
just invalidate and use the client instead.. too many APIs.

[piman, 2013-08-29 04:16:01]

https://codereview.chromium.org/23530003/diff/1/cc/layers/texture_layer_unitt...
File cc/layers/texture_layer_unittest.cc (right):

https://codereview.chromium.org/23530003/diff/1/cc/layers/texture_layer_unitt...
cc/layers/texture_layer_unittest.cc:754: SetMailbox('2');
On 2013/08/29 01:59:16, danakj wrote:
> @piman I am actually not sure if using SetTextureMailbox is bad, and I should
> just invalidate and use the client instead.. too many APIs.

I've generally tried to avoid using both ways at the same time. Either no client
and SetTextureMailbox, or a client and no call to SetTextureMailbox. Either is
still ok, though in term I want to limit it to the client way (I think).

[danakj, 2013-08-29 17:36:14]

Moved the bool to the proxy. I guess I'll leave the SetTextureMailbox() calls as
is in the test since we're not certain that API won't stay forever then :)

[enne (OOO), 2013-08-29 20:01:33]

https://codereview.chromium.org/23530003/diff/14001/cc/layers/tiled_layer.cc
File cc/layers/tiled_layer.cc (left):

https://codereview.chromium.org/23530003/diff/14001/cc/layers/tiled_layer.cc#...
cc/layers/tiled_layer.cc:238: bool TiledLayer::BlocksPendingCommit() const {
return true; }
I don't think this is safe to just remove.

[enne (OOO), 2013-08-29 20:03:22]

https://codereview.chromium.org/23530003/diff/14001/cc/layers/tiled_layer.cc
File cc/layers/tiled_layer.cc (left):

https://codereview.chromium.org/23530003/diff/14001/cc/layers/tiled_layer.cc#...
cc/layers/tiled_layer.cc:238: bool TiledLayer::BlocksPendingCommit() const {
return true; }
On 2013/08/29 20:01:33, enne wrote:
> I don't think this is safe to just remove.

Or, it seems unlikely that anybody would stick a TiledLayer into an impl-side
painting tree, but if they did TiledLayer really doesn't support that.  So,
there either needs to be some safety here via blocking commits or a CHECK or
something?

[danakj, 2013-08-29 20:13:52]

https://codereview.chromium.org/23530003/diff/14001/cc/layers/tiled_layer.cc
File cc/layers/tiled_layer.cc (left):

https://codereview.chromium.org/23530003/diff/14001/cc/layers/tiled_layer.cc#...
cc/layers/tiled_layer.cc:238: bool TiledLayer::BlocksPendingCommit() const {
return true; }
On 2013/08/29 20:03:22, enne wrote:
> On 2013/08/29 20:01:33, enne wrote:
> > I don't think this is safe to just remove.
> 
> Or, it seems unlikely that anybody would stick a TiledLayer into an impl-side
> painting tree, but if they did TiledLayer really doesn't support that.  So,
> there either needs to be some safety here via blocking commits or a CHECK or
> something?

Unit tests do it, but they all pass. So I was like *shrug*. But okay I'll make
this layer set the bit in Update().

[danakj, 2013-08-29 20:15:29]

Done.

[enne (OOO), 2013-08-29 20:19:09]

lgtm

https://codereview.chromium.org/23530003/diff/14001/cc/layers/tiled_layer.cc
File cc/layers/tiled_layer.cc (left):

https://codereview.chromium.org/23530003/diff/14001/cc/layers/tiled_layer.cc#...
cc/layers/tiled_layer.cc:238: bool TiledLayer::BlocksPendingCommit() const {
return true; }
On 2013/08/29 20:13:52, danakj wrote:

> Unit tests do it, but they all pass. So I was like *shrug*. But okay I'll make
> this layer set the bit in Update().

...

I think partial updates would be very wrong with impl-side painting.  Mostly,
it's that this path isn't tested very well, so I'd prefer to avoid it entirely.

[commit-bot: I haz the power, 2013-08-29 20:35:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/23530003/24001

[commit-bot: I haz the power, 2013-08-29 23:05:16]

Change committed as 220418

[danakj, 2013-08-30 02:06:49]

Patch set 6 fixes the flake in the tests.

When DidActivateOnThread() runs, the main thread has already been signaled and
is running. So we had a race between DidCommit reading activate_count_ and
DidActivateOnThread() setting it.

Fixed by moving the setting of activate_count_ to WillActivateOnThread() so it
happens before the main thread unblocks.

[commit-bot: I haz the power, 2013-08-30 02:08:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/23530003/54001

[commit-bot: I haz the power, 2013-08-30 06:29:38]

Change committed as 220515