Not for review: Enable accelerated animations for orphaned layers

Not for review: Enable accelerated animations for orphaned layers.

This will actually need to land as multiple CLs. The planned approach is:

1) (chromium) Make LayerTreeHost::animateLayers take a wall clock time in addition to a monotonic time, just like LayerTreeHostImpl::animateLayers already does (this will be needed in step 3 when LayerTreeHost::animateLayers needs to generate events). In preparation for step 2, define a version of WebLayerTreeViewImpl::updateAnimations that takes a wall clock time in addition to a monotonic time.

2) (WebKit) Make WebLayerTree::updateAnimations take a wall clock time in addition to a monotonic time. Define a method WebViewImpl::registerForAnimations(WebLayer*) that calls a new method WebLayerTreeView::registerForAnimations(WebLayer*). Make GraphicsLayerChromiumFactory call WebViewImpl::registerForAnimations when creating graphics layers.

3) (chromium) Define WebLayerTreeViewImpl::registerForAnimations. Define a method LayerAnimationController::hasNonOrphanedObserver to allow us to identify LayerAnimationControllers that belong to orphaned layers. Change cc::Layer::addAnimation so that animations can be added whenever the layer has an AnimationRegistrar. Change cc::LayerTreeHost::animateLayers so that events are generated when calling |animate| on LayerAnimationControllers belonging to orphaned layers (that have waited for at least one layout to complete).

BUG=129683

[jamesr, 2013-01-08 22:13:36]

Why do we need a new AnimationRegistrar type?  What about adding API to an
existing class with the same lifetime/etc, like WebLayerTreeView?

[ajuma, 2013-01-08 22:30:19]

On 2013/01/08 22:13:36, jamesr wrote:
> Why do we need a new AnimationRegistrar type?  What about adding API to an
> existing class with the same lifetime/etc, like WebLayerTreeView?

The goal is to be able to make a new layer have the same AnimationRegistar as an
existing layer, so one alternative would be to have a
WebLayer::addAnimatiorRegistrarFromLayer that just takes another WebLayer.

[jamesr, 2013-01-08 22:31:15]

On 2013/01/08 22:30:19, ajuma wrote:
> On 2013/01/08 22:13:36, jamesr wrote:
> > Why do we need a new AnimationRegistrar type?  What about adding API to an
> > existing class with the same lifetime/etc, like WebLayerTreeView?
> 
> The goal is to be able to make a new layer have the same AnimationRegistar as
an
> existing layer, so one alternative would be to have a
> WebLayer::addAnimatiorRegistrarFromLayer that just takes another WebLayer.

I thought the goal was to associate animations with a given WebLayerTreeView
(aka LayerTreeHost).

[ajuma, 2013-01-08 22:45:42]

On 2013/01/08 22:31:15, jamesr wrote:
> On 2013/01/08 22:30:19, ajuma wrote:
> > On 2013/01/08 22:13:36, jamesr wrote:
> > > Why do we need a new AnimationRegistrar type?  What about adding API to an
> > > existing class with the same lifetime/etc, like WebLayerTreeView?
> > 
> > The goal is to be able to make a new layer have the same AnimationRegistar
as
> an
> > existing layer, so one alternative would be to have a
> > WebLayer::addAnimatiorRegistrarFromLayer that just takes another WebLayer.
> 
> I thought the goal was to associate animations with a given WebLayerTreeView
> (aka LayerTreeHost).

Right, we want to a new layer to be able to register its
LayerAnimationController with the AnimationRegistrar of the LayerTreeHost, even
before that layer gets added to the tree. The approach here is to accomplish
this by getting the AnimationRegistrar of some layer already in the tree (i.e.
the root layer, which we happen to have convenient access to when constructing
new GraphicsLayerChromiums), and passing it to the new layer.

[jamesr, 2013-01-08 22:54:56]

On 2013/01/08 22:45:42, ajuma wrote:
> On 2013/01/08 22:31:15, jamesr wrote:
> > On 2013/01/08 22:30:19, ajuma wrote:
> > > On 2013/01/08 22:13:36, jamesr wrote:
> > > > Why do we need a new AnimationRegistrar type?  What about adding API to
an
> > > > existing class with the same lifetime/etc, like WebLayerTreeView?
> > > 
> > > The goal is to be able to make a new layer have the same AnimationRegistar
> as
> > an
> > > existing layer, so one alternative would be to have a
> > > WebLayer::addAnimatiorRegistrarFromLayer that just takes another WebLayer.
> > 
> > I thought the goal was to associate animations with a given WebLayerTreeView
> > (aka LayerTreeHost).
> 
> Right, we want to a new layer to be able to register its
> LayerAnimationController with the AnimationRegistrar of the LayerTreeHost,
even
> before that layer gets added to the tree. The approach here is to accomplish
> this by getting the AnimationRegistrar of some layer already in the tree (i.e.
> the root layer, which we happen to have convenient access to when constructing
> new GraphicsLayerChromiums), and passing it to the new layer.

But does the LayerTreeHost exist at the time that you want to register the
animation?  If so, then I don't think you need new API all the way up the stack
(although you may want to split the implementation logic out), just pipe the
call through WebLayerTreeView and have that bottom out into whatever class you
want.

[ajuma, 2013-01-09 22:09:31]

