Title: [279077] branches/safari-611-branch
Revision
279077
Author
repst...@apple.com
Date
2021-06-21 12:37:00 -0700 (Mon, 21 Jun 2021)

Log Message

Cherry-pick r279010. rdar://problem/79574790

    Crash in WebCore::SlotAssignment::assignedNodesForSlot
    https://bugs.webkit.org/show_bug.cgi?id=224408
    <rdar://problem/76805764>

    Reviewed by Michael Catanzaro.

    Source/WebCore:

    Like webkit.org/b/225684, the release assertion failure was caused by RenderTreeUpdater::tearDownRenderers
    traversing the slot element for which we're currently calling Element::insertedIntoAncestor but had not yet
    called SlotAssignment::addSlotElementByName.

    Fixed the bug by returning early in SlotAssignment::assignedNodesForSlot when this condition holds,
    which is when the shadow root is connected to a document and HTMLSlotElement is in the middle of
    HTMLSlotElement::insertedIntoAncestor.

    It's not the most elegant solution but staying safe for now.

    Test: fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html

    * dom/SlotAssignment.cpp:
    (WebCore::SlotAssignment::assignedNodesForSlot):
    * html/HTMLSlotElement.cpp:
    (WebCore::HTMLSlotElement::insertedIntoAncestor):
    * html/HTMLSlotElement.h:
    (WebCore::HTMLSlotElement::isInInsertedIntoAncestor): Added.

    LayoutTests:

    Added a regression test.

    * fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash-expected.txt: Added.
    * fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html: Added.

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@279010 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Added Paths

Diff

Modified: branches/safari-611-branch/LayoutTests/ChangeLog (279076 => 279077)


--- branches/safari-611-branch/LayoutTests/ChangeLog	2021-06-21 19:09:04 UTC (rev 279076)
+++ branches/safari-611-branch/LayoutTests/ChangeLog	2021-06-21 19:37:00 UTC (rev 279077)
@@ -1,3 +1,57 @@
+2021-06-21  Alan Coon  <alanc...@apple.com>
+
+        Cherry-pick r279010. rdar://problem/79574790
+
+    Crash in WebCore::SlotAssignment::assignedNodesForSlot
+    https://bugs.webkit.org/show_bug.cgi?id=224408
+    <rdar://problem/76805764>
+    
+    Reviewed by Michael Catanzaro.
+    
+    Source/WebCore:
+    
+    Like webkit.org/b/225684, the release assertion failure was caused by RenderTreeUpdater::tearDownRenderers
+    traversing the slot element for which we're currently calling Element::insertedIntoAncestor but had not yet
+    called SlotAssignment::addSlotElementByName.
+    
+    Fixed the bug by returning early in SlotAssignment::assignedNodesForSlot when this condition holds,
+    which is when the shadow root is connected to a document and HTMLSlotElement is in the middle of
+    HTMLSlotElement::insertedIntoAncestor.
+    
+    It's not the most elegant solution but staying safe for now.
+    
+    Test: fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html
+    
+    * dom/SlotAssignment.cpp:
+    (WebCore::SlotAssignment::assignedNodesForSlot):
+    * html/HTMLSlotElement.cpp:
+    (WebCore::HTMLSlotElement::insertedIntoAncestor):
+    * html/HTMLSlotElement.h:
+    (WebCore::HTMLSlotElement::isInInsertedIntoAncestor): Added.
+    
+    LayoutTests:
+    
+    Added a regression test.
+    
+    * fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash-expected.txt: Added.
+    * fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html: Added.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@279010 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-06-17  Ryosuke Niwa  <rn...@webkit.org>
+
+            Crash in WebCore::SlotAssignment::assignedNodesForSlot
+            https://bugs.webkit.org/show_bug.cgi?id=224408
+            <rdar://problem/76805764>
+
+            Reviewed by Michael Catanzaro.
+
+            Added a regression test.
+
+            * fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash-expected.txt: Added.
+            * fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html: Added.
+
 2021-06-15  Alan Coon  <alanc...@apple.com>
 
         Cherry-pick r278410. rdar://problem/79355285

Added: branches/safari-611-branch/LayoutTests/fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash-expected.txt (0 => 279077)


