[worklets] Review

[domenic]

This is an overdue review of the latest changes, e.g. #375 and #381. I decided to just do a read-through of the algorithm portions of the spec, instead of those particularly. I've bolded the significant issues; most are minor.

• "Do not obtain any source texts for scripts or modules." is no longer necessary
• "set up a worklet environment settings object" seems to inherit way too much:
  • The API base URL is inherited from the HTML page? Meaning that URL resolution inside a worklet script created via CSS.paintWorklet.loadModule("foo/bar.js") should be relative to index.html, not relative to foo/bar.js?
  • The referrer policy ignores any HTTP headers for the worklet scripts, but instead just uses whatever the HTML page does?
  • The HTTPS state is copied from the document? So if I import a worklet over an insecure transport, it still counts as secure, as long as the hosting page uses HTTPS?
• Technically the id and creation URL for environment settings objects are fields, not algorithms, and so listing them separately might be better.
• Typo "consiting"
• In general we don't italicize "null"
• loadModule():
  • "the current Worklet" -> "this Worklet"
  • "SyntaxError" DOMException and other similar spec text should be using "{{SyntaxError}}" {{DOMException}} markup so it gets code-ified
  • Step 7 which establishes outsideSettings could move earlier, which would make step 3 which does URL parsing shorter
  • "Run a module script" will try to report the exception, but WorkletGlobalScope doesn't have an onerror event handler. Probably it should.
  • Great task-queuing discipline.

[bfgeek]

cc/ @annevk - So a lot of the environment settings object is complex by the fact there isn't one clear "owner" script.

The API base URL is inherited from the HTML page? Meaning that URL resolution inside a worklet script created via CSS.paintWorklet.loadModule("foo/bar.js") should be relative to index.html, not relative to foo/bar.js?

So I think needs to more or less be this way? I.e.

CSS.paintWorklet.addModule('foo/module1.js');
CSS.paintWorklet.addModule('module2.js');

We aren't able to have API base URL switch depending on the currently running script right?

The referrer policy ignores any HTTP headers for the worklet scripts, but instead just uses whatever the HTML page does?
The HTTPS state is copied from the document? So if I import a worklet over an insecure transport, it still counts as secure, as long as the hosting page uses HTTPS?

I'm not sure what is correct for these two... @mkwst might have some thoughts.

[domenic]

We aren't able to have API base URL switch depending on the currently running script right?

Well, it's an algorithm, not a static value, so it certainly could. But maybe the right way to think about this is more like <script src="..."> which will still use the document's base URL, not the script's. I think I was still in a worker mindset, where script URL is always the appropriate base URL. So this part is probably OK.

[bfgeek]

"Run a module script" will try to report the exception, but WorkletGlobalScope doesn't have an onerror event handler. Probably it should.

So part of this is to clamp down on communication channels for the paint api. (in short we don't want any "easy" communication channels out). It's a little weird but not sure what the purpose would be for webaudio either?

[domenic]

I guess yeah, if you have no ability to e.g. send the error somewhere with fetch(), then having a self.onerror handler isn't super-useful. It still feels wrong to just drop the error into developer tools and never let web devs see it... but I'm not sure what the alternative is.

In this case probably what we need is for HTML's "report an exception" to stop assuming the global is always an EventTarget. In the meantime you should probably add a monkeypatch note to worklets saying something like

When asked to [report an exception], do nothing instead, as HTML's [report an exception] needs updating to work with non-EventTarget global objects. That is tracked in whatwg/html#XYZ.

[annevk]

It's a little weird but not sure what the purpose would be for webaudio either?

Remote debugging. We probably do want to enable that I think, especially as things get closer to the metal.