New issue
Advanced search Search tips

Issue 757848 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 469639



Sign in to add a comment

Use CreateWebAudioThread() in AudioWorkletThread

Project Member Reported by hongchan@chromium.org, Aug 22 2017

Issue description

Description: Show this description
Cc: nhiroki@chromium.org
nhiroki@

You mentioned that passing the released pointer of WebThread[1] is not enough because it won't be owned by WorkletThreadHolder[2].

However, we're calling FooWorkletThread::EnsureInstance() as a static function, which can be called even before the actual object is created. So if FooWorkletThread owns the thread as unique_ptr doesn't really solve the problem.

One cheap solution I could think of is to have a static member of std::unique_ptr<WebThread> in AudioWorkletThread. This is okay for now because all the AudioWorkletThread shares the thread and the worklet global scopes. However, this is something I want to avoid because we eventually have to support the multiple instances of AudioWorkletThread in the near future.

WDYT?

[1]: https://chromium-review.googlesource.com/c/chromium/src/+/617588/9/third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp
[2]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/WebThreadSupportingGC.cpp?sq=package:chromium&l=36
[3]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/workers/WorkletThreadHolder.h?q=WorkletThreadHolder&dr=CSs&l=44
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a762c7247e109ae8814ef05e457a209abf13e3b0

commit a762c7247e109ae8814ef05e457a209abf13e3b0
Author: Hongchan Choi <hongchan@chromium.org>
Date: Thu Aug 31 20:36:07 2017

Use CreateWebAudioThread in AudioWorkletThread

This CL changes the WebThread used by AudioWorkletThread from the
one with NORMAL priority to a thread with DISPLAY priority.

AudioWorkletThread now owns a static member of WebThread, which will
be shared by multiple AudioContexts in the same frame.

Locally confirmed the static WebThread is working correctly on Box2D
stress demo. (--enable-blink-features=Worklet,AudioWorklet)

Bug:  757848 
Change-Id: Ifae97d749e08b5f9f232b2a26579e83aae411364
Reviewed-on: https://chromium-review.googlesource.com/626948
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498983}
[modify] https://crrev.com/a762c7247e109ae8814ef05e457a209abf13e3b0/third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp
[modify] https://crrev.com/a762c7247e109ae8814ef05e457a209abf13e3b0/third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.h

Re c#2: Sorry! I missed your comment. I hope your question was already resolved in the review. As I commented there, having separate mechanisms for AudioWorklt and AnimationWorklet sounds reasonable because they will have different threading model.
Re #4: No worries, we already discussed it in the CL and I will think about the right design for it.
Status: Verified (was: Started)

Sign in to add a comment