--- branches/safari-611-branch/LayoutTests/fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash-expected.txt	                        (rev 0)
+++ branches/safari-611-branch/LayoutTests/fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash-expected.txt	2021-06-21 19:37:00 UTC (rev 279077)
@@ -0,0 +1,6 @@
+This tests inserting a slot child under a shadow host.
+WebKit should not hit any assertion or crash and you should see 1, 2, & PASS below each on its own line.
+
+1
+2
+PASS - WebKit did not crash

Added: branches/safari-611-branch/LayoutTests/fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html (0 => 279077)


--- branches/safari-611-branch/LayoutTests/fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html	                        (rev 0)
+++ branches/safari-611-branch/LayoutTests/fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html	2021-06-21 19:37:00 UTC (rev 279077)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests inserting a slot child under a shadow host.<br>
+WebKit should not hit any assertion or crash and you should see 1, 2, & PASS below each on its own line.</p>
+<div id="outerHost"><div slot="slot1">1</div><div slot="slot2">2</div></div>
+<script>
+
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+const outerShadow = outerHost.attachShadow({mode: 'open'});
+outerShadow.innerHTML = `<slot name="slot1"></slot><div id="innerHost">X</div>`;
+
+const innerHost = outerShadow.getElementById('innerHost');
+innerHost.attachShadow({mode: 'closed'}).innerHTML = '<slot></slot>';
+
+innerHost.getBoundingClientRect();
+innerHost.innerHTML = '<slot name="slot2"></slot>';
+
+document.write('<div>PASS - WebKit did not crash</div>');
+
+</script>
+</body>
+</html>

Modified: branches/safari-611-branch/Source/WebCore/ChangeLog (279076 => 279077)


--- branches/safari-611-branch/Source/WebCore/ChangeLog	2021-06-21 19:09:04 UTC (rev 279076)
+++ branches/safari-611-branch/Source/WebCore/ChangeLog	2021-06-21 19:37:00 UTC (rev 279077)
@@ -1,3 +1,71 @@
+2021-06-21  Alan Coon  <alanc...@apple.com>
+
+        Cherry-pick r279010. rdar://problem/79574790
+
+    Crash in WebCore::SlotAssignment::assignedNodesForSlot
+    https://bugs.webkit.org/show_bug.cgi?id=224408
+    <rdar://problem/76805764>
+    
+    Reviewed by Michael Catanzaro.
+    
+    Source/WebCore:
+    
+    Like webkit.org/b/225684, the release assertion failure was caused by RenderTreeUpdater::tearDownRenderers
+    traversing the slot element for which we're currently calling Element::insertedIntoAncestor but had not yet
+    called SlotAssignment::addSlotElementByName.
+    
+    Fixed the bug by returning early in SlotAssignment::assignedNodesForSlot when this condition holds,
+    which is when the shadow root is connected to a document and HTMLSlotElement is in the middle of
+    HTMLSlotElement::insertedIntoAncestor.
+    
+    It's not the most elegant solution but staying safe for now.
+    
+    Test: fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html
+    
+    * dom/SlotAssignment.cpp:
+    (WebCore::SlotAssignment::assignedNodesForSlot):
+    * html/HTMLSlotElement.cpp:
+    (WebCore::HTMLSlotElement::insertedIntoAncestor):
+    * html/HTMLSlotElement.h:
+    (WebCore::HTMLSlotElement::isInInsertedIntoAncestor): Added.
+    
+    LayoutTests:
+    
+    Added a regression test.
+    
+    * fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash-expected.txt: Added.
+    * fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html: Added.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@279010 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-06-17  Ryosuke Niwa  <rn...@webkit.org>
+
+            Crash in WebCore::SlotAssignment::assignedNodesForSlot
+            https://bugs.webkit.org/show_bug.cgi?id=224408
+            <rdar://problem/76805764>
+
+            Reviewed by Michael Catanzaro.
+
+            Like webkit.org/b/225684, the release assertion failure was caused by RenderTreeUpdater::tearDownRenderers
+            traversing the slot element for which we're currently calling Element::insertedIntoAncestor but had not yet
+            called SlotAssignment::addSlotElementByName.
+
+            Fixed the bug by returning early in SlotAssignment::assignedNodesForSlot when this condition holds,
+            which is when the shadow root is connected to a document and HTMLSlotElement is in the middle of
+            HTMLSlotElement::insertedIntoAncestor.
+
+            It's not the most elegant solution but staying safe for now.
+
+            Test: fast/shadow-dom/insert-slot-child-of-shadow-host-render-tree-invalidation-crash.html
+
+            * dom/SlotAssignment.cpp:
+            (WebCore::SlotAssignment::assignedNodesForSlot):
+            * html/HTMLSlotElement.cpp:
+            (WebCore::HTMLSlotElement::insertedIntoAncestor):
+            * html/HTMLSlotElement.h:
+            (WebCore::HTMLSlotElement::isInInsertedIntoAncestor): Added.
+
 2021-06-17  Alan Coon  <alanc...@apple.com>
 
         Cherry-pick r278975. rdar://problem/79474077

