Address review comments

alice · alice · commit 23e800a6d888 · 2025-01-22T14:25:31.000+11:00
diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp
@@ -174,11 +174,8 @@ static bool nodeAndRendererAreValid(Node* node)
 
 static RefPtr<Element> getElementByIdWithReferenceTarget(const TreeScope& treeScope, const AtomString& elementId)
 {
-    if (elementId.isNull())
-        return nullptr;
-    
     RefPtr<Element> element = treeScope.getElementById(elementId);
-    return element->deepShadowRootReferenceTargetOrSelf();
+    return element ? element->deepShadowRootReferenceTargetOrSelf() : nullptr;
 }
 
 AccessibilityObjectInclusion AXComputedObjectAttributeCache::getIgnored(AXID id) const
diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -3252,15 +3252,8 @@ String AccessibilityObject::popupValue() const
 
 bool AccessibilityObject::hasDatalist() const
 {
-    auto datalistId = getAttribute(listAttr);
-    if (datalistId.isEmpty())
-        return false;
-
-    auto element = this->element();
-    if (!element || !is<HTMLInputElement>(element))
-        return false;
-
-    return dynamicDowncast<HTMLInputElement>(element)->hasDataList();
+    RefPtr input = dynamicDowncast<HTMLInputElement>(element());
+    return input && input->hasDataList();
 }
 
 bool AccessibilityObject::supportsSetSize() const
@@ -4051,11 +4044,11 @@ Vector<Ref<Element>> AccessibilityObject::elementsFromAttribute(const QualifiedN
         return elementsFromAttribute.value();
     }
 
-        if (auto* defaultARIA = element->customElementDefaultARIAIfExists()) {
-            return defaultARIA->elementsForAttribute(*element, attribute);
-        }
+    if (auto* defaultARIA = element->customElementDefaultARIAIfExists()) {
+        return defaultARIA->elementsForAttribute(*element, attribute);
+    }
 
-        return { };
+    return { };
 }
 
 #if PLATFORM(COCOA)
diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
@@ -2329,7 +2329,7 @@ ExplicitlySetAttrElementsMap* Element::explicitlySetAttrElementsMapIfExists() co
 
 static RefPtr<Element> getElementByIdInternalIncludingDisconnected(const Element& startElement, const AtomString& id)
 {
-    if (id.isNull() || id.isEmpty())
+    if (id.isEmpty())
         return nullptr;
 
     if (LIKELY(startElement.isInTreeScope()))
@@ -2366,7 +2366,7 @@ RefPtr<Element> Element::getElementForAttributeInternal(const QualifiedName& att
     }
     
     if (!hasExplicitlySetElement) {
-    const AtomString& id = getAttribute(attributeName);
+        const AtomString& id = getAttribute(attributeName);
         element = getElementByIdInternalIncludingDisconnected(*this, id);
     }
 
@@ -2379,16 +2379,13 @@ RefPtr<Element> Element::getElementForAttributeInternal(const QualifiedName& att
 RefPtr<Element> Element::getElementAttributeForBindings(const QualifiedName& attributeName) const
 {
     ASSERT(isElementReflectionAttribute(document().settings(), attributeName));
-    RefPtr<Element> element = getElementForAttributeInternal(attributeName);
+    RefPtr element = getElementForAttributeInternal(attributeName);
 
     if (!element)
         return nullptr;
 
-    if (document().settings().shadowRootReferenceTargetEnabled()) {
-        Ref<Node> retargeted = treeScope().retargetToScope(*element);
-        ASSERT(retargeted->isElementNode());
-        return dynamicDowncast<Element>(retargeted);
-    }
+    if (document().settings().shadowRootReferenceTargetEnabled())
+        return downcast<Element>(treeScope().retargetToScope(*element));
 
     return element;
 }
@@ -2446,10 +2443,9 @@ std::optional<Vector<Ref<Element>>> Element::getElementsArrayForAttributeInterna
 
     if (document().settings().shadowRootReferenceTargetEnabled()) {
         elements = compactMap(elements.value(), [&](Ref<Element>& element) -> std::optional<Ref<Element>> {
-            RefPtr<Element> deepReferenceTarget = element->deepShadowRootReferenceTargetOrSelf();
-            if (!deepReferenceTarget)
-                return std::nullopt;
-            return *deepReferenceTarget;
+            if (RefPtr deepReferenceTarget = element->deepShadowRootReferenceTargetOrSelf())
+                return *deepReferenceTarget;
+            return std::nullopt;
         });
     }
 
@@ -2466,9 +2462,7 @@ std::optional<Vector<Ref<Element>>> Element::getElementsArrayAttributeForBinding
 
     if (document().settings().shadowRootReferenceTargetEnabled()) {
         elements = compactMap(elements.value(), [&](Ref<Element>& element) -> std::optional<Ref<Element>> {
-            Ref<Node> retargeted = treeScope().retargetToScope(element);
-            ASSERT(retargeted->isElementNode());
-            return *dynamicDowncast<Element>(retargeted);
+            return downcast<Element>(treeScope().retargetToScope(element));
         });
     }
 
@@ -3261,8 +3255,7 @@ ExceptionOr<ShadowRoot&> Element::attachShadow(const ShadowRootInit& init)
         init.serializable ? ShadowRoot::Serializable::Yes : ShadowRoot::Serializable::No,
         isPrecustomizedOrDefinedCustomElement() ? ShadowRoot::AvailableToElementInternals::Yes : ShadowRoot::AvailableToElementInternals::No,
         WTFMove(registry), init.registry ? ShadowRoot::ScopedCustomElementRegistry::Yes : ShadowRoot::ScopedCustomElementRegistry::No);
-    if (document().settings().shadowRootReferenceTargetEnabled())
-        shadow->setReferenceTarget(AtomString(init.referenceTarget));
+    shadow->setReferenceTarget(AtomString(init.referenceTarget));
     addShadowRoot(shadow.copyRef());
     return shadow.get();
 }
