Use CheckedPtr<Element> / CheckedRef<Element> less for non-stack vari…

…ables https://bugs.webkit.org/show_bug.cgi?id=266286 Reviewed by Ryosuke Niwa. Use CheckedPtr<Element> / CheckedRef<Element> less for non-stack variables since it introduces crashes that are not actionable. Use WeakRef<Element> / WeakPtr<Element> instead. This tested as performance neutral on Speedometer 2 & 3. * Source/WebCore/dom/Attr.h: * Source/WebCore/dom/DatasetDOMStringMap.h: * Source/WebCore/dom/Document.cpp: (WebCore::findNearestCommonComposedAncestor): (WebCore::Document::registerArticleElement): (WebCore::Document::unregisterArticleElement): (WebCore::Document::updateMainArticleElementAfterLayout): * Source/WebCore/dom/Document.h: * Source/WebCore/dom/Element.cpp: (WebCore::Element::addShadowRoot): * Source/WebCore/dom/NamedNodeMap.h: * Source/WebCore/dom/RadioButtonGroups.cpp: * Source/WebCore/dom/ScriptElement.h: * Source/WebCore/dom/ShadowRoot.cpp: * Source/WebCore/dom/ShadowRoot.h: * Source/WebCore/dom/SimulatedClick.cpp: (WebCore::simulateClick): * Source/WebCore/html/HTMLCollection.h: * Source/WebCore/html/HTMLCollectionInlines.h: (WebCore::CollectionNamedElementCache::findElementsWithId const): (WebCore::CollectionNamedElementCache::findElementsWithName const): (WebCore::CollectionNamedElementCache::find const): (WebCore::CollectionNamedElementCache::append): Canonical link: https://commits.webkit.org/271949@main
WebKit · Dec 12, 2023 · 12e1fe4 · 12e1fe4
1 parent d3e0a4b
commit 12e1fe4
Show file tree

Hide file tree

Showing 13 changed files with 28 additions and 27 deletions.
diff --git a/Source/WebCore/dom/Attr.h b/Source/WebCore/dom/Attr.h
@@ -78,7 +78,7 @@ class Attr final : public Node {
 
     // Attr wraps either an element/name, or a name/value pair (when it's a standalone Node.)
     // Note that m_name is always set, but m_element/m_standaloneValue may be null.
-    CheckedPtr<Element> m_element;
+    WeakPtr<Element, WeakPtrImplWithEventTargetData> m_element;
     QualifiedName m_name;
     AtomString m_standaloneValue;
 

diff --git a/Source/WebCore/dom/DatasetDOMStringMap.h b/Source/WebCore/dom/DatasetDOMStringMap.h
@@ -27,7 +27,7 @@
 
 #include "ExceptionOr.h"
 #include "ScriptWrappable.h"
-#include <wtf/CheckedRef.h>
+#include <wtf/WeakRef.h>
 
 namespace WebCore {
 
@@ -57,7 +57,7 @@ class DatasetDOMStringMap final : public ScriptWrappable {
 private:
     const AtomString* item(const String& name) const;
 
-    CheckedRef<Element> m_element;
+    WeakRef<Element, WeakPtrImplWithEventTargetData> m_element;
 };
 
 } // namespace WebCore
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
@@ -7954,7 +7954,7 @@ static Element* findNearestCommonComposedAncestor(Element* elementA, Element* el
     if (elementA == elementB)
         return elementA;
 
-    HashSet<CheckedRef<Element>> ancestorChain;
+    HashSet<Ref<Element>> ancestorChain;
     for (auto* element = elementA; element; element = element->parentElementInComposedTree())
         ancestorChain.add(*element);
 
@@ -8841,12 +8841,12 @@ std::optional<FrameIdentifier> Document::frameID() const
 
 void Document::registerArticleElement(Element& article)
 {
-    m_articleElements.add(&article);
+    m_articleElements.add(article);
 }
 
 void Document::unregisterArticleElement(Element& article)
 {
-    m_articleElements.remove(&article);
+    m_articleElements.remove(article);
     if (m_mainArticleElement == &article)
         m_mainArticleElement = nullptr;
 }
@@ -8882,7 +8882,7 @@ void Document::updateMainArticleElementAfterLayout()
             secondTallestArticleHeight = tallestArticleHeight;
             tallestArticleHeight = height;
             tallestArticleWidth = box ? box->width().toFloat() : 0;
-            tallestArticle = article.get();
+            tallestArticle = article.ptr();
         } else if (height >= secondTallestArticleHeight)
             secondTallestArticleHeight = height;
     }

diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
@@ -2081,7 +2081,7 @@ class Document
 
     std::unique_ptr<Style::Update> m_pendingRenderTreeUpdate;
 
