Added removal of unload event listener on unload for windowUtils.clos…

…eOnUnload(), as per adw's advice.
mozilla · Apr 15, 2010 · 60d58bf · 60d58bf
1 parent 7d6416a
commit 60d58bf
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 2 deletions.
diff --git a/packages/jetpack-core/lib/window-utils.js b/packages/jetpack-core/lib/window-utils.js
@@ -121,7 +121,9 @@ function onDocUnload(event) {
   var index = gDocsToClose.indexOf(event.target);
   if (index == -1)
     throw new Error("internal error: unloading document not found");
-  gDocsToClose.splice(index, 1);
+  var document = gDocsToClose.splice(index, 1)[0];
+  // Just in case, let's remove the event listener too.
+  document.defaultView.removeEventListener("unload", onDocUnload, false);
 }
 
 onDocUnload = require("errors").catchAndLog(onDocUnload);

diff --git a/packages/jetpack-core/tests/test-window-utils.js b/packages/jetpack-core/tests/test-window-utils.js
@@ -23,13 +23,18 @@ exports.testCloseOnUnload = function(test) {
     addEventListener: function(name, func, bool) {
       this._listeners.push(func);
     },
+    removeEventListener: function(name, func, bool) {
+      var index = this._listeners.indexOf(func);
+      if (index == -1)
+        throw new Error("event listener not found");
+      this._listeners.splice(index, 1);
+    },
     close: function() {
       timesClosed++;
       this._listeners.forEach(
         function(func) { 
           func({target: fakeWindow.document});
         });
-      this._listeners = [];
     },
     document: {
       get defaultView() { return fakeWindow; }
@@ -38,11 +43,15 @@ exports.testCloseOnUnload = function(test) {
 
   var loader = test.makeSandboxedLoader();
   loader.require("window-utils").closeOnUnload(fakeWindow);
+  test.assertEqual(fakeWindow._listeners.length, 1,
+                   "unload listener added on closeOnUnload()");
   test.assertEqual(timesClosed, 0,
                    "window not closed when registered.");
   loader.require("unload").send();
   test.assertEqual(timesClosed, 1,
                    "window closed on module unload.");
+  test.assertEqual(fakeWindow._listeners.length, 0,
+                   "unload event listener removed on module unload");
 
   timesClosed = 0;
   loader.require("window-utils").closeOnUnload(fakeWindow);
@@ -51,6 +60,8 @@ exports.testCloseOnUnload = function(test) {
   fakeWindow.close();
   test.assertEqual(timesClosed, 1,
                    "window closed when close() called.");
+  test.assertEqual(fakeWindow._listeners.length, 0,
+                   "unload event listener removed on window close");
   loader.require("unload").send();
   test.assertEqual(timesClosed, 1,
                    "window not closed again on module unload.");