Changeset 198090 in webkit

Timestamp:

Mar 13, 2016 6:57:17 PM (8 years ago)

Author:

rniwa@webkit.org

Message:

REGRESSION (r190840): crash inside details element's slotNameFunction
​https://bugs.webkit.org/show_bug.cgi?id=155388

Reviewed by Antti Koivisto.

Source/WebCore:

The bug was caused by HTMLDetailsElement::isActiveSummary calling findAssignedSlot with a summary element
inside the shadow tree of the detials element. Fixed it by existing early when the summary element passed
to isActiveSummary is not a direct child of the details element.

Test: fast/html/details-summary-tabindex-crash.html

• dom/ShadowRoot.cpp:

(WebCore::ShadowRoot::findAssignedSlot): Added an assertion for regression testing.

• dom/SlotAssignment.cpp:

(WebCore::SlotAssignment::findAssignedSlot): Removed the superfluous call to assignSlots added in r190840.
There is no need to update the slot assignments here (entires in m_slots are added or removed by
addSlotElementByName or removeSlotElementByName and assignSlots only updates assignedNodes in each SlotInfo
which is never used in this function or findFirstSlotElement.

• html/HTMLDetailsElement.cpp:

(WebCore::HTMLDetailsElement::isActiveSummary): Fixed the bug.

LayoutTests:

Added a regression test.

• fast/html/details-summary-tabindex-crash-expected.txt: Added.
• fast/html/details-summary-tabindex-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/html/details-summary-tabindex-crash-expected.txt (added)
• LayoutTests/fast/html/details-summary-tabindex-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/ShadowRoot.cpp (modified) (1 diff)
• Source/WebCore/dom/SlotAssignment.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLDetailsElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-03-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION (r190840): crash inside details element's slotNameFunction
+        https://bugs.webkit.org/show_bug.cgi?id=155388
+
+        Reviewed by Antti Koivisto.
+
+        Added a regression test.
+
+        * fast/html/details-summary-tabindex-crash-expected.txt: Added.
+        * fast/html/details-summary-tabindex-crash.html: Added.
+
 2016-03-13  Dean Jackson  <dino@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-03-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION (r190840): crash inside details element's slotNameFunction
+        https://bugs.webkit.org/show_bug.cgi?id=155388
+
+        Reviewed by Antti Koivisto.
+
+        The bug was caused by HTMLDetailsElement::isActiveSummary calling findAssignedSlot with a summary element
+        inside the shadow tree of the detials element. Fixed it by existing early when the summary element passed
+        to isActiveSummary is not a direct child of the details element.
+
+        Test: fast/html/details-summary-tabindex-crash.html
+
+        * dom/ShadowRoot.cpp:
+        (WebCore::ShadowRoot::findAssignedSlot): Added an assertion for regression testing.
+        * dom/SlotAssignment.cpp:
+        (WebCore::SlotAssignment::findAssignedSlot): Removed the superfluous call to assignSlots added in r190840.
+        There is no need to update the slot assignments here (entires in m_slots are added or removed by
+        addSlotElementByName or removeSlotElementByName and assignSlots only updates assignedNodes in each SlotInfo
+        which is never used in this function or findFirstSlotElement.
+        * html/HTMLDetailsElement.cpp:
+        (WebCore::HTMLDetailsElement::isActiveSummary): Fixed the bug.
+
 2016-03-13  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/dom/ShadowRoot.cpp

@@ -183 +183 @@
 HTMLSlotElement* ShadowRoot::findAssignedSlot(const Node& node)
 {
+    ASSERT(node.parentNode() == host());
     if (!m_slotAssignment)
         return nullptr;

trunk/Source/WebCore/dom/SlotAssignment.cpp

@@ -65 +65 @@
         return nullptr;
 
-    if (!m_slotAssignmentsIsValid)
-        assignSlots(shadowRoot);
-
     auto slotName = m_slotNameFunction(node);
     if (!slotName)

trunk/Source/WebCore/html/HTMLDetailsElement.cpp

@@ -104 +104 @@
         return &summary == m_defaultSummary;
 
+    if (summary.parentNode() != this)
+        return false;
+
     auto* slot = shadowRoot()->findAssignedSlot(summary);
     if (!slot)