Reduce use of ExecutionContextTask in core/

Reduce use of ExecutionContextTask in core/

ExecutionContextTask is deprecated in favor of WTF::Closure.
This patch replaces ExecutionContextTask::postTask
with WebTaskRunner::postTask in some of the occurrences.

BUG=625927
Review-Url: https://codereview.chromium.org/2680013008
Cr-Commit-Position: refs/heads/master@{#449557}
Committed: https://chromium.googlesource.com/chromium/src/+/5fbd15767912e9546dab933ba377c8ab68a5b3c8

[yuryu, 2017-02-09 05:14:21]

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

[commit-bot: I haz the power, 2017-02-09 05:14:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-09 06:41:38]

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

[commit-bot: I haz the power, 2017-02-09 06:41:39]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[yuryu, 2017-02-09 10:41:32]

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

[commit-bot: I haz the power, 2017-02-09 10:41:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-02-09 12:15:33]

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

[tzik, 2017-02-09 12:15:34]

https://codereview.chromium.org/2680013008/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/clipboard/DataTransferItem.cpp (right):

https://codereview.chromium.org/2680013008/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/clipboard/DataTransferItem.cpp:90:
WTF::bind(&runGetAsStringTask, wrapPersistent(context),
Could you replace wrapPersistent(context) here with wrapWeakPersistent(context),
and add a null check to runGetAsStringTask?
On the previous code, ExecutionContext::postTask cancels the posted task when
ExecutionContext is destroyed, and the posted task doesn't retain the strong
reference to ExecutionContext. I think we should keep previous behavior now.

[commit-bot: I haz the power, 2017-02-09 12:23:05]

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

[commit-bot: I haz the power, 2017-02-09 12:23:05]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[yuryu, 2017-02-09 12:56:42]

yuryu@chromium.org changed reviewers:
+ nhiroki@chromium.org

[yuryu, 2017-02-09 12:56:43]

https://codereview.chromium.org/2680013008/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/clipboard/DataTransferItem.cpp (right):

https://codereview.chromium.org/2680013008/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/clipboard/DataTransferItem.cpp:90:
WTF::bind(&runGetAsStringTask, wrapPersistent(context),
On 2017/02/09 12:15:34, tzik wrote:
> Could you replace wrapPersistent(context) here with
wrapWeakPersistent(context),
> and add a null check to runGetAsStringTask?
> On the previous code, ExecutionContext::postTask cancels the posted task when
> ExecutionContext is destroyed, and the posted task doesn't retain the strong
> reference to ExecutionContext. I think we should keep previous behavior now.

Done.

[yuryu, 2017-02-09 12:56:56]

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

[commit-bot: I haz the power, 2017-02-09 12:57:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-09 14:06:28]

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

[commit-bot: I haz the power, 2017-02-09 14:06:29]

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

[yuryu, 2017-02-09 15:34:47]

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

[commit-bot: I haz the power, 2017-02-09 15:35:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-09 17:10:39]

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

[commit-bot: I haz the power, 2017-02-09 17:10:40]

Dry run: This issue passed the CQ dry run.

[tzik, 2017-02-10 03:25:21]

lgtm

[yuryu, 2017-02-10 03:37:34]

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

[yuryu, 2017-02-10 03:37:35]

haraken, can you take a look? thanks.

[nhiroki, 2017-02-10 04:02:03]

lgtm

Regarding CL subject and description, "Worker: " prefix sounds strange because
this is not related to worker code (i.e., core/workers).

I don't come up with a good alternative... no prefix or "PostTask: "??

[yuryu, 2017-02-10 04:38:09]

Description was changed from

==========
Worker: Reduce use of ExecutionContextTask in core/

ExecutionContextTask is deprecated in favor of WTF::Closure.
This patch replaces ExecutionContextTask::postTask
with WebTaskRunner::postTask in some of the occurrences.

BUG=625927
==========

to

==========
Reduce use of ExecutionContextTask in core/

ExecutionContextTask is deprecated in favor of WTF::Closure.
This patch replaces ExecutionContextTask::postTask
with WebTaskRunner::postTask in some of the occurrences.

BUG=625927
==========

[yuryu, 2017-02-10 04:38:36]

deleted the prefix.

[haraken, 2017-02-10 05:53:14]

LGTM

https://codereview.chromium.org/2680013008/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/clipboard/DataTransferItem.cpp (right):

https://codereview.chromium.org/2680013008/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/clipboard/DataTransferItem.cpp:88: context,
"DataTransferItem.getAsString", callback);

Not directly related to this CL, on what tasks do we need to add the inspector
instrumentation? Normal postTasks don't have the inspector instrumentation.

[tzik, 2017-02-10 06:13:19]

https://codereview.chromium.org/2680013008/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/clipboard/DataTransferItem.cpp (right):

https://codereview.chromium.org/2680013008/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/clipboard/DataTransferItem.cpp:88: context,
"DataTransferItem.getAsString", callback);
On 2017/02/10 05:53:14, haraken wrote:
> 
> Not directly related to this CL, on what tasks do we need to add the inspector
> instrumentation? Normal postTasks don't have the inspector instrumentation.

Conceptually, JS-initiated asynchronous tasks that fires a JS event should use
it for better stack trace on the inspector.

I think it should be done separately outside of postTask.

[yuryu, 2017-02-10 06:30:09]

The CQ bit was checked by yuryu@chromium.org

[commit-bot: I haz the power, 2017-02-10 06:30:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-10 06:34:49]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1486708209142210,
"parent_rev": "ae9e1cac379b3eb4c1b78a0092a7c5f863bc891a", "commit_rev":
"5fbd15767912e9546dab933ba377c8ab68a5b3c8"}

[commit-bot: I haz the power, 2017-02-10 06:36:01]

Description was changed from

==========
Reduce use of ExecutionContextTask in core/

ExecutionContextTask is deprecated in favor of WTF::Closure.
This patch replaces ExecutionContextTask::postTask
with WebTaskRunner::postTask in some of the occurrences.

BUG=625927
==========

to

==========
Reduce use of ExecutionContextTask in core/

ExecutionContextTask is deprecated in favor of WTF::Closure.
This patch replaces ExecutionContextTask::postTask
with WebTaskRunner::postTask in some of the occurrences.

BUG=625927

Review-Url: https://codereview.chromium.org/2680013008
Cr-Commit-Position: refs/heads/master@{#449557}
Committed:
https://chromium.googlesource.com/chromium/src/+/5fbd15767912e9546dab933ba377...
==========

[commit-bot: I haz the power, 2017-02-10 06:36:02]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/5fbd15767912e9546dab933ba377...