Bug 393974 - Tree walkers leak with a non-null filter. r=smaug, sr=sicking, a=blocking1.9
authorjwalden@mit.edu
Tue, 28 Aug 2007 16:42:41 -0700
changeset 5402 80131b88a3cc34269594073092d8e5ca15bca062
parent 5401 2bfe36ffbfbe737af68f1a771a9b2812aa4a9cd3
child 5403 b25941106951afce9e46cd7cb7e128136b8d4e90
push idunknown
push userunknown
push dateunknown
reviewerssmaug, sicking, blocking1
bugs393974
milestone1.9a8pre
Bug 393974 - Tree walkers leak with a non-null filter. r=smaug, sr=sicking, a=blocking1.9
content/base/src/nsTreeWalker.cpp
content/base/src/nsTreeWalker.h
dom/tests/mochitest/bugs/Makefile.in
dom/tests/mochitest/bugs/test_bug393974.html
--- a/content/base/src/nsTreeWalker.cpp
+++ b/content/base/src/nsTreeWalker.cpp
@@ -95,28 +95,32 @@ nsTreeWalker::nsTreeWalker(nsINode *aRoo
 }
 
 nsTreeWalker::~nsTreeWalker()
 {
     /* destructor code */
 }
 
 /*
- * nsISupports stuff
+ * nsISupports and cycle collection stuff
  */
 
+NS_IMPL_CYCLE_COLLECTION_3(nsTreeWalker, mFilter, mCurrentNode, mRoot)
+
 // QueryInterface implementation for nsTreeWalker
-NS_INTERFACE_MAP_BEGIN(nsTreeWalker)
+NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsTreeWalker)
     NS_INTERFACE_MAP_ENTRY(nsIDOMTreeWalker)
     NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIDOMTreeWalker)
     NS_INTERFACE_MAP_ENTRY_CONTENT_CLASSINFO(TreeWalker)
 NS_INTERFACE_MAP_END
 
-NS_IMPL_ADDREF(nsTreeWalker)
-NS_IMPL_RELEASE(nsTreeWalker)
+NS_IMPL_CYCLE_COLLECTING_ADDREF(nsTreeWalker)
+NS_IMPL_CYCLE_COLLECTING_RELEASE(nsTreeWalker)
+
+
 
 /*
  * nsIDOMTreeWalker Getters/Setters
  */
 
 /* readonly attribute nsIDOMNode root; */
 NS_IMETHODIMP nsTreeWalker::GetRoot(nsIDOMNode * *aRoot)
 {
--- a/content/base/src/nsTreeWalker.h
+++ b/content/base/src/nsTreeWalker.h
@@ -43,33 +43,36 @@
 
 #ifndef nsTreeWalker_h___
 #define nsTreeWalker_h___
 
 #include "nsIDOMTreeWalker.h"
 #include "nsCOMPtr.h"
 #include "nsVoidArray.h"
 #include "nsJSUtils.h"
+#include "nsCycleCollectionParticipant.h"
 
 class nsINode;
 class nsIDOMNode;
 class nsIDOMNodeFilter;
 
 class nsTreeWalker : public nsIDOMTreeWalker
 {
 public:
-    NS_DECL_ISUPPORTS
+    NS_DECL_CYCLE_COLLECTING_ISUPPORTS
     NS_DECL_NSIDOMTREEWALKER
 
     nsTreeWalker(nsINode *aRoot,
                  PRUint32 aWhatToShow,
                  nsIDOMNodeFilter *aFilter,
                  PRBool aExpandEntityReferences);
     virtual ~nsTreeWalker();
-    /* additional members */
+
+    NS_DECL_CYCLE_COLLECTION_CLASS(nsTreeWalker)
+
 private:
     nsCOMPtr<nsINode> mRoot;
     PRUint32 mWhatToShow;
     nsCOMPtr<nsIDOMNodeFilter> mFilter;
     PRBool mExpandEntityReferences;
     nsCOMPtr<nsINode> mCurrentNode;
     
     /*
--- a/dom/tests/mochitest/bugs/Makefile.in
+++ b/dom/tests/mochitest/bugs/Makefile.in
@@ -50,12 +50,13 @@ include $(topsrcdir)/config/rules.mk
 		test_bug308856.html \
 		test_bug333983.html \
 		test_bug342448.html \
 		test_bug345521.html \
 		test_bug351601.html \
 		test_bug370098.html \
 		test_bug377539.html \
 		test_bug384122.html \
+		test_bug393974.html \
 		$(NULL)
 
 libs:: 	$(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/bugs/test_bug393974.html
@@ -0,0 +1,70 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=393974
+-->
+<head>
+  <title>Test for Bug 393974</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/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=393974">Mozilla Bug 393974</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+SimpleTest.waitForExplicitFinish();
+
+function test()
+{
+  // Create a tree walker with a filter which creates a cycle...
+  var tw = document.createTreeWalker(document, NodeFilter.SHOW_ALL,
+                                     function(n)
+                                     {
+                                       // force the closure to contain the
+                                       // global object, in case a future
+                                       // optimization might minimize the
+                                       // function's captured environment
+                                       if ("foo" + window == "fooPIRATES!")
+                                         return NodeFilter.FILTER_ACCEPT;
+                                       return NodeFilter.FILTER_REJECT;
+                                     },
+                                     true);
+
+  // That should have been enough to create a leak, but we should do at least
+  // a couple tests while we're here so that this document doesn't show up as
+  // having no tests pass *or* fail.
+
+  ok(tw.firstChild() === null, "shouldn't be a first child");
+  ok(tw.currentNode === document, "should be unchanged");
+
+  ok(tw.lastChild() === null, "shouldn't be a last child");
+  ok(tw.currentNode === document, "should be unchanged");
+
+  ok(tw.nextNode() === null, "shouldn't be a next node");
+  ok(tw.currentNode === document, "should be unchanged");
+
+  ok(tw.nextSibling() === null, "shouldn't be a next sibling");
+  ok(tw.currentNode === document, "should be unchanged");
+
+  ok(tw.parentNode() === null, "shouldn't be a parent node");
+  ok(tw.currentNode === document, "should be unchanged");
+
+  ok(tw.previousNode() === null, "shouldn't be a previous node");
+  ok(tw.currentNode === document, "should be unchanged");
+
+  ok(tw.previousSibling() === null, "shouldn't be a previous sibling");
+  ok(tw.currentNode === document, "should be unchanged");
+}
+
+addLoadEvent(test);
+addLoadEvent(SimpleTest.finish);
+</script>
+</pre>
+</body>
+</html>