On 2013/01/08 22:54:56, jamesr wrote:
> On 2013/01/08 22:45:42, ajuma wrote:
> > On 2013/01/08 22:31:15, jamesr wrote:
> > > On 2013/01/08 22:30:19, ajuma wrote:
> > > > On 2013/01/08 22:13:36, jamesr wrote:
> > > > > Why do we need a new AnimationRegistrar type?  What about adding API
to
> an
> > > > > existing class with the same lifetime/etc, like WebLayerTreeView?
> > > > 
> > > > The goal is to be able to make a new layer have the same
AnimationRegistar
> > as
> > > an
> > > > existing layer, so one alternative would be to have a
> > > > WebLayer::addAnimatiorRegistrarFromLayer that just takes another
WebLayer.
> > > 
> > > I thought the goal was to associate animations with a given
WebLayerTreeView
> > > (aka LayerTreeHost).
> > 
> > Right, we want to a new layer to be able to register its
> > LayerAnimationController with the AnimationRegistrar of the LayerTreeHost,
> even
> > before that layer gets added to the tree. The approach here is to accomplish
> > this by getting the AnimationRegistrar of some layer already in the tree
(i.e.
> > the root layer, which we happen to have convenient access to when
constructing
> > new GraphicsLayerChromiums), and passing it to the new layer.
> 
> But does the LayerTreeHost exist at the time that you want to register the
> animation?  If so, then I don't think you need new API all the way up the
stack
> (although you may want to split the implementation logic out), just pipe the
> call through WebLayerTreeView and have that bottom out into whatever class you
> want.

There are two cases we have to deal with:
1) Layers constructed by GraphicsLayerChromiumFactory. The
GraphicsLayerChromiumFactory is owned by a ChromeClientImpl, which has a pointer
to a WebViewImpl, which has a pointer to the WebLayerTreeView. For this case, we
could add a registerForAnimation(WebLayer*) method the WebViewImpl which calls a
registerForAnimation(WebLayer*) method in WebLayerTreeView.

2) Layers constructed by GraphicsLayerChromium::updateLayerPreserves3D. This is
the trickier case, since it's not clear where to get the appropriate
WebLayerTreeView. At this point, the GraphicsLayerChromium itself may be a
detached layer. One approach here would be to cache the WebLayerTreeView used in
case 1 inside WebLayer, updating it whenever the WebLayer is added to a tree,
and provide a WebLayer method for accessing this. This would allow
GraphicsLayerChromium::updateLayerPreserves3D to use m_layer's WebLayerTreeView
when constructing m_transformLayer.

[jamesr, 2013-01-09 22:16:54]

On 2013/01/09 22:09:31, ajuma wrote:
> 
> There are two cases we have to deal with:
> 1) Layers constructed by GraphicsLayerChromiumFactory. The
> GraphicsLayerChromiumFactory is owned by a ChromeClientImpl, which has a
pointer
> to a WebViewImpl, which has a pointer to the WebLayerTreeView. For this case,
we
> could add a registerForAnimation(WebLayer*) method the WebViewImpl which calls
a
> registerForAnimation(WebLayer*) method in WebLayerTreeView.

Right, that's what I would suggest.

> 2) Layers constructed by GraphicsLayerChromium::updateLayerPreserves3D. This
is
> the trickier case, since it's not clear where to get the appropriate
> WebLayerTreeView. At this point, the GraphicsLayerChromium itself may be a
> detached layer. One approach here would be to cache the WebLayerTreeView used
in
> case 1 inside WebLayer, updating it whenever the WebLayer is added to a tree,
> and provide a WebLayer method for accessing this. This would allow
> GraphicsLayerChromium::updateLayerPreserves3D to use m_layer's
WebLayerTreeView
> when constructing m_transformLayer.

Is this the code you are referring to:

void GraphicsLayerChromium::updateLayerPreserves3D()
{
    if (m_preserves3D && !m_transformLayer) {
        m_transformLayer =
adoptPtr(Platform::current()->compositorSupport()->createLayer());
        m_transformLayer->setPreserves3D(true);
        m_transformLayer->setAnimationDelegate(this);
        m_layer->layer()->transferAnimationsTo(m_transformLayer.get());


?  Could we make the transferAnimationsTo() call update the correct metadata? 
That call has an association from m_layer->layer(), which was created through
the factory, to the newly created m_transformLayer.

[ajuma, 2013-01-09 22:24:39]

On 2013/01/09 22:16:54, jamesr wrote:
> ?  Could we make the transferAnimationsTo() call update the correct metadata? 
> That call has an association from m_layer->layer(), which was created through
> the factory, to the newly created m_transformLayer.

Indeed, that will work. Thanks!

[ajuma, 2013-01-10 19:08:55]

Updated patch and description to follow the approach discussed above.

[jamesr, 2013-01-10 19:25:58]

I'm just seeing "Error: old chunk mismatch" on all the side-by-side diffs -
could you try uploading again?

[jamesr, 2013-01-10 19:26:17]

Also, do you want a review now?  The patch title and description both say "Not
for review"

[ajuma, 2013-01-10 20:41:56]

On 2013/01/10 19:25:58, jamesr wrote:
> I'm just seeing "Error: old chunk mismatch" on all the side-by-side diffs -
> could you try uploading again?

Done.

[ajuma, 2013-01-10 20:44:04]

On 2013/01/10 19:26:17, jamesr wrote:
> Also, do you want a review now?  The patch title and description both say "Not
> for review"

Yes, please. I've uploaded the 3 parts separately for review:
1) https://chromiumcodereview.appspot.com/11783101
2) https://bugs.webkit.org/show_bug.cgi?id=106594
3) https://chromiumcodereview.appspot.com/11830056

[ajuma, 2013-01-30 21:48:40]

Based on discussion in https://chromiumcodereview.appspot.com/11783101 (the
first part of this combined patch) and offline, I've updated the combined patch
so that, rather than starting animations on orphaned layers at the first
available opportunity, we instead wait until layout completes. This should
reduce the number of instances in which we have to start animations on orphaned
layers (since we're allowing more time for the orphaned layer to get added to
the tree and get started in the usual way).