Do not invoke async method on a GC-unreachable object.

Do not invoke async method on a GC-unreachable object.

For AsyncMethodRunner<Target> where Target is a garbage collected type,
we cannot have the async method runner safely call a method on an object
instance if that object has been determined as being unreachable by the
GC marking phase, but it _or_ any of the objects that it refers to hasn't
been swept yet.

This aligns the treatment of AsyncMethodRunner<> over garbage collected
objects with that of Timer<>.

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

[sof, 2015-04-24 21:38:41]

sigbjornf@opera.com changed reviewers:
+ haraken@chromium.org

[sof, 2015-04-24 21:38:42]

Please take a look.

[haraken, 2015-04-25 08:02:08]

LGTM

https://codereview.chromium.org/1108663002/diff/1/Source/platform/AsyncMethod...
File Source/platform/AsyncMethodRunner.h (right):

https://codereview.chromium.org/1108663002/diff/1/Source/platform/AsyncMethod...
Source/platform/AsyncMethodRunner.h:125: if
(!TimerIsObjectAliveTrait<TargetClass>::isHeapObjectAlive(m_object))

Can we have a comment to mention that this hack will be gone once we move
AsyncMethodRunner to the heap?

[sof, 2015-04-25 09:10:26]

https://codereview.chromium.org/1108663002/diff/1/Source/platform/AsyncMethod...
File Source/platform/AsyncMethodRunner.h (right):

https://codereview.chromium.org/1108663002/diff/1/Source/platform/AsyncMethod...
Source/platform/AsyncMethodRunner.h:125: if
(!TimerIsObjectAliveTrait<TargetClass>::isHeapObjectAlive(m_object))
On 2015/04/25 08:02:07, haraken wrote:
> 
> Can we have a comment to mention that this hack will be gone once we move
> AsyncMethodRunner to the heap?

Good idea, done.

[sof, 2015-04-25 09:10:34]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-04-25 09:10: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/1108663002/#ps20001
(title: "Clarify that added is-alive check will be redundant once object is on
the heap")

[commit-bot: I haz the power, 2015-04-25 09:10:52]

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

[sof, 2015-04-25 10:20:31]

Somewhat related, so let me make this observation/comment here (for lack of a
better place to put it before I forget :) ) -- there's a wish to reduce the
Blink use of one-shot timers on the main thread as part of scheduler work.
Replacing them instead with posted tasks.

For Oilpan, there's a subtle implication of this -- timers (and this async
method runner) do not create Persistent<> references to their objects, whereas
tasks do. It is something to keep in mind, there might be costs associated with
this switch to being task-based, which we may have to address.

[haraken, 2015-04-25 13:21:55]

On 2015/04/25 10:20:31, sof wrote:
> Somewhat related, so let me make this observation/comment here (for lack of a
> better place to put it before I forget :) ) -- there's a wish to reduce the
> Blink use of one-shot timers on the main thread as part of scheduler work.
> Replacing them instead with posted tasks.
> 
> For Oilpan, there's a subtle implication of this -- timers (and this async
> method runner) do not create Persistent<> references to their objects, whereas
> tasks do. It is something to keep in mind, there might be costs associated
with
> this switch to being task-based, which we may have to address.

My gut feeling is that the overhead of creating a Persistent handle would be
negligible compared to the overhead of a task invokation and the task execution
itself, but just my 2 cents. (Also switching the timer to the task won't change
# of live objects in most cases.) :)

[commit-bot: I haz the power, 2015-04-25 16:35:45]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194437