New issue
Advanced search Search tips

Issue 774566 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Page crashes when passing an invalid audio context to AudioWorkerNode

Reported by a...@skratchdot.com, Oct 13 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.0 Safari/537.36

Steps to reproduce the problem:
1. Visit https://plnkr.co/edit/BP3pRj4ZrtficRhJUTMb?p=preview
2. Click the "crash button"
3. Confirm the page crashes

What is the expected behavior?
If I pass an invalid context, I would expect an error message, not a page crash.  But it would also be nice if I could pass an audio context created in the root page scope.

What went wrong?
It seems like there might be a scoping issue with where you can create audio contexts for use with an audioWorklet.

In any case, if I'm using the api incorrectly, it would be better to throw an error than crash the page.

Did this work before? No 

Does this work in other browsers? N/A

Chrome version: 63.0.3239.0  Channel: canary
OS Version: OS X 10.13.0
Flash Version: 

Since this api was just released today (behind a flag), I would expect there to be some kinks.  I'm not entirely sure what's expected here, but wanted to log an issue in case it helps.
 
Labels: OS-Chrome OS-Linux OS-Windows
Owner: hongchan@chromium.org
Status: Assigned (was: Unconfirmed)
The minimum repro:
---
let context = new AudioContext();
  let osc = new OscillatorNode(context);
  osc.connect(context.destination);
  osc.start();
  console.log('creating worklet: before');
  window.audioWorklet.addModule('custom.js').then(() => {
    console.log('in callback');
    let custom = new AudioWorkletNode(context, 'Custom');
    custom.connect(context.destination);
    console.log(custom);
  }).catch((err) => {
    console.log(err);
  });
  console.log('creating worklet: after');
---

The problem here is AudioContext starts with the regular thread and the render thread needs to be changed when .addModule() called. The WebAudio engine needs to do dynamic thread switching and it's not there yet.

It's a known issue for the initial experimental release and will fix it soon.
Labels: ReleaseBlock-Stable M-63
manoranjanr@ This isn't RB for M63. The feature is behind the experimental flag.
Labels: -ReleaseBlock-Stable
Ok, i'm taking it off.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 17 2017

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

commit e9624faf40045c7bae27acca932936849a7f336d
Author: Hongchan Choi <hongchan@chromium.org>
Date: Tue Oct 17 23:06:17 2017

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/722126
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509580}
[rename] https://crrev.com/e9624faf40045c7bae27acca932936849a7f336d/third_party/WebKit/LayoutTests/http/tests/webaudio/audio-worklet/audio-worklet-node-construction.html
[add] https://crrev.com/e9624faf40045c7bae27acca932936849a7f336d/third_party/WebKit/LayoutTests/http/tests/webaudio/audio-worklet/worklet-context-interaction.html
[modify] https://crrev.com/e9624faf40045c7bae27acca932936849a7f336d/third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.h
[modify] https://crrev.com/e9624faf40045c7bae27acca932936849a7f336d/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
[modify] https://crrev.com/e9624faf40045c7bae27acca932936849a7f336d/third_party/WebKit/Source/modules/webaudio/DefaultAudioDestinationNode.cpp
[modify] https://crrev.com/e9624faf40045c7bae27acca932936849a7f336d/third_party/WebKit/Source/modules/webaudio/DefaultAudioDestinationNode.h
[modify] https://crrev.com/e9624faf40045c7bae27acca932936849a7f336d/third_party/WebKit/Source/modules/webaudio/OfflineAudioDestinationNode.cpp
[modify] https://crrev.com/e9624faf40045c7bae27acca932936849a7f336d/third_party/WebKit/Source/modules/webaudio/OfflineAudioDestinationNode.h

Status: Verified (was: Assigned)
The patch has been landed and the repro case does not crash anymore.

Sign in to add a comment