Changeset 266212 in webkit

Timestamp:

Aug 26, 2020 6:26:24 PM (4 years ago)

Author:

rniwa@webkit.org

Message:

Make it possible to create a WeakPtr to Node and use it store assigned nodes in SlotAssignment
​https://bugs.webkit.org/show_bug.cgi?id=215673
<rdar://problem/67440818>

Reviewed by Darin Adler.

This patch makes Node inherit from CanMakeWeakPtr<Node> and uses it in SlotAssignment's assignedNodes
and related member variables and functions.

No new tests since there should be no behavioral difference.

• dom/ComposedTreeIterator.cpp:

(WebCore::ComposedTreeIterator::traverseNextInShadowTree):
(WebCore::ComposedTreeIterator::advanceInSlot):

• dom/ComposedTreeIterator.h:

(WebCore::firstChildInComposedTreeIgnoringUserAgentShadow):
(WebCore::nextSiblingInComposedTreeIgnoringUserAgentShadow):

• dom/ContainerNode.h:
• dom/Node.h:
• dom/ShadowRoot.cpp:

(WebCore::ShadowRoot::assignedNodesForSlot):

• dom/ShadowRoot.h:
• dom/SlotAssignment.cpp:

(WebCore::SlotAssignment::assignedNodesForSlot):
(WebCore::SlotAssignment::assignToSlot):

• dom/SlotAssignment.h:
• html/HTMLSlotElement.cpp:

(WebCore::HTMLSlotElement::assignedNodes const):
(WebCore::flattenAssignedNodes):
(WebCore:: const): Deleted.

• html/HTMLSlotElement.h:
• page/FocusController.cpp:

(WebCore::FocusNavigationScope::firstNodeInScope const):
(WebCore::FocusNavigationScope::lastNodeInScope const):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/ComposedTreeIterator.cpp (modified) (2 diffs)
• dom/ComposedTreeIterator.h (modified) (2 diffs)
• dom/ContainerNode.h (modified) (2 diffs)
• dom/Node.h (modified) (2 diffs)
• dom/ShadowRoot.cpp (modified) (1 diff)
• dom/ShadowRoot.h (modified) (1 diff)
• dom/SlotAssignment.cpp (modified) (3 diffs)
• dom/SlotAssignment.h (modified) (2 diffs)
• html/HTMLSlotElement.cpp (modified) (3 diffs)
• html/HTMLSlotElement.h (modified) (1 diff)
• page/FocusController.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-08-25  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Make it possible to create a WeakPtr to Node and use it store assigned nodes in SlotAssignment
+        https://bugs.webkit.org/show_bug.cgi?id=215673
+        <rdar://problem/67440818>
+
+        Reviewed by Darin Adler.
+
+        This patch makes Node inherit from CanMakeWeakPtr<Node> and uses it in SlotAssignment's assignedNodes
+        and related member variables and functions.
+
+        No new tests since there should be no behavioral difference.
+
+        * dom/ComposedTreeIterator.cpp:
+        (WebCore::ComposedTreeIterator::traverseNextInShadowTree):
+        (WebCore::ComposedTreeIterator::advanceInSlot):
+        * dom/ComposedTreeIterator.h:
+        (WebCore::firstChildInComposedTreeIgnoringUserAgentShadow):
+        (WebCore::nextSiblingInComposedTreeIgnoringUserAgentShadow):
+        * dom/ContainerNode.h:
+        * dom/Node.h:
+        * dom/ShadowRoot.cpp:
+        (WebCore::ShadowRoot::assignedNodesForSlot):
+        * dom/ShadowRoot.h:
+        * dom/SlotAssignment.cpp:
+        (WebCore::SlotAssignment::assignedNodesForSlot):
+        (WebCore::SlotAssignment::assignToSlot):
+        * dom/SlotAssignment.h:
+        * html/HTMLSlotElement.cpp:
+        (WebCore::HTMLSlotElement::assignedNodes const):
+        (WebCore::flattenAssignedNodes):
+        (WebCore:: const): Deleted.
+        * html/HTMLSlotElement.h:
+        * page/FocusController.cpp:
+        (WebCore::FocusNavigationScope::firstNodeInScope const):
+        (WebCore::FocusNavigationScope::lastNodeInScope const):
+
 2020-08-26  Andres Gonzalez  <andresg_22@apple.com>
 

