Replace AllowCrossThreadAccess() + GCed pointers with wrapCrossThreadPersistent()

Replace AllowCrossThreadAccess() + GCed pointers with wrapCrossThreadPersistent()

This CL disallows AllowCrossThreadAccess() with GCed pointers in
platform/CrossThreadCopier.h and replaces AllowCrossThreadAccess() with
wrapCrossThreadPersistent() in other files where compile failed.

BUG=597856
Committed: https://crrev.com/7e43c8dae9203ccc418452fafdea69a0f3a7eaef
Cr-Commit-Position: refs/heads/master@{#394076}

[hiroshige, 2016-04-22 12:29:26]

Description was changed from

==========
Replace AllowCrossThreadAccess() with GCed pointers with retainedRef()

BUG=
==========

to

==========
Replace AllowCrossThreadAccess() with GCed pointers with retainedRef()

BUG=
==========

[hiroshige, 2016-04-22 13:20:18]

The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-22 13:20:30]

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

[commit-bot: I haz the power, 2016-04-22 15:00:05]

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

[commit-bot: I haz the power, 2016-04-22 15:00:06]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-04-26 11:47:19]

Description was changed from

==========
Replace AllowCrossThreadAccess() with GCed pointers with retainedRef()

BUG=
==========

to

==========
Replace AllowCrossThreadAccess() + GCed pointers with crossThreadRetainedRef()

BUG=
==========

[hiroshige, 2016-04-27 06:37:46]

Description was changed from

==========
Replace AllowCrossThreadAccess() + GCed pointers with crossThreadRetainedRef()

BUG=
==========

to

==========
Replace AllowCrossThreadAccess() + GCed pointers with crossThreadRetainedRef()


BUG=597856
==========

[hiroshige, 2016-04-27 06:40:23]

The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-27 06:40:32]

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

[commit-bot: I haz the power, 2016-04-27 08:06:19]

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

[commit-bot: I haz the power, 2016-04-27 08:06:20]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-04-27 09:27:22]

The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run

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

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

[commit-bot: I haz the power, 2016-04-27 12:24:11]

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

[commit-bot: I haz the power, 2016-04-27 12:24:11]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hiroshige, 2016-05-17 03:23:25]

The CQ bit was checked by hiroshige@chromium.org

[hiroshige, 2016-05-17 03:23:26]

The CQ bit was unchecked by hiroshige@chromium.org

[hiroshige, 2016-05-17 03:23:35]

The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-17 03:23:49]

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

[hiroshige, 2016-05-17 03:26:05]

Description was changed from

==========
Replace AllowCrossThreadAccess() + GCed pointers with crossThreadRetainedRef()


BUG=597856
==========

to

==========
Replace AllowCrossThreadAccess() + GCed pointers with
wrapCrossThreadPersistent()

This CL disallows AllowCrossThreadAccess() with GCed pointers in
platform/CrossThreadCopier.h and replaces AllowCrossThreadAccess() with
wrapCrossThreadPersistent() in other files where compile failed.

BUG=597856
==========

[hiroshige, 2016-05-17 03:26:05]

hiroshige@chromium.org changed reviewers:
+ haraken@chromium.org, kinuko@chromium.org, tzik@chromium.org

[hiroshige, 2016-05-17 03:46:31]

PTAL.

[haraken, 2016-05-17 04:18:56]

LGTM

[commit-bot: I haz the power, 2016-05-17 04:27:44]

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

[commit-bot: I haz the power, 2016-05-17 04:27:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tzik, 2016-05-17 05:10:40]

lgtm

[kinuko, 2016-05-17 05:21:52]

lgtm!

[hiroshige, 2016-05-17 05:39:53]

The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-17 05:40:11]

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

[sof, 2016-05-17 05:44:45]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2016-05-17 05:44:46]

https://codereview.chromium.org/1914453002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
(right):

https://codereview.chromium.org/1914453002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp:223:
BackgroundTaskRunner::postOnBackgroundThread(BLINK_FROM_HERE,
threadSafeBind(&ImageBitmapFactories::ImageBitmapLoader::decodeImageOnDecoderThread,
wrapCrossThreadPersistent(this), AllowCrossThreadAccess(taskRunner),
wrapCrossThreadPersistent(arrayBuffer)), taskSize);
Clarifying Q: why is it considered preferable to have the user/developer know
who manages the argument pointer being passed, and choose
wrapCrossThreadPersistent() / AllowCrossThreadAccess() accordingly?

[hiroshige, 2016-05-17 05:57:12]

https://codereview.chromium.org/1914453002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
(right):