-    CheckedPtr<Element> m_cssTarget;
+    WeakPtr<Element, WeakPtrImplWithEventTargetData> m_cssTarget;
 
     std::unique_ptr<LazyLoadImageObserver> m_lazyLoadImageObserver;
 
@@ -2139,8 +2139,8 @@ class Document
     WeakPtr<HTMLMediaElement, WeakPtrImplWithEventTargetData> m_mediaElementShowingTextTrack;
 #endif
 
-    CheckedPtr<Element> m_mainArticleElement;
-    HashSet<CheckedPtr<Element>> m_articleElements;
+    WeakPtr<Element, WeakPtrImplWithEventTargetData> m_mainArticleElement;
+    HashSet<WeakRef<Element, WeakPtrImplWithEventTargetData>> m_articleElements;
 
     WeakHashSet<VisibilityChangeClient> m_visibilityStateCallbackClients;
 

diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
@@ -2896,7 +2896,7 @@ void Element::addShadowRoot(Ref<ShadowRoot>&& newShadowRoot)
 
         ensureElementRareData().setShadowRoot(WTFMove(newShadowRoot));
 
-        shadowRoot->setHost(this);
+        shadowRoot->setHost(*this);
         shadowRoot->setParentTreeScope(treeScope());
 
         NodeVector postInsertionNotificationTargets;

diff --git a/Source/WebCore/dom/NamedNodeMap.h b/Source/WebCore/dom/NamedNodeMap.h
@@ -27,6 +27,7 @@
 #include "Element.h"
 #include "ExceptionOr.h"
 #include "ScriptWrappable.h"
+#include <wtf/WeakRef.h>
 
 namespace WebCore {
 
@@ -59,7 +60,7 @@ class NamedNodeMap final : public ScriptWrappable {
     Ref<Element> protectedElement() const;
 
 private:
-    CheckedRef<Element> m_element;
+    WeakRef<Element, WeakPtrImplWithEventTargetData> m_element;
 };
 
 } // namespace WebCore
diff --git a/Source/WebCore/dom/RadioButtonGroups.cpp b/Source/WebCore/dom/RadioButtonGroups.cpp
@@ -47,8 +47,8 @@ class RadioButtonGroup {
     bool isValid() const;
     void setCheckedButton(HTMLInputElement*);
 
-    HashSet<CheckedRef<HTMLInputElement>> m_members;
-    CheckedPtr<HTMLInputElement> m_checkedButton;
+    HashSet<WeakRef<HTMLInputElement, WeakPtrImplWithEventTargetData>> m_members;
+    WeakPtr<HTMLInputElement, WeakPtrImplWithEventTargetData> m_checkedButton;
     size_t m_requiredCount { 0 };
 };
 

