Oilpan: provide a weak 'this' pointer abstraction for cancellable closures.

Oilpan: provide a weak 'this' pointer abstraction for cancellable closures.

For CancellableTaskFactory objects owned by an Oilpan heap object, the
factory's closure maintain a weak reference back to its heap
object owner -- the closure must not invoke a method on that heap object
once the weak reference is cleared.

That latter check for a cleared weak reference wasn't in place; provide
it here. Due to wtf/ and platform/heap/ dependency constraints, we're
forced to do that indirectly by way of using a WeakPtr<>.

R=haraken
BUG=575272
Committed: https://crrev.com/88e7f2572537569a657d3e1b94f3cd1a221d1eb1
Cr-Commit-Position: refs/heads/master@{#368851}

[sof, 2016-01-11 21:49:48]

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

[commit-bot: I haz the power, 2016-01-11 21:51:25]

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

[sof, 2016-01-11 22:14:43]

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

[sof, 2016-01-11 22:14:44]

please take a look.

Ad-hoc solution given that wtf/Functional.h cannot work over
CrossThreadWeakPersistent<>.

[commit-bot: I haz the power, 2016-01-11 23:25:42]

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

[commit-bot: I haz the power, 2016-01-11 23:25:43]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-01-12 00:04:56]

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

[haraken, 2016-01-12 00:04:57]

LGTM

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

