Describe integration with the WeakRefs proposal

[littledan]

The main observable semantics here are that both clearing the KeptAlive
list and calling the finalization callback on any particular
FinalizationGroup happen on individual tasks, on a separate task source.

---

/index.html ( diff )
/infrastructure.html ( diff )
/webappapis.html ( diff )

[littledan]

Thanks to @domenic for explaining to me why it makes sense to use tasks for these cases. Note, this is not ready to merge until (at least) WeakRefs reaches Stage 3. Companion PR: tc39/proposal-weakrefs#86

[domenic]

Great stuff, thank you! Just some clarity nits.

[annevk]

Thanks for putting this up!

[littledan]

WeakRefs reached Stage 3 in TC39, so this should be ready to merge soon. However, there were some recommended editorial changes to the host hooks, so we should probably land those before merging this patch. (I'm not sure whether those changes will be ready in the next week, or in September when I'm back from my leave.)

[syg]

Checking my understanding: there's a microtask checkpoint after running the finalizer of each FinalizationGroup, meaning implementations are not free to coalesce finalizers of multiple FinalizationGroups into a single task without running microtasks in between. Is that right?

[littledan]

@syg Yes, as each FinalizationGroup is called out to in its own task, a microtask checkpoint is required to happen after each callback to them. This fits with HTML's general mental model that a microtask checkpoint happens every time we "return from JavaScript".

HTML's scheduling of tasks is loose enough that implementations may coalesce things in terms of running one right after the other, but a microtask checkpoint is required after each one.

Is this definition OK for implementations? Is there any particular reason we might want to loosen the specification and make the timing of microtask checkpoints less well-defined? All else being equal, I'd prefer if we define things as strictly as possible, to minimize potential interoperability issues.

[syg]

Is this definition OK for implementations? Is there any particular reason we might want to loosen the specification and make the timing of microtask checkpoints less well-defined? All else being equal, I'd prefer if we define things as strictly as possible, to minimize potential interoperability issues.

It seems perfectly fine to me. Thanks for the clarification.

[bzbarsky]

That's going to run script, right? That means it needs to at the very least prepare to run a script and then clean up.

Depending on how that script gets entered (via a pre-defined function, think?), we may also need the other work invoking callback functions does: prepare to run a callback and clean up after running a callback. At the very least we should have tests that test for whether that preparation happens or not.

[littledan]

Ah, that's right. Thanks for pointing this out. This should all be the same as EnqueueJob. We should actually share spec text once tc39/ecma262#1597 lands; once it does, I'll rebase this on top of the generalized version of #4722 and that should be that.

However, this algorithm may be invoked from the GC, so the "incumbent" concept might not be well-defined. We could get a relevant realm from the FinalizationGroup or the callback itself. I'm not sure what would be preferable.

[bzbarsky]

Yeah, we need to think a bit about how the incumbent bit should work. Web IDL callbacks handle that by capturing that at callback-creation time, but it's not clear to me that there's a corresponding thing here...

[syg]

Mainly this matters for exceptions. Domenic knows much better below.

In chromium/v8 we decided to use the context (I assume that's the "environment settings object" in HTML speak) of the FinalizationGroup.

[domenic]

I think this matters for any API which looks up the incumbent or entry settings objects. Notable examples are window.postMessage() (uses incumbent; see "the following more complicated example") and window.open() (uses entry; see https://html.spec.whatwg.org/#window-open-steps).

So in particular if any of those APIs are called during finalization callbacks, how should they behave?

/cc @yuki3 who was working on a proper incumbent implementation in Blink (I think that landed by now?).

Also see #1426 for the analogous issue with promise callbacks, which has been on my to-do list to fix (per #1426 (comment)) for a while. Although I've also been waiting for tc39/ecma262#735 to land since that'd change things out from under us a bit.

[bzbarsky]

Interesting. At one point Blink absolutely ran functions in the "removed from the DOM" case, and we had to fix Gecko for compat with that. But I just tried a simple testcase like so:

<!doctype html>
<html>
  <div style="display: none">
    <iframe srcdoc="<script>function f(callback) { return () => callback('first'); }</script>"></iframe>
    <iframe srcdoc="<script>function f(callback) { return () => callback('second'); }</script>"></iframe>
  </div>
  <div id="target">
    Click me and look at your console to see which listeners fire
  </div>
  <script>
    function logger(str) {
      console.log(str);
    }
    onload = () => {
      let t = document.getElementById("target");
      t.addEventListener("click", frames[0].f(logger));
      t.addEventListener("click", frames[1].f(logger));
      frames[0].location.href = "about:blank";
      frames[1].frameElement.remove();
    }
  </script>
</html>

and at first glance Blink is not running either of the functions. If that's the case, we could align on that, yes. I'd want some pretty careful testing of the Blink behavior, or code inspection that says this case really never runs script there.

[syg]

The more I think about it the more I am convinced that detaching a document -- whether by navigation or removal of an iframe -- should cancel any scheduled finalizers.

We may not be able to forbid code execution for detached iframes easily, but having scheduled finalizers arbitrarily extend the lifetime of navigated-away-from frames is bad enough that we need to live with FinalizationGroups from removed iframes being useless (i.e. their finalizers will never run).

[syg]

At one point Blink absolutely ran functions in the "removed from the DOM" case, and we had to fix Gecko for compat with that.

Sorry, let me clarify. Blink still allows referencing functions from removed iframes and invoking those functions. I mean that from the scheduled FinalizationGroup cleanup task's point of view, it cannot tell if the context of the FinalizationGroup is detached due to navigation or detached due to being removed from the DOM.

[bzbarsky]

Sure, everyone except Edge (with the non-Blink engine) allowed such references and calls if you're already in JS. I was specifically talking about cases when functions from a removed iframe are set as event listeners, passed as callbacks to DOM APIs, handed as callbacks to promises, etc.

[syg]

The more I think about it the more I am convinced that detaching a document -- whether by navigation or removal of an iframe -- should cancel any scheduled finalizers.

To give more detail, what I mean is that upon detach and navigation, existing scheduled finalization tasks are canceled. If, at the next full GC, that frame's FinalizationGroups are still alive due to, for instance, another realm keeping references to a removed iframe's FinalizationGroups, new tasks may be scheduled. This latter case is keeping consistent with the current widespread ability to directly call into the removed iframe.

[syg]

Is this definition OK for implementations? Is there any particular reason we might want to loosen the specification and make the timing of microtask checkpoints less well-defined? All else being equal, I'd prefer if we define things as strictly as possible, to minimize potential interoperability issues.

Coming back to this, it might be desirable to allow coalescing of the tasks somehow...

The current wording ties the scalability of FinalizationGroups to the task scheduler, basically. Do we want to enable the use case of thousands of FGs? Tens of thousands?

[domenic]

Looking great. I like the grouping under job-related host hooks.

[syg]

Rebased and addressed the last review.

tc39/ecma262#2316 got consensus at the March 2021 TC39, so once that merges we should be good to go for this PR.

[syg]

tc39/ecma262#2316 is merged, so let's hit.

[annevk]

Proposed commit message:

Integrate with JavaScript WeakRefs

The role of HTML here is strictly limited to being a task scheduling
service for JavaScript and tracking incumbent globals.

Co-authored-by: Ms2ger <[email protected]>
Co-authored-by: Shu-yu Guo <[email protected]>

(I think it should ack @littledan by virtue of having created the PR.)