New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 892067 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression

Blocking:
issue 857476



Sign in to add a comment

AudioWorkletNode port.postMessage not sending message with SharedArrayBuffer as argument

Reported by pszewczy...@gmail.com, Oct 4

Issue description

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

Steps to reproduce the problem:
1. Download attached zipped and extract it
2. Enter extracted folder content and start any http local server (i.e in terminal type npx http-server)
3. Open Chrome Beta Version 70.0.3538.35 using command line:
/Applications/Google\ Chrome\ Beta.app/Contents/MacOS/Google\ Chrome --js-flags=--experimental-wasm-threads --enable-features=WebAssembly,SharedArrayBuffer --disable-features=WebAssemblyTrapHandler
4. Go to localhost:8080 (default for http-server)
5. Open developer tools, refresh browser and observe that port.onmessage in worklet.js in never called

What is the expected behavior?
port.onmessage for AudioWorkletProcessor instance is invoked when AudioWorkletNode instance port.postMessage is called with SharedArrayBuffer as argument

What went wrong?
port.onmessage for AudioWorkletProcessor instance is not invoked when AudioWorkletNode instance port.postMessage is called with SharedArrayBuffer as argument; it works fine for other types of data like Object or ArrayBuffers

Did this work before? Yes 69.0.3497.100

Does this work in other browsers? N/A

Chrome version: 70.0.3538.35  Channel: beta
OS Version: OS X 10.13.6
Flash Version:
 
workletnode_problem.zip
1.9 KB Download
Cc: dschuff@chromium.org binji@chromium.org hablich@chromium.org
Labels: -Pri-2 Needs-Bisect Pri-1
Labels: Needs-Triage-M70
Owner: binji@chromium.org
Status: Assigned (was: Unconfirmed)
First off, thanks for the great repro zip, with included steps! It was very helpful.

I took a look, and I see the behavior you describe, even with ToT Chrome.

If you add the following code to the worklet constructor, you see that the message was sent, but produced an error:

```
    this.port.onmessageerror = (e) => {
      console.log('error', e)
    }
```

This is occurring because the spec currently doesn't allow transferring a SharedArrayBuffer to a worklet. See https://html.spec.whatwg.org/multipage/webappapis.html#integration-with-the-javascript-agent-cluster-formalism

This bounces through a number of GH issues, but eventually ends up here: https://github.com/w3c/css-houdini-drafts/issues/380

Where passing SAB to AudioWorklets is mentioned explicitly, but there seems to be no resolution. I'll follow up on that thread.
Cc: ikilpatrick@chromium.org
Components: Blink>Workers
binji@

I think this is a regression. It's working in the current stable (69.0.3497) and I even wrote an article/example for this feature. But this doesn't work anymore in the current Canary (71.0.3570.0) without any visible error or warning.

Please try this example with Stable and Canary:
https://googlechromelabs.github.io/web-audio-samples/audio-worklet/design-pattern/shared-buffer/

I was not aware of the change in agent cluster in the spec - but now sure why Worklet's owner cannot be undefined. ikilpatrick@ should know more about this.

* FYI the article: https://developers.google.com/web/updates/2018/06/audio-worklet-design-pattern#webaudio_powerhouse_audio_worklet_and_sharedarraybuffer
binji@

What's the change that excluded the Worklet from the agent cluster? Can we defer that change until we have a clear resolution on the spec?
hongchan@:

It was added in https://chromium-review.googlesource.com/1130505. The relevant code is at https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/workers/worklet_global_scope.h?sq=package:chromium&g=0&l=56, where it gets the AgentClusterId.

I see what you mean that it is a regression; though the previous behavior was actually not spec compliant, as we allowed SABs to be transferred where they should not be allowed.

I think to fix this we'd want to do something similar to Worker, where we pass the AgentClusterId down from the document that created it. Not sure where that happens, however.
Thanks for the link. I agree that it's bad for being non-compliant but I am concerned that there are some projects already rely on this feature. I hope we can reach the resolution before it gets out to the stable.
Cc: adamk@chromium.org
I have a WIP CL here https://chromium-review.googlesource.com/c/chromium/src/+/1262932, currently missing tests. I'm not sure this is the correct behavior, however.
Cc: swarnasree.mukkala@chromium.org
Labels: Needs-Feedback Triaged-ET
Tried testing the issue on reported chrome version #70.0.3538.35 using Mac OS 10.13.6 by following below steps.

