Changeset 207212 in webkit

Timestamp:

Oct 12, 2016 1:41:58 AM (8 years ago)

Author:

matthew_hanson@apple.com

Message:

Merge r206975. rdar://problem/28545012

Location:

branches/safari-602-branch

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/NodeList/form-labels-length-expected.txt (added)
• LayoutTests/fast/dom/NodeList/form-labels-length.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (2 diffs)
• Source/WebCore/dom/Document.h (modified) (1 diff)
• Source/WebCore/dom/Node.cpp (modified) (1 diff)

branches/safari-602-branch/LayoutTests/ChangeLog

@@ +1 @@
+2016-10-12  Matthew Hanson  <matthew_hanson@apple.com>
+
+        Merge r206975. rdar://problem/28545012
+
+    2016-10-07  Ryosuke Niwa  <rniwa@webkit.org>
+
+            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.
+
+            Added a regression test.
+
+            * fast/dom/NodeList/form-labels-length-expected.txt: Added.
+            * fast/dom/NodeList/form-labels-length.html: Added.
+
 2016-10-12  Matthew Hanson  <matthew_hanson@apple.com>
 

branches/safari-602-branch/Source/WebCore/ChangeLog

@@ +1 @@
+2016-10-12  Matthew Hanson  <matthew_hanson@apple.com>
+
+        Merge r206975. rdar://problem/28545012
+
+    2016-10-07  Ryosuke Niwa  <rniwa@webkit.org>
+
+            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.
+
+            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.
+
 2016-10-12  Matthew Hanson  <matthew_hanson@apple.com>
 

branches/safari-602-branch/Source/WebCore/dom/Document.cpp

@@ -485 +485 @@
     , m_hasXMLDeclaration(false)
     , m_designMode(inherit)
-#if !ASSERT_DISABLED
-    , m_inInvalidateNodeListAndCollectionCaches(false)
-#endif
 #if ENABLE(DASHBOARD_SUPPORT)
     , m_hasAnnotatedRegions(false)
@@ -4002 +3999 @@
 
     list.setRegisteredForInvalidationAtDocument(false);
-    ASSERT(m_inInvalidateNodeListAndCollectionCaches
-        ? m_listsInvalidatedAtDocument.isEmpty()
-        : m_listsInvalidatedAtDocument.contains(&list));
+    ASSERT(m_listsInvalidatedAtDocument.contains(&list));
     m_listsInvalidatedAtDocument.remove(&list);
 }

branches/safari-602-branch/Source/WebCore/dom/Document.h

@@ -1586 +1586 @@
     HashSet<LiveNodeList*> m_listsInvalidatedAtDocument;
     HashSet<HTMLCollection*> m_collectionsInvalidatedAtDocument;
-#if !ASSERT_DISABLED
-    bool m_inInvalidateNodeListAndCollectionCaches;
-#endif
-
     unsigned m_nodeListAndCollectionCounts[numNodeListInvalidationTypes];
 

branches/safari-602-branch/Source/WebCore/dom/Node.cpp

@@ -815 +815 @@
 void Document::invalidateNodeListAndCollectionCaches(const QualifiedName* attrName)
 {
-#if !ASSERT_DISABLED
-    m_inInvalidateNodeListAndCollectionCaches = true;
-#endif
-    HashSet<LiveNodeList*> lists = WTFMove(m_listsInvalidatedAtDocument);
-    m_listsInvalidatedAtDocument.clear();
+    Vector<LiveNodeList*, 8> lists;
+    copyToVector(m_listsInvalidatedAtDocument, lists);
     for (auto* list : lists)
         list->invalidateCacheForAttribute(attrName);
-    HashSet<HTMLCollection*> collections = WTFMove(m_collectionsInvalidatedAtDocument);
+
+    Vector<HTMLCollection*, 8> collections;
+    copyToVector(m_collectionsInvalidatedAtDocument, collections);
     for (auto* collection : collections)
         collection->invalidateCacheForAttribute(attrName);
-#if !ASSERT_DISABLED
-    m_inInvalidateNodeListAndCollectionCaches = false;
-#endif
 }