Skip to content

GitLab

T
tangled
Commit e9624faf authored by Hongchan Choi's avatar Hongchan Choi Committed by Commit Bot
Browse files
Options

Reset thread when audioWorklet.addModule called after context starts 

Before audioWorklet.addModule() gets called, BaseAudioContext does not
have a valid AudioWorkletThread so it cannot access to the worklet
functionality, eventually causing the page to crash.

This CL introduces the dynamic thread switching: if the context already
started with the regular thread, then switch it with the worklet thread
when audioWorklet.addModule() call is resolved.

The dynamic thread switching on the real-time audio context makes
the context rendering to stop and restart. This might cause audio
glitches in the audio stream.

With this fix, the attached layout test does not crash anymore.

Bug: 774566
Test: worklet-context-interaction.html
Change-Id: I2dc8e723a1b88d3a56fbd49aeb4e64c304d48dad
Reviewed-on: https://chromium-review.googlesource.com/722126Reviewed-by: default avatarRaymond Toy <rtoy@chromium.org>
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509580}
parent 0c621390
Hide whitespace changes
Inline Side-by-side
Showing with 99 additions and 7 deletions
third_party/WebKit/LayoutTests/http/tests/webaudio/audio-worklet/audio-worklet-node.html → third_party/WebKit/LayoutTests/http/tests/webaudio/audio-worklet/audio-worklet-node-construction.html
View file @ e9624faf
......@@ -2,7 +2,7 @@
<html>
<head>
<title>
Test the construction of AudioWorkletNode
Test the construction of AudioWorkletNode with real-time context
</title>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
......@@ -15,14 +15,15 @@
assertAudioWorklet();
let audit = Audit.createTaskRunner();
let context = new AudioContext();
let realtimeContext = new AudioContext();
// Test if an exception is thrown correctly when AWN constructor is
// invoked before resolving |.addModule()| promise.
audit.define(
{label: 'construction-before-module-loading'},
(task, should) => {
should(() => new AudioWorkletNode(context, 'dummy'),
should(() => new AudioWorkletNode(realtimeContext, 'dummy'),
'Creating a node before loading a module should throw.')
.throw('InvalidStateError');
......@@ -36,11 +37,12 @@
{label: 'construction-after-module-loading'},
(task, should) => {
audioWorklet.addModule('dummy-processor.js').then(() => {
let dummyWorkletNode = new AudioWorkletNode(context, 'dummy');
let dummyWorkletNode =
new AudioWorkletNode(realtimeContext, 'dummy');
should(dummyWorkletNode instanceof AudioWorkletNode,
'"dummyWorkletNode" is an instance of AudioWorkletNode')
.beTrue();
should(() => new AudioWorkletNode(context, 'foobar'),
should(() => new AudioWorkletNode(realtimeContext, 'foobar'),
'Unregistered name "foobar" must throw an exception.')
.throw();
task.done();
......
third_party/WebKit/LayoutTests/http/tests/webaudio/audio-worklet/worklet-context-interaction.html 0 → 100644
View file @ e9624faf
<!DOCTYPE html>
<html>
<head>
<title>
Test the invocation order of AudioWorklet.addModule() and BaseAudioContext
</title>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../../../webaudio-resources/audit.js"></script>
<script src="audio-worklet-common.js"></script>
</head>
<body>
<script id="layout-test-code">
// TODO(hongchan): remove this assertion when AudioWorklet shipped.
assertAudioWorklet();
let audit = Audit.createTaskRunner();
let sampleRate = 48000;
let realtimeContext = new AudioContext();
let offlineContext = new OfflineAudioContext(1, sampleRate, sampleRate);
// Test if the browser does not crash upon addModule() call after the
// realtime context construction.
audit.define(
{label: 'module-loading-after-realtime-context-creation'},
(task, should) => {
let dummyWorkletNode =
new AudioWorkletNode(realtimeContext, 'dummy');
dummyWorkletNode.connect(realtimeContext.destination);
should(dummyWorkletNode instanceof AudioWorkletNode,
'"dummyWorkletNode" is an instance of AudioWorkletNode ' +
'from realtime context')
.beTrue();
task.done();
});
// Test if the browser does not crash upon addModule() call after the
// offline context construction.
audit.define(
{label: 'module-loading-after-offline-context-creation'},
(task, should) => {
let dummyWorkletNode =
new AudioWorkletNode(offlineContext, 'dummy');
dummyWorkletNode.connect(offlineContext.destination);
should(dummyWorkletNode instanceof AudioWorkletNode,
'"dummyWorkletNode" is an instance of AudioWorkletNode ' +
'from offline context')
.beTrue();
task.done();
});
audioWorklet.addModule('dummy-processor.js').then(() => {
audit.run();
});
</script>
</body>
</html>
third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.h
View file @ e9624faf
......@@ -66,6 +66,11 @@ class AudioDestinationHandler : public AudioHandler, public AudioIOCallback {
virtual void StartRendering() = 0;
virtual void StopRendering() = 0;
// The render thread needs to be changed after Worklet JS code is loaded by
// AudioWorklet. This method ensures the switching of render thread and the
// restart of the context.
virtual void RestartDestination() = 0;
// Returns the rendering callback buffer size.
virtual size_t CallbackBufferSize() const = 0;
virtual double SampleRate() const = 0;
......
third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
View file @ e9624faf
......@@ -1040,8 +1040,14 @@ void BaseAudioContext::SetWorkletMessagingProxy(
DCHECK(!worklet_messaging_proxy_);
worklet_messaging_proxy_ = proxy;
has_worklet_messaging_proxy_ = true;
// TODO(hongchan): If the context was already running, stop the destination
// and restart the context with the worklet thread.
// If the context is running or suspended, restart the destination to switch
// the render thread with the worklet thread. Note that restarting can happen
// right after the context construction.
// TODO(hongchan): consider removing redundant restart.
if (ContextState() != kClosed) {
destination()->GetAudioDestinationHandler().RestartDestination();
}
}
AudioWorkletMessagingProxy* BaseAudioContext::WorkletMessagingProxy() {
......
third_party/WebKit/Source/modules/webaudio/DefaultAudioDestinationNode.cpp
View file @ e9624faf
......@@ -117,6 +117,11 @@ void DefaultAudioDestinationHandler::StopRendering() {
}
}
void DefaultAudioDestinationHandler::RestartDestination() {
StopDestination();
StartDestination();
}
unsigned long DefaultAudioDestinationHandler::MaxChannelCount() const {
return AudioDestination::MaxChannelCount();
}
......
third_party/WebKit/Source/modules/webaudio/DefaultAudioDestinationNode.h
View file @ e9624faf
......@@ -53,6 +53,7 @@ class DefaultAudioDestinationHandler final : public AudioDestinationHandler {
// AudioDestinationHandler
void StartRendering() override;
void StopRendering() override;
void RestartDestination() override;
unsigned long MaxChannelCount() const override;
// Returns the rendering callback buffer size.
size_t CallbackBufferSize() const override;
......
third_party/WebKit/Source/modules/webaudio/OfflineAudioDestinationNode.cpp
View file @ e9624faf
......@@ -381,6 +381,19 @@ WebThread* OfflineAudioDestinationHandler::GetRenderingThread() {
return render_thread_.get();
}
void OfflineAudioDestinationHandler::RestartDestination() {
// If the worklet thread is not assigned yet, that means the context has
// started without a valid WorkletGlobalScope. Assign the worklet thread,
// and it will be picked up when the GetRenderingThread() is called next.
if (RuntimeEnabledFeatures::AudioWorkletEnabled() &&
Context()->HasWorkletMessagingProxy() &&
!worklet_backing_thread_) {
DCHECK(Context()->WorkletMessagingProxy()->GetWorkletBackingThread());
worklet_backing_thread_ =
Context()->WorkletMessagingProxy()->GetWorkletBackingThread();
}
};
// ----------------------------------------------------------------
OfflineAudioDestinationNode::OfflineAudioDestinationNode(
......
third_party/WebKit/Source/modules/webaudio/OfflineAudioDestinationNode.h
View file @ e9624faf
......@@ -64,6 +64,8 @@ class OfflineAudioDestinationHandler final : public AudioDestinationHandler {
void StopRendering() override;
unsigned long MaxChannelCount() const override;
void RestartDestination() override;
// Returns the rendering callback buffer size. This should never be
// called.
size_t CallbackBufferSize() const override;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment