Fix a crash in AutoplayUmaHelper when releasing visibility observer

Fix a crash and possible memory leak in AutoplayUmaHelper

There's a crash in
AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod().
It might be possible that the callback is called after the observer
is released. This CL adds additional nullcheck to avoid the crash.

This CL also fixes a possible memory leak by changing
wrapPersistent to wrapWeakPersistent.

BUG=638595
Committed: https://crrev.com/403b4008f974e12adcc9ba95036120cc299784fb
Cr-Commit-Position: refs/heads/master@{#412574}

[Zhiqiang Zhang (Slow), 2016-08-17 15:15:32]

Description was changed from

==========
Fix a crash in AutoplayUmaHelper when releasing visibility observer

BUG=638595
==========

to

==========
Fix a crash in AutoplayUmaHelper when releasing visibility observer

There's a crash in
AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod().
It might be possible that the callback is called after the observer
is released. Adding additional nullcheck to avoid the crash.

BUG=638595
==========

[Zhiqiang Zhang (Slow), 2016-08-17 15:15:52]

Description was changed from

==========
Fix a crash in AutoplayUmaHelper when releasing visibility observer

There's a crash in
AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod().
It might be possible that the callback is called after the observer
is released. Adding additional nullcheck to avoid the crash.

BUG=638595
==========

to

==========
Fix a crash in AutoplayUmaHelper when releasing visibility observer

There's a crash in
AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod().
It might be possible that the callback is called after the observer
is released. This CL adds additional nullcheck to avoid the crash.

BUG=638595
==========

[Zhiqiang Zhang (Slow), 2016-08-17 15:17:19]

zqzhang@chromium.org changed reviewers:
+ mlamouri@chromium.org

[Zhiqiang Zhang (Slow), 2016-08-17 15:17:20]

PTAL. I don't have any other explanations for the crash. Maybe we should land
this null-check first.

[mlamouri (slow - plz ping), 2016-08-17 15:42:38]

On 2016/08/17 at 15:17:20, zqzhang wrote:
> PTAL. I don't have any other explanations for the crash. Maybe we should land
this null-check first.

Could AutoplayUmaHelper be null? According to the stack,
ElementVisibilityObserver::onVisibilityChanged was called and it was able to get
the callback from it so the object is likely there. I wonder if it might still
be alive while the element is not.  WDYT?

[Zhiqiang Zhang (Slow), 2016-08-17 16:08:08]

On 2016/08/17 15:42:38, Mounir Lamouri wrote:
> On 2016/08/17 at 15:17:20, zqzhang wrote:
> > PTAL. I don't have any other explanations for the crash. Maybe we should
land
> this null-check first.
> 
> Could AutoplayUmaHelper be null? According to the stack,
> ElementVisibilityObserver::onVisibilityChanged was called and it was able to
get
> the callback from it so the object is likely there. I wonder if it might still
> be alive while the element is not.  WDYT?

As our offline discussion, I think it's possible that there are two events
queued: unload and visibilitychanged.
When they fire in the order of "unload->visibilitychanged", the crash can
happen.

Also, using wrapWeakPersistent instead of wrapPersistent. Will change the CL
description.

[Zhiqiang Zhang (Slow), 2016-08-17 16:09:13]

Description was changed from

==========
Fix a crash in AutoplayUmaHelper when releasing visibility observer

There's a crash in
AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod().
It might be possible that the callback is called after the observer
is released. This CL adds additional nullcheck to avoid the crash.

BUG=638595
==========

to

==========
Fix a crash and possible memory leak in AutoplayUmaHelper

There's a crash in
AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod().
It might be possible that the callback is called after the observer
is released. This CL adds additional nullcheck to avoid the crash.

This CL also fixes a possible memory leak by changing
wrapPersistent to wrapWeakPersistent.

BUG=638595
==========

[mlamouri (slow - plz ping), 2016-08-17 16:11:45]

lgtm

[Zhiqiang Zhang (Slow), 2016-08-17 16:16:23]

The CQ bit was checked by zqzhang@chromium.org

[commit-bot: I haz the power, 2016-08-17 16:16:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 17:29:15]

Description was changed from

==========
Fix a crash and possible memory leak in AutoplayUmaHelper

There's a crash in
AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod().
It might be possible that the callback is called after the observer
is released. This CL adds additional nullcheck to avoid the crash.

This CL also fixes a possible memory leak by changing
wrapPersistent to wrapWeakPersistent.

BUG=638595
==========

to

==========
Fix a crash and possible memory leak in AutoplayUmaHelper

There's a crash in
AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod().
It might be possible that the callback is called after the observer
is released. This CL adds additional nullcheck to avoid the crash.

This CL also fixes a possible memory leak by changing
wrapPersistent to wrapWeakPersistent.

BUG=638595
==========

[commit-bot: I haz the power, 2016-08-17 17:29:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-17 17:32:35]

Description was changed from

==========
Fix a crash and possible memory leak in AutoplayUmaHelper

There's a crash in
AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod().
It might be possible that the callback is called after the observer
is released. This CL adds additional nullcheck to avoid the crash.

This CL also fixes a possible memory leak by changing
wrapPersistent to wrapWeakPersistent.

BUG=638595
==========

to

==========
Fix a crash and possible memory leak in AutoplayUmaHelper

There's a crash in
AutoplayUmaHelper::onVisibilityChangedForVideoMutedPlayMethod().
It might be possible that the callback is called after the observer
is released. This CL adds additional nullcheck to avoid the crash.

This CL also fixes a possible memory leak by changing
wrapPersistent to wrapWeakPersistent.

BUG=638595

Committed: https://crrev.com/403b4008f974e12adcc9ba95036120cc299784fb
Cr-Commit-Position: refs/heads/master@{#412574}
==========

[commit-bot: I haz the power, 2016-08-17 17:32:36]

Patchset 2 (id:??) landed as
https://crrev.com/403b4008f974e12adcc9ba95036120cc299784fb
Cr-Commit-Position: refs/heads/master@{#412574}