Cherry-pick 265870.486@safari-7616-branch (3fc6931). https://bugs.web…

…kit.org/show_bug.cgi?id=260907 Crash under HTMLBodyElement::didFinishInsertingNode() https://bugs.webkit.org/show_bug.cgi?id=260907 rdar://114177696 Reviewed by Ryosuke Niwa. When a <body> is inserted into the document, `HTMLBodyElement::insertedIntoAncestor()` gets called. This function would only return `InsertedIntoAncestorResult::NeedsPostInsertionCallback` if `is<HTMLFrameElementBase>(document().ownerElement())`, causing `HTMLBodyElement::didFinishInsertingNode()` to get called later on. We would then assume in didFinishInsertingNode() that the document's owner element is a non-null HTMLFrameElementBase. However, as proven by the test, DOM manipulations can happen in between the 2 calls causing the assertion to no longer hold. To address the issue we now early return if `is<HTMLFrameElementBase>(document().ownerElement())` is no longer true in `HTMLBodyElement::didFinishInsertingNode()`. In the case of the test, `document().frame()` becomes null because the frame gets detached, causing `document().ownerElement()` to return null as well. * LayoutTests/fast/frames/frame-append-body-child-crash-expected.txt: Added. * LayoutTests/fast/frames/frame-append-body-child-crash.html: Added. * Source/WebCore/html/HTMLBodyElement.cpp: (WebCore::HTMLBodyElement::didFinishInsertingNode): Canonical link: https://commits.webkit.org/265870.486@safari-7616-branch
WebKit · Oct 19, 2023 · a305325 · a305325
1 parent 20c4be2
commit a305325
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 1 deletion.
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -237,6 +237,9 @@ imported/w3c/web-platform-tests/html/cross-origin-embedder-policy/reporting-to-f
 imported/w3c/web-platform-tests/html/cross-origin-embedder-policy/reporting-to-worker-owner.https.html
 imported/w3c/web-platform-tests/html/cross-origin-embedder-policy/cache-storage-reporting-document.https.html [ Skip ]
 
+# This test is a bit slow.
+fast/frames/frame-append-body-child-crash.html [ Slow ]
+
 # This test is failing in all major browser engines as of Sept 2022. The reason is that the Reporting-Endpoints header
 # is served on the parent of the reporting frame, not the reporting frame.
 imported/w3c/web-platform-tests/html/cross-origin-embedder-policy/reporting-to-endpoint.https.html

diff --git a/LayoutTests/fast/frames/frame-append-body-child-crash-expected.txt b/LayoutTests/fast/frames/frame-append-body-child-crash-expected.txt
@@ -0,0 +1,3 @@
+This test passes if it doesn't crash.
+
+
diff --git a/LayoutTests/fast/frames/frame-append-body-child-crash.html b/LayoutTests/fast/frames/frame-append-body-child-crash.html
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function runTest() {
+    document.body.appendChild(testFrame);
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>This test passes if it doesn't crash.</p>
+<iframe id="testFrame" name="testFrameName"></iframe>
+<form id="testForm" target="testFrameName" method="GET">
+    <input type=hidden id="runInput" name="run" value="1">
+</form>
+<script>
+    const urlParams = new URLSearchParams(window.location.search);
+    if (run = urlParams.get("run"))
+        document.getElementById("runInput").setAttribute("value", Number.parseInt(run) + 1);
+    else {
+        setTimeout(() => {
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }, 3000);
+    }
+
+    if (run < 2) {
+        testForm.submit();
+        var video = document.createElement("video");
+        video.src = "data:";
+        testFrame.appendChild(document.createElement("body"));
+    } else if (window.testRunner)
+        testRunner.notifyDone();
+</script>
+</body>
+</html>
+
diff --git a/Source/WebCore/html/HTMLBodyElement.cpp b/Source/WebCore/html/HTMLBodyElement.cpp
@@ -183,7 +183,11 @@ Node::InsertedIntoAncestorResult HTMLBodyElement::insertedIntoAncestor(Insertion
 
 void HTMLBodyElement::didFinishInsertingNode()
 {
-    ASSERT(is<HTMLFrameElementBase>(document().ownerElement()));
+    // A DOM mutation could have happened in between the call to insertedIntoAncestor() and the
+    // call to didFinishInsertingNode().
+    if (!is<HTMLFrameElementBase>(document().ownerElement()))
+        return;
+
     Ref ownerElement = *document().ownerElement();
 
     // FIXME: It's surprising this is web compatible since it means marginwidth and marginheight attributes