Skip to content

Commit 9e25f51

Browse files
cdumezcdumez
committed
Adopt more smart pointers in WebCore/
https://bugs.webkit.org/show_bug.cgi?id=263268 Reviewed by Ryosuke Niwa. * Source/WebCore/dom/ContainerNode.h: (WebCore::ContainerNode::protectedRootNode const): * Source/WebCore/dom/CurrentScriptIncrementer.h: (WebCore::CurrentScriptIncrementer::CurrentScriptIncrementer): (WebCore::CurrentScriptIncrementer::~CurrentScriptIncrementer): (WebCore::CurrentScriptIncrementer::protectedDocument const): * Source/WebCore/dom/CustomElementDefaultARIA.cpp: (WebCore::isElementVisible): (WebCore::CustomElementDefaultARIA::valueForAttribute const): (WebCore::CustomElementDefaultARIA::setElementsForAttribute): * Source/WebCore/dom/CustomElementReactionQueue.cpp: (WebCore::CustomElementReactionQueue::tryToUpgradeElement): (WebCore::CustomElementQueue::invokeAll): (WebCore::CustomElementReactionQueue::enqueueElementOnAppropriateElementQueue): (WebCore::CustomElementReactionStack::processQueue): (WebCore::CustomElementReactionStack::takeElements): * Source/WebCore/dom/CustomElementReactionQueue.h: * Source/WebCore/dom/CustomElementRegistry.cpp: (WebCore::CustomElementRegistry::document const): (WebCore::enqueueUpgradeInShadowIncludingTreeOrder): (WebCore::CustomElementRegistry::addElementDefinition): (WebCore::CustomElementRegistry::get): (WebCore::CustomElementRegistry::getName): (WebCore::upgradeElementsInShadowIncludingDescendants): * Source/WebCore/dom/CustomElementRegistry.h: * Source/WebCore/dom/DatasetDOMStringMap.cpp: (WebCore::DatasetDOMStringMap::ref): (WebCore::DatasetDOMStringMap::deref): (WebCore::DatasetDOMStringMap::isSupportedPropertyName const): (WebCore::DatasetDOMStringMap::supportedPropertyNames const): (WebCore::DatasetDOMStringMap::item const): (WebCore::DatasetDOMStringMap::setNamedItem): (WebCore::DatasetDOMStringMap::deleteNamedProperty): (WebCore::DatasetDOMStringMap::protectedElement const): * Source/WebCore/dom/DatasetDOMStringMap.h: Canonical link: https://commits.webkit.org/269465@main
1 parent 1b7f44c commit 9e25f51

10 files changed

+69
-59
lines changed

