New issue
Advanced search Search tips

Issue 823431 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 469639



Sign in to add a comment

Prototype chain in AudioWorkletNode is not correct

Project Member Reported by hongchan@chromium.org, Mar 19 2018

Issue description

const context = new AudioContext();
  context.audioWorklet.addModule('js/processor.js').then(() => {       
    let node = new AudioWorkletNode(context, 'processor');

    console.log('port' in node); // expect: true
    console.log('parameters' in node); // expect: true
    console.log('port' in AudioWorkletNode.prototype); // expect: false
    console.log('parameters' in AudioWorkletNode.prototype); // expect: false
    console.log(node.hasOwnProperty('port')); // expect: false
    console.log(node.hasOwnProperty('parameters')); // expect: false
});

And the actual result is:
false
false
true
true
true
true
 

Comment 1 by jsb...@chromium.org, Mar 19 2018

    console.log('port' in AudioWorkletNode.prototype); // expect: false
    console.log('parameters' in AudioWorkletNode.prototype); // expect: false

I would expect true for these.

From the results above, it seems like non-enumerable own-properties are being assigned to instances?

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 19 2018

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

commit 2ea3396c11d748579c305232a5306c0ad498e616
Author: Joshua Bell <jsbell@chromium.org>
Date: Mon Mar 19 21:49:59 2018

Web Platform Tests: add /interfaces/webaudio.idl and corresponding test

The Web Audio tests (in /webaudio) have idlharness test on an
interface-by-interface basis, but the new hotness is to have the
IDL fragments consolidated. This also allows dependent specs to
include the master interface definitions.

Bug: 679813, 697123 ,785409, 823431 

Change-Id: I60bc8415627b8b3f43c13060e4bf65f47bc17c2b
Reviewed-on: https://chromium-review.googlesource.com/965401
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544173}
[add] https://crrev.com/2ea3396c11d748579c305232a5306c0ad498e616/third_party/WebKit/LayoutTests/external/wpt/interfaces/webaudio.idl
[add] https://crrev.com/2ea3396c11d748579c305232a5306c0ad498e616/third_party/WebKit/LayoutTests/external/wpt/webaudio/idlharness.https-expected.txt
[add] https://crrev.com/2ea3396c11d748579c305232a5306c0ad498e616/third_party/WebKit/LayoutTests/external/wpt/webaudio/idlharness.https.html

Comment 3 by jsb...@chromium.org, Mar 19 2018

Oh!

Blink's AudioWorkletNode has [Global] on it, so it's being treated as a Global (like Window, WorkerGlobalScope, etc) which get special bindings treatment (i.e. own properties, not prototype properties)

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/webaudio/AudioWorkletNode.idl?q=audioworkletnode.idl&sq=package:chromium&dr&l=11

That shouldn't be there. Only AudioWorkletGlobalScope should have [Global].

Looks like AudioWorkletProcessor has that too.
Oh!

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/csspaint/PaintWorkletGlobalScope.idl?q=PaintWorkletGlobalScope&dr=C

I basically followed the pattern here, but PaintWorklet does not have entities like AudioWorkletNode or AudioWorkletProcessor. Also I did not understand the implication of Global() in the IDL.

A good catch! I will fix this ASAP.
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 27 2018

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

commit 579cd590b0948c1c793842cdfa83aefc84295638
Author: Joshua Bell <jsbell@chromium.org>
Date: Tue Mar 27 03:18:59 2018

WebAudio: Remove incorrect [Global] IDL attributes on some non-globals

The [Global] extended IDL attribute is used to mark up types like
Window and WorkerGlobalSope that get special treatment, like attribute
getters exposed as own-properties rather than on the prototype.

This was accidentally applied to a couple of Web Audio interfaces
thanks to the magic of copy/paste and obscure documentation, but was
revealed by new idlharness tests. Yay tests!

Bug:  823431 
Change-Id: I1e79a8942e2e6bb9072ac1cf7b5d39ead5f88047
Reviewed-on: https://chromium-review.googlesource.com/969798
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545951}
[modify] https://crrev.com/579cd590b0948c1c793842cdfa83aefc84295638/third_party/WebKit/LayoutTests/external/wpt/webaudio/idlharness.https-expected.txt
[modify] https://crrev.com/579cd590b0948c1c793842cdfa83aefc84295638/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/579cd590b0948c1c793842cdfa83aefc84295638/third_party/WebKit/LayoutTests/webaudio/audio-worklet/audio-worklet-node-idl-expected.txt
[modify] https://crrev.com/579cd590b0948c1c793842cdfa83aefc84295638/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/579cd590b0948c1c793842cdfa83aefc84295638/third_party/WebKit/Source/modules/webaudio/AudioWorkletNode.idl
[modify] https://crrev.com/579cd590b0948c1c793842cdfa83aefc84295638/third_party/WebKit/Source/modules/webaudio/AudioWorkletProcessor.idl

Comment 6 by jsb...@chromium.org, Mar 27 2018

Status: Fixed (was: Assigned)
Blocking: 469639
Thanks for fixing this, jsbell@! :)

Sign in to add a comment