Conversation
annevk
left a comment
annevk
left a comment
There was a problem hiding this comment.
Ooh, exciting.
Should we annotate these IDL blocks with SecureContext?
| <var>reservedEnvironment</var>'s <span | ||
| data-x="concept-environment-target-browsing-context">target browsing context</span>, and <span | ||
| data-x="concept-environment-active-service-worker">active service worker</span> to | ||
| <var>reservedEnvironment</var>'s <span |
There was a problem hiding this comment.
I'm not sure I find this as clear as the previous wording.
There was a problem hiding this comment.
Hmm. I preferred the way the worklets spec does it, which involves less repetition, so I normalized both to this style.
| <ul> | ||
| <li><p>Are thread-agnostic. That is, they are not designed to run on a dedicated separate thread, | ||
| like each worker is. Implementations can run worklets wherever they choose (including on the main | ||
| thread).</p></li> |
There was a problem hiding this comment.
I'm wondering if this holds for audio worklets with shared memory, but I guess it does in theory, but not really in practice.
There was a problem hiding this comment.
I think they're thread-agnostic, just not process-agnostic.
There was a problem hiding this comment.
It's just that you wouldn't have a good time if you ran the audio worklet on the main thread.
|
Note to self: I'm also hoping to land #4352 soon, so this will need some minor updates there, by incorporating @Ms2ger's work in w3c/css-houdini-drafts#984 |
annevk
left a comment
annevk
left a comment
There was a problem hiding this comment.
I found a few potential issues and a couple nits, but largely looks good to me.
@sideshowbarker are there any special considerations with adding an additional file to the multipage version?
| <ul> | ||
| <li><p>Are thread-agnostic. That is, they are not designed to run on a dedicated separate thread, | ||
| like each worker is. Implementations can run worklets wherever they choose (including on the main | ||
| thread).</p></li> |
There was a problem hiding this comment.
It's just that you wouldn't have a good time if you ran the audio worklet on the main thread.
|
|
||
| <dt>The <span data-x="concept-settings-object-cross-origin-isolated-capability">cross-origin | ||
| isolated capability</span></dt> | ||
| <dd><p>Return <span class="XXX">TODO</span>.</p></dd> |
There was a problem hiding this comment.
This we can inherit, no? I guess there's again the question as to whether we require COEP to be specified because of imports.
There was a problem hiding this comment.
Yeah, I prefer leaving this as a TODO for now, ideally getting some tests.
| <li> | ||
| <p><span>Fetch a worklet script graph</span> given <var>moduleURL</var>, | ||
| <var>insideSettings</var>, <var>worklet</var>'s <span>worklet destination type</span>, <span | ||
| class="XXX">what credentials mode?</span>, <var>insideSettings</var>, and <var>worklet</var>'s |
There was a problem hiding this comment.
It's either same-origin or include, right? Any reason not to go with same-origin? Or do we want to write tests before resolving this?
There was a problem hiding this comment.
I suspect it's whatever was originally used with addModule(). Fixing this seems to require some plumbing to store the credential alongside the URL. (Which leads to complicated questions about what if you addModule() the same module twice with two different credentials...) Maybe related: w3c/css-houdini-drafts#264
padenot
left a comment
padenot
left a comment
There was a problem hiding this comment.
A couple comments from an Audio WG point of view, let me know if I can be clearer, audio programming has very strict constraints that are a bit unusual.
| <code>WorkletGlobalScope</code> is expected to be shared between multiple separate scripts. (This | ||
| is similar to how a single <code>Window</code> is shared between multiple separate scripts.)</p> | ||
|
|
||
| <h4 id="worklets-idempotent">Code idempotence</h4> |
There was a problem hiding this comment.
This section doesn't reflect the reality. The AudioWorklet requires the opposite: a single scope, statefullness, memory sharing via accessing a global, etc.
This is the case for rather fundamental reasons: audio programming is very often stateful (to output an audio sample at time t, we need to know the value of the sample at t-1, and state that was computed prior to compute t), and of course performance: sharing large audio buffers.
There was a problem hiding this comment.
Interesting. This is ported straight from the worklets spec, but I guess we shouldn't add things that are known-wrong in the porting process.
I guess the simplest way to fix this would be to say "some worklets" instead of "worklets"... I'll try something like that.
There was a problem hiding this comment.
I guess this section already says "Some specifications which use worklets", not all. So I think this section already reflects reality. Do you disagree?
There was a problem hiding this comment.
It's probably worth calling out that worklets serve both use cases. Both stateless and stateful, and short and long-running operations.
There was a problem hiding this comment.
I added such a paragraph; PTAL.
| data-x="">process</code>" property and convert it into a <code | ||
| data-x="idl-Function">Function</code> at registration time, as part of the <code | ||
| data-x="dom-FakeWorkletGlobalScope-registerFake">registerFake()</code> method steps.</p> | ||
|
|
There was a problem hiding this comment.
The Audio WG is exploring this. Classes are nice, but are problematic for WASM integration.
There was a problem hiding this comment.
To be clear, the idea here is to use classes, but instead of using the style in this toy example which Get()s and converts the method every time, use the architecture that audio worklet and paint worklet use, where you Get() and convert the method at registration time.
|
I just realized that w3c/css-houdini-drafts#264 is a pretty important bug fix (it fixes the problem described in w3c/css-houdini-drafts#264 (comment)) so I think I should pull that in before we land this. |
None that come to mind. We’re not maintaining a static list of output filenames anywhere — not in any code nor in any support files — so there’s nothing to update as far as that goes. Everything’s just keyed off the |
sideshowbarker
changed the title
annevk
left a comment
annevk
left a comment
There was a problem hiding this comment.
I'm happy with this. Lots of details to figure out still, but that's no different from the status quo and this does improve on that, e.g., by having worklets limited to secure contexts.
sideshowbarker
added
the
impacts documentation
|
cc @whatwg/documentation |
4 participants
Closes w3c/css-houdini-drafts#1000.
This provides a baseline by porting over all the existing text from
https://drafts.css-houdini.org/worklets/#lifetime-of-the-worklet,
modernizing and restructuring it along the way. It does not yet fix many
of the open logged issues (although it does fix some; see below).
Notable changes from that document:
Rearranged sections to better match workers, and my sense of flow.
Moved worklet script fetching to be siblings with all the other script
fetching algorithms.
Improved clarity and guidance on what specifications that define
worklets should do, including fleshing out the fake worklet example.
Changed "create a WorkletGlobalScope", which took one set of
arguments, to "create a worklet global scope", which just takes a
Worklet instance. This appears to match better how the algorithm is
used, e.g. in
https://drafts.css-houdini.org/css-paint-api/#draw-a-paint-image step
10.
Updated "report an error" to bail out for non-EventTarget globals,
like WorkletGlobalScope. Closes Update [report an exception] to work with globals which aren't EventTargets. #2611.
Updated worklets to only be exposed in secure contexts. Closes
[worklets] restrict worklets to SecureContexts w3c/css-houdini-drafts#505.
Makes the lifetime of creating and terminating WorkletGlobalScopes
more explicit. Closes
Why is there a responsible browsing context, but no document? w3c/css-houdini-drafts#224. Closes
[worklets] Introduce an "owner document" for each WorkletGlobalScope. w3c/css-houdini-drafts#389.
Explicitly start and stop the event loop for a given
WorkletGlobalScope upon creation/termination. Closes
Worklets does not integrate with the event loop w3c/css-houdini-drafts#843.
Closes Worklet event loop is not run? w3c/css-houdini-drafts#318 for real.
Fixes creation of new worklet global scopes to only run the top-level
module scripts added via addModule(), which will automatically run
their dependencies. Previously it would run all module scripts loaded
into the worklet, so dependencies would be run in the order they were
fetched, not as part of the top-down module evaluation process. Closes
[worklets] Fix module loading when creating a new worklet global scope. w3c/css-houdini-drafts#264.
This PR has three TODOs in the source:
The credentials mode is missing from one call site of "fetch a worklet script graph". This is a preexisting problem, and probably we can survive with this and merge it. (I made it an XXX.) I think doing this properly might involve converting the module responses map from url-keyed to (url, credentials mode) keyed? I'll want to research what implementations do...
The "cross-origin isolated capability" is a TODO. Also a preexisting problem; can also probably be merged and fixed later.
The sentence "TODO Whenever a Document object is discarded, each WorkletGlobalScope whose owner document is that Document object, should clear its owner document." is not carried over from the Worklets spec. This is a bit trickier:Attempted to fix this with the latest commits; PTALThe only use of "owner document" in the HTML spec is to figure out if a WorkletGlobalScope is a secure context. There's probably an easier way to do this, e.g. copying over secure-ness at creation time, which would allow us to eliminate the "owner document" field entirely?
But, I think that "owner document" was added to try to address Why is there a responsible browsing context, but no document? w3c/css-houdini-drafts#224. Hmm, I guess I should maybe add the contents of https://github.com/w3c/css-houdini-drafts/pull/389/files into this PR? @annevk, what do you think?
After we get this merged, we can tackle some of the easier open issues, ideally with tests. For example, the minor TODOs above, or w3c/css-houdini-drafts#506, or w3c/css-houdini-drafts#985 .
Probably also w3c/css-houdini-drafts#505 since at least Chrome appears to be restricting worklets to secure contexts.Fixed this in this PRBefore merging this, I need to canvas the worklet-using specs and make sure they'll keep working, and update their links as appropriate (e.g. changing
create a WorkletGlobalScopetocreate a worklet global scope). That may reveal more<dfn>s to export./acknowledgements.html ( diff )
/iana.html ( diff )
/index.html ( diff )
/indices.html ( diff )
/infrastructure.html ( diff )
/named-characters.html ( diff )
/obsolete.html ( diff )
/parsing.html ( diff )
/references.html ( diff )
/rendering.html ( diff )
/syntax.html ( diff )
/webappapis.html ( diff )
/webstorage.html ( diff )
/window-object.html ( diff )
/workers.html ( diff )
/xhtml.html ( diff )
/worklets.html ( diff )