Steps:
=====
1.Launched chrome by" --js-flags=--experimental-wasm-threads --enable-features=WebAssembly,SharedArrayBuffer --disable-features=WebAssemblyTrapHandler".
2.Ran a local host and opened the file.
3.Opened dev tools and refreshed the page, observed blank page
Note: Tested same on chrome version #69.0.3497.100 but unable to see any difference between both.

Attached screencast for reference.
@reporter: Could you please  verify above screencast and let us know if anything is being missed here. Request you to provide a screencast/screenshot of the issue so that it would be really helpful for better triaging of issue.
Thanks.!
892067.mp4
1.0 MB View Download
Re #11:

This issue is confirmed, and I believe it is kind of a regression but more of "external dependency" due to the spec change that needs to be made. I will follow up on the spec issue and report back here.
Labels: -Needs-Bisect
Is there a temporary workaround one can employ until the spec is hashed out?
Re #13

There is no known workaround for this issue in our audio processing solution. We're using SAB for data control flow and as audio data feed for AudioWorklet.
Blocking: 857476
Cc: domenic@chromium.org
+domenic@

We have a plan to change the spec so the worklet agent can be supported officially. I believe it was a simple disconnection between two spec documents.

https://github.com/w3c/css-houdini-drafts/issues/380#issuecomment-427443234
Cc: nattestad@chromium.org
Labels: -Needs-Feedback -Needs-Triage-M70
I am currently trying to decide if we should actually block the launch of Chrome 70 to stable because of this or release the fix on a follow-up Chrome 70 stable push.

hongchan@ et. al. who else is using such a setup and might be affected by the behavior change?
Labels: OS-Chrome OS-Linux OS-Windows
A couple of audio developers is using this scheme, since I published the article a while ago (2018 June).

I don't think it is widely used because of the complexity of the design, but it would be great if we don't break the known behavior just because the spec was not fully updated.
We have an approval for the spec change:
https://github.com/whatwg/html/pull/4069#pullrequestreview-163099529

The CL + tests can be landed now.
Labels: Merge-Review-70 ReleaseBlock-Stable M-70
Great, please merge to 70 and 71 (I think this might miss branch cut tomorrow)
Labels: Merge-Review-71
Project Member

Comment 22 by bugdroid1@chromium.org, Oct 11

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

commit 97167bddb398b0adb7e06f6f5c68e9f5d556ad96
Author: Ben Smith <binji@chromium.org>
Date: Thu Oct 11 01:24:49 2018

Allow posting a SharedArrayBuffer to AudioWorklet

The HTML spec only allows passing SharedArrayBuffers to an agent in the
same agent cluster. Worklets currently do not belong to any agent
cluster, so a SharedArrayBuffer can not be shared with a worklet.

However, it is intended that this is possible; see, for example,
https://github.com/w3c/css-houdini-drafts/issues/380 and the
AudioWorklet article here:
https://developers.google.com/web/updates/2018/06/audio-worklet-design-pattern#webaudio_powerhouse_audio_worklet_and_sharedarraybuffer

This change funnels the agent cluster ID through when creating a
ThreadedWorklet, so a SharedArrayBuffer can be shared as long as the
creator of the Worklet's thread is in the same agent cluster. It is not
clear that this is the behavior that will be specified, however.

Bug:  chromium:892067 
Change-Id: If1a2187ae38da41f2389538c07e7b04921c6128f
Reviewed-on: https://chromium-review.googlesource.com/c/1262932
Commit-Queue: Ben Smith <binji@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598619}
[add] https://crrev.com/97167bddb398b0adb7e06f6f5c68e9f5d556ad96/third_party/WebKit/LayoutTests/external/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-postmessage-sharedarraybuffer.https.html
[add] https://crrev.com/97167bddb398b0adb7e06f6f5c68e9f5d556ad96/third_party/WebKit/LayoutTests/external/wpt/webaudio/the-audio-api/the-audioworklet-interface/processors/sharedarraybuffer-processor.js
[modify] https://crrev.com/97167bddb398b0adb7e06f6f5c68e9f5d556ad96/third_party/blink/renderer/core/workers/threaded_worklet_messaging_proxy.cc
[modify] https://crrev.com/97167bddb398b0adb7e06f6f5c68e9f5d556ad96/third_party/blink/renderer/core/workers/worklet_global_scope.cc
[modify] https://crrev.com/97167bddb398b0adb7e06f6f5c68e9f5d556ad96/third_party/blink/renderer/core/workers/worklet_global_scope.h

