[worklets] Add AudioWorkletGlobalScope and AudioWorkletThread

[worklets] Add AudioWorkletGlobalScope and AudioWorkletThread

This CL is to add AudioWorkletGlobalScope and AudioWorkletThread. This
change is largely based on the https://codereview.chromium.org/2254593002,
but some adaptations have been made to accommodate the AudioWorklet
design.

- AudioWorkletGlobalScope extends ThreadedWorkletGlobalScope
- AudioWorklerThread uses WebThread(SupportingGC)

BUG=469639
TEST=AudioWorkletThreadTest.cpp

Committed: https://crrev.com/1b37f672598fd11ebeb8e329dc257f62fbd8232e
Cr-Commit-Position: refs/heads/master@{#425278}

[hongchan, 2016-09-27 18:28:55]

Description was changed from

==========
[worklets] Add AudioWorkletGlobalScope and AudioWorkletThread

This CL is to add AudioWorkletGlobalScope and AudioWorkletThread. This
change is largely based on the https://codereview.chromium.org/2254593002,
but some adaptations have been made to accommodate the AudioWorklet
design.

- AudioWorkletGlobalScope extends ThreadedWorkletGlobalScope
- AudioWorklerThread uses WebThread(SupportingGC)

BUG=469639
==========

to

==========
[worklets] Add AudioWorkletGlobalScope and AudioWorkletThread

This CL is to add AudioWorkletGlobalScope and AudioWorkletThread. This
change is largely based on the https://codereview.chromium.org/2254593002,
but some adaptations have been made to accommodate the AudioWorklet
design.

- AudioWorkletGlobalScope extends ThreadedWorkletGlobalScope
- AudioWorklerThread uses WebThread(SupportingGC)

BUG=469639
==========

[hongchan, 2016-09-27 18:28:56]

hongchan@chromium.org changed reviewers:
+ haraken@chromium.org, ikillbert@gmail.com, rtoy@chromium.org

[hongchan, 2016-09-27 18:29:25]

hongchan@chromium.org changed reviewers:
+ ikilpatrick@chromium.org
- ikillbert@gmail.com

[hongchan, 2016-09-29 20:11:56]

PTAL.

[hongchan, 2016-09-29 20:25:38]

Description was changed from

==========
[worklets] Add AudioWorkletGlobalScope and AudioWorkletThread

This CL is to add AudioWorkletGlobalScope and AudioWorkletThread. This
change is largely based on the https://codereview.chromium.org/2254593002,
but some adaptations have been made to accommodate the AudioWorklet
design.

- AudioWorkletGlobalScope extends ThreadedWorkletGlobalScope
- AudioWorklerThread uses WebThread(SupportingGC)

BUG=469639
==========

to

==========
[worklets] Add AudioWorkletGlobalScope and AudioWorkletThread

This CL is to add AudioWorkletGlobalScope and AudioWorkletThread. This
change is largely based on the https://codereview.chromium.org/2254593002,
but some adaptations have been made to accommodate the AudioWorklet
design.

- AudioWorkletGlobalScope extends ThreadedWorkletGlobalScope
- AudioWorklerThread uses WebThread(SupportingGC)

BUG=469639
TEST=AudioWorkletThreadTest.cpp
==========

[ikilpatrick, 2016-09-29 21:31:45]

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

[ikilpatrick, 2016-09-29 21:31:45]

+nhiroki

[haraken, 2016-09-30 00:18:39]

I'm curious if we can share more code with AbstractAnimationWorkletThread.

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp (right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:24: class
BackingThreadHolder {

Redundant indentation.

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:24: class
BackingThreadHolder {

Can we share the implementation with BackingThreadHolder in
AbstractAnimationWorkletThread.cpp ?

[hongchan, 2016-09-30 02:37:56]

On 2016/09/30 00:18:39, haraken wrote:
> I'm curious if we can share more code with AbstractAnimationWorkletThread.
> 
>
https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp
(right):
> 
>
https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:24: class
> BackingThreadHolder {
> 
> Redundant indentation.
> 
>
https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:24: class
> BackingThreadHolder {
> 
> Can we share the implementation with BackingThreadHolder in
> AbstractAnimationWorkletThread.cpp ?

FWIW, AbstractAnimationWorkletThread will be merged into AnimationWorkletThread
according to the comment inside.

Perhaps we can refactor BackingThreadHolder out or create ThreadedWorkletThread
class. ikilpatrick@ WDYT?

[nhiroki, 2016-09-30 11:20:46]

On 2016/09/30 02:37:56, hoch wrote:
> On 2016/09/30 00:18:39, haraken wrote:
> > I'm curious if we can share more code with AbstractAnimationWorkletThread.
> > 
> >
>
https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp
> (right):
> > 
> >
>
https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:24: class
> > BackingThreadHolder {
> > 
> > Redundant indentation.
> > 
> >
>
https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:24: class
> > BackingThreadHolder {
> > 
> > Can we share the implementation with BackingThreadHolder in
> > AbstractAnimationWorkletThread.cpp ?
> 
> FWIW, AbstractAnimationWorkletThread will be merged into
AnimationWorkletThread
> according to the comment inside.
> 
> Perhaps we can refactor BackingThreadHolder out or create
ThreadedWorkletThread
> class. ikilpatrick@ WDYT?

I'd prefer to factor out BackingThreadHolder into its own file and make
xxxWorkletThread that needs BackingThreadHolder functions include it because I
think ThreadedWorkletThread doesn't always need BackingThreadHolder. For
example, we may add a new Worklet that one WorkletGlobalScope occupies a whole
dedicated thread.

[nhiroki, 2016-09-30 11:28:58]

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/modules_idl_files.gni (right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/modules_idl_files.gni:15:
"AudioWorkletGlobalScope",
Can you sort this in alphabetical order?

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/modules_idl_files.gni:24:
"AudioWorkletGlobalScope",
ditto.

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.h
(right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.h:12: class
AudioWorkletGlobalScope : public ThreadedWorkletGlobalScope {
final?

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.h (right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.h:35: static void
collectAllGarbage();
Can you add a comment that this may block the main thread?

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp
(right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp:32: // A
null WorkerReportingProxy, supplied when creating AudioWorkletThreads.
Indents are not necessary.

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp:93:
std::unique_ptr<WaitableEvent> waitEvent = wrapUnique(new WaitableEvent());
You can allocate WaitableEvent on stack.

[hongchan, 2016-10-04 17:07:15]

On 2016/09/30 00:18:39, haraken wrote:
> I'm curious if we can share more code with AbstractAnimationWorkletThread.
> 
>
https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp
(right):
> 
>
https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:24: class
> BackingThreadHolder {
> 
> Redundant indentation.
> 
>
https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:24: class
> BackingThreadHolder {
> 
> Can we share the implementation with BackingThreadHolder in
> AbstractAnimationWorkletThread.cpp ?

Just submitted the CL for the refactoring: 
https://codereview.chromium.org/2389193003/

[hongchan, 2016-10-11 23:42:39]

PTAL. 

I applied the refactored WorkletBackingThreadHolder to AudioWorkletThread.

@nhiroki Can you explain about your comment on WaitableEvent being on stack? Do
you want me to create a local instance of WaitableEvent in the test?

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/modules_idl_files.gni (right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/modules_idl_files.gni:15:
"AudioWorkletGlobalScope",
On 2016/09/30 11:28:57, nhiroki (slow) wrote:
> Can you sort this in alphabetical order?

Done.

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/modules_idl_files.gni:24:
"AudioWorkletGlobalScope",
On 2016/09/30 11:28:57, nhiroki (slow) wrote:
> ditto.

Done.

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.h
(right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.h:12: class
AudioWorkletGlobalScope : public ThreadedWorkletGlobalScope {
On 2016/09/30 11:28:57, nhiroki (slow) wrote:
> final?

Done.

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp (right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:24: class
BackingThreadHolder {
On 2016/09/30 00:18:38, haraken wrote:
> 
> Redundant indentation.

Done.

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.h (right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.h:35: static void
collectAllGarbage();
On 2016/09/30 11:28:57, nhiroki (slow) wrote:
> Can you add a comment that this may block the main thread?

Done.

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp
(right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp:32: // A
null WorkerReportingProxy, supplied when creating AudioWorkletThreads.
On 2016/09/30 11:28:57, nhiroki (slow) wrote:
> Indents are not necessary.

Done.

[hongchan, 2016-10-11 23:42:46]

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

[commit-bot: I haz the power, 2016-10-11 23:43:16]

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-10-12 01:27:17]

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

[commit-bot: I haz the power, 2016-10-12 01:27:18]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-10-12 01:56:43]

LGTM on my side, but I hope you remove the duplicated code using a C++ template
in a follow-up CL.

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.h
(right):

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.h:19:
v8::Isolate*,

Put Isolate* on the first argument.

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.h:27:
v8::Isolate*,

Ditto.

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp (right):

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:26: // a
template class.

OK :)

[nhiroki, 2016-10-12 02:27:44]

lgtm

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.h (right):

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.h:44:
WorkerOrWorkletGlobalScope* createWorkerGlobalScope(
explicit

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp
(right):

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp:12:
#include "core/workers/InProcessWorkerObjectProxy.h"
Maybe core/workers/WorkerReportingProxy.h? InProcessWorkerObejctProxy is used
for DedicatedWorker and CompositorWorker.

[nhiroki, 2016-10-12 02:30:13]

On 2016/10/11 23:42:39, hoch wrote:
> PTAL. 
> 
> I applied the refactored WorkletBackingThreadHolder to AudioWorkletThread.
> 
> @nhiroki Can you explain about your comment on WaitableEvent being on stack?
Do
> you want me to create a local instance of WaitableEvent in the test?

Yes, I meant a local instance :)

[ikilpatrick, 2016-10-12 16:54:24]

lgtm

[hongchan, 2016-10-12 17:24:05]

haraken@ Can you take one more look at your feedback on Isolate*?

rtoy@ PTAL.

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp
(right):

https://codereview.chromium.org/2372303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp:93:
std::unique_ptr<WaitableEvent> waitEvent = wrapUnique(new WaitableEvent());
On 2016/09/30 11:28:58, nhiroki wrote:
> You can allocate WaitableEvent on stack.

Done.

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.h
(right):

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.h:19:
v8::Isolate*,
On 2016/10/12 01:56:43, haraken wrote:
> 
> Put Isolate* on the first argument.

Can you elaborate? Do you want me to remove |v8::|? Or to replace the first
argument with Isolate*? Not sure I am following you here. I looked into the
AnimationWorkletGlobalScope and the code is identical.

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp (right):

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp:26: // a
template class.
On 2016/10/12 01:56:43, haraken wrote:
> 
> OK :)

Hopefully I can make this refactoring soon enough.

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.h (right):

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.h:44:
WorkerOrWorkletGlobalScope* createWorkerGlobalScope(
On 2016/10/12 02:27:44, nhiroki wrote:
> explicit

This is not a constructor. I can add explicit to the actual constructor down
below.

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp
(right):

https://codereview.chromium.org/2372303002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp:12:
#include "core/workers/InProcessWorkerObjectProxy.h"
On 2016/10/12 02:27:44, nhiroki wrote:
> Maybe core/workers/WorkerReportingProxy.h? InProcessWorkerObejctProxy is used
> for DedicatedWorker and CompositorWorker.

I merely followed AnimationWorkletThread's test file.

[haraken, 2016-10-13 01:54:41]

LGTM

[Raymond Toy, 2016-10-13 21:54:15]

lgtm with nit.

https://codereview.chromium.org/2372303002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.idl
(right):

https://codereview.chromium.org/2372303002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.idl:12: };
The current spec says there's a registerProcessor method. Presumably you're not
ready to implement that right now, but it might be useful to include that now,
commented out of course.

[hongchan, 2016-10-13 23:10:22]

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

[commit-bot: I haz the power, 2016-10-13 23:10:49]

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-10-14 01:37:10]

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

[commit-bot: I haz the power, 2016-10-14 01:37:11]

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

[hongchan, 2016-10-14 08:07:52]

The CQ bit was checked by hongchan@chromium.org

[hongchan, 2016-10-14 08:07:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
ikilpatrick@chromium.org, nhiroki@chromium.org, haraken@chromium.org,
rtoy@chromium.org
Link to the patchset: https://codereview.chromium.org/2372303002/#ps100001
(title: "Fixing nits after l-g-t-m")

[commit-bot: I haz the power, 2016-10-14 08:08:06]

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-10-14 09:31:16]

Description was changed from

==========
[worklets] Add AudioWorkletGlobalScope and AudioWorkletThread

This CL is to add AudioWorkletGlobalScope and AudioWorkletThread. This
change is largely based on the https://codereview.chromium.org/2254593002,
but some adaptations have been made to accommodate the AudioWorklet
design.

- AudioWorkletGlobalScope extends ThreadedWorkletGlobalScope
- AudioWorklerThread uses WebThread(SupportingGC)

BUG=469639
TEST=AudioWorkletThreadTest.cpp
==========

to

==========
[worklets] Add AudioWorkletGlobalScope and AudioWorkletThread

This CL is to add AudioWorkletGlobalScope and AudioWorkletThread. This
change is largely based on the https://codereview.chromium.org/2254593002,
but some adaptations have been made to accommodate the AudioWorklet
design.

- AudioWorkletGlobalScope extends ThreadedWorkletGlobalScope
- AudioWorklerThread uses WebThread(SupportingGC)

BUG=469639
TEST=AudioWorkletThreadTest.cpp
==========

[commit-bot: I haz the power, 2016-10-14 09:31:17]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-14 09:33:23]

Description was changed from

==========
[worklets] Add AudioWorkletGlobalScope and AudioWorkletThread

This CL is to add AudioWorkletGlobalScope and AudioWorkletThread. This
change is largely based on the https://codereview.chromium.org/2254593002,
but some adaptations have been made to accommodate the AudioWorklet
design.

- AudioWorkletGlobalScope extends ThreadedWorkletGlobalScope
- AudioWorklerThread uses WebThread(SupportingGC)

BUG=469639
TEST=AudioWorkletThreadTest.cpp
==========

to

==========
[worklets] Add AudioWorkletGlobalScope and AudioWorkletThread

This CL is to add AudioWorkletGlobalScope and AudioWorkletThread. This
change is largely based on the https://codereview.chromium.org/2254593002,
but some adaptations have been made to accommodate the AudioWorklet
design.

- AudioWorkletGlobalScope extends ThreadedWorkletGlobalScope
- AudioWorklerThread uses WebThread(SupportingGC)

BUG=469639
TEST=AudioWorkletThreadTest.cpp

Committed: https://crrev.com/1b37f672598fd11ebeb8e329dc257f62fbd8232e
Cr-Commit-Position: refs/heads/master@{#425278}
==========

[commit-bot: I haz the power, 2016-10-14 09:33:24]

Patchset 6 (id:??) landed as
https://crrev.com/1b37f672598fd11ebeb8e329dc257f62fbd8232e
Cr-Commit-Position: refs/heads/master@{#425278}