trunk/Source/WebCore/dom/ComposedTreeIterator.cpp

@@ -164 +164 @@
         if (auto* assignedNodes = slot.assignedNodes()) {
             context().slotNodeIndex = 0;
-            auto* assignedNode = assignedNodes->at(0);
+            auto* assignedNode = assignedNodes->at(0).get();
             m_contextStack.append(Context(*assignedNode->parentElement(), *assignedNode, Context::Slotted));
             return;
@@ -198 +198 @@
         return false;
 
-    auto* slotNode = assignedNodes.at(context().slotNodeIndex);
+    auto* slotNode = assignedNodes.at(context().slotNodeIndex).get();
+    if (!slotNode)
+        return false;
     m_contextStack.append(Context(*slotNode->parentElement(), *slotNode, Context::Slotted));
     return true;

trunk/Source/WebCore/dom/ComposedTreeIterator.h

@@ -227 +227 @@
     if (is<HTMLSlotElement>(node)) {
         if (auto* assignedNodes = downcast<HTMLSlotElement>(node).assignedNodes())
-            return assignedNodes->at(0);
+            return assignedNodes->at(0).get();
     }
     return node.firstChild();
@@ -240 +240 @@
         ASSERT(nodeIndex != notFound);
         if (assignedNodes->size() > nodeIndex + 1)
-            return assignedNodes->at(nodeIndex + 1);
+            return assignedNodes->at(nodeIndex + 1).get();
         return nullptr;
     }

trunk/Source/WebCore/dom/ContainerNode.h

@@ -26 +26 @@
 #include "CollectionType.h"
 #include "Node.h"
-#include <wtf/WeakPtr.h>
 
 namespace WebCore {
@@ -37 +36 @@
 typedef Vector<Ref<Node>, initialNodeVectorSize> NodeVector;
 
-class ContainerNode : public CanMakeWeakPtr<ContainerNode>, public Node {
+class ContainerNode : public Node {
     WTF_MAKE_ISO_ALLOCATED(ContainerNode);
 public:

trunk/Source/WebCore/dom/Node.h

@@ -38 +38 @@
 #include <wtf/MainThread.h>
 #include <wtf/URLHash.h>
+#include <wtf/WeakPtr.h>
 
 // This needs to be here because Document.h also depends on it.
@@ -70 +71 @@
 using NodeOrString = Variant<RefPtr<Node>, String>;
 
-class Node : public EventTarget {
+class Node : public CanMakeWeakPtr<Node>, public EventTarget {
     WTF_MAKE_ISO_ALLOCATED(Node);
 

trunk/Source/WebCore/dom/ShadowRoot.cpp

@@ -254 +254 @@
 }
 
-const Vector<Node*>* ShadowRoot::assignedNodesForSlot(const HTMLSlotElement& slot)
+const Vector<WeakPtr<Node>>* ShadowRoot::assignedNodesForSlot(const HTMLSlotElement& slot)
 {
     if (!m_slotAssignment)

trunk/Source/WebCore/dom/ShadowRoot.h

@@ -100 +100 @@
     void hostChildElementDidChangeSlotAttribute(Element&, const AtomString& oldValue, const AtomString& newValue);
 
-    const Vector<Node*>* assignedNodesForSlot(const HTMLSlotElement&);
+    const Vector<WeakPtr<Node>>* assignedNodesForSlot(const HTMLSlotElement&);
 
     void moveShadowRootToNewParentScope(TreeScope&, Document&);

trunk/Source/WebCore/dom/SlotAssignment.cpp

@@ -312 +312 @@
 }
 
-const Vector<Node*>* SlotAssignment::assignedNodesForSlot(const HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
+const Vector<WeakPtr<Node>>* SlotAssignment::assignedNodesForSlot(const HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
 {
     ASSERT(slotElement.containingShadowRoot() == &shadowRoot);
@@ -401 +401 @@
         auto defaultSlotEntry = m_slots.find(defaultSlotName());
         if (defaultSlotEntry != m_slots.end())
-            defaultSlotEntry->value->assignedNodes.append(&child);
+            defaultSlotEntry->value->assignedNodes.append(makeWeakPtr(child));
         return;
     }
@@ -408 +408 @@
         return makeUnique<Slot>();
     });
-    addResult.iterator->value->assignedNodes.append(&child);
-}
-
-}
-
-
+    addResult.iterator->value->assignedNodes.append(makeWeakPtr(child));
+}
+
+}
+
+

