Bug 235322 – Adding iframe flushes microtasks synchronously with dirty stack

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Dan Abramov]

Created attachment 449401 [details]
repro

Microtasks are supposed to run on a clean stack. However, Safari flushes microtasks synchronously if you append an iframe to the page. This breaks expectations for libraries like React/Preact, and possibly others:

- https://github.com/facebook/react/issues/22459
- https://github.com/preactjs/preact/issues/2814


Here is a simple repro:

```
queueMicrotask(() => {
  console.log("--- in microtask ---");
});
console.log("will add iframe");
const iframe = document.createElement("iframe");
iframe.src = "localhost";
document.body.appendChild(iframe);
console.log("did add iframe");
```

You need to run it with http server to repro. It doesn't repro from file://.

Expected output:

```
will add iframe
did add iframe
--- in microtask ---
```

Safari output:

```
will add iframe
--- in microtask ---
did add iframe
```

[Jason Miller]

I spent a bit of time checking various forms of this issue:
https://jsfiddle.net/developit/u6xztc93/

I was able to narrow the issue down:

- microtasks are only flushed in the realm of the frame's ownerDocument.
- flush is *not* triggered for sandboxed iframes.
- flush *is* triggered for cross-origin iframes.
    (presumably because it's not known to be cross-origin at append time)
- flush *is* triggered for iframes within <svg><foreignObject>.
- other elements (svg/img/video/object/embed) do not trigger flush.

[Radar WebKit Bug Importer]

<rdar://problem/88031232>

[Ryosuke Niwa]

I don't think this is a regression. The issue really stems from the fact we're still firing DOMContentLoaded and load events synchronously in Document::finishedParsing(). 

Since this code gets to run when inserting an iframe as a part of creating the initial empty document, we end up performing the microtask checkpoint.

Making DOMContentLoaded and load events fully asynchronous is a major undertaking but we can workaround this specific issue by detecting that's what's about to happen and avoid performing microtask checkpoint.

[Ryosuke Niwa]

Created attachment 460532 [details]
WIP

We can detect the case where we're creating initial empty document like this.
But this doesn't work when we're implicitly navigating to about:blank (blank iframe).

[Ryosuke Niwa]

Yikes. This must be a regression from https://trac.webkit.org/r234944.

[Ryosuke Niwa]

Created attachment 460534 [details]
WIP2

[Ryosuke Niwa]

Pull request: https://github.com/WebKit/WebKit/pull/1907

[Ryosuke Niwa]

Created attachment 460549 [details]
Patch

[Ryosuke Niwa]

Pull request: https://github.com/WebKit/WebKit/pull/1909

[EWS]

Committed 252015@main (ea74655e102a): <https://commits.webkit.org/252015@main>

Reviewed commits have been landed. Closing PR #1909 and removing active labels.

[Ryosuke Niwa]

*** Bug 243488 has been marked as a duplicate of this bug. ***

[Matt d'Entremont]

The commit for the fix doesn't seem to be part of any Safari-specific git tags, but it is in Safari branches safari-7614.1.20-branch all the way up to safari-7615.1.9-branch.

I see it mentioned in the Safari tech preview 150 release notes, but not in any of the mainline release notes.

Is this released to the general public yet?

[Ryosuke Niwa]

(In reply to Matt d'Entremont from comment #12)
> The commit for the fix doesn't seem to be part of any Safari-specific git
> tags, but it is in Safari branches safari-7614.1.20-branch all the way up to
> safari-7615.1.9-branch.
> 
> I see it mentioned in the Safari tech preview 150 release notes, but not in
> any of the mainline release notes.
> 
> Is this released to the general public yet?

It should be in Safari 16.