Revise AWN/AWP construction process #2022
Conversation
|
|
||
| 1. Make <dfn><var>nodeReference</var></dfn>, | ||
| <dfn><var>deserializedPort</var></dfn>, and | ||
| <var>deserializedOptions</var> available |
There was a problem hiding this comment.
This is a new concept of "making things available" for the AWP constructor.
There was a problem hiding this comment.
OK, but what does this concept mean? How do we expect a UA to actually implement this?
As discussed earlier, fixing this in a separate PR is fine if that helps.
There was a problem hiding this comment.
Sure. I would like to merge the progress in this PR, and follow-up on this part later.
index.bs
Outdated
| <var>deserializedOptions</var> available | ||
| for the execution of <var>processorCtor</var>. | ||
|
|
||
| 1. Invoke the <var>processorCtor</var> with the argument of |
There was a problem hiding this comment.
What does "Invoke" mean here? Does this mean https://heycam.github.io/webidl/#invoke-a-callback-function or https://heycam.github.io/webidl/#construct-a-callback-function or something else?
Seems to me like this should https://heycam.github.io/webidl/#construct-a-callback-function at first glance, since processorCtor is an AudioWorkletProcessorConstructor, right?
There was a problem hiding this comment.
Perhaps it's better to say Perform Construct(processorCtor, deserializedOptions) like the step 9 of https://heycam.github.io/webidl/#construct-a-callback-function.
I am not sure deserializedOptions is a valid ES value. Probably it needs a conversion?
There was a problem hiding this comment.
Why would we prefer Construct over "construct a callback function", especially since what we have is in fact a callback function? I mean, we can do a manual Construct but then we will need to also manually duplicate a lot of the bookkeeping "construct a callback function" does for us, no?
deserializedOptions is the return value of a StructuredDeserialize call, right? That would be an ES value, as defined at https://html.spec.whatwg.org/multipage/structured-data.html#structureddeserialize
There was a problem hiding this comment.
I don't have any preference here. I am merely focusing two things: your suggestion and minimizing changes.
I mean, we can do a manual Construct but then we will need to also manually duplicate a lot of the bookkeeping "construct a callback function" does for us, no?
It sounds like you're constructing a callback function, but not executing the constructor itself. I was mistaken. If this is the correct way of performing the constructor (processorCtor), then I am all for it.
Also the algorithm has some interesting bits:
"Let relevant settings be realm’s settings object."
Is this "setting object" similar to "processor construction data"? Then this algorithm might be a solution for our situation.
deserializedOptions is the return value of a StructuredDeserialize call, right? That would be an ES value
Understood.
There was a problem hiding this comment.
If this is the correct way of performing the constructor
Yes, it is.
Is this "setting object" similar to "processor construction data"?
No, it's the thing defined at https://html.spec.whatwg.org/multipage/webappapis.html#settings-object
|
|
||
| 1. Make <dfn><var>nodeReference</var></dfn>, | ||
| <dfn><var>deserializedPort</var></dfn>, and | ||
| <var>deserializedOptions</var> available |
There was a problem hiding this comment.
OK, but what does this concept mean? How do we expect a UA to actually implement this?
As discussed earlier, fixing this in a separate PR is fine if that helps.
|
|
||
| 1. Let <var>deserializedPort</var> be the result of | ||
| [$StructuredDeserialize$](<var>serializedPort</var>, | ||
| the current Realm). |
There was a problem hiding this comment.
What current Realm? There is no current Realm here.
There was a problem hiding this comment.
Okay, then how do we deserialize the transferred data without a realm? I took the text for granted when I got some help from @domenic. Perhaps the realm should be given to the algorithm from the call site?
There was a problem hiding this comment.
Also this algorithm is performed in AudioWorkletGlobalScope so there is a defined "current realm", I think.
There was a problem hiding this comment.
You want the realm of the AudioWorkletGlobalScope, but it's not the current realm at that point. In order for it to be the current realm, you'd have to have a running execution context, which can only happen if something pushes it on the execution context stack, etc. None of that is happening here.
What you probably want to say is that deserialization is passed the realm of the AudioWorkletGlobalScope involved. It doesn't look like ES really defines getting the realm of a global; I guess you can say you use the realm whose [[GlobalObject]] is the AudioWorkletGlobalScope that corresponds to the BaseAudioContext of the AudioWorkletNode that we're in the middle of creating....
There was a problem hiding this comment.
I guess maybe https://html.spec.whatwg.org/multipage/webappapis.html#concept-global-object-realm offers a shortcut, so you can just talk about taking the realm of the AudioWorkletGlobalScope etc.
There was a problem hiding this comment.
Thanks for the pointer, but this is really out of scope of this PR. I file a new issue and work on a new PR for it.
|
Merging after an approval. I opened #2027 for some issues raised in this PR. |
This is an attempt to fix #2021.
Key changes:
cc @bzbarsky
Preview | Diff