git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 13601f1)
REGRESSION(r104210): Crash inside DynamicSubtreeNodeList::length
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Jan 2012 09:12:01 +0000 (09:12 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Jan 2012 09:12:01 +0000 (09:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=75731

Reviewed by Andreas Kling.

Source/WebCore:

The crash was caused by DynamicSubtreeNodeList::SubtreeCaches::domVersionIsConsistent
using m_cachedItem as a way to access the document. Changed SubtreeCaches to use
DynamicSubtreeNodeList's m_node instead.

Test: fast/dom/node-list-length-after-removing-node.html

* dom/DynamicNodeList.cpp:
(WebCore::DynamicSubtreeNodeList::SubtreeCaches::setLengthCache):
(WebCore::DynamicSubtreeNodeList::SubtreeCaches::setItemCache):
(WebCore::DynamicSubtreeNodeList::length):
(WebCore::DynamicSubtreeNodeList::item):
* dom/DynamicNodeList.h:
(WebCore::DynamicSubtreeNodeList::SubtreeCaches::isLengthCacheValid):
(WebCore::DynamicSubtreeNodeList::SubtreeCaches::isItemCacheValid):
(WebCore::DynamicSubtreeNodeList::SubtreeCaches::cachedItem):
(WebCore::DynamicSubtreeNodeList::SubtreeCaches::domVersionIsConsistent):

LayoutTests:

Add a regression test. It reproduces the crash with a very high probability.

* fast/dom/node-list-length-after-removing-node-expected.txt: Added.
* fast/dom/node-list-length-after-removing-node.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@104381 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog patch | blob | history
LayoutTests/fast/dom/node-list-length-after-removing-node-expected.txt [new file with mode: 0644] patch | blob
LayoutTests/fast/dom/node-list-length-after-removing-node.html [new file with mode: 0644] patch | blob
Source/WebCore/ChangeLog patch | blob | history
Source/WebCore/dom/DynamicNodeList.cpp patch | blob | history
Source/WebCore/dom/DynamicNodeList.h patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 70dbb364b6b456d84cd920bc018a79a5b58fee2e..8294d831c90f9039d097cf91665a6775fd1aa740 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2012-01-06  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION(r104210): Crash inside DynamicSubtreeNodeList::length
+        https://bugs.webkit.org/show_bug.cgi?id=75731
+
+        Reviewed by Andreas Kling.
+
+        Add a regression test. It reproduces the crash with a very high probability.
+
+        * fast/dom/node-list-length-after-removing-node-expected.txt: Added.
+        * fast/dom/node-list-length-after-removing-node.html: Added.
+
 2012-01-06  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r104373 and r104374.
diff --git a/LayoutTests/fast/dom/node-list-length-after-removing-node-expected.txt b/LayoutTests/fast/dom/node-list-length-after-removing-node-expected.txt
new file mode 100644 (file)
index 0000000..3688b0e
--- /dev/null
+++ b/LayoutTests/fast/dom/node-list-length-after-removing-node-expected.txt
@@ -0,0 +1,3 @@
+This tests accessing an item in node list and then querying the length of the node list after removing the item. WebKit should not crash.
+
+PASS
diff --git a/LayoutTests/fast/dom/node-list-length-after-removing-node.html b/LayoutTests/fast/dom/node-list-length-after-removing-node.html
new file mode 100644 (file)
index 0000000..df832ce
--- /dev/null
+++ b/LayoutTests/fast/dom/node-list-length-after-removing-node.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<body onload="runTest()">
+<p>This tests accessing an item in node list and then querying the length of the node list after removing the item.
+WebKit should not crash.</p>
+<span></span><span></span>
+<div id="result"></div>
+<script>
+
+var nodeList = document.getElementsByTagName('span');
+document.body.removeChild(nodeList[0]);
+
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+if (window.GCController)
+   GCController.collect();
+else {
+    for (var i = 0; i < 10000; ++i)
+        new Object;
+}
+
+document.body.offsetLeft;
+
+function runTest() {
+    document.getElementById('result').innerHTML = nodeList.length == 1 ? 'PASS' : 'FAIL';
+}
+
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 833f03e6d537198fdcaad447b45b047b0b38b70c..6c53453b2060c00bc825776ab9f84a8771c1cbfb 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2012-01-06  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION(r104210): Crash inside DynamicSubtreeNodeList::length
+        https://bugs.webkit.org/show_bug.cgi?id=75731
+
+        Reviewed by Andreas Kling.
+
+        The crash was caused by DynamicSubtreeNodeList::SubtreeCaches::domVersionIsConsistent
+        using m_cachedItem as a way to access the document. Changed SubtreeCaches to use
+        DynamicSubtreeNodeList's m_node instead.
+
+        Test: fast/dom/node-list-length-after-removing-node.html
+
+        * dom/DynamicNodeList.cpp:
+        (WebCore::DynamicSubtreeNodeList::SubtreeCaches::setLengthCache):
+        (WebCore::DynamicSubtreeNodeList::SubtreeCaches::setItemCache):
+        (WebCore::DynamicSubtreeNodeList::length):
+        (WebCore::DynamicSubtreeNodeList::item):
+        * dom/DynamicNodeList.h:
+        (WebCore::DynamicSubtreeNodeList::SubtreeCaches::isLengthCacheValid):
+        (WebCore::DynamicSubtreeNodeList::SubtreeCaches::isItemCacheValid):
+        (WebCore::DynamicSubtreeNodeList::SubtreeCaches::cachedItem):
+        (WebCore::DynamicSubtreeNodeList::SubtreeCaches::domVersionIsConsistent):
+
 2012-01-07  Adam Barth  <abarth@webkit.org>
 
         Disconnecting DOMWindow properties is fragile and overly complicated
diff --git a/Source/WebCore/dom/DynamicNodeList.cpp b/Source/WebCore/dom/DynamicNodeList.cpp
index 10cfcce390f3a0bb4c48f5b2cd14195d2ef9e710..6e4ef818bbffab96459645c19edc0315b7967437 100644 (file)
--- a/Source/WebCore/dom/DynamicNodeList.cpp
+++ b/Source/WebCore/dom/DynamicNodeList.cpp
@@ -36,26 +36,24 @@ DynamicSubtreeNodeList::SubtreeCaches::SubtreeCaches()
 {
 }
 
-void DynamicSubtreeNodeList::SubtreeCaches::setLengthCache(Node* node, unsigned length)
+void DynamicSubtreeNodeList::SubtreeCaches::setLengthCache(Document* document, unsigned length)
 {
-    if (m_isItemCacheValid && !domVersionIsConsistent()) {
-        m_cachedItem = node;
+    if (m_isItemCacheValid && !domVersionIsConsistent(document))
         m_isItemCacheValid = false;
-    } else if (!m_isItemCacheValid)
-        m_cachedItem = node; // Used in domVersionIsConsistent.
     m_cachedLength = length;
     m_isLengthCacheValid = true;
-    m_domTreeVersionAtTimeOfCaching = node->document()->domTreeVersion();
+    m_domTreeVersionAtTimeOfCaching = document->domTreeVersion();
 }
 
 void DynamicSubtreeNodeList::SubtreeCaches::setItemCache(Node* item, unsigned offset)
 {
-    if (m_isLengthCacheValid && !domVersionIsConsistent())
+    Document* document = item->document();
+    if (m_isLengthCacheValid && !domVersionIsConsistent(document))
         m_isLengthCacheValid = false;
     m_cachedItem = item;
     m_cachedItemOffset = offset;
     m_isItemCacheValid = true;
-    m_domTreeVersionAtTimeOfCaching = item->document()->domTreeVersion();
+    m_domTreeVersionAtTimeOfCaching = document->domTreeVersion();
 }
 
 void DynamicSubtreeNodeList::SubtreeCaches::reset()
@@ -78,7 +76,8 @@ DynamicSubtreeNodeList::~DynamicSubtreeNodeList()
 
 unsigned DynamicSubtreeNodeList::length() const
 {
-    if (m_caches.isLengthCacheValid())
+    Document* document = node()->document();
+    if (m_caches.isLengthCacheValid(document))
         return m_caches.cachedLength();
 
     unsigned length = 0;
@@ -86,7 +85,7 @@ unsigned DynamicSubtreeNodeList::length() const
     for (Node* n = node()->firstChild(); n; n = n->traverseNextNode(rootNode()))
         length += n->isElementNode() && nodeMatches(static_cast<Element*>(n));
 
-    m_caches.setLengthCache(node(), length);
+    m_caches.setLengthCache(document, length);
 
     return length;
 }
@@ -127,7 +126,7 @@ Node* DynamicSubtreeNodeList::item(unsigned offset) const
 {
     int remainingOffset = offset;
     Node* start = node()->firstChild();
-    if (m_caches.isItemCacheValid()) {
+    if (m_caches.isItemCacheValid(node()->document())) {
         if (offset == m_caches.cachedItemOffset())
             return m_caches.cachedItem();
         if (offset > m_caches.cachedItemOffset() || m_caches.cachedItemOffset() - offset < offset) {
diff --git a/Source/WebCore/dom/DynamicNodeList.h b/Source/WebCore/dom/DynamicNodeList.h
index a90455006cd3af81c34dd88cce196eeea1ddfa21..a4022929cdada2501c6df0b60861a895dea68831 100644 (file)
--- a/Source/WebCore/dom/DynamicNodeList.h
+++ b/Source/WebCore/dom/DynamicNodeList.h
@@ -86,14 +86,14 @@ private:
     public:
         SubtreeCaches();
 
-        bool isLengthCacheValid() const { return m_isLengthCacheValid && domVersionIsConsistent(); }
-        bool isItemCacheValid() const { return m_isItemCacheValid && domVersionIsConsistent(); }
+        bool isLengthCacheValid(Document* document) const { return m_isLengthCacheValid && domVersionIsConsistent(document); }
+        bool isItemCacheValid(Document* document) const { return m_isItemCacheValid && domVersionIsConsistent(document); }
 
         unsigned cachedLength() const { return m_cachedLength; }
         Node* cachedItem() const { return m_cachedItem; }
         unsigned cachedItemOffset() const { return m_cachedItemOffset; }
 
-        void setLengthCache(Node* nodeForDocumentVersion, unsigned length);
+        void setLengthCache(Document*, unsigned length);
         void setItemCache(Node*, unsigned offset);
         void reset();
 
@@ -104,9 +104,10 @@ private:
         unsigned m_isLengthCacheValid : 1;
         unsigned m_isItemCacheValid : 1;
 
-        bool domVersionIsConsistent() const
+        bool domVersionIsConsistent(Document* document) const
         {
-            return m_cachedItem && m_cachedItem->document()->domTreeVersion() == m_domTreeVersionAtTimeOfCaching;
+            ASSERT(document);
+            return document->domTreeVersion() == m_domTreeVersionAtTimeOfCaching;
         }
         uint64_t m_domTreeVersionAtTimeOfCaching;
     };
Mirror of the WebKit open source project from svn.webkit.org.