https://codereview.chromium.org/1573283004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/Handle.h:1572: struct
ParamStorageTraits<blink::CrossThreadWeakPersistentThisPointer<T>> {

CrossThreadWeakPersistentThisPointer => WeakPersistentForFunctionBind ?

[sof, 2016-01-12 07:23:40]

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

https://codereview.chromium.org/1573283004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/Handle.h:1572: struct
ParamStorageTraits<blink::CrossThreadWeakPersistentThisPointer<T>> {
On 2016/01/12 00:04:57, haraken wrote:
> 
> CrossThreadWeakPersistentThisPointer => WeakPersistentForFunctionBind ?

That name promises a little bit more than I want it to,
WeakPersistentThisPointerForFunctionBind ?

(i.e., we shouldn't support weak persistents elsewhere in closures until
wtf/Functional.h can work with (CrossThread)(Weak)Persistents properly.)

[haraken, 2016-01-12 07:27:55]

On 2016/01/12 07:23:40, sof wrote:
>
https://codereview.chromium.org/1573283004/diff/1/third_party/WebKit/Source/p...
> File third_party/WebKit/Source/platform/heap/Handle.h (right):
> 
>
https://codereview.chromium.org/1573283004/diff/1/third_party/WebKit/Source/p...
> third_party/WebKit/Source/platform/heap/Handle.h:1572: struct
> ParamStorageTraits<blink::CrossThreadWeakPersistentThisPointer<T>> {
> On 2016/01/12 00:04:57, haraken wrote:
> > 
> > CrossThreadWeakPersistentThisPointer => WeakPersistentForFunctionBind ?
> 
> That name promises a little bit more than I want it to,
> WeakPersistentThisPointerForFunctionBind ?
> 
> (i.e., we shouldn't support weak persistents elsewhere in closures until
> wtf/Functional.h can work with (CrossThread)(Weak)Persistents properly.)

Ah, got it. CrossThreadWeakPersistentThisPointer sounds fine.

Shall we add a comment and mention that this should not be used in other places?

[sof, 2016-01-12 07:29:46]

On 2016/01/12 07:27:55, haraken wrote:
> On 2016/01/12 07:23:40, sof wrote:
> >
>
https://codereview.chromium.org/1573283004/diff/1/third_party/WebKit/Source/p...
> > File third_party/WebKit/Source/platform/heap/Handle.h (right):
> > 
> >
>
https://codereview.chromium.org/1573283004/diff/1/third_party/WebKit/Source/p...
> > third_party/WebKit/Source/platform/heap/Handle.h:1572: struct
> > ParamStorageTraits<blink::CrossThreadWeakPersistentThisPointer<T>> {
> > On 2016/01/12 00:04:57, haraken wrote:
> > > 
> > > CrossThreadWeakPersistentThisPointer => WeakPersistentForFunctionBind ?
> > 
> > That name promises a little bit more than I want it to,
> > WeakPersistentThisPointerForFunctionBind ?
> > 
> > (i.e., we shouldn't support weak persistents elsewhere in closures until
> > wtf/Functional.h can work with (CrossThread)(Weak)Persistents properly.)
> 
> Ah, got it. CrossThreadWeakPersistentThisPointer sounds fine.
> 
> Shall we add a comment and mention that this should not be used in other
places?

Will do, prominent TODO needed as well.

[sof, 2016-01-12 08:45:20]

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

[sof, 2016-01-12 08:45:42]

On 2016/01/12 07:29:46, sof wrote:
> On 2016/01/12 07:27:55, haraken wrote:
> > On 2016/01/12 07:23:40, sof wrote:
> > >
> >
>
https://codereview.chromium.org/1573283004/diff/1/third_party/WebKit/Source/p...
> > > File third_party/WebKit/Source/platform/heap/Handle.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1573283004/diff/1/third_party/WebKit/Source/p...
> > > third_party/WebKit/Source/platform/heap/Handle.h:1572: struct
> > > ParamStorageTraits<blink::CrossThreadWeakPersistentThisPointer<T>> {
> > > On 2016/01/12 00:04:57, haraken wrote:
> > > > 
> > > > CrossThreadWeakPersistentThisPointer => WeakPersistentForFunctionBind ?
> > > 
> > > That name promises a little bit more than I want it to,
> > > WeakPersistentThisPointerForFunctionBind ?
> > > 
> > > (i.e., we shouldn't support weak persistents elsewhere in closures until
> > > wtf/Functional.h can work with (CrossThread)(Weak)Persistents properly.)
> > 
> > Ah, got it. CrossThreadWeakPersistentThisPointer sounds fine.
> > 
> > Shall we add a comment and mention that this should not be used in other
> places?
> 
> Will do, prominent TODO needed as well.

Done.

[commit-bot: I haz the power, 2016-01-12 08:45:54]

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

[sof, 2016-01-12 09:49:55]

Description was changed from

==========
Oilpan: provide a weak 'this' pointer abstraction for cancellable closures.

For CancellableTaskFactory objects owned by an Oilpan heap object, the
factory's closure maintain a weak reference back to its heap
object owner -- the closure must not invoke a method on that heap object
once the weak reference is cleared.

That latter check for a cleared weak reference wasn't in place; provide
it here. Due to wtf/ and platform/heap/ dependency constraints, we're
forced to do that indirectly by way of using a WeakPtr<>.

R=
BUG=575272
==========

to

==========
Oilpan: provide a weak 'this' pointer abstraction for cancellable closures.

For CancellableTaskFactory objects owned by an Oilpan heap object, the
factory's closure maintain a weak reference back to its heap
object owner -- the closure must not invoke a method on that heap object
once the weak reference is cleared.

That latter check for a cleared weak reference wasn't in place; provide
it here. Due to wtf/ and platform/heap/ dependency constraints, we're
forced to do that indirectly by way of using a WeakPtr<>.

R=haraken
BUG=575272
==========

[commit-bot: I haz the power, 2016-01-12 10:49:32]

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

[commit-bot: I haz the power, 2016-01-12 10:49:34]

Dry run: This issue passed the CQ dry run.

[sof, 2016-01-12 11:52:10]

webkit_tests results looks fine on the Oilpan bot, landing.

[sof, 2016-01-12 11:52:18]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2016-01-12 11:52:19]

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/1573283004/#ps20001
(title: "add comments + TODOs")

[commit-bot: I haz the power, 2016-01-12 11:52:33]

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

[commit-bot: I haz the power, 2016-01-12 11:56:29]

Description was changed from

==========
Oilpan: provide a weak 'this' pointer abstraction for cancellable closures.

For CancellableTaskFactory objects owned by an Oilpan heap object, the
factory's closure maintain a weak reference back to its heap
object owner -- the closure must not invoke a method on that heap object
once the weak reference is cleared.

That latter check for a cleared weak reference wasn't in place; provide
it here. Due to wtf/ and platform/heap/ dependency constraints, we're
forced to do that indirectly by way of using a WeakPtr<>.

R=haraken
BUG=575272
==========

to

==========
Oilpan: provide a weak 'this' pointer abstraction for cancellable closures.

For CancellableTaskFactory objects owned by an Oilpan heap object, the
factory's closure maintain a weak reference back to its heap
object owner -- the closure must not invoke a method on that heap object
once the weak reference is cleared.

That latter check for a cleared weak reference wasn't in place; provide
it here. Due to wtf/ and platform/heap/ dependency constraints, we're
forced to do that indirectly by way of using a WeakPtr<>.

R=haraken
BUG=575272
==========

[commit-bot: I haz the power, 2016-01-12 11:56:30]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-12 11:57:21]

Description was changed from

==========
Oilpan: provide a weak 'this' pointer abstraction for cancellable closures.

For CancellableTaskFactory objects owned by an Oilpan heap object, the
factory's closure maintain a weak reference back to its heap
object owner -- the closure must not invoke a method on that heap object
once the weak reference is cleared.

That latter check for a cleared weak reference wasn't in place; provide
it here. Due to wtf/ and platform/heap/ dependency constraints, we're
forced to do that indirectly by way of using a WeakPtr<>.

R=haraken
BUG=575272
==========

to

==========
Oilpan: provide a weak 'this' pointer abstraction for cancellable closures.

For CancellableTaskFactory objects owned by an Oilpan heap object, the
factory's closure maintain a weak reference back to its heap
object owner -- the closure must not invoke a method on that heap object
once the weak reference is cleared.

That latter check for a cleared weak reference wasn't in place; provide
it here. Due to wtf/ and platform/heap/ dependency constraints, we're
forced to do that indirectly by way of using a WeakPtr<>.

R=haraken
BUG=575272

Committed: https://crrev.com/88e7f2572537569a657d3e1b94f3cd1a221d1eb1
Cr-Commit-Position: refs/heads/master@{#368851}
==========

[commit-bot: I haz the power, 2016-01-12 11:57:22]

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