LayerAnimator must be prepared for its owning layer's deletion whenever it notifies observers.

I had originally tried to build upon http://codereview.chromium.org/10869066/, but the number of functions that needed to return (or needed to cope with) DestroyedType's was spiralling out of control and it seemed likely that a mistake would be made and bugs introduced. pkotwicz suggested I make the layer animator ref counted, and this seemed to be a much simpler and safer approach. This way, whenever we're in a LayerAnimator function that may notify observers, we create a scoped_refptr<LayerAnimator> for |this|. If the animator's owning layer gets deleted by an observer, then |this| will be safely destroyed when the function exits and the scoped_refptr falls out of scope.

BUG=
TEST=LayerAnimatorTest.ObserverDeletesAnimatorAfter[FinishingAnimation|StoppingAnimation|Scheduling|Aborted]


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=156318

[sky, 2012-09-10 21:18:53]

If you go this route, won't it mean observers may outlast the layer?

[Ian Vollick, 2012-09-11 01:30:16]

On 2012/09/10 21:18:53, sky wrote:
> If you go this route, won't it mean observers may outlast the layer?

It's true that the animator may be alive shortly after its owning layer dies,
but I don't think it will pose any more risk for observers. Neither the layer
nor the animator own the observers, so they could always have outlived the
destruction of either of these. Typically, I think the observer will delete
itself when notified, and if this results in the destruction of the animator's
owning layer, I think the animator, layer and observer will effectively end
their lifetimes together, even if the animator outlives the layer a little.

[sky, 2012-09-11 15:37:02]

Fair enough. Please document in layer_animator.h why it's ref counted.

LGTM

[Ian Vollick, 2012-09-11 21:02:52]

On 2012/09/11 15:37:02, sky wrote:
> Fair enough. Please document in layer_animator.h why it's ref counted.
> 
> LGTM

I've added the comment. I also noticed a few other small things I wanted to
address in this patch. The layer needed to set its animator's delegate to NULL
in its destructor, and we also needed to make sure that request our draws
_before_ we notify our observers (moved the schedule draw from the animator to
the sequence). SLGTY?

[sky, 2012-09-11 23:00:20]

LGTM

[Ian Vollick, 2012-09-12 03:17:36]

On 2012/09/11 23:00:20, sky wrote:
> LGTM

I'm sorry for continuing to muck with this patch, but I found a number of places
where the animator is assuming a non-NULL delegate, which is no longer
guaranteed to be true now that the animator can outlive the layer. In addition
to cleaning these up, I've fixed the return type for
LayerAnimationSequence::Progress now that it schedules its own draws, and
updated the LayerAnimator subclasses in the unit tests so they compile again.

[sky, 2012-09-12 04:34:14]

I had a feeling setting the delegate_ to NULL was going to require some
additional checks. SLGTM

[commit-bot: I haz the power, 2012-09-12 12:56:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/10919195/7

[commit-bot: I haz the power, 2012-09-12 17:08:24]

Change committed as 156318

[aarya, 2012-09-23 00:42:26]

On 2012/09/12 17:08:24, I haz the power (commit-bot) wrote:
> Change committed as 156318

you have wrong bug number in the changelog which is causing unrelated comments
on another bug. can you please fix.