‎Source/WebCore/bindings/js/JSHTMLElementCustom.cpp

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,36 +65,33 @@ EncodedJSValue constructJSHTMLElement(JSGlobalObject* lexicalGlobalObject, CallF
6565
if (newTarget == htmlElementConstructorValue)
6666
return throwVMTypeError(lexicalGlobalObject, scope, "new.target is not a valid custom element constructor"_s);
6767

68-
auto& document = downcast<Document>(*context);
68+
Ref document = downcast<Document>(*context);
6969

70-
auto* window = document.domWindow();
70+
RefPtr window = document->domWindow();
7171
if (!window)
7272
return throwVMTypeError(lexicalGlobalObject, scope, "new.target is not a valid custom element constructor"_s);
7373

74-
auto* registry = window->customElementRegistry();
74+
RefPtr registry = window->customElementRegistry();
7575
if (!registry)
7676
return throwVMTypeError(lexicalGlobalObject, scope, "new.target is not a valid custom element constructor"_s);
7777

78-
auto* elementInterface = registry->findInterface(newTarget);
78+
RefPtr elementInterface = registry->findInterface(newTarget);
7979
if (!elementInterface)
8080
return throwVMTypeError(lexicalGlobalObject, scope, "new.target does not define a custom element"_s);
8181

8282
if (!elementInterface->isUpgradingElement()) {
83-
Ref<Document> protectedDocument(document);
84-
Ref<JSCustomElementInterface> protectedElementInterface(*elementInterface);
85-
8683
Structure* baseStructure = getDOMStructure<JSHTMLElement>(vm, *newTargetGlobalObject);
8784
auto* newElementStructure = InternalFunction::createSubclassStructure(lexicalGlobalObject, newTarget, baseStructure);
8885
RETURN_IF_EXCEPTION(scope, { });
8986

90-
Ref<HTMLElement> element = elementInterface->createElement(document);
87+
Ref element = elementInterface->createElement(document);
9188
element->setIsDefinedCustomElement(*elementInterface);
9289
auto* jsElement = JSHTMLElement::create(newElementStructure, newTargetGlobalObject, element.get());
9390
cacheWrapper(newTargetGlobalObject->world(), element.ptr(), jsElement);
9491
return JSValue::encode(jsElement);
9592
}
9693

97-
Element* elementToUpgrade = elementInterface->lastElementInConstructionStack();
94+
RefPtr elementToUpgrade = elementInterface->lastElementInConstructionStack();
9895
if (!elementToUpgrade) {
9996
throwTypeError(lexicalGlobalObject, scope, "Cannot instantiate a custom element inside its own constructor during upgrades"_s);
10097
return JSValue::encode(jsUndefined());

‎Source/WebCore/dom/ContainerNode.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ class ContainerNode : public Node {
6363
void replaceAll(Node*);
6464

6565
ContainerNode& rootNode() const { return downcast<ContainerNode>(Node::rootNode()); }
66+
Ref<ContainerNode> protectedRootNode() const { return downcast<ContainerNode>(Node::rootNode()); }
6667

6768
// These methods are only used during parsing.
6869
// They don't send DOM mutation events or handle reparenting.

‎Source/WebCore/dom/CurrentScriptIncrementer.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
#pragma once
3030

31+
#include <wtf/CheckedRef.h>
3132
#include "Document.h"
3233
#include "ScriptElement.h"
3334

@@ -40,16 +41,18 @@ class CurrentScriptIncrementer {
4041
: m_document(document)
4142
{
4243
bool shouldPushNullForCurrentScript = scriptElement.element().isInShadowTree() || scriptElement.scriptType() != ScriptType::Classic;
43-
m_document.pushCurrentScript(shouldPushNullForCurrentScript ? nullptr : &scriptElement.element());
44+
protectedDocument()->pushCurrentScript(shouldPushNullForCurrentScript ? nullptr : &scriptElement.element());
4445
}
4546

4647
~CurrentScriptIncrementer()
4748
{
48-
m_document.popCurrentScript();
49+
protectedDocument()->popCurrentScript();
4950
}
5051

5152
private:
52-
Document& m_document;
53+
Ref<Document> protectedDocument() const { return m_document.get(); }
54+
55+
CheckedRef<Document> m_document;
5356
};
5457

5558
} // namespace WebCore

‎Source/WebCore/dom/CustomElementDefaultARIA.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ void CustomElementDefaultARIA::setValueForAttribute(const QualifiedName& name, c
4747

4848
static bool isElementVisible(const Element& element, const Element& thisElement)
4949
{
50-
return !element.isConnected() || element.isInDocumentTree() || thisElement.isDescendantOrShadowDescendantOf(element.rootNode());
50+
return !element.isConnected() || element.isInDocumentTree() || thisElement.isDescendantOrShadowDescendantOf(element.protectedRootNode());
5151
}
5252

5353
const AtomString& CustomElementDefaultARIA::valueForAttribute(const Element& thisElement, const QualifiedName& name) const
@@ -56,14 +56,14 @@ const AtomString& CustomElementDefaultARIA::valueForAttribute(const Element& thi
5656
if (it == m_map.end())
5757
return nullAtom();
5858

59-
const AtomString* result = &nullAtom();
60-
std::visit(WTF::makeVisitor([&](const AtomString& stringValue) {
61-
result = &stringValue;
62-
}, [&](const WeakPtr<Element, WeakPtrImplWithEventTargetData>& weakElementValue) {
59+
return std::visit(WTF::makeVisitor([&](const AtomString& stringValue) -> const AtomString& {
60+
return stringValue;
61+
}, [&](const WeakPtr<Element, WeakPtrImplWithEventTargetData>& weakElementValue) -> const AtomString& {
6362
RefPtr elementValue = weakElementValue.get();
6463
if (elementValue && isElementVisible(*elementValue, thisElement))
65-
result = &elementValue->attributeWithoutSynchronization(HTMLNames::idAttr);
66-
}, [&](const Vector<WeakPtr<Element, WeakPtrImplWithEventTargetData>>& elements) {
64+
return elementValue->attributeWithoutSynchronization(HTMLNames::idAttr);
65+
return nullAtom();
66+
}, [&](const Vector<WeakPtr<Element, WeakPtrImplWithEventTargetData>>& elements) -> const AtomString& {
6767
StringBuilder idList;
6868
for (auto& weakElement : elements) {
6969
RefPtr element = weakElement.get();
@@ -73,9 +73,9 @@ const AtomString& CustomElementDefaultARIA::valueForAttribute(const Element& thi
7373
idList.append(element->attributeWithoutSynchronization(HTMLNames::idAttr));
7474
}
7575
}
76+
// FIXME: This should probably be using the idList we just built.
77+
return nullAtom();
7678
}), it->value);
77-
78-
return *result;
7979
}
8080

8181
bool CustomElementDefaultARIA::hasAttribute(const QualifiedName& name) const
@@ -144,7 +144,7 @@ void CustomElementDefaultARIA::setElementsForAttribute(const QualifiedName& name
144144
elements.append(WeakPtr<Element, WeakPtrImplWithEventTargetData> { *element });
145145
}
146146
}
147-
m_map.set(name, elements);
147+
m_map.set(name, WTFMove(elements));
148148
}
149149

150-
}; // namespace WebCore
150+
} // namespace WebCore

‎Source/WebCore/dom/CustomElementReactionQueue.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -142,15 +142,15 @@ void CustomElementReactionQueue::tryToUpgradeElement(Element& element)
142142
{
143143
ASSERT(CustomElementReactionDisallowedScope::isReactionAllowed());
144144
ASSERT(element.isCustomElementUpgradeCandidate());
145-
auto* window = element.document().domWindow();
145+
RefPtr window = element.document().domWindow();
146146
if (!window)
147147
return;
148148

149-
auto* registry = window->customElementRegistry();
149+
RefPtr registry = window->customElementRegistry();
150150
if (!registry)
151151
return;
152152

153-
auto* elementInterface = registry->findInterface(element);
153+
RefPtr elementInterface = registry->findInterface(element);
154154
if (!elementInterface)
155155
return;
156156

@@ -337,8 +337,8 @@ inline void CustomElementQueue::invokeAll()
337337
// It's possible for more elements to be enqueued if some IDL attributes were missing CEReactions.
338338
// Invoke callbacks slightly later here instead of crashing / ignoring those cases.
339339
for (unsigned i = 0; i < m_elements.size(); ++i) {
340-
auto& element = m_elements[i].get();
341-
auto* queue = element.reactionQueue();
340+
Ref element = m_elements[i].get();
341+
auto* queue = element->reactionQueue();
342342
ASSERT(queue);
343343
queue->invokeAll(element);
344344
}
@@ -393,9 +393,9 @@ void CustomElementReactionQueue::enqueueElementOnAppropriateElementQueue(Element
393393
return;
394394
}
395395

396-
auto*& queue = CustomElementReactionStack::s_currentProcessingStack->m_queue;
397-
if (!queue) // We use a raw pointer to avoid genearing code to delete it in ~CustomElementReactionStack.
398-
queue = new CustomElementQueue;
396+
auto& queue = CustomElementReactionStack::s_currentProcessingStack->m_queue;
397+
if (!queue)
398+
queue = makeUnique<CustomElementQueue>();
399399
queue->add(element);
400400
}
401401

@@ -409,7 +409,6 @@ void CustomElementReactionStack::processQueue(JSC::JSGlobalObject* state)
409409
{
410410
ASSERT(m_queue);
411411
m_queue->processQueue(state);
412-
delete m_queue;
413412
m_queue = nullptr;
414413
}
415414

@@ -418,7 +417,6 @@ Vector<GCReachableRef<Element>, 4> CustomElementReactionStack::takeElements()
418417
if (!m_queue)
419418
return { };
420419
auto elements = m_queue->takeElements();
421-
delete m_queue;
422420
m_queue = nullptr;
423421
return elements;
424422
}

‎Source/WebCore/dom/CustomElementReactionQueue.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ class CustomElementQueue {
9999
WTF_MAKE_NONCOPYABLE(CustomElementQueue);
100100
public:
101101
CustomElementQueue();
102-
~CustomElementQueue();
102+
WEBCORE_EXPORT ~CustomElementQueue();
103103

104104
void add(Element&);
105105
void processQueue(JSC::JSGlobalObject*);
@@ -233,7 +233,7 @@ class CustomElementReactionStack : public CustomElementReactionDisallowedScope::
233233
private:
234234
WEBCORE_EXPORT void processQueue(JSC::JSGlobalObject*);
235235

236-
CustomElementQueue* m_queue { nullptr }; // Use raw pointer to avoid generating delete in the destructor.
236+
std::unique_ptr<CustomElementQueue> m_queue;
237237
CustomElementReactionStack* const m_previousProcessingStack;
238238
JSC::JSGlobalObject* const m_state;
239239

‎Source/WebCore/dom/CustomElementRegistry.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,16 @@ CustomElementRegistry::~CustomElementRegistry() = default;
5858

5959
Document* CustomElementRegistry::document() const
6060
{
61-
return m_window.document();
61+
return m_window ? m_window->document() : nullptr;
6262
}
6363

6464
// https://dom.spec.whatwg.org/#concept-shadow-including-tree-order
6565
static void enqueueUpgradeInShadowIncludingTreeOrder(ContainerNode& node, JSCustomElementInterface& elementInterface)
6666
{
67-
for (Element* element = ElementTraversal::firstWithin(node); element; element = ElementTraversal::next(*element)) {
67+
for (RefPtr element = ElementTraversal::firstWithin(node); element; element = ElementTraversal::next(*element)) {
6868
if (element->isCustomElementUpgradeCandidate() && element->tagQName().matches(elementInterface.name()))
6969
element->enqueueToUpgrade(elementInterface);
70-
if (auto* shadowRoot = element->shadowRoot()) {
70+
if (RefPtr shadowRoot = element->shadowRoot()) {
7171
if (shadowRoot->mode() != ShadowRootMode::UserAgent)
7272
enqueueUpgradeInShadowIncludingTreeOrder(*shadowRoot, elementInterface);
7373
}
@@ -89,7 +89,7 @@ RefPtr<DeferredPromise> CustomElementRegistry::addElementDefinition(Ref<JSCustom
8989
if (elementInterface->isShadowDisabled())
9090
m_disabledShadowSet.add(localName);
9191

92-
if (auto* document = m_window.document()) {
92+
if (RefPtr document = this->document()) {
9393
// ungap/@custom-elements detection for quirk (rdar://problem/111008826).
9494
if (localName == extendsLi.get())
9595
document->quirks().setNeedsConfigurableIndexedPropertiesQuirk();
@@ -116,7 +116,7 @@ JSCustomElementInterface* CustomElementRegistry::findInterface(const AtomString&
116116
return m_nameMap.get(name);
117117
}
118118

119-
JSCustomElementInterface* CustomElementRegistry::findInterface(const JSC::JSObject* constructor) const
119+
RefPtr<JSCustomElementInterface> CustomElementRegistry::findInterface(const JSC::JSObject* constructor) const
120120
{
121121
Locker locker { m_constructorMapLock };
122122
return m_constructorMap.get(constructor);
@@ -130,7 +130,7 @@ bool CustomElementRegistry::containsConstructor(const JSC::JSObject* constructor
130130

131131
JSC::JSValue CustomElementRegistry::get(const AtomString& name)
132132
{
133-
if (auto* elementInterface = m_nameMap.get(name))
133+
if (RefPtr elementInterface = m_nameMap.get(name))
134134
return elementInterface->constructor();
135135
return JSC::jsUndefined();
136136
}
@@ -140,18 +140,18 @@ String CustomElementRegistry::getName(JSC::JSValue constructorValue)
140140
auto* constructor = constructorValue.getObject();
141141
if (!constructor)
142142
return String { };
143-
auto* elementInterface = findInterface(constructor);
143+
RefPtr elementInterface = findInterface(constructor);
144144
if (!elementInterface)
145145
return String { };
146146
return elementInterface->name().localName();
147147
}
148148

149149
static void upgradeElementsInShadowIncludingDescendants(ContainerNode& root)
150150
{
151-
for (auto& element : descendantsOfType<Element>(root)) {
152-
if (element.isCustomElementUpgradeCandidate())
151+
for (Ref element : descendantsOfType<Element>(root)) {
152+
if (element->isCustomElementUpgradeCandidate())
153153
CustomElementReactionQueue::tryToUpgradeElement(element);
154-
if (auto* shadowRoot = element.shadowRoot())
154+
if (RefPtr shadowRoot = element->shadowRoot())
155155
upgradeElementsInShadowIncludingDescendants(*shadowRoot);
156156
}
157157
}

‎Source/WebCore/dom/CustomElementRegistry.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <wtf/Lock.h>
3131
#include <wtf/RobinHoodHashMap.h>
3232
#include <wtf/RobinHoodHashSet.h>
33+
#include <wtf/WeakPtr.h>
3334
#include <wtf/text/AtomString.h>
3435
#include <wtf/text/AtomStringHash.h>
3536

@@ -66,7 +67,7 @@ class CustomElementRegistry : public RefCounted<CustomElementRegistry>, public C
6667
JSCustomElementInterface* findInterface(const Element&) const;
6768
JSCustomElementInterface* findInterface(const QualifiedName&) const;
6869
JSCustomElementInterface* findInterface(const AtomString&) const;
69-
JSCustomElementInterface* findInterface(const JSC::JSObject*) const;
70+
RefPtr<JSCustomElementInterface> findInterface(const JSC::JSObject*) const;
7071
bool containsConstructor(const JSC::JSObject*) const;
7172

7273
JSC::JSValue get(const AtomString&);
@@ -80,7 +81,7 @@ class CustomElementRegistry : public RefCounted<CustomElementRegistry>, public C
8081
private:
8182
CustomElementRegistry(LocalDOMWindow&, ScriptExecutionContext*);
8283

83-
LocalDOMWindow& m_window;
84+
WeakPtr<LocalDOMWindow, WeakPtrImplWithEventTargetData> m_window;
8485
HashMap<AtomString, Ref<JSCustomElementInterface>> m_nameMap;
8586
HashMap<const JSC::JSObject*, JSCustomElementInterface*> m_constructorMap WTF_GUARDED_BY_LOCK(m_constructorMapLock);
8687
MemoryCompactRobinHoodHashMap<AtomString, Ref<DeferredPromise>> m_promiseMap;

‎Source/WebCore/dom/DatasetDOMStringMap.cpp

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -118,20 +118,21 @@ static AtomString convertPropertyNameToAttributeName(const String& name)
118118

119119
void DatasetDOMStringMap::ref()
120120
{
121-
m_element.ref();
121+
m_element->ref();
122122
}
123123

124124
void DatasetDOMStringMap::deref()
125125
{
126-
m_element.deref();
126+
m_element->deref();
127127
}
128128

129129
bool DatasetDOMStringMap::isSupportedPropertyName(const String& propertyName) const
130130
{
131-
if (!m_element.hasAttributes())
131+
Ref element = m_element.get();
132+
if (!element->hasAttributes())
132133
return false;
133134

134-
auto attributeIteratorAccessor = m_element.attributesIterator();
135+
auto attributeIteratorAccessor = element->attributesIterator();
135136
if (attributeIteratorAccessor.attributeCount() == 1) {
136137
// Avoid creating AtomString when there is only one attribute.
137138
const auto& attribute = *attributeIteratorAccessor.begin();
@@ -152,10 +153,11 @@ Vector<String> DatasetDOMStringMap::supportedPropertyNames() const
152153
{
153154
Vector<String> names;
154155

155-
if (!m_element.hasAttributes())
156+
Ref element = m_element.get();
157+
if (!element->hasAttributes())
156158
return names;
157159

158-
for (auto& attribute : m_element.attributesIterator()) {
160+
for (auto& attribute : element->attributesIterator()) {
159161
if (isValidAttributeName(attribute.localName()))
160162
names.append(convertAttributeNameToPropertyName(attribute.localName()));
161163
}
@@ -165,8 +167,9 @@ Vector<String> DatasetDOMStringMap::supportedPropertyNames() const
165167

166168
const AtomString* DatasetDOMStringMap::item(const String& propertyName) const
167169
{
168-
if (m_element.hasAttributes()) {
169-
AttributeIteratorAccessor attributeIteratorAccessor = m_element.attributesIterator();
170+
Ref element = m_element.get();
171+
if (element->hasAttributes()) {
172+
AttributeIteratorAccessor attributeIteratorAccessor = element->attributesIterator();
170173

171174
if (attributeIteratorAccessor.attributeCount() == 1) {
172175
// Avoid creating AtomString when there is only one attribute.
@@ -196,12 +199,17 @@ ExceptionOr<void> DatasetDOMStringMap::setNamedItem(const String& name, const At
196199
{
197200
if (!isValidPropertyName(name))
198201
return Exception { SyntaxError };
199-
return m_element.setAttribute(convertPropertyNameToAttributeName(name), value);
202+
return protectedElement()->setAttribute(convertPropertyNameToAttributeName(name), value);
200203
}
201204

202205
bool DatasetDOMStringMap::deleteNamedProperty(const String& name)
203206
{
204-
return m_element.removeAttribute(convertPropertyNameToAttributeName(name));
207+
return protectedElement()->removeAttribute(convertPropertyNameToAttributeName(name));
208+
}
209+
210+
Ref<Element> DatasetDOMStringMap::protectedElement() const
211+
{
212+
return m_element.get();
205213
}
206214

207215
} // namespace WebCore

0 commit comments

Comments
 (0)