trunk/Source/WebCore/dom/SlotAssignment.h

@@ -61 +61 @@
     void enqueueSlotChangeEvent(const AtomString&, ShadowRoot&);
 
-    const Vector<Node*>* assignedNodesForSlot(const HTMLSlotElement&, ShadowRoot&);
+    const Vector<WeakPtr<Node>>* assignedNodesForSlot(const HTMLSlotElement&, ShadowRoot&);
 
     virtual void hostChildElementDidChange(const Element&, ShadowRoot&);
@@ -79 +79 @@
         unsigned elementCount { 0 };
         bool seenFirstElement { false };
-        Vector<Node*> assignedNodes;
+        Vector<WeakPtr<Node>> assignedNodes;
     };
 

trunk/Source/WebCore/html/HTMLSlotElement.cpp

@@ -96 +96 @@
 }
 
-const Vector<Node*>* HTMLSlotElement::assignedNodes() const
+const Vector<WeakPtr<Node>>* HTMLSlotElement::assignedNodes() const
 {
     auto shadowRoot = makeRefPtr(containingShadowRoot());
@@ -120 +120 @@
         return;
     }
-    for (const RefPtr<Node>& node : *assignedNodes) {
+    for (auto& nodeWeakPtr : *assignedNodes) {
+        auto* node = nodeWeakPtr.get();
+        if (UNLIKELY(!node)) {
+            ASSERT_NOT_REACHED();
+            continue;
+        }
         if (is<HTMLSlotElement>(*node) && downcast<HTMLSlotElement>(*node).containingShadowRoot())
             flattenAssignedNodes(nodes, downcast<HTMLSlotElement>(*node));
@@ -140 +145 @@
     if (!assignedNodes)
         return { };
-    return assignedNodes->map([] (Node* node) { return makeRef(*node); });
+
+    Vector<Ref<Node>> nodes;
+    nodes.reserveInitialCapacity(assignedNodes->size());
+    for (auto& nodePtr : *assignedNodes) {
+        auto* node = nodePtr.get();
+        if (UNLIKELY(!node))
+            continue;
+        nodes.uncheckedAppend(*node);
+    }
+
+    return nodes;
 }
 

trunk/Source/WebCore/html/HTMLSlotElement.h

@@ -36 +36 @@
     static Ref<HTMLSlotElement> create(const QualifiedName&, Document&);
 
-    const Vector<Node*>* assignedNodes() const;
+    const Vector<WeakPtr<Node>>* assignedNodes() const;
     struct AssignedNodesOptions {
         bool flatten;

trunk/Source/WebCore/page/FocusController.cpp

@@ -181 +181 @@
         if (m_slotKind == SlotKind::Assigned) {
             ASSERT(assigneNodes);
-            return assigneNodes->first();
+            return assigneNodes->first().get();
         }
         ASSERT(m_slotKind == SlotKind::Fallback);
@@ -196 +196 @@
         if (m_slotKind == SlotKind::Assigned) {
             ASSERT(assigneNodes);
-            return assigneNodes->last();
+            return assigneNodes->last().get();
         }
         ASSERT(m_slotKind == SlotKind::Fallback);