search

WebAudio / web-audio-api

The Web Audio API v1.0, developed by the W3C Audio WG
https://webaudio.github.io/web-audio-api/
Other
1.04k stars 166 forks source link

Restore empty of pending processor construction data after successful… #2318

Closed karlt closed 3 years ago

karlt commented 3 years ago

… initialization of AudioWorkletProcessor#port

https://github.com/WebAudio/web-audio-api/pull/2306/files moved two "Empty the [=pending processor construction data=] slot" steps from the AudioWorkletProcessor constructor into one step in "The instantiation of AudioWorkletProcessor" algorithm.

However, we need to keep one of the steps in the AudioWorkletProcessor constructor in order to keep the behavior of allowing only one AudioWorkletProcessor with the construction data, as intended in https://github.com/WebAudio/web-audio-api/pull/2049#pullrequestreview-280367894 and tested in https://github.com/web-platform-tests/wpt/pull/21454

Followup for https://github.com/WebAudio/web-audio-api/issues/2119

:boom: Error: 500 Internal Server Error :boom:

PR Preview failed to build. (Last tried on Apr 13, 2021, 8:22 AM UTC).

More PR Preview relies on a number of web services to run. There seems to be an issue with the following one: :rotating_light: [HTML Diff Service](http://services.w3.org/htmldiff) - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request. :link: [Related URL](https://services.w3.org/htmldiff?doc1=https%3A%2F%2Fpr-preview.s3.amazonaws.com%2FWebAudio%2Fweb-audio-api%2Fpull%2F2318%2Fe29a1cd.html&doc2=https%3A%2F%2Fpr-preview.s3.amazonaws.com%2Fkarlt%2Fweb-audio-api%2Fpull%2F2318.html) ``` 500 Internal Server Error

Internal Server Error

The server encountered an internal error or misconfiguration and was unable to complete your request.

Please contact the server administrator at sysreq@w3.org to inform them of the time this error occurred, and the actions you performed just before this error.

More information about this error may be available in the server error log.

``` _If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please [file an issue](https://github.com/tobie/pr-preview/issues/new?title=Error%20not%20surfaced%20properly&body=See%20WebAudio/web-audio-api%232318.)._
karlt commented 3 years ago

Non-substantive as editorial in restoring previous text.

w3cbot commented 3 years ago

rtoy marked as non substantive for IPR from ash-nazg.

rtoy commented 3 years ago

Adding @hoch to review since he made the original change.

hoch commented 3 years ago

@karlt

Please help me understand. My understanding is that the current algorithm always performs 1.32.2.3 step 9 performed no matter what, even when the step 8 throws an exception in the constructor callback. (At least that was my intention)

allowing only one AudioWorkletProcessor with the construction data

Do you believe the current text allows multiple/concurrent constructor invocations?

karlt commented 3 years ago

My understanding is that the current algorithm always performs 1.32.2.3 step 9 performed no matter what, even when the step 8 throws an exception in the constructor callback. (At least that was my intention)

Yes, and that's all good and necessary, thanks, because the AudioWorkletProcessor constructor may or may not be invoked, but consider what may happen before step 9 is reached.

Do you believe the current text allows multiple/concurrent constructor invocations?

Yes, there is an example in the test where processorCtor invoked in step 8 of the "instantiation" algorithm attempts to construct two AudioWorkletProcessor objects before returning to the instantiation algorithm.

hoch commented 3 years ago

Yes, there is an example in the test where processorCtor invoked in step 8 of the "instantiation" algorithm attempts to construct two AudioWorkletProcessor objects before returning to the instantiation algorithm.

Ah. That makes sense. Can you share the link to the test? I would like to take a look.

That said, this PR looks good to me.

karlt commented 3 years ago

Can you share the link to the test?

https://github.com/web-platform-tests/wpt/blob/master/webaudio/the-audio-api/the-audioworklet-interface/processors/construction-port-new-after-new.js#L3-L6

Thanks for your work.

hoch commented 3 years ago

Yup, that test case confirms my guess. Thanks for catching this!