Fix a crash in dtor of CallbackAnimationObserver

Fix a crash in dtor of CallbackAnimationObserver

It might be possible that animator is destroyed when desstruct CallbackAnimationObserver.
Use a scoped_refptr to make sure it wont happen.

BUG=478016
Committed: https://crrev.com/d8322ad28b115e844805d3046c02180f702e253e
Cr-Commit-Position: refs/heads/master@{#327113}

[bshe, 2015-04-23 22:15:34]

bshe@chromium.org changed reviewers:
+ sadrul@chromium.org

[bshe, 2015-04-23 22:18:42]

Hey Sadrul.

Do you mind to take a look at this CL? It should fix a crash in the dtor of
CallbackAnimationObserver. The animator_ is a LayerAnimator and it seems
LayerAnimator doesnt care if its observer list is not empty. So this should be
safe.
I can't not reproduce the crash though. So I can't verify if it really fix the
crash. But it should in theory. An example crash is here:
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_ChromeOS%27%2...

Thanks!

[bshe, 2015-04-23 22:27:06]

On 2015/04/23 22:18:42, bshe wrote:
> Hey Sadrul.
> 
> Do you mind to take a look at this CL? It should fix a crash in the dtor of
> CallbackAnimationObserver. The animator_ is a LayerAnimator and it seems
> LayerAnimator doesnt care if its observer list is not empty. So this should be
> safe.
> I can't not reproduce the crash though. So I can't verify if it really fix the
> crash. But it should in theory. An example crash is here:
>
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_ChromeOS%27%2...
> 
> Thanks!

Sorry. Forgot that the animator observer could be destroyed before any animation
notification. Please wait for my new patch before review.

[bshe, 2015-04-23 22:36:37]

On 2015/04/23 22:27:06, bshe wrote:
> On 2015/04/23 22:18:42, bshe wrote:
> > Hey Sadrul.
> > 
> > Do you mind to take a look at this CL? It should fix a crash in the dtor of
> > CallbackAnimationObserver. The animator_ is a LayerAnimator and it seems
> > LayerAnimator doesnt care if its observer list is not empty. So this should
be
> > safe.
> > I can't not reproduce the crash though. So I can't verify if it really fix
the
> > crash. But it should in theory. An example crash is here:
> >
>
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_ChromeOS%27%2...
> > 
> > Thanks!
> 
> Sorry. Forgot that the animator observer could be destroyed before any
animation
> notification. Please wait for my new patch before review.

uploaded patchset 3. I am not too confident that it will fix the crash since I
am unable to reproduce the crash locally. But the crash looks like because of
animator_ is somehow destroyed and CallbackAnimationObserver is not aware of it.
So in the dtor, when we try to remove the observer it crashed. Anyhow,
do you mind to take a look? Thanks!

[sadrul, 2015-04-23 23:07:59]

https://codereview.chromium.org/1094863005/diff/40001/ui/keyboard/keyboard_co...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/1094863005/diff/40001/ui/keyboard/keyboard_co...
ui/keyboard/keyboard_controller.cc:129: ui::LayerAnimator* animator_;
Should this be a scoped_refptr<> instead?

[bshe, 2015-04-24 14:24:37]

https://codereview.chromium.org/1094863005/diff/40001/ui/keyboard/keyboard_co...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/1094863005/diff/40001/ui/keyboard/keyboard_co...
ui/keyboard/keyboard_controller.cc:129: ui::LayerAnimator* animator_;
On 2015/04/23 23:07:59, sadrul wrote:
> Should this be a scoped_refptr<> instead?

Great suggestion! Done.

[sadrul, 2015-04-27 18:56:16]

lgtm

https://codereview.chromium.org/1094863005/diff/60001/ui/keyboard/keyboard_co...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/1094863005/diff/60001/ui/keyboard/keyboard_co...
ui/keyboard/keyboard_controller.cc:118:
CallbackAnimationObserver(scoped_refptr<ui::LayerAnimator> animator,
const scoped_refptr<>&

[bshe, 2015-04-27 19:52:54]

Thanks!

https://codereview.chromium.org/1094863005/diff/60001/ui/keyboard/keyboard_co...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/1094863005/diff/60001/ui/keyboard/keyboard_co...
ui/keyboard/keyboard_controller.cc:118:
CallbackAnimationObserver(scoped_refptr<ui::LayerAnimator> animator,
On 2015/04/27 18:56:16, sadrul wrote:
> const scoped_refptr<>&

Done.

[bshe, 2015-04-27 19:53:03]

The CQ bit was checked by bshe@chromium.org

[bshe, 2015-04-27 19:53:03]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/1094863005/#ps80001
(title: "use ref")

[commit-bot: I haz the power, 2015-04-27 19:53:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1094863005/80001

[commit-bot: I haz the power, 2015-04-27 20:17:24]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-04-27 20:18:10]

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