Deploy smart pointers in RadioButtonGroups and RadioButtonGroup
https://bugs.webkit.org/show_bug.cgi?id=202942
Reviewed by Zalan Bujtas.
Use Ref & WeakPtr instead of raw pointers in RadioButtonGroups and RadioButtonGroup.
Also made RadioButtonGroups::m_nameToGroupMap a HashMap instead of a unique_ptr of HashMap
since RadioButtonGroups is lazily created in TreeScope as of r250708.
No new tests since there should be no observable behavioral change.
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::addRadioButtonGroupMembers const):
* dom/RadioButtonGroups.cpp:
(WebCore::RadioButtonGroup): Use WeakHashSet and WeakPtr instead of raw pointers.
(WebCore::RadioButtonGroup::isEmpty const):
(WebCore::RadioButtonGroup::checkedButton const):
(WebCore::RadioButtonGroup::members const):
(WebCore::RadioButtonGroup::setCheckedButton):
(WebCore::RadioButtonGroup::updateCheckedState):
(WebCore::RadioButtonGroup::requiredStateChanged):
(WebCore::RadioButtonGroup::remove):
(WebCore::RadioButtonGroup::setNeedsStyleRecalcForAllButtons):
(WebCore::RadioButtonGroup::updateValidityForAllButtons):
(WebCore::RadioButtonGroup::contains const):
(WebCore::RadioButtonGroups::addButton):
(WebCore::RadioButtonGroups::groupMembers const):
(WebCore::RadioButtonGroups::updateCheckedState):
(WebCore::RadioButtonGroups::requiredStateChanged):
(WebCore::RadioButtonGroups::checkedButtonForGroup const):
(WebCore::RadioButtonGroups::hasCheckedButton const):
(WebCore::RadioButtonGroups::isInRequiredGroup const):
(WebCore::RadioButtonGroups::removeButton):
* dom/RadioButtonGroups.h:
(WebCore::RadioButtonGroups):
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::radioButtonGroup const):
(WebCore::HTMLInputElement::checkedRadioButtonForGroup const):
* html/HTMLInputElement.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251110 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 0030d2d..76789f4 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,46 @@
+2019-10-14 Ryosuke Niwa <rniwa@webkit.org>
+
+ Deploy smart pointers in RadioButtonGroups and RadioButtonGroup
+ https://bugs.webkit.org/show_bug.cgi?id=202942
+
+ Reviewed by Zalan Bujtas.
+
+ Use Ref & WeakPtr instead of raw pointers in RadioButtonGroups and RadioButtonGroup.
+
+ Also made RadioButtonGroups::m_nameToGroupMap a HashMap instead of a unique_ptr of HashMap
+ since RadioButtonGroups is lazily created in TreeScope as of r250708.
+
+ No new tests since there should be no observable behavioral change.
+
+ * accessibility/AccessibilityRenderObject.cpp:
+ (WebCore::AccessibilityRenderObject::addRadioButtonGroupMembers const):
+ * dom/RadioButtonGroups.cpp:
+ (WebCore::RadioButtonGroup): Use WeakHashSet and WeakPtr instead of raw pointers.
+ (WebCore::RadioButtonGroup::isEmpty const):
+ (WebCore::RadioButtonGroup::checkedButton const):
+ (WebCore::RadioButtonGroup::members const):
+ (WebCore::RadioButtonGroup::setCheckedButton):
+ (WebCore::RadioButtonGroup::updateCheckedState):
+ (WebCore::RadioButtonGroup::requiredStateChanged):
+ (WebCore::RadioButtonGroup::remove):
+ (WebCore::RadioButtonGroup::setNeedsStyleRecalcForAllButtons):
+ (WebCore::RadioButtonGroup::updateValidityForAllButtons):
+ (WebCore::RadioButtonGroup::contains const):
+ (WebCore::RadioButtonGroups::addButton):
+ (WebCore::RadioButtonGroups::groupMembers const):
+ (WebCore::RadioButtonGroups::updateCheckedState):
+ (WebCore::RadioButtonGroups::requiredStateChanged):
+ (WebCore::RadioButtonGroups::checkedButtonForGroup const):
+ (WebCore::RadioButtonGroups::hasCheckedButton const):
+ (WebCore::RadioButtonGroups::isInRequiredGroup const):
+ (WebCore::RadioButtonGroups::removeButton):
+ * dom/RadioButtonGroups.h:
+ (WebCore::RadioButtonGroups):
+ * html/HTMLInputElement.cpp:
+ (WebCore::HTMLInputElement::radioButtonGroup const):
+ (WebCore::HTMLInputElement::checkedRadioButtonForGroup const):
+ * html/HTMLInputElement.h:
+
2019-10-14 David Quesada <david_quesada@apple.com>
Remove WebCore::IOSApplication::isWebApp()
diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
index e95228e..5334dc7 100644
--- a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
+++ b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
@@ -995,7 +995,7 @@
if (is<HTMLInputElement>(node)) {
HTMLInputElement& input = downcast<HTMLInputElement>(*node);
for (auto& radioSibling : input.radioButtonGroup()) {
- if (AccessibilityObject* object = axObjectCache()->getOrCreate(radioSibling))
+ if (AccessibilityObject* object = axObjectCache()->getOrCreate(radioSibling.ptr()))
linkedUIElements.append(object);
}
} else {
diff --git a/Source/WebCore/dom/RadioButtonGroups.cpp b/Source/WebCore/dom/RadioButtonGroups.cpp
index 61bbc4e..74b4f89 100644
--- a/Source/WebCore/dom/RadioButtonGroups.cpp
+++ b/Source/WebCore/dom/RadioButtonGroups.cpp
@@ -23,22 +23,23 @@
#include "HTMLInputElement.h"
#include "Range.h"
-#include <wtf/HashSet.h>
+#include <wtf/WeakHashSet.h>
+#include <wtf/WeakPtr.h>
namespace WebCore {
class RadioButtonGroup {
WTF_MAKE_FAST_ALLOCATED;
public:
- bool isEmpty() const { return m_members.isEmpty(); }
+ bool isEmpty() const { return m_members.computesEmpty(); }
bool isRequired() const { return m_requiredCount; }
- HTMLInputElement* checkedButton() const { return m_checkedButton; }
+ RefPtr<HTMLInputElement> checkedButton() const { return m_checkedButton.get(); }
void add(HTMLInputElement&);
void updateCheckedState(HTMLInputElement&);
void requiredStateChanged(HTMLInputElement&);
void remove(HTMLInputElement&);
bool contains(HTMLInputElement&) const;
- Vector<HTMLInputElement*> members() const;
+ Vector<Ref<HTMLInputElement>> members() const;
private:
void setNeedsStyleRecalcForAllButtons();
@@ -46,8 +47,8 @@
bool isValid() const;
void setCheckedButton(HTMLInputElement*);
- HashSet<HTMLInputElement*> m_members;
- HTMLInputElement* m_checkedButton { nullptr };
+ WeakHashSet<HTMLInputElement> m_members;
+ WeakPtr<HTMLInputElement> m_checkedButton;
size_t m_requiredCount { 0 };
};
@@ -56,25 +57,29 @@
return !isRequired() || m_checkedButton;
}
-Vector<HTMLInputElement*> RadioButtonGroup::members() const
+Vector<Ref<HTMLInputElement>> RadioButtonGroup::members() const
{
- auto members = copyToVector(m_members);
- std::sort(members.begin(), members.end(), documentOrderComparator);
- return members;
+ Vector<Ref<HTMLInputElement>> sortedMembers;
+ for (auto& memeber : m_members)
+ sortedMembers.append(memeber);
+ std::sort(sortedMembers.begin(), sortedMembers.end(), [](auto& a, auto& b) {
+ return documentOrderComparator(a.ptr(), b.ptr());
+ });
+ return sortedMembers;
}
void RadioButtonGroup::setCheckedButton(HTMLInputElement* button)
{
- RefPtr<HTMLInputElement> oldCheckedButton = m_checkedButton;
+ RefPtr<HTMLInputElement> oldCheckedButton = m_checkedButton.get();
if (oldCheckedButton == button)
return;
- bool hadCheckedButton = m_checkedButton;
+ bool hadCheckedButton = m_checkedButton.get();
bool willHaveCheckedButton = button;
if (hadCheckedButton != willHaveCheckedButton)
setNeedsStyleRecalcForAllButtons();
- m_checkedButton = button;
+ m_checkedButton = makeWeakPtr(button);
if (oldCheckedButton)
oldCheckedButton->setChecked(false);
}
@@ -103,7 +108,7 @@
void RadioButtonGroup::updateCheckedState(HTMLInputElement& button)
{
ASSERT(button.isRadioButton());
- ASSERT(m_members.contains(&button));
+ ASSERT(m_members.contains(button));
bool wasValid = isValid();
if (button.checked())
setCheckedButton(&button);
@@ -118,7 +123,7 @@
void RadioButtonGroup::requiredStateChanged(HTMLInputElement& button)
{
ASSERT(button.isRadioButton());
- ASSERT(m_members.contains(&button));
+ ASSERT(m_members.contains(button));
bool wasValid = isValid();
if (button.isRequired())
++m_requiredCount;
@@ -133,12 +138,11 @@
void RadioButtonGroup::remove(HTMLInputElement& button)
{
ASSERT(button.isRadioButton());
- auto it = m_members.find(&button);
- if (it == m_members.end())
+ if (!m_members.contains(button))
return;
bool wasValid = isValid();
- m_members.remove(it);
+ m_members.remove(button);
if (button.isRequired()) {
ASSERT(m_requiredCount);
--m_requiredCount;
@@ -151,7 +155,7 @@
}
}
- if (m_members.isEmpty()) {
+ if (m_members.computesEmpty()) {
ASSERT(!m_requiredCount);
ASSERT(!m_checkedButton);
} else if (wasValid != isValid())
@@ -166,22 +170,22 @@
void RadioButtonGroup::setNeedsStyleRecalcForAllButtons()
{
for (auto& button : m_members) {
- ASSERT(button->isRadioButton());
- button->invalidateStyleForSubtree();
+ ASSERT(button.isRadioButton());
+ button.invalidateStyleForSubtree();
}
}
void RadioButtonGroup::updateValidityForAllButtons()
{
for (auto& button : m_members) {
- ASSERT(button->isRadioButton());
- button->updateValidity();
+ ASSERT(button.isRadioButton());
+ button.updateValidity();
}
}
bool RadioButtonGroup::contains(HTMLInputElement& button) const
{
- return m_members.contains(&button);
+ return m_members.contains(button);
}
// ----------------------------------------------------------------
@@ -197,16 +201,13 @@
if (element.name().isEmpty())
return;
- if (!m_nameToGroupMap)
- m_nameToGroupMap = makeUnique<NameToGroupMap>();
-
- auto& group = m_nameToGroupMap->add(element.name().impl(), nullptr).iterator->value;
+ auto& group = m_nameToGroupMap.add(element.name().impl(), nullptr).iterator->value;
if (!group)
group = makeUnique<RadioButtonGroup>();
group->add(element);
}
-Vector<HTMLInputElement*> RadioButtonGroups::groupMembers(const HTMLInputElement& element) const
+Vector<Ref<HTMLInputElement>> RadioButtonGroups::groupMembers(const HTMLInputElement& element) const
{
ASSERT(element.isRadioButton());
if (!element.isRadioButton())
@@ -216,10 +217,7 @@
if (!name)
return { };
- if (!m_nameToGroupMap)
- return { };
-
- auto* group = m_nameToGroupMap->get(name);
+ auto* group = m_nameToGroupMap.get(name);
if (!group)
return { };
return group->members();
@@ -230,10 +228,7 @@
ASSERT(element.isRadioButton());
if (element.name().isEmpty())
return;
- ASSERT(m_nameToGroupMap);
- if (!m_nameToGroupMap)
- return;
- m_nameToGroupMap->get(element.name().impl())->updateCheckedState(element);
+ m_nameToGroupMap.get(element.name().impl())->updateCheckedState(element);
}
void RadioButtonGroups::requiredStateChanged(HTMLInputElement& element)
@@ -241,20 +236,15 @@
ASSERT(element.isRadioButton());
if (element.name().isEmpty())
return;
- ASSERT(m_nameToGroupMap);
- if (!m_nameToGroupMap)
- return;
- auto* group = m_nameToGroupMap->get(element.name().impl());
+ auto* group = m_nameToGroupMap.get(element.name().impl());
ASSERT(group);
group->requiredStateChanged(element);
}
-HTMLInputElement* RadioButtonGroups::checkedButtonForGroup(const AtomString& name) const
+RefPtr<HTMLInputElement> RadioButtonGroups::checkedButtonForGroup(const AtomString& name) const
{
- if (!m_nameToGroupMap)
- return nullptr;
- m_nameToGroupMap->checkConsistency();
- RadioButtonGroup* group = m_nameToGroupMap->get(name.impl());
+ m_nameToGroupMap.checkConsistency();
+ RadioButtonGroup* group = m_nameToGroupMap.get(name.impl());
return group ? group->checkedButton() : nullptr;
}
@@ -262,9 +252,9 @@
{
ASSERT(element.isRadioButton());
const AtomString& name = element.name();
- if (name.isEmpty() || !m_nameToGroupMap)
+ if (name.isEmpty())
return element.checked();
- return m_nameToGroupMap->get(name.impl())->checkedButton();
+ return m_nameToGroupMap.get(name.impl())->checkedButton();
}
bool RadioButtonGroups::isInRequiredGroup(HTMLInputElement& element) const
@@ -272,9 +262,7 @@
ASSERT(element.isRadioButton());
if (element.name().isEmpty())
return false;
- if (!m_nameToGroupMap)
- return false;
- auto* group = m_nameToGroupMap->get(element.name().impl());
+ auto* group = m_nameToGroupMap.get(element.name().impl());
return group && group->isRequired() && group->contains(element);
}
@@ -283,22 +271,14 @@
ASSERT(element.isRadioButton());
if (element.name().isEmpty())
return;
- if (!m_nameToGroupMap)
- return;
- m_nameToGroupMap->checkConsistency();
- auto it = m_nameToGroupMap->find(element.name().impl());
- if (it == m_nameToGroupMap->end())
+ m_nameToGroupMap.checkConsistency();
+ auto it = m_nameToGroupMap.find(element.name().impl());
+ if (it == m_nameToGroupMap.end())
return;
it->value->remove(element);
- if (it->value->isEmpty()) {
- // FIXME: We may skip deallocating the empty RadioButtonGroup for
- // performance improvement. If we do so, we need to change the key type
- // of m_nameToGroupMap from AtomStringImpl* to RefPtr<AtomStringImpl>.
- m_nameToGroupMap->remove(it);
- if (m_nameToGroupMap->isEmpty())
- m_nameToGroupMap = nullptr;
- }
+ if (it->value->isEmpty())
+ m_nameToGroupMap.remove(it);
}
} // namespace
diff --git a/Source/WebCore/dom/RadioButtonGroups.h b/Source/WebCore/dom/RadioButtonGroups.h
index 52b336f..ef87610 100644
--- a/Source/WebCore/dom/RadioButtonGroups.h
+++ b/Source/WebCore/dom/RadioButtonGroups.h
@@ -39,14 +39,14 @@
void updateCheckedState(HTMLInputElement&);
void requiredStateChanged(HTMLInputElement&);
void removeButton(HTMLInputElement&);
- HTMLInputElement* checkedButtonForGroup(const AtomString& groupName) const;
+ RefPtr<HTMLInputElement> checkedButtonForGroup(const AtomString& groupName) const;
bool hasCheckedButton(const HTMLInputElement&) const;
bool isInRequiredGroup(HTMLInputElement&) const;
- Vector<HTMLInputElement*> groupMembers(const HTMLInputElement&) const;
+ Vector<Ref<HTMLInputElement>> groupMembers(const HTMLInputElement&) const;
private:
- typedef HashMap<AtomStringImpl*, std::unique_ptr<RadioButtonGroup>> NameToGroupMap;
- std::unique_ptr<NameToGroupMap> m_nameToGroupMap;
+ typedef HashMap<AtomString, std::unique_ptr<RadioButtonGroup>> NameToGroupMap;
+ NameToGroupMap m_nameToGroupMap;
};
} // namespace WebCore
diff --git a/Source/WebCore/html/HTMLInputElement.cpp b/Source/WebCore/html/HTMLInputElement.cpp
index fb59207..ddf55b7 100644
--- a/Source/WebCore/html/HTMLInputElement.cpp
+++ b/Source/WebCore/html/HTMLInputElement.cpp
@@ -1898,7 +1898,7 @@
return false;
}
-Vector<HTMLInputElement*> HTMLInputElement::radioButtonGroup() const
+Vector<Ref<HTMLInputElement>> HTMLInputElement::radioButtonGroup() const
{
RadioButtonGroups* buttons = radioButtonGroups();
if (!buttons)
@@ -1906,7 +1906,7 @@
return buttons->groupMembers(*this);
}
-HTMLInputElement* HTMLInputElement::checkedRadioButtonForGroup() const
+RefPtr<HTMLInputElement> HTMLInputElement::checkedRadioButtonForGroup() const
{
if (RadioButtonGroups* buttons = radioButtonGroups())
return buttons->checkedButtonForGroup(name());
diff --git a/Source/WebCore/html/HTMLInputElement.h b/Source/WebCore/html/HTMLInputElement.h
index f868eee..efe58b0 100644
--- a/Source/WebCore/html/HTMLInputElement.h
+++ b/Source/WebCore/html/HTMLInputElement.h
@@ -279,8 +279,8 @@
void listAttributeTargetChanged();
#endif
- Vector<HTMLInputElement*> radioButtonGroup() const;
- HTMLInputElement* checkedRadioButtonForGroup() const;
+ Vector<Ref<HTMLInputElement>> radioButtonGroup() const;
+ RefPtr<HTMLInputElement> checkedRadioButtonForGroup() const;
bool isInRequiredRadioButtonGroup();
// Returns null if this isn't associated with any radio button group.
RadioButtonGroups* radioButtonGroups() const;
