Changeset 206975 in webkit


Ignore:
Timestamp:
Oct 9, 2016 7:03:35 PM (7 years ago)
Author:
rniwa@webkit.org
Message:

REGRESSION(r165103): labels list doesn't get invalidated when other lists are invalidated at document level
https://bugs.webkit.org/show_bug.cgi?id=163145

Reviewed by Darin Adler.

Source/WebCore:

The bug was caused by Document::invalidateNodeListAndCollectionCaches removing all node lists regardless
of whether they have been invalidated or not.

Fixed the bug by removing only those node lists that got invalidated via LiveNodeList::invalidateCache.

Test: fast/dom/NodeList/form-labels-length.html

  • dom/Document.cpp:

(WebCore::Document::Document):
(WebCore::Document::unregisterNodeListForInvalidation): Removed the conditional which allowed removal to
happen while m_listsInvalidatedAtDocument is empty inside invalidateNodeListAndCollectionCaches.

  • dom/Document.h:
  • dom/Node.cpp:

(WebCore::Document::invalidateNodeListAndCollectionCaches): Just remove the node lists being invalidated via
LiveNodeList's invalidateCache, which calls unregisterNodeListForInvalidation, instead of removing them all.
We make a copy of the list of node lists into a local vector because mutating HashMap while iterating over it
is not a safe operation.

LayoutTests:

Added a regression test.

  • fast/dom/NodeList/form-labels-length-expected.txt: Added.
  • fast/dom/NodeList/form-labels-length.html: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r206971 r206975  
     12016-10-07  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        REGRESSION(r165103): labels list doesn't get invalidated when other lists are invalidated at document level
     4        https://bugs.webkit.org/show_bug.cgi?id=163145
     5
     6        Reviewed by Darin Adler.
     7
     8        Added a regression test.
     9
     10        * fast/dom/NodeList/form-labels-length-expected.txt: Added.
     11        * fast/dom/NodeList/form-labels-length.html: Added.
     12
    1132016-10-09  Chris Dumez  <cdumez@apple.com>
    214
  • trunk/Source/WebCore/ChangeLog

    r206974 r206975  
     12016-10-07  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        REGRESSION(r165103): labels list doesn't get invalidated when other lists are invalidated at document level
     4        https://bugs.webkit.org/show_bug.cgi?id=163145
     5
     6        Reviewed by Darin Adler.
     7
     8        The bug was caused by Document::invalidateNodeListAndCollectionCaches removing all node lists regardless
     9        of whether they have been invalidated or not.
     10
     11        Fixed the bug by removing only those node lists that got invalidated via LiveNodeList::invalidateCache.
     12
     13        Test: fast/dom/NodeList/form-labels-length.html
     14
     15        * dom/Document.cpp:
     16        (WebCore::Document::Document):
     17        (WebCore::Document::unregisterNodeListForInvalidation): Removed the conditional which allowed removal to
     18        happen while m_listsInvalidatedAtDocument is empty inside invalidateNodeListAndCollectionCaches.
     19        * dom/Document.h:
     20        * dom/Node.cpp:
     21        (WebCore::Document::invalidateNodeListAndCollectionCaches): Just remove the node lists being invalidated via
     22        LiveNodeList's invalidateCache, which calls unregisterNodeListForInvalidation, instead of removing them all.
     23        We make a copy of the list of node lists into a local vector because mutating HashMap while iterating over it
     24        is not a safe operation.
     25
    1262016-10-09  Chris Dumez  <cdumez@apple.com>
    227
  • trunk/Source/WebCore/dom/Document.cpp

    r206964 r206975  
    478478    , m_hasXMLDeclaration(false)
    479479    , m_designMode(inherit)
    480 #if !ASSERT_DISABLED
    481     , m_inInvalidateNodeListAndCollectionCaches(false)
    482 #endif
    483480#if ENABLE(DASHBOARD_SUPPORT)
    484481    , m_hasAnnotatedRegions(false)
     
    38403837
    38413838    list.setRegisteredForInvalidationAtDocument(false);
    3842     ASSERT(m_inInvalidateNodeListAndCollectionCaches
    3843         ? m_listsInvalidatedAtDocument.isEmpty()
    3844         : m_listsInvalidatedAtDocument.contains(&list));
     3839    ASSERT(m_listsInvalidatedAtDocument.contains(&list));
    38453840    m_listsInvalidatedAtDocument.remove(&list);
    38463841}
  • trunk/Source/WebCore/dom/Document.h

    r206960 r206975  
    15441544    HashSet<LiveNodeList*> m_listsInvalidatedAtDocument;
    15451545    HashSet<HTMLCollection*> m_collectionsInvalidatedAtDocument;
    1546 #if !ASSERT_DISABLED
    1547     bool m_inInvalidateNodeListAndCollectionCaches;
    1548 #endif
    1549 
    15501546    unsigned m_nodeListAndCollectionCounts[numNodeListInvalidationTypes];
    15511547
  • trunk/Source/WebCore/dom/Node.cpp

    r206956 r206975  
    838838void Document::invalidateNodeListAndCollectionCaches(const QualifiedName* attrName)
    839839{
    840 #if !ASSERT_DISABLED
    841     m_inInvalidateNodeListAndCollectionCaches = true;
    842 #endif
    843     HashSet<LiveNodeList*> lists = WTFMove(m_listsInvalidatedAtDocument);
    844     m_listsInvalidatedAtDocument.clear();
     840    Vector<LiveNodeList*, 8> lists;
     841    copyToVector(m_listsInvalidatedAtDocument, lists);
    845842    for (auto* list : lists)
    846843        list->invalidateCacheForAttribute(attrName);
    847     HashSet<HTMLCollection*> collections = WTFMove(m_collectionsInvalidatedAtDocument);
     844
     845    Vector<HTMLCollection*, 8> collections;
     846    copyToVector(m_collectionsInvalidatedAtDocument, collections);
    848847    for (auto* collection : collections)
    849848        collection->invalidateCacheForAttribute(attrName);
    850 #if !ASSERT_DISABLED
    851     m_inInvalidateNodeListAndCollectionCaches = false;
    852 #endif
    853849}
    854850
Note: See TracChangeset for help on using the changeset viewer.