Merge pull request #761 from ZER0/selection-gc/836930

Fix Bug 836930 - selection module may have early gc issues for system/event listeners r=@erikvold(cherry picked from commit df9c6ff)
mozilla · Feb 21, 2013 · b360cb7 · b360cb7
1 parent f496376
commit b360cb7
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 15 deletions.
diff --git a/lib/sdk/selection.js b/lib/sdk/selection.js
@@ -20,7 +20,7 @@ const { Ci, Cc } = require("chrome"),
     { when: unload } = require("./system/unload"),
     { getTabs, getTabContentWindow, getTabForContentWindow,
       getAllTabContentWindows } = require('./tabs/utils'),
-    { getInnerId, getMostRecentBrowserWindow,
+    { getMostRecentBrowserWindow,
       windows, getFocusedWindow, getFocusedElement } = require("./window/utils"),
     events = require("./system/events");
 
@@ -321,17 +321,6 @@ function addSelectionListener(window) {
   window.addEventListener("select", selectionListener.onSelect, true);
 
   selections(window).selection = selection;
-
-  let innerId = getInnerId(window);
-
-  events.on("inner-window-destroyed", function destroyed (event) {
-    let destroyedId = event.subject.QueryInterface(Ci.nsISupportsPRUint64).data;
-
-    if (destroyedId === innerId) {
-      removeSelectionListener(window);
-      events.off("inner-window-destroyed", destroyed);
-    }
-  });
 };
 
 /**
@@ -382,7 +371,7 @@ getAllTabContentWindows().forEach(addSelectionListener);
 //
 // See bug 665386 for further details.
 
-events.on("document-shown", function (event) {
+function onShown(event) {
   let window = event.subject.defaultView;
 
   // We are not interested in documents without valid defaultView.
@@ -408,17 +397,21 @@ events.on("document-shown", function (event) {
     if (currentSelection instanceof Ci.nsISelectionPrivate &&
       currentSelection !== selection) {
 
+      window.addEventListener("select", selectionListener.onSelect, true);
       currentSelection.addSelectionListener(selectionListener);
       selections(window).selection = currentSelection;
     }
   }
-});
+}
+
+events.on("document-shown", onShown, true);
 
 // Removes Selection listeners when the add-on is unloaded
 unload(function(){
   getAllTabContentWindows().forEach(removeSelectionListener);
 
   events.off("document-element-inserted", onContent);
+  events.off("document-shown", onShown);
 
   off(exports);
 });

diff --git a/test/test-selection.js b/test/test-selection.js
@@ -14,12 +14,16 @@ const HTML = "<html>\
 
 const URL = "data:text/html;charset=utf-8," + encodeURIComponent(HTML);
 
+const FRAME_HTML = "<iframe src='" + URL + "'><iframe>";
+const FRAME_URL = "data:text/html;charset=utf-8," + encodeURIComponent(FRAME_HTML);
+
 const { defer } = require("sdk/core/promise");
 const tabs = require("sdk/tabs");
 const { getActiveTab, getTabContentWindow, closeTab } = require("sdk/tabs/utils")
 const { getMostRecentBrowserWindow } = require("sdk/window/utils");
 const { Loader } = require("sdk/test/loader");
 const { setTimeout } = require("sdk/timers");
+const { Cu } = require("chrome");
 
 // General purpose utility functions
 
@@ -82,6 +86,55 @@ function reload(window) {
 
 // Selection's unit test utility function
 
+/**
+ * Returns the frame's window once the document is loaded
+ */
+function getFrameWindow(window) {
+  let { promise, resolve } = defer();
+
+  let frame = window.frames[0];
+  let { document } = frame;
+
+  frame.focus();
+
+  if (document.readyState === "complete")
+    return frame;
+
+  document.addEventListener("readystatechange", function readystate() {
+    if (this.readyState === "complete") {
+      this.removeEventListener("readystatechange", readystate);
+      frame.focus();
+      resolve(frame);
+    }
+  });
+
+  return promise;
+}
+
+/**
+ * Hide the frame in order to destroy the selection object, and show it again
+ * after ~500 msec, to give time to attach the code on `document-shown`
+ * notification.
+ * In the process, call `Cu.forgeGC` to ensure that the `document-shown` code
+ * is not garbaged.
+ */
+function hideAndShowFrame(window) {
+  let { promise, resolve } = defer();
+  let iframe = window.document.querySelector("iframe");
+
+  iframe.style.display = "none";
+
+  Cu.forceGC();
+
+  setTimeout(function(){
+    iframe.style.display = "";
+
+    setTimeout(resolve, 500, window);
+  }, 0)
+
+  return promise;
+}
+
 /**
  * Select the first div in the page, adding the range to the selection.
  */
@@ -174,7 +227,7 @@ function dispatchOnSelectEvent(window) {
 
   event.initUIEvent("select", true, true, window, 1);
 
-  textarea.dispatchEvent(event)
+  textarea.dispatchEvent(event);
 }
 
 /**
@@ -730,6 +783,42 @@ exports["test Textarea OnSelect Listener on document reload"] = function(assert,
     then(loader.unload);
 };
 
+exports["test Selection Listener on frame"] = function(assert, done) {
+  let loader = Loader(module);
+  let selection = loader.require("sdk/selection");
+
+  selection.once("select", function() {
+    assert.equal(selection.text, "fo");
+    done();
+  });
+
+  open(FRAME_URL).
+    then(hideAndShowFrame).
+    then(getFrameWindow).
+    then(selectContentFirstDiv).
+    then(dispatchSelectionEvent).
+    then(close).
+    then(loader.unload)
+};
+
+exports["test Textarea onSelect Listener on frame"] = function(assert, done) {
+  let loader = Loader(module);
+  let selection = loader.require("sdk/selection");
+
+  selection.once("select", function() {
+    assert.equal(selection.text, "noodles");
+    done();
+  });
+
+  open(FRAME_URL).
+    then(hideAndShowFrame).
+    then(getFrameWindow).
+    then(selectTextarea).
+    then(dispatchOnSelectEvent).
+    then(close).
+    then(loader.unload)
+};
+
 // TODO: test Selection Listener on long-held connection (Bug 661884)
 //
 //  I didn't find a way to do so with httpd, using `processAsync` I'm able to