Prevent reentrant cleanup events for idle delegates.

Prevent reentrant cleanup events for idle delegates.

Crash stacks show nested calls into the cleanup method, we shouldn't
invoke such cases. It may be worth investigating how much overhead
shotgunning a bunch of post tasks adds here and if that is too much
on JellyBean.

BUG=650663
TEST=new unittest

Committed: https://crrev.com/5f256fb8fa34e627947d0f49720546e855f794df
Cr-Commit-Position: refs/heads/master@{#421347}

[DaleCurtis, 2016-09-27 20:10:39]

dalecurtis@chromium.org changed reviewers:
+ watk@chromium.org

[watk, 2016-09-27 20:58:30]

lgtm

https://codereview.chromium.org/2367253007/diff/1/content/renderer/media/rend...
File content/renderer/media/renderer_webmediaplayer_delegate.cc (right):

https://codereview.chromium.org/2367253007/diff/1/content/renderer/media/rend...
content/renderer/media/renderer_webmediaplayer_delegate.cc:229: // OnHidden()
can trigger calls into RemoveIdleDelegate(), so for iterator
unrelated, but if you're making another change, "OnHidden" is out of date I
think.

https://codereview.chromium.org/2367253007/diff/1/content/renderer/media/rend...
content/renderer/media/renderer_webmediaplayer_delegate.cc:232:
base::AutoReset<bool> scoper(&idle_cleanup_running_, true);
Might make sense to consolidate the comments and put this right below the early
return.

[DaleCurtis, 2016-09-27 21:03:48]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2016-09-27 21:03:49]

https://codereview.chromium.org/2367253007/diff/1/content/renderer/media/rend...
File content/renderer/media/renderer_webmediaplayer_delegate.cc (right):

https://codereview.chromium.org/2367253007/diff/1/content/renderer/media/rend...
content/renderer/media/renderer_webmediaplayer_delegate.cc:229: // OnHidden()
can trigger calls into RemoveIdleDelegate(), so for iterator
On 2016/09/27 at 20:58:30, watk wrote:
> unrelated, but if you're making another change, "OnHidden" is out of date I
think.

Done.

https://codereview.chromium.org/2367253007/diff/1/content/renderer/media/rend...
content/renderer/media/renderer_webmediaplayer_delegate.cc:232:
base::AutoReset<bool> scoper(&idle_cleanup_running_, true);
On 2016/09/27 at 20:58:30, watk wrote:
> Might make sense to consolidate the comments and put this right below the
early return.

Hmm, don't like separating the comments like that, so going to keep it as is
unless you feel strongly.

[DaleCurtis, 2016-09-27 21:03:50]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org
Link to the patchset: https://codereview.chromium.org/2367253007/#ps20001
(title: "Comments.")

[commit-bot: I haz the power, 2016-09-27 21:04:13]

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-09-27 21:47:47]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-27 21:49:55]

Description was changed from

==========
Prevent reentrant cleanup events for idle delegates.

Crash stacks show nested calls into the cleanup method, we shouldn't
invoke such cases. It may be worth investigating how much overhead
shotgunning a bunch of post tasks adds here and if that is too much
on JellyBean.

BUG=650663
TEST=new unittest
==========

to

==========
Prevent reentrant cleanup events for idle delegates.

Crash stacks show nested calls into the cleanup method, we shouldn't
invoke such cases. It may be worth investigating how much overhead
shotgunning a bunch of post tasks adds here and if that is too much
on JellyBean.

BUG=650663
TEST=new unittest

Committed: https://crrev.com/5f256fb8fa34e627947d0f49720546e855f794df
Cr-Commit-Position: refs/heads/master@{#421347}
==========

[commit-bot: I haz the power, 2016-09-27 21:49:56]

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