@@ -3310,12 +3303,10 @@ RefPtr<const Element> Element::deepShadowRootReferenceTargetOrSelf() const {
 
     RefPtr<const Element> element = this;
 
-    ShadowRoot* shadow = shadowRoot();
+    RefPtr shadow = shadowRoot();
     while (shadow && shadow->hasReferenceTarget()) {
         element = shadow->referenceTargetElementOrHost();
-        if (!element)
-            return nullptr;
-        shadow = element->shadowRoot();
+        shadow = element ? element->shadowRoot() : nullptr;
     }
     return element;
 }
diff --git a/Source/WebCore/html/FormAssociatedElement.cpp b/Source/WebCore/html/FormAssociatedElement.cpp
@@ -37,14 +37,13 @@ FormAssociatedElement::FormAssociatedElement(HTMLFormElement* form)
 
 HTMLFormElement* FormAssociatedElement::formForBindings() const
 {
-    HTMLFormElement* formElement = form();
-    if (!formElement)
-        return nullptr;
-
-    if (asHTMLElement().document().settings().shadowRootReferenceTargetEnabled() && &formElement->treeScope() != &asHTMLElement().treeScope())
-        return nullptr;
-
-    return formElement;
+    if (asHTMLElement().document().settings().shadowRootReferenceTargetEnabled()) {
+        auto* form = this->form();
+        if (!form || &form->treeScope() != &asHTMLElement().treeScope())
+            return nullptr;
+        return form;
+    }
+    return form();
 }
 
 void FormAssociatedElement::setFormInternal(RefPtr<HTMLFormElement>&& newForm)
diff --git a/Source/WebCore/html/HTMLLabelElement.cpp b/Source/WebCore/html/HTMLLabelElement.cpp
@@ -79,15 +79,11 @@ RefPtr<HTMLElement> HTMLLabelElement::control() const
         // the form element must be "labelable form-associated element".
         for (const HTMLElement& descendant : descendantsOfType<HTMLElement>(*this)) {
             if (document().settings().shadowRootReferenceTargetEnabled()) {
-                RefPtr<const Element> referenceTarget = descendant.deepShadowRootReferenceTargetOrSelf();
-                if (!referenceTarget || !is<HTMLElement>(referenceTarget))
-                    continue;
-                const HTMLElement* htmlReferenceTarget = dynamicDowncast<HTMLElement>(referenceTarget.get());
-                if (htmlReferenceTarget->isLabelable())
-                    return const_cast<HTMLElement*>(htmlReferenceTarget);
-            } else {
-                if (descendant.isLabelable())
-                    return const_cast<HTMLElement*>(&descendant);
+                RefPtr referenceTarget = dynamicDowncast<const HTMLElement>(descendant.deepShadowRootReferenceTargetOrSelf());
+                if (referenceTarget && referenceTarget->isLabelable())
+                    return const_cast<HTMLElement*>(referenceTarget.get());
+            } else if (descendant.isLabelable()) {
+                return const_cast<HTMLElement*>(&descendant);
             }
         }
         return nullptr;
