Title: [104381] trunk
- Revision
- 104381
- Author
- [email protected]
- Date
- 2012-01-07 01:12:01 -0800 (Sat, 07 Jan 2012)
Log Message
REGRESSION(r104210): Crash inside DynamicSubtreeNodeList::length
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (104380 => 104381)
--- trunk/LayoutTests/ChangeLog 2012-01-07 08:46:38 UTC (rev 104380)
+++ trunk/LayoutTests/ChangeLog 2012-01-07 09:12:01 UTC (rev 104381)
@@ -1,3 +1,15 @@
+2012-01-06 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
Unreviewed, rolling out r104373 and r104374.
Added: trunk/LayoutTests/fast/dom/node-list-length-after-removing-node-expected.txt (0 => 104381)
--- trunk/LayoutTests/fast/dom/node-list-length-after-removing-node-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/node-list-length-after-removing-node-expected.txt 2012-01-07 09:12:01 UTC (rev 104381)
@@ -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
Added: trunk/LayoutTests/fast/dom/node-list-length-after-removing-node.html (0 => 104381)
--- trunk/LayoutTests/fast/dom/node-list-length-after-removing-node.html (rev 0)
+++ trunk/LayoutTests/fast/dom/node-list-length-after-removing-node.html 2012-01-07 09:12:01 UTC (rev 104381)
@@ -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>
Modified: trunk/Source/WebCore/ChangeLog (104380 => 104381)
--- trunk/Source/WebCore/ChangeLog 2012-01-07 08:46:38 UTC (rev 104380)
+++ trunk/Source/WebCore/ChangeLog 2012-01-07 09:12:01 UTC (rev 104381)
@@ -1,3 +1,27 @@
+2012-01-06 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
Disconnecting DOMWindow properties is fragile and overly complicated
Modified: trunk/Source/WebCore/dom/DynamicNodeList.cpp (104380 => 104381)
--- trunk/Source/WebCore/dom/DynamicNodeList.cpp 2012-01-07 08:46:38 UTC (rev 104380)
+++ trunk/Source/WebCore/dom/DynamicNodeList.cpp 2012-01-07 09:12:01 UTC (rev 104381)
@@ -36,26 +36,24 @@
{
}
-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 @@
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 @@
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 @@
{
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) {
Modified: trunk/Source/WebCore/dom/DynamicNodeList.h (104380 => 104381)
--- trunk/Source/WebCore/dom/DynamicNodeList.h 2012-01-07 08:46:38 UTC (rev 104380)
+++ trunk/Source/WebCore/dom/DynamicNodeList.h 2012-01-07 09:12:01 UTC (rev 104381)
@@ -86,14 +86,14 @@
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 @@
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;
};
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
