Changeset 259513 in webkit


Ignore:
Timestamp:
Apr 3, 2020 2:05:19 PM (4 years ago)
Author:
rniwa@webkit.org
Message:

HTMLFormElement should use WeakPtr to keep track of its FormNamedItem
https://bugs.webkit.org/show_bug.cgi?id=209925

Reviewed by Wenson Hsieh.

Like r259393, this patch replaces the HashMap of AtomString to the raw pointer of a FormNamedItem
by a HashMap of AtomString to WeakPtr of a FormNamedItem.

It also replaces a bunch of ASSERT_WITH_SECURITY_IMPLICATIONs with ASSERTs since there are no more
security implications left after this patch.

  • html/HTMLFormElement.cpp:

(WebCore::HTMLFormElement::formElementIndex):
(WebCore::HTMLFormElement::removeFormElement):
(WebCore::HTMLFormElement::assertItemCanBeInPastNamesMap const):
(WebCore::HTMLFormElement::elementFromPastNamesMap const):
(WebCore::HTMLFormElement::addToPastNamesMap):
(WebCore::HTMLFormElement::removeFromPastNamesMap):

  • html/HTMLFormElement.h:
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r259512 r259513  
     12020-04-02  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        HTMLFormElement should use WeakPtr to keep track of its FormNamedItem
     4        https://bugs.webkit.org/show_bug.cgi?id=209925
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        Like r259393, this patch replaces the HashMap of AtomString to the raw pointer of a FormNamedItem
     9        by a HashMap of AtomString to WeakPtr of a FormNamedItem.
     10
     11        It also replaces a bunch of ASSERT_WITH_SECURITY_IMPLICATIONs with ASSERTs since there are no more
     12        security implications left after this patch.
     13
     14        * html/HTMLFormElement.cpp:
     15        (WebCore::HTMLFormElement::formElementIndex):
     16        (WebCore::HTMLFormElement::removeFormElement):
     17        (WebCore::HTMLFormElement::assertItemCanBeInPastNamesMap const):
     18        (WebCore::HTMLFormElement::elementFromPastNamesMap const):
     19        (WebCore::HTMLFormElement::addToPastNamesMap):
     20        (WebCore::HTMLFormElement::removeFromPastNamesMap):
     21        * html/HTMLFormElement.h:
     22
    1232020-04-03  Tim Horton  <timothy_horton@apple.com>
    224
  • trunk/Source/WebCore/html/HTMLFormElement.cpp

    r259394 r259513  
    505505    if (associatedHTMLElement.hasAttributeWithoutSynchronization(formAttr) && associatedHTMLElement.isConnected()) {
    506506        unsigned short position = compareDocumentPosition(associatedHTMLElement);
    507         ASSERT_WITH_SECURITY_IMPLICATION(!(position & DOCUMENT_POSITION_DISCONNECTED));
     507        ASSERT(!(position & DOCUMENT_POSITION_DISCONNECTED));
    508508        if (position & DOCUMENT_POSITION_PRECEDING) {
    509509            ++m_associatedElementsBeforeIndex;
     
    561561{
    562562    unsigned index = m_associatedElements.find(&e->asHTMLElement());
    563     ASSERT_WITH_SECURITY_IMPLICATION(index < m_associatedElements.size());
     563    ASSERT(index < m_associatedElements.size());
    564564    if (index < m_associatedElementsBeforeIndex)
    565565        --m_associatedElementsBeforeIndex;
     
    765765}
    766766
    767 #ifndef NDEBUG
     767#if ASSERT_ENABLED
    768768void HTMLFormElement::assertItemCanBeInPastNamesMap(FormNamedItem* item) const
    769769{
    770     ASSERT_WITH_SECURITY_IMPLICATION(item);
     770    ASSERT(item);
    771771    HTMLElement& element = item->asHTMLElement();
    772     ASSERT_WITH_SECURITY_IMPLICATION(element.form() == this);
     772    ASSERT(element.form() == this);
    773773
    774774    if (item->isFormAssociatedElement()) {
    775         ASSERT_WITH_SECURITY_IMPLICATION(m_associatedElements.find(&element) != notFound);
    776         return;
    777     }
    778 
    779     ASSERT_WITH_SECURITY_IMPLICATION(element.hasTagName(imgTag));
    780     ASSERT_WITH_SECURITY_IMPLICATION(m_imageElements.find(&downcast<HTMLImageElement>(element)) != notFound);
    781 }
    782 #else
    783 inline void HTMLFormElement::assertItemCanBeInPastNamesMap(FormNamedItem*) const
    784 {
     775        ASSERT(m_associatedElements.find(&element) != notFound);
     776        return;
     777    }
     778
     779    ASSERT(element.hasTagName(imgTag));
     780    ASSERT(m_imageElements.find(&downcast<HTMLImageElement>(element)) != notFound);
    785781}
    786782#endif
     
    788784RefPtr<HTMLElement> HTMLFormElement::elementFromPastNamesMap(const AtomString& pastName) const
    789785{
    790     if (pastName.isEmpty() || !m_pastNamesMap)
     786    if (pastName.isEmpty() || m_pastNamesMap.isEmpty())
    791787        return nullptr;
    792     FormNamedItem* item = m_pastNamesMap->get(pastName.impl());
    793     if (!item)
     788    auto weakElement = m_pastNamesMap.get(pastName);
     789    if (!weakElement)
    794790        return nullptr;
     791    auto element = makeRefPtr(weakElement.get());
     792#if ASSERT_ENABLED
     793    assertItemCanBeInPastNamesMap(element->asFormNamedItem());
     794#endif
     795    return element;
     796}
     797
     798void HTMLFormElement::addToPastNamesMap(FormNamedItem* item, const AtomString& pastName)
     799{
     800#if ASSERT_ENABLED || ENABLE(SECURITY_ASSERTIONS)
    795801    assertItemCanBeInPastNamesMap(item);
    796     return &item->asHTMLElement();
    797 }
    798 
    799 void HTMLFormElement::addToPastNamesMap(FormNamedItem* item, const AtomString& pastName)
    800 {
    801     assertItemCanBeInPastNamesMap(item);
     802#endif
    802803    if (pastName.isEmpty())
    803804        return;
    804     if (!m_pastNamesMap)
    805         m_pastNamesMap = makeUnique<PastNamesMap>();
    806     m_pastNamesMap->set(pastName.impl(), item);
     805    m_pastNamesMap.set(pastName.impl(), makeWeakPtr(item->asHTMLElement()));
    807806}
    808807
     
    810809{
    811810    ASSERT(item);
    812     if (!m_pastNamesMap)
    813         return;
    814 
    815     for (auto& pastName : m_pastNamesMap->values()) {
    816         if (pastName == item)
    817             pastName = nullptr; // Keep looping. Single element can have multiple names.
    818     }
     811    if (m_pastNamesMap.isEmpty())
     812        return;
     813
     814    m_pastNamesMap.removeIf([&element = item->asHTMLElement()] (auto& iterator) {
     815        return iterator.value == &element;
     816    });
    819817}
    820818
  • trunk/Source/WebCore/html/HTMLFormElement.h

    r259393 r259513  
    157157    RefPtr<HTMLElement> elementFromPastNamesMap(const AtomString&) const;
    158158    void addToPastNamesMap(FormNamedItem*, const AtomString& pastName);
     159#if ASSERT_ENABLED
    159160    void assertItemCanBeInPastNamesMap(FormNamedItem*) const;
     161#endif
    160162    void removeFromPastNamesMap(FormNamedItem*);
    161163
     
    165167    void resetAssociatedFormControlElements();
    166168
    167     typedef HashMap<RefPtr<AtomStringImpl>, FormNamedItem*> PastNamesMap;
    168 
    169169    FormSubmission::Attributes m_attributes;
    170     std::unique_ptr<PastNamesMap> m_pastNamesMap;
     170    HashMap<AtomString, WeakPtr<HTMLElement>> m_pastNamesMap;
    171171
    172172    RadioButtonGroups m_radioButtonGroups;
Note: See TracChangeset for help on using the changeset viewer.