Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NewTarget check for AudioWorkletProcessor isn't actually possible with a Web IDL constructor #1963

Closed
bzbarsky opened this issue Jul 5, 2019 · 6 comments
Assignees

Comments

@bzbarsky
Copy link

bzbarsky commented Jul 5, 2019

https://webaudio.github.io/web-audio-api/#AudioWorketProcessor-constructors step 1 says:

Compare the value of NewTarget with the active function object; if the two are equal, throw a TypeError.

but there is no NewTarget or "active function object" available in a Web IDL constructor's steps. Those steps are handed the already-created to-be-initialized object and IDL values for the arguments; see https://heycam.github.io/webidl/#create-an-interface-object step 9.

Presumably this is trying to do something like what [HTMLConstructor] does, but note that this defined a separate extended attribute, with different semantics from IDL's [Constructor].

Is there a specific reason to disallow directly constructing AudioWorkletProcessor instances?

@hoch hoch self-assigned this Jul 11, 2019
@hoch
Copy link
Member

hoch commented Jul 11, 2019

@bzbarsky
Copy link
Author

Yeah, so #1447 (comment) was just not correct. :(

@hoch
Copy link
Member

hoch commented Jul 12, 2019

Is there a specific reason to disallow directly constructing AudioWorkletProcessor instances?

Yes. AudioWorkletProcessor (render thread) is a counterpart of AudioWorkletNode (main thread). Without a node, a processor is meaningless because it can't be a part of the audio graph.

As we already have seen, the current text is based on #1447 (comment), and we'll appreciate the guidance to fix this. Perhaps things have changed over 2 years...

@bzbarsky
Copy link
Author

Without a node, a processor is meaningless because it can't be a part of the audio graph.

OK, but is that a reason to prevent it being constructed, at the cost of specification and implementation complexity? If someone just constructs a processor, they can't do anything useful with it. Fine. That means no one will do that in practice...

But also, this approach doesn't actually solve that problem as described. If we take that current spec at face value, and code in the worklet does:

    class Foo extends AudioWorkletProcessor {};
    let foo = new Foo;

that is allowed by the current spec (because now NewTarget is Foo so https://webaudio.github.io/web-audio-api/#dom-audioworkletprocessor-audioworkletprocessor step 1 does not throw) and results in an AudioWorkletProcessor instance that is not hooked up to an AudioWorkletNode.

the current text is based on #1447 (comment)

That comment is wrong in terms of what things [Constructor] lets you specify and in terms of what [HTMLConstructor] actually is. It was wrong at the time too...

we'll appreciate the guidance to fix this

Here are some options:

  1. Just allow AudioWorkletProcessor to be (uselessly) constructible. This is the simplest option in spec and implementation terms at first glance, though there might be some complications around the port stuff; see below.
  2. Add a new extended attribute like [HTMLConstructor] that has the different semantics you want. That said, just doing a NewTarget check does not seem to give the semantics you want, per above.
  3. Change the Web IDL spec to pass more information to IDL constructors (e.g. whether NewTarget was the constructor function itself). Again, this doesn't really let you solve the problem you describe, just like the current spec.
  4. Solve this as part of fixing VUMeter example uses AudioWorkletProcessor's port in a way that can't work per spec #1973 by throwing from the AudioWorkletProcessor constructor if whatever magic out-of-band thing is being used to initialize the port is not available. This will make my new Foo example above throw, as well as new AudioWorkletProcessor. At least in general. What happens if Foo is registered as an actual processor and then the UA calls its constructor and that constructor does new Foo before calling super() is an interesting question that we're going to need to sort out for VUMeter example uses AudioWorkletProcessor's port in a way that can't work per spec #1973 anyway.

If we do any of options 1-3 we still have to figure out what happens with the port stuff in those cases, exactly, so we might end up at option 4 by default.

@hoch
Copy link
Member

hoch commented Jul 12, 2019

Thanks for your input. This is really helpful. I'll try to come up with a plan with the option 4.

@hoch
Copy link
Member

hoch commented Aug 8, 2019

@hoch hoch closed this as completed Aug 8, 2019
hoch referenced this issue in web-platform-tests/wpt Apr 28, 2020
…construction

Differential Revision: https://phabricator.services.mozilla.com/D55228

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1599952
gecko-commit: e56cf2dc1636e0dbb01632f9039ab0a7ad143dc1
gecko-integration-branch: autoland
gecko-reviewers: bzbarsky
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants