Oilpan: move AsyncMethodRunner to the heap.

Oilpan: move AsyncMethodRunner to the heap.

Simplify and keep AsyncMethodRunner<T> on the heap always.

R=haraken
BUG=363031
Committed: https://crrev.com/3757f656d8bb15d11d9af2b2a0093b4b734df817
Cr-Commit-Position: refs/heads/master@{#369141}

[sof, 2016-01-12 21:46:19]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-12 21:47:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1580883002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1580883002/1

[commit-bot: I haz the power, 2016-01-12 23:00:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-12 23:00:14]

Dry run: This issue passed the CQ dry run.

[sof, 2016-01-13 06:15:09]

Description was changed from

==========
Oilpan: move AsyncMethodRunner to the heap.

Simplify and keep AsyncMethodRunner<T> on the heap always.

R=
BUG=
==========

to

==========
Oilpan: move AsyncMethodRunner to the heap.

Simplify and keep AsyncMethodRunner<T> on the heap always.

R=
BUG=363031
==========

[sof, 2016-01-13 06:19:51]

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

[sof, 2016-01-13 06:19:52]

please take a look.

I think it is time to remove these AsyncMethodRunner wrinkles, but no great rush
(wrt branch cut.)

(..and just after having considered PointerFieldStorageTrait<> too specialized
and removed it from the blink:: namespace, it found another use :) )

[haraken, 2016-01-13 07:41:55]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2016-01-13 07:41:56]

LGTM

Just to confirm: Assume that X and AsyncMethodRunner<X> die together and due to
the lazy sweeping, X is swept before the AsyncMethodRunner<X> is swept. In this
case, it is not possible that the timer of the AsyncMethodRunner<X> fires
because we've already added a guard to the timer
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...).
In other words, we don't need to implement something special on
AsyncMethodRunner to make it workable with lazy sweeping. Am I understanding
correctly?

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/FontFaceSet.cpp (right):

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/FontFaceSet.cpp:129: stop();

Do we really need to call stop() here?

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/heap/Handle.h (right):

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/Handle.h:1200: class
PointerFieldStorageTrait {

RawPtrOrMemberTrait ?

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/Handle.h:1202: using Type = RawPtr<T>;

4 indentation

[sof, 2016-01-13 07:49:46]

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/FontFaceSet.cpp (right):

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/FontFaceSet.cpp:129: stop();
On 2016/01/13 07:41:56, haraken wrote:
> 
> Do we really need to call stop() here?

It is required.

If we don't (from this non-heap object), then the Persistent<> will be cleared
and make the pointed-to AsyncMethodRunner<> garbage..to be collected by the next
GC. Until that time we want to protect timers from firing and calling into this
object, which has now been freed.

[sof, 2016-01-13 07:52:16]

On 2016/01/13 07:41:56, haraken wrote:
> LGTM
> 
> Just to confirm: Assume that X and AsyncMethodRunner<X> die together and due
to
> the lazy sweeping, X is swept before the AsyncMethodRunner<X> is swept. In
this
> case, it is not possible that the timer of the AsyncMethodRunner<X> fires
> because we've already added a guard to the timer
>
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...).
> In other words, we don't need to implement something special on
> AsyncMethodRunner to make it workable with lazy sweeping. Am I understanding
> correctly?
> 

Yes, exactly so: lifetimes are identical (and a reasonable
assumption/constraint), so having Timer<> use AsyncMethodRunner<>'s liveness to
determine if it is safe to fire, is sufficient.

[haraken, 2016-01-13 07:57:34]

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/FontFaceSet.cpp (right):

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/FontFaceSet.cpp:129: stop();
On 2016/01/13 07:49:46, sof wrote:
> On 2016/01/13 07:41:56, haraken wrote:
> > 
> > Do we really need to call stop() here?
> 
> It is required.
> 
> If we don't (from this non-heap object), then the Persistent<> will be cleared
> and make the pointed-to AsyncMethodRunner<> garbage..to be collected by the
next
> GC. Until that time we want to protect timers from firing and calling into
this
> object, which has now been freed.

Ah, makes sense.

[sof, 2016-01-13 09:39:42]

Description was changed from

==========
Oilpan: move AsyncMethodRunner to the heap.

Simplify and keep AsyncMethodRunner<T> on the heap always.

R=
BUG=363031
==========

to

==========
Oilpan: move AsyncMethodRunner to the heap.

Simplify and keep AsyncMethodRunner<T> on the heap always.

R=haraken
BUG=363031
==========

[sof, 2016-01-13 09:41:17]

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/heap/Handle.h (right):

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/Handle.h:1200: class
PointerFieldStorageTrait {
On 2016/01/13 07:41:56, haraken wrote:
> 
> RawPtrOrMemberTrait ?
> 

Done, little confusion about what it offers with such a name.

https://codereview.chromium.org/1580883002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/Handle.h:1202: using Type = RawPtr<T>;
On 2016/01/13 07:41:56, haraken wrote:
> 
> 4 indentation

Done.

[sof, 2016-01-13 09:41:35]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2016-01-13 09:41:35]

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/1580883002/#ps20001
(title: "rename to RawPtrOrMemberTrait<>")

[commit-bot: I haz the power, 2016-01-13 09:42:02]

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

[commit-bot: I haz the power, 2016-01-13 11:07:49]

Description was changed from

==========
Oilpan: move AsyncMethodRunner to the heap.

Simplify and keep AsyncMethodRunner<T> on the heap always.

R=haraken
BUG=363031
==========

to

==========
Oilpan: move AsyncMethodRunner to the heap.

Simplify and keep AsyncMethodRunner<T> on the heap always.

R=haraken
BUG=363031
==========

[commit-bot: I haz the power, 2016-01-13 11:07:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-13 11:08:54]

Description was changed from

==========
Oilpan: move AsyncMethodRunner to the heap.

Simplify and keep AsyncMethodRunner<T> on the heap always.

R=haraken
BUG=363031
==========

to

==========
Oilpan: move AsyncMethodRunner to the heap.

Simplify and keep AsyncMethodRunner<T> on the heap always.

R=haraken
BUG=363031

Committed: https://crrev.com/3757f656d8bb15d11d9af2b2a0093b4b734df817
Cr-Commit-Position: refs/heads/master@{#369141}
==========

[commit-bot: I haz the power, 2016-01-13 11:08:55]

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