- Revision
- 276404
- Author
- [email protected]
- Date
- 2021-04-21 16:39:33 -0700 (Wed, 21 Apr 2021)
Log Message
Cherry-pick r276010. rdar://problem/76962988
Integrator's note, used bit 27 instead of 26 to avoid conflict.
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):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276010 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-611-branch/Source/WebCore/ChangeLog (276403 => 276404)
--- branches/safari-611-branch/Source/WebCore/ChangeLog 2021-04-21 23:39:29 UTC (rev 276403)
+++ branches/safari-611-branch/Source/WebCore/ChangeLog 2021-04-21 23:39:33 UTC (rev 276404)
@@ -1,5 +1,66 @@
2021-04-21 Ruben Turcios <[email protected]>
+ Cherry-pick r276010. rdar://problem/76962988
+
+ 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):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276010 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 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-21 Ruben Turcios <[email protected]>
+
Cherry-pick r276206. rdar://problem/76962916
Perform port blocking earlier in the load
Modified: branches/safari-611-branch/Source/WebCore/dom/ComposedTreeIterator.cpp (276403 => 276404)
--- branches/safari-611-branch/Source/WebCore/dom/ComposedTreeIterator.cpp 2021-04-21 23:39:29 UTC (rev 276403)
+++ branches/safari-611-branch/Source/WebCore/dom/ComposedTreeIterator.cpp 2021-04-21 23:39:33 UTC (rev 276404)
@@ -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: branches/safari-611-branch/Source/WebCore/dom/ContainerNode.cpp (276403 => 276404)
--- branches/safari-611-branch/Source/WebCore/dom/ContainerNode.cpp 2021-04-21 23:39:29 UTC (rev 276403)
+++ branches/safari-611-branch/Source/WebCore/dom/ContainerNode.cpp 2021-04-21 23:39:33 UTC (rev 276404)
@@ -630,6 +630,9 @@
destroyRenderTreeIfNeeded(oldChild);
+ if (UNLIKELY(hasShadowRootContainingSlots()))
+ shadowRoot()->willRemoveAssignedNode(oldChild);
+
if (nextChild) {
nextChild->setPreviousSibling(previousChild);
oldChild.setNextSibling(nullptr);
Modified: branches/safari-611-branch/Source/WebCore/dom/Node.h (276403 => 276404)
--- branches/safari-611-branch/Source/WebCore/dom/Node.h 2021-04-21 23:39:29 UTC (rev 276403)
+++ branches/safari-611-branch/Source/WebCore/dom/Node.h 2021-04-21 23:39:33 UTC (rev 276404)
@@ -226,6 +226,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;
@@ -558,7 +561,9 @@
#endif
IsComputedStyleInvalidFlag = 1 << 26,
- // Bits 27-31 are free.
+ HasShadowRootContainingSlots = 1 << 27,
+
+ // Bits 28-31 are free.
};
enum class TabIndexState : uint8_t {
Modified: branches/safari-611-branch/Source/WebCore/dom/ShadowRoot.h (276403 => 276404)
--- branches/safari-611-branch/Source/WebCore/dom/ShadowRoot.h 2021-04-21 23:39:29 UTC (rev 276403)
+++ branches/safari-611-branch/Source/WebCore/dom/ShadowRoot.h 2021-04-21 23:39:33 UTC (rev 276404)
@@ -93,6 +93,7 @@
void slotFallbackDidChange(HTMLSlotElement&);
void resolveSlotsBeforeNodeInsertionOrRemoval();
void willRemoveAllChildren(ContainerNode&);
+ void willRemoveAssignedNode(const Node&);
void didRemoveAllChildrenOfShadowHost();
void didChangeDefaultSlot();
Modified: branches/safari-611-branch/Source/WebCore/dom/SlotAssignment.cpp (276403 => 276404)
--- branches/safari-611-branch/Source/WebCore/dom/SlotAssignment.cpp 2021-04-21 23:39:29 UTC (rev 276403)
+++ branches/safari-611-branch/Source/WebCore/dom/SlotAssignment.cpp 2021-04-21 23:39:33 UTC (rev 276404)
@@ -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: branches/safari-611-branch/Source/WebCore/dom/SlotAssignment.h (276403 => 276404)
--- branches/safari-611-branch/Source/WebCore/dom/SlotAssignment.h 2021-04-21 23:39:29 UTC (rev 276403)
+++ branches/safari-611-branch/Source/WebCore/dom/SlotAssignment.h 2021-04-21 23:39:33 UTC (rev 276404)
@@ -61,6 +61,7 @@
void enqueueSlotChangeEvent(const AtomString&, ShadowRoot&);
const Vector<WeakPtr<Node>>* assignedNodesForSlot(const HTMLSlotElement&, ShadowRoot&);
+ void willRemoveAssignedNode(const Node&);
virtual void hostChildElementDidChange(const Element&, ShadowRoot&);
@@ -104,6 +105,7 @@
bool m_willBeRemovingAllChildren { false };
unsigned m_slotMutationVersion { 0 };
unsigned m_slotResolutionVersion { 0 };
+ unsigned m_slotElementCount { 0 };
};
inline void ShadowRoot::resolveSlotsBeforeNodeInsertionOrRemoval()
@@ -145,4 +147,10 @@
RenderTreeUpdater::tearDownRenderers(element);
}
+inline void ShadowRoot::willRemoveAssignedNode(const Node& node)
+{
+ if (m_slotAssignment)
+ m_slotAssignment->willRemoveAssignedNode(node);
+}
+
} // namespace WebCore
