Worklet: Enable TaskRunnerHelper to handle MainThreadWorkletGlobalScope

Worklet: Enable TaskRunnerHelper to handle MainThreadWorkletGlobalScope

MainThreadWorklet runs on the main thread and tasks on that should be managed by
the per-frame task runners. This CL associates them in TaskRunnerHelper::Get().

This is necessary for implementing module loading on main thread worklets.
(see https://codereview.chromium.org/2826313003/)

BUG=627945
Review-Url: https://codereview.chromium.org/2870273002
Cr-Commit-Position: refs/heads/master@{#470851}
Committed: https://chromium.googlesource.com/chromium/src/+/905a5fd6040cc73a6adaff129faa5bf0b1edfc2a

[nhiroki, 2017-05-10 00:13:41]

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

[nhiroki, 2017-05-10 00:14:23]

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

[nhiroki, 2017-05-10 00:14:24]

PTAL, thanks!

[commit-bot: I haz the power, 2017-05-10 00:14:28]

Dry run: CQ is trying da patch.

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

[kinuko (google), 2017-05-10 01:04:45]

kinuko@google.com changed reviewers:
+ kinuko@google.com

[kinuko (google), 2017-05-10 01:04:46]

https://codereview.chromium.org/2870273002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp (right):

https://codereview.chromium.org/2870273002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp:82: return Get(type,
ToDocument(execution_context));
would Get(type, ToDocument(nullptr)) work?

actually what we want to call in this case is frame version of Get, is that
right?

[nhiroki, 2017-05-10 01:15:41]

Thanks for your comments.

https://codereview.chromium.org/2870273002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp (right):

https://codereview.chromium.org/2870273002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp:82: return Get(type,
ToDocument(execution_context));
On 2017/05/10 01:04:46, kinuko (google) wrote:
> would Get(type, ToDocument(nullptr)) work?

I tried it before but the compiler complained:
https://codereview.chromium.org/2806623004/diff/300001/third_party/WebKit/Sou...

> actually what we want to call in this case is frame version of Get, is that
> right?

Yes, that's right. Do you prefer ToLocalFrame()?

[kinuko (google), 2017-05-10 01:27:18]

On 2017/05/10 01:15:41, nhiroki (maybe slow) wrote:
> Thanks for your comments.
> 
>
https://codereview.chromium.org/2870273002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp (right):
> 
>
https://codereview.chromium.org/2870273002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp:82: return Get(type,
> ToDocument(execution_context));
> On 2017/05/10 01:04:46, kinuko (google) wrote:
> > would Get(type, ToDocument(nullptr)) work?
> 
> I tried it before but the compiler complained:

Would compiler complain if we do Get(type, nullptr)?  We have some Get(type, x ?
x->Frame() : nullptr) code, not super sure which order they're resolved...

>
https://codereview.chromium.org/2806623004/diff/300001/third_party/WebKit/Sou...
> 
> > actually what we want to call in this case is frame version of Get, is that
> > right?
> 
> Yes, that's right. Do you prefer ToLocalFrame()?

[kinuko (google), 2017-05-10 01:29:52]

On 2017/05/10 01:27:18, kinuko (google) wrote:
> On 2017/05/10 01:15:41, nhiroki (maybe slow) wrote:
> > Thanks for your comments.
> > 
> >
>
https://codereview.chromium.org/2870273002/diff/1/third_party/WebKit/Source/c...
> > File third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2870273002/diff/1/third_party/WebKit/Source/c...
> > third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp:82: return Get(type,
> > ToDocument(execution_context));
> > On 2017/05/10 01:04:46, kinuko (google) wrote:
> > > would Get(type, ToDocument(nullptr)) work?
> > 
> > I tried it before but the compiler complained:
> 
> Would compiler complain if we do Get(type, nullptr)?  We have some Get(type, x
?
> x->Frame() : nullptr) code, not super sure which order they're resolved...
oh ok sorry, in this case we can get the right frame one. hm.
> >
>
https://codereview.chromium.org/2806623004/diff/300001/third_party/WebKit/Sou...
> > 
> > > actually what we want to call in this case is frame version of Get, is
that
> > > right?
> > 
> > Yes, that's right. Do you prefer ToLocalFrame()?

[kinuko, 2017-05-10 01:39:08]

lgtm as we already have had the code.

I feel this overload resolution is getting weird though, we could probably have
the canonical frame version with a differen name like GetInternal... but that's
unrelated to this particular change.

[nhiroki, 2017-05-10 01:45:27]

On 2017/05/10 01:39:08, kinuko wrote:
> lgtm as we already have had the code.
> 
> I feel this overload resolution is getting weird though, we could probably
have
> the canonical frame version with a differen name like GetInternal... but
that's
> unrelated to this particular change.

Sounds reasonable. I'll make a separate patch.

[nhiroki, 2017-05-10 02:05:28]

The CQ bit was unchecked by nhiroki@chromium.org

[nhiroki, 2017-05-10 02:05:32]

The CQ bit was checked by nhiroki@chromium.org

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

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-05-10 04:09:24]

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

[commit-bot: I haz the power, 2017-05-10 04:09:25]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[nhiroki, 2017-05-10 04:39:15]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2017-05-10 04:39:59]

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-05-10 07:08:10]

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

[commit-bot: I haz the power, 2017-05-10 07:08:11]

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_...)

[nhiroki, 2017-05-11 01:55:39]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2017-05-11 01:56:35]

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-05-11 03:07:58]

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

[commit-bot: I haz the power, 2017-05-11 03:07:58]

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

[nhiroki, 2017-05-11 03:32:19]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2017-05-11 03:34:26]

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-05-11 05:29:48]

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

[commit-bot: I haz the power, 2017-05-11 05:29:49]

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_...)

[nhiroki, 2017-05-11 05:30:56]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2017-05-11 05:32:34]

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-05-11 06:51:52]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1494480656855390, "parent_rev":
"afaa2a65a0d50a88e08fd595507668538cb6689e", "commit_rev":
"905a5fd6040cc73a6adaff129faa5bf0b1edfc2a"}

[commit-bot: I haz the power, 2017-05-11 06:52:02]

Description was changed from

==========
Worklet: Enable TaskRunnerHelper to handle MainThreadWorkletGlobalScope

MainThreadWorklet runs on the main thread and tasks on that should be managed by
the per-frame task runners. This CL associates them in TaskRunnerHelper::Get().

This is necessary for implementing module loading on main thread worklets.
(see https://codereview.chromium.org/2826313003/)

BUG=627945
==========

to

==========
Worklet: Enable TaskRunnerHelper to handle MainThreadWorkletGlobalScope

MainThreadWorklet runs on the main thread and tasks on that should be managed by
the per-frame task runners. This CL associates them in TaskRunnerHelper::Get().

This is necessary for implementing module loading on main thread worklets.
(see https://codereview.chromium.org/2826313003/)

BUG=627945

Review-Url: https://codereview.chromium.org/2870273002
Cr-Commit-Position: refs/heads/master@{#470851}
Committed:
https://chromium.googlesource.com/chromium/src/+/905a5fd6040cc73a6adaff129faa...
==========

[commit-bot: I haz the power, 2017-05-11 06:52:03]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/905a5fd6040cc73a6adaff129faa...