[worklets] Make WorkerThread handle both Worker and Worklet global scopes.

[worklets] Make WorkerThread handle both Worker and Worklet global scopes.

This is to allow the WorkerThread infrastructure be used for ThreadedWorkletGlobalScope (see WIP here: https://codereview.chromium.org/2171973002/)

This class could also be renamed to something like ThreadScriptEnvironment or ThreadContextEnvironment, but can also be done later in just a rename patch.

BUG=567358
Committed: https://crrev.com/9639129aab764e7597ab62c98dc27605d0a0b668
Cr-Commit-Position: refs/heads/master@{#407911}

[ikilpatrick, 2016-07-21 18:00:36]

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

[commit-bot: I haz the power, 2016-07-21 18:01:43]

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, 2016-07-21 19:44:14]

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

[commit-bot: I haz the power, 2016-07-21 19:44:16]

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

[ikilpatrick, 2016-07-21 22:57:43]

Description was changed from

==========
[worklets] Make WorkerThread handle both Worker and Worklet global scopes.

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

to

==========
[worklets] Make WorkerThread handle both Worker and Worklet global scopes.

This is to allow the WorkerThread infrastructure be used for
ThreadedWorkletGlobalScope (see WIP here:
https://codereview.chromium.org/2171973002/)

This class could also be renamed to something like ThreadScriptEnvironment or
ThreadContextEnvironment, but can also be done later in just a rename patch.

BUG=567358
==========

[ikilpatrick, 2016-07-21 22:57:43]

ikilpatrick@chromium.org changed reviewers:
+ hongchan@chromium.org, nhiroki@chromium.org, yhirano@chromium.org

[ikilpatrick, 2016-07-21 23:35:12]

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

[commit-bot: I haz the power, 2016-07-21 23:35:44]

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

[ikilpatrick, 2016-07-21 23:36:20]

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

[ikilpatrick, 2016-07-21 23:40:41]

This is the first patch to make the worker thread infrastructure to handle both
workers and worklets. There isn't actually too much more required than this. See
WIP for ThreadedWorkletGlobalScope https://codereview.chromium.org/2171973002/

Next change would be deciding which parts of the messaging infrastructure makes
sense to share.

There is a much older WIP patch for this here:
https://codereview.chromium.org/1992933002/ but is out of date now.

[commit-bot: I haz the power, 2016-07-22 01:45:49]

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

[commit-bot: I haz the power, 2016-07-22 01:45:50]

Dry run: This issue passed the CQ dry run.

[kinuko, 2016-07-22 05:44:34]

At a quick glance this looks reasonable to me, I'd like to defer the detailed
review to yhirano/nhiroki

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h
(right):

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h:20: virtual
void dispose() = 0;
comments?

[yhirano, 2016-07-22 06:23:41]

The change looks good but I would be happy if we had a right word for
"WorkerOrWorklet"...

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h
(right):

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h:19: virtual
bool isClosing() const = 0;
Could you add comments for these new functions?

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/WorkerReportingProxy.h (right):

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:46: class
CORE_EXPORT WorkerReportingProxy {
Should this class and functions be renamed to WorkerOrWorklet*?

[kinuko, 2016-07-22 06:57:26]

On 2016/07/22 06:23:41, yhirano wrote:
> The change looks good but I would be happy if we had a right word for
> "WorkerOrWorklet"...

Yeah. It'd become more and more awkward if we start to have more common
classes..

>

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h
> (right):
> 
>
https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h:19:
virtual
> bool isClosing() const = 0;
> Could you add comments for these new functions?
> 
>
https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/workers/WorkerReportingProxy.h (right):
> 
>
https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:46: class
> CORE_EXPORT WorkerReportingProxy {
> Should this class and functions be renamed to WorkerOrWorklet*?

[nhiroki, 2016-07-22 07:43:00]

LGTM after kinuko and yhirano's comments are addressed.

[haraken, 2016-07-22 07:57:20]

LGTM

[ikilpatrick, 2016-07-22 18:14:31]

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

[commit-bot: I haz the power, 2016-07-22 18:14:57]

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

[ikilpatrick, 2016-07-22 18:21:59]

Right, my thoughts for naming was to make it
"ThreadedGlobal*"

Thoughts?

Was going to followup renames in other patches.

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h
(right):

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h:19: virtual
bool isClosing() const = 0;
On 2016/07/22 06:23:40, yhirano wrote:
> Could you add comments for these new functions?

Done.

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h:20: virtual
void dispose() = 0;
On 2016/07/22 05:44:34, kinuko wrote:
> comments?

Done.

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/WorkerReportingProxy.h (right):

https://codereview.chromium.org/2116113002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:46: class
CORE_EXPORT WorkerReportingProxy {
On 2016/07/22 06:23:40, yhirano wrote:
> Should this class and functions be renamed to WorkerOrWorklet*?

I was going to follow up with a rename patch(es) for this things that are now
common. Is that ok with you?

For this class either:
WorkerOrWorkletReportingProxy
ThreadedGlobalReportingProxy

Thoughts?

(then remove "Worker" from most of the methods here).

[commit-bot: I haz the power, 2016-07-22 19:38:17]

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

[commit-bot: I haz the power, 2016-07-22 19:38:18]

Dry run: This issue passed the CQ dry run.

[yhirano, 2016-07-26 02:16:17]

Sorry for the late response. LGTM.

Regarding ThreadedGlobal:

According to your design doc, both ThreadedWorkletGlobalScope and
MainThreadWorkletGlobalScope inherit WorkerOrWorkletGlobalScope (a.k.a.
ThreadedGlobalScope, I assume?) which sounds a bit confusing to me.

[nhiroki, 2016-07-26 04:39:52]

On 2016/07/26 02:16:17, yhirano wrote:
> Sorry for the late response. LGTM.
> 
> Regarding ThreadedGlobal:
> 
> According to your design doc, both ThreadedWorkletGlobalScope and
> MainThreadWorkletGlobalScope inherit WorkerOrWorkletGlobalScope (a.k.a.
> ThreadedGlobalScope, I assume?) which sounds a bit confusing to me.

FYI: To drive a discussion about naming convention, I'm now composing a document
and will share it with you soon.

[nhiroki, 2016-07-26 08:47:26]

On 2016/07/26 04:39:52, nhiroki (slow) wrote:
> On 2016/07/26 02:16:17, yhirano wrote:
> > Sorry for the late response. LGTM.
> > 
> > Regarding ThreadedGlobal:
> > 
> > According to your design doc, both ThreadedWorkletGlobalScope and
> > MainThreadWorkletGlobalScope inherit WorkerOrWorkletGlobalScope (a.k.a.
> > ThreadedGlobalScope, I assume?) which sounds a bit confusing to me.
> 
> FYI: To drive a discussion about naming convention, I'm now composing a
document
> and will share it with you soon.

https://docs.google.com/document/d/1IWgnaDMkEZq1TSXCywE9LwxfvNgtZ9-O76JhDzWI1...

[ikilpatrick, 2016-07-26 18:57:56]

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

[commit-bot: I haz the power, 2016-07-26 18:58:51]

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, 2016-07-26 20:54:20]

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

[commit-bot: I haz the power, 2016-07-26 20:54:21]

Dry run: This issue passed the CQ dry run.

[ikilpatrick, 2016-07-26 20:54:59]

The CQ bit was checked by ikilpatrick@chromium.org

[ikilpatrick, 2016-07-26 20:54:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, nhiroki@chromium.org, yhirano@chromium.org
Link to the patchset: https://codereview.chromium.org/2116113002/#ps160001
(title: "rebase again")

[commit-bot: I haz the power, 2016-07-26 20:55:41]

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, 2016-07-26 21:00:32]

Description was changed from

==========
[worklets] Make WorkerThread handle both Worker and Worklet global scopes.

This is to allow the WorkerThread infrastructure be used for
ThreadedWorkletGlobalScope (see WIP here:
https://codereview.chromium.org/2171973002/)

This class could also be renamed to something like ThreadScriptEnvironment or
ThreadContextEnvironment, but can also be done later in just a rename patch.

BUG=567358
==========

to

==========
[worklets] Make WorkerThread handle both Worker and Worklet global scopes.

This is to allow the WorkerThread infrastructure be used for
ThreadedWorkletGlobalScope (see WIP here:
https://codereview.chromium.org/2171973002/)

This class could also be renamed to something like ThreadScriptEnvironment or
ThreadContextEnvironment, but can also be done later in just a rename patch.

BUG=567358
==========

[commit-bot: I haz the power, 2016-07-26 21:00:34]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-07-26 21:02:04]

Description was changed from

==========
[worklets] Make WorkerThread handle both Worker and Worklet global scopes.

This is to allow the WorkerThread infrastructure be used for
ThreadedWorkletGlobalScope (see WIP here:
https://codereview.chromium.org/2171973002/)

This class could also be renamed to something like ThreadScriptEnvironment or
ThreadContextEnvironment, but can also be done later in just a rename patch.

BUG=567358
==========

to

==========
[worklets] Make WorkerThread handle both Worker and Worklet global scopes.

This is to allow the WorkerThread infrastructure be used for
ThreadedWorkletGlobalScope (see WIP here:
https://codereview.chromium.org/2171973002/)

This class could also be renamed to something like ThreadScriptEnvironment or
ThreadContextEnvironment, but can also be done later in just a rename patch.

BUG=567358

Committed: https://crrev.com/9639129aab764e7597ab62c98dc27605d0a0b668
Cr-Commit-Position: refs/heads/master@{#407911}
==========

[commit-bot: I haz the power, 2016-07-26 21:02:06]

Patchset 9 (id:??) landed as
https://crrev.com/9639129aab764e7597ab62c98dc27605d0a0b668
Cr-Commit-Position: refs/heads/master@{#407911}