diff --git a/Source/WebCore/dom/ScriptElement.h b/Source/WebCore/dom/ScriptElement.h
@@ -128,7 +128,7 @@ class ScriptElement {
 
     virtual bool isScriptPreventedByAttributes() const { return false; }
 
-    CheckedRef<Element> m_element;
+    WeakRef<Element, WeakPtrImplWithEventTargetData> m_element;
     OrdinalNumber m_startLineNumber { OrdinalNumber::beforeFirst() };
     JSC::SourceTaintedOrigin m_taintedOrigin;
     ParserInserted m_parserInserted : bitWidthOfParserInserted;

diff --git a/Source/WebCore/dom/ShadowRoot.cpp b/Source/WebCore/dom/ShadowRoot.cpp
@@ -51,7 +51,7 @@ WTF_MAKE_ISO_ALLOCATED_IMPL(ShadowRoot);
 
 struct SameSizeAsShadowRoot : public DocumentFragment, public TreeScope {
     uint8_t flagsAndModes[3];
-    CheckedPtr<Element> host;
+    WeakPtr<Element, WeakPtrImplWithEventTargetData> host;
     void* styleSheetList;
     void* styleScope;
     void* slotAssignment;

diff --git a/Source/WebCore/dom/ShadowRoot.h b/Source/WebCore/dom/ShadowRoot.h
@@ -97,7 +97,7 @@ class ShadowRoot final : public DocumentFragment, public TreeScope {
 
     Element* host() const { return m_host.get(); }
     RefPtr<Element> protectedHost() const { return m_host.get(); }
-    void setHost(CheckedPtr<Element>&& host) { m_host = WTFMove(host); }
+    void setHost(WeakPtr<Element, WeakPtrImplWithEventTargetData>&& host) { m_host = WTFMove(host); }
 
     ExceptionOr<void> setHTMLUnsafe(const String&);
 
@@ -168,7 +168,7 @@ class ShadowRoot final : public DocumentFragment, public TreeScope {
     ShadowRootMode m_mode { ShadowRootMode::UserAgent };
     SlotAssignmentMode m_slotAssignmentMode { SlotAssignmentMode::Named };
 
-    CheckedPtr<Element> m_host;
+    WeakPtr<Element, WeakPtrImplWithEventTargetData> m_host;
     RefPtr<StyleSheetList> m_styleSheetList;
 
     std::unique_ptr<Style::Scope> m_styleScope;

diff --git a/Source/WebCore/dom/SimulatedClick.cpp b/Source/WebCore/dom/SimulatedClick.cpp
@@ -86,7 +86,7 @@ bool simulateClick(Element& element, Event* underlyingEvent, SimulatedClickMouse
     if (element.isDisabledFormControl())
         return false;
 
-    static MainThreadNeverDestroyed<HashSet<CheckedRef<Element>>> elementsDispatchingSimulatedClicks;
+    static MainThreadNeverDestroyed<HashSet<Ref<Element>>> elementsDispatchingSimulatedClicks;
     if (!elementsDispatchingSimulatedClicks.get().add(element).isNewEntry)
         return false;
 

diff --git a/Source/WebCore/html/HTMLCollection.h b/Source/WebCore/html/HTMLCollection.h
@@ -33,8 +33,8 @@ namespace WebCore {
 class CollectionNamedElementCache {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    inline const Vector<CheckedRef<Element>>* findElementsWithId(const AtomString& id) const;
-    inline const Vector<CheckedRef<Element>>* findElementsWithName(const AtomString& name) const;
+    inline const Vector<WeakRef<Element, WeakPtrImplWithEventTargetData>>* findElementsWithId(const AtomString& id) const;
+    inline const Vector<WeakRef<Element, WeakPtrImplWithEventTargetData>>* findElementsWithName(const AtomString& name) const;
     const Vector<AtomString>& propertyNames() const { return m_propertyNames; }
 
     inline void appendToIdCache(const AtomString& id, Element&);
@@ -44,9 +44,9 @@ class CollectionNamedElementCache {
     inline size_t memoryCost() const;
 
 private:
-    typedef HashMap<AtomStringImpl*, Vector<CheckedRef<Element>>> StringToElementsMap;
+    typedef HashMap<AtomStringImpl*, Vector<WeakRef<Element, WeakPtrImplWithEventTargetData>>> StringToElementsMap;
 
-    inline const Vector<CheckedRef<Element>>* find(const StringToElementsMap&, const AtomString& key) const;
+    inline const Vector<WeakRef<Element, WeakPtrImplWithEventTargetData>>* find(const StringToElementsMap&, const AtomString& key) const;
     inline void append(StringToElementsMap&, const AtomString& key, Element&);
 
     StringToElementsMap m_idMap;

diff --git a/Source/WebCore/html/HTMLCollectionInlines.h b/Source/WebCore/html/HTMLCollectionInlines.h
@@ -37,12 +37,12 @@ inline ContainerNode& HTMLCollection::rootNode() const
     return ownerNode();
 }
 
-inline const Vector<CheckedRef<Element>>* CollectionNamedElementCache::findElementsWithId(const AtomString& id) const
+inline const Vector<WeakRef<Element, WeakPtrImplWithEventTargetData>>* CollectionNamedElementCache::findElementsWithId(const AtomString& id) const
 {
     return find(m_idMap, id);
 }
 
-inline const Vector<CheckedRef<Element>>* CollectionNamedElementCache::findElementsWithName(const AtomString& name) const
+inline const Vector<WeakRef<Element, WeakPtrImplWithEventTargetData>>* CollectionNamedElementCache::findElementsWithName(const AtomString& name) const
 {
     return find(m_nameMap, name);
 }
@@ -66,7 +66,7 @@ inline void CollectionNamedElementCache::didPopulate()
         reportExtraMemoryAllocatedForCollectionIndexCache(cost);
 }
 
-inline const Vector<CheckedRef<Element>>* CollectionNamedElementCache::find(const StringToElementsMap& map, const AtomString& key) const
+inline const Vector<WeakRef<Element, WeakPtrImplWithEventTargetData>>* CollectionNamedElementCache::find(const StringToElementsMap& map, const AtomString& key) const
 {
     ASSERT(m_didPopulate);
     auto it = map.find(key.impl());
@@ -77,7 +77,7 @@ inline void CollectionNamedElementCache::append(StringToElementsMap& map, const
 {
     if (!m_idMap.contains(key.impl()) && !m_nameMap.contains(key.impl()))
         m_propertyNames.append(key);
-    map.add(key.impl(), Vector<CheckedRef<Element>>()).iterator->value.append(element);
+    map.add(key.impl(), Vector<WeakRef<Element, WeakPtrImplWithEventTargetData>>()).iterator->value.append(element);
 }
 
 inline bool HTMLCollection::isRootedAtTreeScope() const