Labels: -Merge-Review-70 -Merge-Review-71 Merge-Approved-71 Merge-Approved-70
Please merge to 70 and 71 after this has enough canary coverage.
Cc: abdulsyed@chromium.org
Looks like this made the M71 branch cut. I'll merge back to M70 the beginning of next week.
Pls merge your change to M71 branch #3578 ASAP so we can pick it up for next M71 dev release. Thank you.
FYI M70 stable is tomorrow. This has to be in M70 by today in order to make it to stable.
Project Member

Comment 28 by bugdroid1@chromium.org, Oct 15

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db54921dea346d5d2676be91eba718a311916db1

commit db54921dea346d5d2676be91eba718a311916db1
Author: Ben Smith <binji@chromium.org>
Date: Mon Oct 15 15:22:07 2018

Allow posting a SharedArrayBuffer to AudioWorklet

The HTML spec only allows passing SharedArrayBuffers to an agent in the
same agent cluster. Worklets currently do not belong to any agent
cluster, so a SharedArrayBuffer can not be shared with a worklet.

However, it is intended that this is possible; see, for example,
https://github.com/w3c/css-houdini-drafts/issues/380 and the
AudioWorklet article here:
https://developers.google.com/web/updates/2018/06/audio-worklet-design-pattern#webaudio_powerhouse_audio_worklet_and_sharedarraybuffer

This change funnels the agent cluster ID through when creating a
ThreadedWorklet, so a SharedArrayBuffer can be shared as long as the
creator of the Worklet's thread is in the same agent cluster. It is not
clear that this is the behavior that will be specified, however.

Bug:  chromium:892067 
Change-Id: If1a2187ae38da41f2389538c07e7b04921c6128f
Reviewed-on: https://chromium-review.googlesource.com/c/1262932
Commit-Queue: Ben Smith <binji@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#598619}(cherry picked from commit 97167bddb398b0adb7e06f6f5c68e9f5d556ad96)
Reviewed-on: https://chromium-review.googlesource.com/c/1280843
Reviewed-by: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#998}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[add] https://crrev.com/db54921dea346d5d2676be91eba718a311916db1/third_party/WebKit/LayoutTests/external/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-postmessage-sharedarraybuffer.https.html
[add] https://crrev.com/db54921dea346d5d2676be91eba718a311916db1/third_party/WebKit/LayoutTests/external/wpt/webaudio/the-audio-api/the-audioworklet-interface/processors/sharedarraybuffer-processor.js
[modify] https://crrev.com/db54921dea346d5d2676be91eba718a311916db1/third_party/blink/renderer/core/workers/threaded_worklet_messaging_proxy.cc
[modify] https://crrev.com/db54921dea346d5d2676be91eba718a311916db1/third_party/blink/renderer/core/workers/worklet_global_scope.cc
[modify] https://crrev.com/db54921dea346d5d2676be91eba718a311916db1/third_party/blink/renderer/core/workers/worklet_global_scope.h

Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision db54921dea346d5d2676be91eba718a311916db1 was merged to refs/branch-heads/3538 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/db54921dea346d5d2676be91eba718a311916db1

Commit: db54921dea346d5d2676be91eba718a311916db1
Author: binji@chromium.org
Commiter: binji@chromium.org
Date: 2018-10-15 15:22:07 +0000 UTC

Allow posting a SharedArrayBuffer to AudioWorklet

The HTML spec only allows passing SharedArrayBuffers to an agent in the
same agent cluster. Worklets currently do not belong to any agent
cluster, so a SharedArrayBuffer can not be shared with a worklet.

However, it is intended that this is possible; see, for example,
https://github.com/w3c/css-houdini-drafts/issues/380 and the
AudioWorklet article here:
https://developers.google.com/web/updates/2018/06/audio-worklet-design-pattern#webaudio_powerhouse_audio_worklet_and_sharedarraybuffer

This change funnels the agent cluster ID through when creating a
ThreadedWorklet, so a SharedArrayBuffer can be shared as long as the
creator of the Worklet's thread is in the same agent cluster. It is not
clear that this is the behavior that will be specified, however.

Bug:  chromium:892067 
Change-Id: If1a2187ae38da41f2389538c07e7b04921c6128f
Reviewed-on: https://chromium-review.googlesource.com/c/1262932
Commit-Queue: Ben Smith <binji@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#598619}(cherry picked from commit 97167bddb398b0adb7e06f6f5c68e9f5d556ad96)
Reviewed-on: https://chromium-review.googlesource.com/c/1280843
Reviewed-by: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#998}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
Project Member

Comment 31 by sheriffbot@chromium.org, Oct 15

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-71
Status: Fixed (was: Assigned)

Sign in to add a comment