Switch SuspendableScriptExecutor over to using SelfKeepAlive<>.

Switch SuspendableScriptExecutor over to using SelfKeepAlive<>.

Have this Oilpan-hosted object keep itself alive using SelfKeepAlive<>,
moving it onto the Oilpan heap by default in the process.

R=haraken
BUG=420515
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200036

[sof, 2015-08-04 14:05:46]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2015-08-04 14:05:47]

please take a look.

[sof, 2015-08-04 14:14:40]

https://codereview.chromium.org/1258653003/diff/1/Source/web/SuspendableScrip...
File Source/web/SuspendableScriptExecutor.h (left):

https://codereview.chromium.org/1258653003/diff/1/Source/web/SuspendableScrip...
Source/web/SuspendableScriptExecutor.h:48: bool m_disposed;
With the self-keepalive, having this object be finalized without calling
dispose() isn't possible; an invariant it shouldn't be too hard to maintain. And
should you fail to call dispose(), you'll experience a bad leak. Hence, I don't
think this flag adds much value now & can be convinced to remove it.

[haraken, 2015-08-04 23:20:36]

LGTM

On 2015/08/04 14:14:40, sof wrote:
>
https://codereview.chromium.org/1258653003/diff/1/Source/web/SuspendableScrip...
> File Source/web/SuspendableScriptExecutor.h (left):
> 
>
https://codereview.chromium.org/1258653003/diff/1/Source/web/SuspendableScrip...
> Source/web/SuspendableScriptExecutor.h:48: bool m_disposed;
> With the self-keepalive, having this object be finalized without calling
> dispose() isn't possible; an invariant it shouldn't be too hard to maintain.
And
> should you fail to call dispose(), you'll experience a bad leak. Hence, I
don't
> think this flag adds much value now & can be convinced to remove it.

Agreed. Maybe we can add an assert to SelfKeepAlive to verify that no
SelfKeepAlive is left when shutting down the main thread. (This is already
verified for other threads by ASSERT(remainingPersistentHandles == 0) when
shutting down the threads.)

[sof, 2015-08-05 06:32:01]

On 2015/08/04 23:20:36, haraken wrote:
> LGTM
> 
> On 2015/08/04 14:14:40, sof wrote:
> >
>
https://codereview.chromium.org/1258653003/diff/1/Source/web/SuspendableScrip...
> > File Source/web/SuspendableScriptExecutor.h (left):
> > 
> >
>
https://codereview.chromium.org/1258653003/diff/1/Source/web/SuspendableScrip...
> > Source/web/SuspendableScriptExecutor.h:48: bool m_disposed;
> > With the self-keepalive, having this object be finalized without calling
> > dispose() isn't possible; an invariant it shouldn't be too hard to maintain.
> And
> > should you fail to call dispose(), you'll experience a bad leak. Hence, I
> don't
> > think this flag adds much value now & can be convinced to remove it.
> 
> Agreed. Maybe we can add an assert to SelfKeepAlive to verify that no
> SelfKeepAlive is left when shutting down the main thread. (This is already
> verified for other threads by ASSERT(remainingPersistentHandles == 0) when
> shutting down the threads.)

Removed the ref-ptr misuse assert from SuspendableScriptExecutor, I'll
separately follow up on detecting leaks due to SelfKeepAlive<>.

[sof, 2015-08-05 06:46:53]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-08-05 06:46:53]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1258653003/#ps60001
(title: "retire m_disposed checking")

[commit-bot: I haz the power, 2015-08-05 06:47:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1258653003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1258653003/60001

[commit-bot: I haz the power, 2015-08-05 07:49:44]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200036