- Revision
- 282505
- Author
- [email protected]
- Date
- 2021-09-16 01:26:42 -0700 (Thu, 16 Sep 2021)
Log Message
Merge r276010 - REGRESSION(r272900): Nullptr crash in ComposedTreeIterator::traverseNextInShadowTree() via ShadowRoot::hostChildElementDidChange
https://bugs.webkit.org/show_bug.cgi?id=222720
Patch by Carlos Garcia Campos <[email protected]> on 2021-04-15
Reviewed by Ryosuke Niwa.
This patch reverts r274064 to apply a different fix. Instead of null-checking the nodes returned by
SlotAssignment::assignedNodesForSlot(), assigned nodes are removed from the list when they are about to be
removed from the parent. That ensures we never return nullptr nodes nor nodes with a nullptr parent from the
assigned nodes vector.
* dom/ComposedTreeIterator.cpp:
(WebCore::ComposedTreeIterator::traverseNextInShadowTree):
(WebCore::ComposedTreeIterator::advanceInSlot):
* dom/ContainerNode.cpp:
(WebCore::ContainerNode::removeBetween):
* dom/Node.h:
(WebCore::Node::hasShadowRootContainingSlots const):
(WebCore::Node::setHasShadowRootContainingSlots):
* dom/ShadowRoot.h:
* dom/SlotAssignment.cpp:
(WebCore::SlotAssignment::addSlotElementByName):
(WebCore::SlotAssignment::removeSlotElementByName):
(WebCore::SlotAssignment::willRemoveAssignedNode):
* dom/SlotAssignment.h:
(WebCore::ShadowRoot::willRemoveAssignedNode):
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog (282504 => 282505)
--- releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog 2021-09-16 08:14:51 UTC (rev 282504)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog 2021-09-16 08:26:42 UTC (rev 282505)
@@ -1,3 +1,31 @@
+2021-04-15 Carlos Garcia Campos <[email protected]>
+
+ REGRESSION(r272900): Nullptr crash in ComposedTreeIterator::traverseNextInShadowTree() via ShadowRoot::hostChildElementDidChange
+ https://bugs.webkit.org/show_bug.cgi?id=222720
+
+ Reviewed by Ryosuke Niwa.
+
+ This patch reverts r274064 to apply a different fix. Instead of null-checking the nodes returned by
+ SlotAssignment::assignedNodesForSlot(), assigned nodes are removed from the list when they are about to be
+ removed from the parent. That ensures we never return nullptr nodes nor nodes with a nullptr parent from the
+ assigned nodes vector.
+
+ * dom/ComposedTreeIterator.cpp:
+ (WebCore::ComposedTreeIterator::traverseNextInShadowTree):
+ (WebCore::ComposedTreeIterator::advanceInSlot):
+ * dom/ContainerNode.cpp:
+ (WebCore::ContainerNode::removeBetween):
+ * dom/Node.h:
+ (WebCore::Node::hasShadowRootContainingSlots const):
+ (WebCore::Node::setHasShadowRootContainingSlots):
+ * dom/ShadowRoot.h:
+ * dom/SlotAssignment.cpp:
+ (WebCore::SlotAssignment::addSlotElementByName):
+ (WebCore::SlotAssignment::removeSlotElementByName):
+ (WebCore::SlotAssignment::willRemoveAssignedNode):
+ * dom/SlotAssignment.h:
+ (WebCore::ShadowRoot::willRemoveAssignedNode):
+
2021-04-14 Ryosuke Niwa <[email protected]>
Crash in in ReplaceSelectionCommand::mergeEndIfNeeded()
Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/ComposedTreeIterator.cpp (282504 => 282505)
--- releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/ComposedTreeIterator.cpp 2021-09-16 08:14:51 UTC (rev 282504)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/ComposedTreeIterator.cpp 2021-09-16 08:26:42 UTC (rev 282505)
@@ -162,11 +162,12 @@
if (is<HTMLSlotElement>(current())) {
auto& slot = downcast<HTMLSlotElement>(current());
if (auto* assignedNodes = slot.assignedNodes()) {
- if (auto assignedNode = assignedNodes->at(0)) {
- context().slotNodeIndex = 0;
- m_contextStack.append(Context(*assignedNode->parentElement(), *assignedNode, Context::Slotted));
- return;
- }
+ context().slotNodeIndex = 0;
+ auto* assignedNode = assignedNodes->at(0).get();
+ ASSERT(assignedNode);
+ ASSERT(assignedNode->parentElement());
+ m_contextStack.append(Context(*assignedNode->parentElement(), *assignedNode, Context::Slotted));
+ return;
}
}
@@ -199,8 +200,8 @@
return false;
auto* slotNode = assignedNodes.at(context().slotNodeIndex).get();
- if (!slotNode)
- return false;
+ ASSERT(slotNode);
+ ASSERT(slotNode->parentElement());
m_contextStack.append(Context(*slotNode->parentElement(), *slotNode, Context::Slotted));
return true;
}
Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/ContainerNode.cpp (282504 => 282505)
--- releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/ContainerNode.cpp 2021-09-16 08:14:51 UTC (rev 282504)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/ContainerNode.cpp 2021-09-16 08:26:42 UTC (rev 282505)
@@ -630,6 +630,9 @@
destroyRenderTreeIfNeeded(oldChild);
+ if (UNLIKELY(hasShadowRootContainingSlots()))
+ shadowRoot()->willRemoveAssignedNode(oldChild);
+
if (nextChild) {
nextChild->setPreviousSibling(previousChild);
oldChild.setNextSibling(nullptr);
Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Node.h (282504 => 282505)
--- releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Node.h 2021-09-16 08:14:51 UTC (rev 282504)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Node.h 2021-09-16 08:26:42 UTC (rev 282505)
@@ -222,6 +222,9 @@
bool hasSyntheticAttrChildNodes() const { return hasNodeFlag(NodeFlag::HasSyntheticAttrChildNodes); }
void setHasSyntheticAttrChildNodes(bool flag) { setNodeFlag(NodeFlag::HasSyntheticAttrChildNodes, flag); }
+ bool hasShadowRootContainingSlots() const { return hasNodeFlag(NodeFlag::HasShadowRootContainingSlots); }
+ void setHasShadowRootContainingSlots(bool flag) { setNodeFlag(NodeFlag::HasShadowRootContainingSlots, flag); }
+
// If this node is in a shadow tree, returns its shadow host. Otherwise, returns null.
WEBCORE_EXPORT Element* shadowHost() const;
ShadowRoot* containingShadowRoot() const;
@@ -566,7 +569,8 @@
InclusiveAncestorStateForForm = 1 << 28,
InclusiveAncestorStateForCanvas = 1 << 29,
- // Bits 30-31 are free.
+ HasShadowRootContainingSlots = 1 << 30,
+ // Bit 31 is free.
};
enum class TabIndexState : uint8_t {
Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/ShadowRoot.h (282504 => 282505)
--- releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/ShadowRoot.h 2021-09-16 08:14:51 UTC (rev 282504)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/ShadowRoot.h 2021-09-16 08:26:42 UTC (rev 282505)
@@ -93,6 +93,7 @@
void slotFallbackDidChange(HTMLSlotElement&);
void resolveSlotsBeforeNodeInsertionOrRemoval();
void willRemoveAllChildren(ContainerNode&);
+ void willRemoveAssignedNode(const Node&);
void didRemoveAllChildrenOfShadowHost();
void didChangeDefaultSlot();
Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/SlotAssignment.cpp (282504 => 282505)
--- releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/SlotAssignment.cpp 2021-09-16 08:14:51 UTC (rev 282504)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/SlotAssignment.cpp 2021-09-16 08:26:42 UTC (rev 282505)
@@ -124,6 +124,9 @@
if (!m_slotAssignmentsIsValid)
assignSlots(shadowRoot);
+ shadowRoot.host()->setHasShadowRootContainingSlots(true);
+ m_slotElementCount++;
+
bool needsSlotchangeEvent = shadowRoot.shouldFireSlotchangeEvent() && hasAssignedNodes(shadowRoot, slot);
slot.elementCount++;
@@ -151,8 +154,15 @@
m_slotElementsForConsistencyCheck.remove(&slotElement);
#endif
- if (auto* host = shadowRoot.host()) // FIXME: We should be able to do a targeted reconstruction.
+ ASSERT(m_slotElementCount > 0);
+ m_slotElementCount--;
+
+ if (auto host = makeRefPtr(shadowRoot.host())) {
+ // FIXME: We should be able to do a targeted reconstruction.
host->invalidateStyleAndRenderersForSubtree();
+ if (!m_slotElementCount)
+ host->setHasShadowRootContainingSlots(false);
+ }
auto* slot = m_slots.get(slotNameFromAttributeValue(name));
RELEASE_ASSERT(slot && slot->hasSlotElements());
@@ -337,6 +347,23 @@
return &slot->assignedNodes;
}
+void SlotAssignment::willRemoveAssignedNode(const Node& node)
+{
+ if (!m_slotAssignmentsIsValid)
+ return;
+
+ if (!is<Text>(node) && !is<Element>(node))
+ return;
+
+ auto* slot = m_slots.get(slotNameForHostChild(node));
+ if (!slot || slot->assignedNodes.isEmpty())
+ return;
+
+ slot->assignedNodes.removeFirstMatching([&node](const auto& item) {
+ return item.get() == &node;
+ });
+}
+
const AtomString& SlotAssignment::slotNameForHostChild(const Node& child) const
{
return slotNameFromSlotAttribute(child);
Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/SlotAssignment.h (282504 => 282505)
--- releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/SlotAssignment.h 2021-09-16 08:14:51 UTC (rev 282504)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/SlotAssignment.h 2021-09-16 08:26:42 UTC (rev 282505)
@@ -60,6 +60,7 @@
void didChangeSlot(const AtomString&, ShadowRoot&);
const Vector<WeakPtr<Node>>* assignedNodesForSlot(const HTMLSlotElement&, ShadowRoot&);
+ void willRemoveAssignedNode(const Node&);
virtual void hostChildElementDidChange(const Element&, ShadowRoot&);
@@ -103,6 +104,7 @@
bool m_willBeRemovingAllChildren { false };
unsigned m_slotMutationVersion { 0 };
unsigned m_slotResolutionVersion { 0 };
+ unsigned m_slotElementCount { 0 };
};
inline void ShadowRoot::resolveSlotsBeforeNodeInsertionOrRemoval()
@@ -144,4 +146,10 @@
RenderTreeUpdater::tearDownRenderers(element);
}
+inline void ShadowRoot::willRemoveAssignedNode(const Node& node)
+{
+ if (m_slotAssignment)
+ m_slotAssignment->willRemoveAssignedNode(node);
+}
+
} // namespace WebCore