https://codereview.chromium.org/1914453002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp:223:
BackgroundTaskRunner::postOnBackgroundThread(BLINK_FROM_HERE,
threadSafeBind(&ImageBitmapFactories::ImageBitmapLoader::decodeImageOnDecoderThread,
wrapCrossThreadPersistent(this), AllowCrossThreadAccess(taskRunner),
wrapCrossThreadPersistent(arrayBuffer)), taskSize);
On 2016/05/17 05:44:46, sof wrote:
> Clarifying Q: why is it considered preferable to have the user/developer know
> who manages the argument pointer being passed, and choose
> wrapCrossThreadPersistent() / AllowCrossThreadAccess() accordingly?

|AllowCrossThreadAccess(taskRunner)| will be replaced by
|crossThreadUnretained(taskRunner)|.

This is to make retainedness of bound arguments explicit.
Binding CrossThreadPersistent clearly retains strong reference, while
crossThreadUnretained() means WTF::Function doesn't retain strong reference nor
ownership.

Previously, AllowCrossThreadAccess(pointerToGCedObject) retains strong reference
while AllowCrossThreadAccess(pointerToNONGCedObject) does NOT retain strong
reference/ownership.
Issue 597856 is cleaning up such confusing cases.

[sof, 2016-05-17 06:05:24]

https://codereview.chromium.org/1914453002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
(right):

https://codereview.chromium.org/1914453002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp:223:
BackgroundTaskRunner::postOnBackgroundThread(BLINK_FROM_HERE,
threadSafeBind(&ImageBitmapFactories::ImageBitmapLoader::decodeImageOnDecoderThread,
wrapCrossThreadPersistent(this), AllowCrossThreadAccess(taskRunner),
wrapCrossThreadPersistent(arrayBuffer)), taskSize);
On 2016/05/17 05:57:12, hiroshige wrote:
> On 2016/05/17 05:44:46, sof wrote:
> > Clarifying Q: why is it considered preferable to have the user/developer
know
> > who manages the argument pointer being passed, and choose
> > wrapCrossThreadPersistent() / AllowCrossThreadAccess() accordingly?
> 
> |AllowCrossThreadAccess(taskRunner)| will be replaced by
> |crossThreadUnretained(taskRunner)|.
> 
> This is to make retainedness of bound arguments explicit.
> Binding CrossThreadPersistent clearly retains strong reference, while
> crossThreadUnretained() means WTF::Function doesn't retain strong reference
nor
> ownership.
> 
> Previously, AllowCrossThreadAccess(pointerToGCedObject) retains strong
reference
> while AllowCrossThreadAccess(pointerToNONGCedObject) does NOT retain strong
> reference/ownership.
> Issue 597856 is cleaning up such confusing cases.

ok, we'll arrive at names that work better iteratively possibly -- having to be
explicit about something be Persistent-backed (in whatever variant) seems like
an implementation detail escaping too far out.

[commit-bot: I haz the power, 2016-05-17 06:54:11]

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

[commit-bot: I haz the power, 2016-05-17 06:54:12]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-05-17 07:23:43]

The CQ bit was checked by hiroshige@chromium.org

[commit-bot: I haz the power, 2016-05-17 07:24:01]

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

[commit-bot: I haz the power, 2016-05-17 07:29:09]

Description was changed from

==========
Replace AllowCrossThreadAccess() + GCed pointers with
wrapCrossThreadPersistent()

This CL disallows AllowCrossThreadAccess() with GCed pointers in
platform/CrossThreadCopier.h and replaces AllowCrossThreadAccess() with
wrapCrossThreadPersistent() in other files where compile failed.

BUG=597856
==========

to

==========
Replace AllowCrossThreadAccess() + GCed pointers with
wrapCrossThreadPersistent()

This CL disallows AllowCrossThreadAccess() with GCed pointers in
platform/CrossThreadCopier.h and replaces AllowCrossThreadAccess() with
wrapCrossThreadPersistent() in other files where compile failed.

BUG=597856
==========

[commit-bot: I haz the power, 2016-05-17 07:29:11]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-17 07:31:05]

Description was changed from

==========
Replace AllowCrossThreadAccess() + GCed pointers with
wrapCrossThreadPersistent()

This CL disallows AllowCrossThreadAccess() with GCed pointers in
platform/CrossThreadCopier.h and replaces AllowCrossThreadAccess() with
wrapCrossThreadPersistent() in other files where compile failed.

BUG=597856
==========

to

==========
Replace AllowCrossThreadAccess() + GCed pointers with
wrapCrossThreadPersistent()

This CL disallows AllowCrossThreadAccess() with GCed pointers in
platform/CrossThreadCopier.h and replaces AllowCrossThreadAccess() with
wrapCrossThreadPersistent() in other files where compile failed.

BUG=597856

Committed: https://crrev.com/7e43c8dae9203ccc418452fafdea69a0f3a7eaef
Cr-Commit-Position: refs/heads/master@{#394076}
==========

[commit-bot: I haz the power, 2016-05-17 07:31:07]

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