Cherry-pick 275221@main (9a1d1c7). https://bugs.webkit.org/show_bug.c…

…gi?id=269959 HTMLDataListElement::childrenChanged doesn't call HTMLElement::childrenChanged https://bugs.webkit.org/show_bug.cgi?id=269959 <rdar://123388053> Reviewed by Chris Dumez. This can lead to an inconsistent state in DOM so call that. * LayoutTests/fast/dom/datalist-children-changed-crash-expected.txt: Added. * LayoutTests/fast/dom/datalist-children-changed-crash.html: Added. * Source/WebCore/dom/ContainerNode.cpp: (WebCore::ContainerNode::removeAllChildrenWithScriptAssertion): Added a debug assertion to catch a similar mistake in the future. * Source/WebCore/html/HTMLDataListElement.cpp: (WebCore::HTMLDataListElement::childrenChanged): Canonical link: https://commits.webkit.org/275221@main Canonical link: https://commits.webkit.org/274313.180@webkitglib/2.44
WebKit · Apr 30, 2024 · e8ae6ff · e8ae6ff
1 parent c65b681
commit e8ae6ff
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 1 deletion.
diff --git a/LayoutTests/fast/dom/datalist-children-changed-crash-expected.txt b/LayoutTests/fast/dom/datalist-children-changed-crash-expected.txt
@@ -0,0 +1,2 @@
+This test passes if WebKit does not hit any ASAN assertions.
+PASS
diff --git a/LayoutTests/fast/dom/datalist-children-changed-crash.html b/LayoutTests/fast/dom/datalist-children-changed-crash.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+function runTest() {
+    document.all.length;
+    document.all[0];
+    span.cloneNode(false);
+    document.getElementById('datalist').innerHTML = '';
+    if (window.GCController)
+        GCController.collect();
+    document.all.length;
+    document.body.innerHTML = 'This test passes if WebKit does not hit any ASAN assertions.<br>PASS';
+}
+</script>
+<body onload=runTest()><span id="span">U<datalist id="datalist"><span>
diff --git a/Source/WebCore/dom/ContainerNode.cpp b/Source/WebCore/dom/ContainerNode.cpp
@@ -151,8 +151,13 @@ ALWAYS_INLINE auto ContainerNode::removeAllChildrenWithScriptAssertion(ChildChan
 
     ASSERT_WITH_SECURITY_IMPLICATION(!document().selection().selection().isOrphan());
 
-    if (deferChildrenChanged == DeferChildrenChanged::No)
+    if (deferChildrenChanged == DeferChildrenChanged::No) {
+#if ASSERT_ENABLED
+        auto treeVersion = document().domTreeVersion();
+#endif
         childrenChanged(childChange);
+        ASSERT_WITH_SECURITY_IMPLICATION(document().domTreeVersion() > treeVersion);
+    }
 
     return hadElementChild ? DidRemoveElements::Yes : DidRemoveElements::No;
 }

diff --git a/Source/WebCore/html/HTMLDataListElement.cpp b/Source/WebCore/html/HTMLDataListElement.cpp
@@ -77,6 +77,7 @@ Ref<HTMLCollection> HTMLDataListElement::options()
 
 void HTMLDataListElement::childrenChanged(const ChildChange& change)
 {
+    HTMLElement::childrenChanged(change);
     if (change.source == ChildChange::Source::API)
         optionElementChildrenChanged();
 }