[css-paint-api] Running author function without proper preparation.

[yuki3]

In short, invoke a paint callback does not follow Web IDL nor HTML specs. The CSS Painting API spec and its algorithms are defined using ECMAScript directly, and it violates Web IDL and HTML specs.

I cannot list up all the places that are conflicting with Web IDL and/or HTML, so let me show an example.

In invoke a paint callback, step 5.3. "Let paintInstance be the result of Construct(paintCtor)."; this step invokes an author function |paintCtor| without performing prepare to run script nor prepare to run a callback. This is NOT allowed in Web IDL nor HTML. |paintCtor| will run without the Incumbent realm having been set up, so we cannot tell what would happen if |paintCtor| invoked an Web API that uses the Incumbent realm.

I originally reported the same issue to Animation Worklet. It seems better to report this to CSS Paint API, too, so I'm now reporting here.

Ideally, each Web API spec should not be defined with ECMAScript. Web IDL provides all the necessary abstraction and it's easier with Web IDL to make each spec consistent.

[yuki3]

@annevk , you seem active in this spec, too. Could you triage this issue?

[annevk]

@yuki3 I've only been active as reviewer, but I didn't really review these aspects carefully.

@bfgeek should look at this and maybe @domenic can help.

What did Chrome end up shipping here?

[yuki3]

As Chromium has not yet supported the true Incumbent realm and Chromium is kind-of simulating the Incumbent realm with a hack, it's currently working fine. However, Blink bindings team is currently working to support the true Incumbent realm. Once the true Incumbent realm gets supported, Chromium will be in trouble with this issue.

Given that Firefox correctly implements the Incumbent realm, Firefox should have difficulty to implement CSS Paint API as is.

[yuki3]

Chromium is kind-of simulating the Incumbent realm with a hack, it's currently working fine

I meant that the current implementation/hack for the Incumbent realm in Chromium doesn't rely on "prepare to run script" nor "prepare to run a callback". But Blink bindings team is planning/working to implement it correctly as spec'ed (using "prepare to run script" and "prepare to run a callback").

[annevk]

Looking at the open bugs (e.g., https://bugzilla.mozilla.org/show_bug.cgi?id=1302328 and dependencies) it doesn't seem like we've started doing much yet. However, it seems this would also affect https://webaudio.github.io/web-audio-api/#audioworklet which we are working on somewhat as far as I know.

Having said that, given that a worklet only ever has one global, wouldn't the incumbent always be the same as the current global anyway?

cc @jetvillegas @padenot

[bfgeek]

I think we should be able to change:
Let paintInstance be the result of Construct(paintCtor).
to:
Let paintInstance be the result of constructing paintCtor, with no arguments.

I think I based this off the old web components spec initially, which also performed a direct construct (https://w3c.github.io/webcomponents/spec/custom/)

The only concrete difference I can see in our implementation is this will run microtasks at the end of the ctor, when it doesn't look like we currently are. (This will be minor enough for us to fix).

[yuki3]

I didn't know that Web IDL now supports Construct a callback function type value. That's great and matches this use case.

I have another request, somewhat orthogonal to this, but somewhat relevant to this.

In registerPaint algorithm:
The algorithm is preparing "paint function" as a callback function (but exactly speaking, the manner to create a callback function is not correct. This is another place to be fixed. But this is a side note).
The "paint function" is invoked as a callback function in invoke a paint callback with "paintInstance" as the callback this value.

This is pretty similar to "callback interface" (not callback function) in Web IDL. Why don't we define these algorithms using "callback interface"? I'd recommend a well-defined common manner rather than a unique way.

[yuki3]

(I'm sorry for my misoperation of closing this issue.)

[bfgeek]

callback interface has different semantics to what we are doing here.

Using a callback interface would mean each time we need to access the paint or animate function off the instance, instead what we do here is look up the paint method upon registration, and then invoke that each time.

This was done to be consistent with custom elements which uses the exact same semantics.
@domenic has some of the history here, but this pattern is consistent with the other "register a class" methods on the platform.

[bfgeek]

Also according to https://heycam.github.io/webidl/#es-callback-function a callback function and a JS Function represent the same thing. I can change the registerPaint algorithm to use the converting WebIDL, as this should have no side-effects.

[domenic]

callback function = JS function + callback context. Callback context is important for computing the incumbent realm, indeed. Probably you'll want to copy the changes from https://github.com/whatwg/html/pull/1471/files#diff-36cd38f49b9afa08222c0dc9ebfe35ebL66547