Bug 559526 - Moving a NodeIterator from its NodeFilter causes a crash, r=sicking
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Fri, 16 Apr 2010 21:45:13 +0300
changeset 40942 c2e522dc68b486db114e0d091c7263c7cfc9ca54
parent 40941 935bb616b8a699537f2bbc2ef6392be1a263f4d3
child 40943 f743ed8fb2c566fad9972a22846745108e1221e7
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs559526
milestone1.9.3a5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
Bug 559526 - Moving a NodeIterator from its NodeFilter causes a crash, r=sicking
content/base/src/nsNodeIterator.cpp
content/base/src/nsTraversal.cpp
content/base/src/nsTraversal.h
content/base/test/Makefile.in
content/base/test/test_bug559526.html
--- a/content/base/src/nsNodeIterator.cpp
+++ b/content/base/src/nsNodeIterator.cpp
@@ -296,17 +296,17 @@ nsresult
 nsNodeIterator::NextOrPrevNode(NodePointer::MoveToMethodType aMove,
                                nsIDOMNode **_retval)
 {
     nsresult rv;
     PRInt16 filtered;
 
     *_retval = nsnull;
 
-    if (mDetached)
+    if (mDetached || mInAcceptNode)
         return NS_ERROR_DOM_INVALID_STATE_ERR;
 
     mWorkingPointer = mPointer;
 
     struct AutoClear {
         NodePointer* mPtr;
         AutoClear(NodePointer* ptr) : mPtr(ptr) {}
        ~AutoClear() { mPtr->Clear(); }
--- a/content/base/src/nsTraversal.cpp
+++ b/content/base/src/nsTraversal.cpp
@@ -49,17 +49,18 @@
 
 nsTraversal::nsTraversal(nsINode *aRoot,
                          PRUint32 aWhatToShow,
                          nsIDOMNodeFilter *aFilter,
                          PRBool aExpandEntityReferences) :
     mRoot(aRoot),
     mWhatToShow(aWhatToShow),
     mFilter(aFilter),
-    mExpandEntityReferences(aExpandEntityReferences)
+    mExpandEntityReferences(aExpandEntityReferences),
+    mInAcceptNode(PR_FALSE)
 {
     NS_ASSERTION(aRoot, "invalid root in call to nsTraversal constructor");
 }
 
 nsTraversal::~nsTraversal()
 {
     /* destructor code */
 }
@@ -68,16 +69,18 @@ nsTraversal::~nsTraversal()
  * Tests if and how a node should be filtered. Uses mWhatToShow and
  * mFilter to test the node.
  * @param aNode     Node to test
  * @param _filtered Returned filtervalue. See nsIDOMNodeFilter.idl
  * @returns         Errorcode
  */
 nsresult nsTraversal::TestNode(nsINode* aNode, PRInt16* _filtered)
 {
+    NS_ENSURE_TRUE(!mInAcceptNode, NS_ERROR_DOM_INVALID_STATE_ERR);
+
     nsresult rv;
 
     *_filtered = nsIDOMNodeFilter::FILTER_SKIP;
 
     PRUint16 nodeType = 0;
     // Check the most common cases
     if (aNode->IsNodeOfType(nsINode::eELEMENT)) {
         nodeType = nsIDOMNode::ELEMENT_NODE;
@@ -109,14 +112,17 @@ nsresult nsTraversal::TestNode(nsINode* 
         return NS_OK;
     }
 
     if (mFilter) {
         if (!domNode) {
             domNode = do_QueryInterface(aNode);
         }
 
-        return mFilter->AcceptNode(domNode, _filtered);
+        mInAcceptNode = PR_TRUE;
+        rv = mFilter->AcceptNode(domNode, _filtered);
+        mInAcceptNode = PR_FALSE;
+        return rv;
     }
 
     *_filtered = nsIDOMNodeFilter::FILTER_ACCEPT;
     return NS_OK;
 }
--- a/content/base/src/nsTraversal.h
+++ b/content/base/src/nsTraversal.h
@@ -57,17 +57,18 @@ public:
                 nsIDOMNodeFilter *aFilter,
                 PRBool aExpandEntityReferences);
     virtual ~nsTraversal();
 
 protected:
     nsCOMPtr<nsINode> mRoot;
     PRUint32 mWhatToShow;
     nsCOMPtr<nsIDOMNodeFilter> mFilter;
-    PRBool mExpandEntityReferences;
+    PRPackedBool mExpandEntityReferences;
+    PRPackedBool mInAcceptNode;
 
     /*
      * Tests if and how a node should be filtered. Uses mWhatToShow and
      * mFilter to test the node.
      * @param aNode     Node to test
      * @param _filtered Returned filtervalue. See nsIDOMNodeFilter.idl
      * @returns         Errorcode
      */
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -367,16 +367,17 @@ include $(topsrcdir)/config/rules.mk
 		test_bug545644.xhtml \
 		test_bug553896.xhtml \
 		test_bug541937.html \
 		file_bug541937.html \
 		file_bug541937.xhtml \
 		test_bug558726.html \
 		test_bug557892.html \
 		file_bug557892.html \
+		test_bug559526.html \
 		$(NULL)
 
 # This test fails on the Mac for some reason
 ifneq (,$(filter gtk2 windows,$(MOZ_WIDGET_TOOLKIT)))
 _TEST_FILES += 	test_copyimage.html \
 		$(NULL)
 endif
 
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_bug559526.html
@@ -0,0 +1,97 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=559526
+-->
+<head>
+  <title>Test for Bug 559526</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=559526">Mozilla Bug 559526</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<div id="nodes" style="display:none">
+  <div id="child1"></div>
+  <div id="child2"></div>
+
+  <div id="child3"></div>
+  <div id="child4"></div>
+  <div id="child5"></div>
+  <div id="child6"></div>
+  <div id="child7"></div>
+  <div id="child8"></div>
+</div>
+<script type="application/javascript">
+
+/** Test for Bug 559526 **/
+
+var it;
+var recurse = false;
+var testCount = 0;
+function filter(node) {
+  if (node.id == "child3" && ! recurse) {
+    recurse = true;
+    var ex = null;
+    try {
+      var foo = it.nextNode();
+    } catch(e) {
+      ex = e;
+    }
+    ++testCount;
+    is(ex.code, DOMException.INVALID_STATE_ERR, "Should have thrown an exception!");
+    recurse = false;
+  }
+  return NodeFilter.FILTER_ACCEPT;
+}
+
+(function testNodeIterator() {
+
+  it = document.createNodeIterator(
+    document.getElementById("nodes"),
+    NodeFilter.SHOW_ELEMENT,
+    filter,
+    false
+  );
+  while (it.nextNode());
+})();
+
+(function testTreeWalker() {
+  it = document.createTreeWalker(
+    document.getElementById("nodes"),
+    NodeFilter.SHOW_ELEMENT,
+    filter,
+    false
+  );
+  while(it.nextNode());
+
+  it = document.createTreeWalker(
+    document.getElementById("nodes"),
+    NodeFilter.SHOW_ELEMENT,
+    filter,
+    false
+  );
+  it.firstChild();
+  while(it.nextSibling());
+
+  it = document.createTreeWalker(
+    document.getElementById("nodes"),
+    NodeFilter.SHOW_ELEMENT,
+    filter,
+    false
+  );
+  it.lastChild();
+  while(it.previousSibling());
+})();
+
+is(testCount, 4, "Should have tests 3 filter calls!");
+
+</script>
+</pre>
+</body>
+</html>