Modified: branches/safari-611-branch/Source/WebCore/dom/SlotAssignment.cpp (279076 => 279077)


--- branches/safari-611-branch/Source/WebCore/dom/SlotAssignment.cpp	2021-06-21 19:09:04 UTC (rev 279076)
+++ branches/safari-611-branch/Source/WebCore/dom/SlotAssignment.cpp	2021-06-21 19:37:00 UTC (rev 279077)
@@ -333,8 +333,8 @@
     const AtomString& slotName = slotNameFromAttributeValue(slotElement.attributeWithoutSynchronization(nameAttr));
     auto* slot = m_slots.get(slotName);
 
-    bool hasNotCalledInsertedIntoAncestorOnSlot = shadowRoot.isConnected() && !slotElement.isConnected();
-    if (hasNotCalledInsertedIntoAncestorOnSlot)
+    bool hasNotAddedSlotInInsertedIntoAncestorYet = shadowRoot.isConnected() && (!slotElement.isConnected() || slotElement.isInInsertedIntoAncestor());
+    if (hasNotAddedSlotInInsertedIntoAncestorYet)
         return nullptr;
     RELEASE_ASSERT(slot);
 

Modified: branches/safari-611-branch/Source/WebCore/html/HTMLSlotElement.cpp (279076 => 279077)


--- branches/safari-611-branch/Source/WebCore/html/HTMLSlotElement.cpp	2021-06-21 19:09:04 UTC (rev 279076)
+++ branches/safari-611-branch/Source/WebCore/html/HTMLSlotElement.cpp	2021-06-21 19:37:00 UTC (rev 279077)
@@ -33,6 +33,7 @@
 #include "ShadowRoot.h"
 #include "Text.h"
 #include <wtf/IsoMallocInlines.h>
+#include <wtf/SetForScope.h>
 
 namespace WebCore {
 
@@ -53,6 +54,8 @@
 
 HTMLSlotElement::InsertedIntoAncestorResult HTMLSlotElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
 {
+    SetForScope isInInsertedIntoAncestor { m_isInInsertedIntoAncestor, true };
+
     auto insertionResult = HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
     ASSERT_UNUSED(insertionResult, insertionResult == InsertedIntoAncestorResult::Done);
 

Modified: branches/safari-611-branch/Source/WebCore/html/HTMLSlotElement.h (279076 => 279077)


--- branches/safari-611-branch/Source/WebCore/html/HTMLSlotElement.h	2021-06-21 19:09:04 UTC (rev 279076)
+++ branches/safari-611-branch/Source/WebCore/html/HTMLSlotElement.h	2021-06-21 19:37:00 UTC (rev 279077)
@@ -47,6 +47,8 @@
 
     void dispatchSlotChangeEvent();
 
+    bool isInInsertedIntoAncestor() const { return m_isInInsertedIntoAncestor; }
+
 private:
     HTMLSlotElement(const QualifiedName&, Document&);
 
@@ -56,6 +58,7 @@
     void attributeChanged(const QualifiedName&, const AtomString& oldValue, const AtomString& newValue, AttributeModificationReason) final;
 
     bool m_inSignalSlotList { false };
+    bool m_isInInsertedIntoAncestor { false };
 };
 
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to