@@ -97,18 +93,16 @@ RefPtr<HTMLElement> HTMLLabelElement::control() const
 
 RefPtr<HTMLElement> HTMLLabelElement::controlForBindings() const
 {
-    RefPtr<HTMLElement> controlElement = control();
-
-    if (!controlElement)
-        return nullptr;
-
     if (document().settings().shadowRootReferenceTargetEnabled()) {
-        Ref<Node> retargeted = treeScope().retargetToScope(*controlElement);
-        ASSERT(retargeted->isHTMLElement());
-        controlElement = dynamicDowncast<HTMLElement>(retargeted);
+        RefPtr<HTMLElement> control = this->control();
+        if (!control)
+            return nullptr;
+
+        Ref<Node> retargeted = treeScope().retargetToScope(*control);
+        return downcast<HTMLElement>(retargeted);
     }
 
-    return controlElement;
+    return control();
 }
 
 HTMLFormElement* HTMLLabelElement::form() const
@@ -122,15 +116,13 @@ HTMLFormElement* HTMLLabelElement::form() const
 
 HTMLFormElement* HTMLLabelElement::formForBindings() const
 {
-    HTMLFormElement* formElement = form();
-
-    if (!formElement)
-        return nullptr;
-
-    if (document().settings().shadowRootReferenceTargetEnabled() && &formElement->treeScope() != &treeScope())
-        return nullptr;
-
-    return formElement;
+    if (document().settings().shadowRootReferenceTargetEnabled()) {
+        auto* form = this->form();
+        if (!form || &form->treeScope() != &treeScope())
+            return nullptr;
+        return form;
+    }
+    return form();
 }
 
 void HTMLLabelElement::setActive(bool down, Style::InvalidationScope invalidationScope)
diff --git a/Source/WebCore/html/HTMLLegendElement.cpp b/Source/WebCore/html/HTMLLegendElement.cpp
@@ -58,15 +58,13 @@ HTMLFormElement* HTMLLegendElement::form() const
 
 HTMLFormElement* HTMLLegendElement::formForBindings() const
 {
-    HTMLFormElement* formElement = form();
-
-    if (!formElement)
-        return nullptr;
-
-    if (document().settings().shadowRootReferenceTargetEnabled() && &formElement->treeScope() != &treeScope())
-        return nullptr;
-
-    return formElement;
+    if (document().settings().shadowRootReferenceTargetEnabled()) {
+        auto* form = this->form();
+        if (!form || &form->treeScope() != &treeScope())
+            return nullptr;
+        return form;
+    }
+    return form();
 }
     
 } // namespace
diff --git a/Source/WebCore/html/HTMLOptionElement.cpp b/Source/WebCore/html/HTMLOptionElement.cpp
@@ -147,15 +147,13 @@ HTMLFormElement* HTMLOptionElement::form() const
 
 HTMLFormElement* HTMLOptionElement::formForBindings() const
 {
-    HTMLFormElement* formElement = form();
-
-    if (!formElement)
-        return nullptr;
-
-    if (document().settings().shadowRootReferenceTargetEnabled() && &formElement->treeScope() != &treeScope())
-        return nullptr;
-
-    return formElement;
+    if (document().settings().shadowRootReferenceTargetEnabled()) {
+        auto* form = this->form();
+        if (!form || &form->treeScope() != &treeScope())
+            return nullptr;
+        return form;
+    }
+    return form();
 }
 
 int HTMLOptionElement::index() const

Original file line number	Diff line number	Diff line change
`@@ -174,11 +174,8 @@ static bool nodeAndRendererAreValid(Node* node)`
`174`	`174`
`175`	`175`	`static RefPtr<Element> getElementByIdWithReferenceTarget(const TreeScope& treeScope, const AtomString& elementId)`
`176`	`176`	`{`
`177`		`- if (elementId.isNull())`
`178`		`- return nullptr;`
`179`		`-`
`180`	`177`	`RefPtr<Element> element = treeScope.getElementById(elementId);`
`181`		`- return element->deepShadowRootReferenceTargetOrSelf();`
	`178`	`+ return element ? element->deepShadowRootReferenceTargetOrSelf() : nullptr;`
`182`	`179`	`}`
`183`	`180`
`184`	`181`	`AccessibilityObjectInclusion AXComputedObjectAttributeCache::getIgnored(AXID id) const`