Bug 1115248 - let DownloadLastDir work even if the window it depends on goes away, r=felipe
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 16 Jan 2015 11:50:32 +0000
changeset 224317 120b108aa17634ccc58cc6ddbfac6e2cd4677a17
parent 224316 b1424a861ccab26af2f6591c715b6d51a23f24bd
child 224318 fb5bebd170baed4bcb5f88590bf316142febde4a
push id54190
push userkwierso@gmail.com
push dateSat, 17 Jan 2015 02:06:29 +0000
treeherdermozilla-inbound@369a8f14ccf8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfelipe
bugs1115248
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1115248 - let DownloadLastDir work even if the window it depends on goes away, r=felipe
browser/components/downloads/test/browser/browser.ini
browser/components/downloads/test/browser/browser_iframe_gone_mid_download.js
browser/components/downloads/test/browser/head.js
toolkit/mozapps/downloads/DownloadLastDir.jsm
--- a/browser/components/downloads/test/browser/browser.ini
+++ b/browser/components/downloads/test/browser/browser.ini
@@ -3,8 +3,12 @@ support-files = head.js
 
 [browser_basic_functionality.js]
 skip-if = buildapp == "mulet" || e10s
 [browser_first_download_panel.js]
 skip-if = os == "linux" # Bug 949434
 [browser_overflow_anchor.js]
 skip-if = os == "linux" # Bug 952422
 [browser_confirm_unblock_download.js]
+
+[browser_iframe_gone_mid_download.js]
+skip-if = e10s
+
new file mode 100644
--- /dev/null
+++ b/browser/components/downloads/test/browser/browser_iframe_gone_mid_download.js
@@ -0,0 +1,62 @@
+const SAVE_PER_SITE_PREF = "browser.download.lastDir.savePerSite";
+
+function test_deleted_iframe(perSitePref, windowOptions={}) {
+  return function*() {
+    Services.prefs.setBoolPref(SAVE_PER_SITE_PREF, perSitePref);
+    let {DownloadLastDir} = Cu.import("resource://gre/modules/DownloadLastDir.jsm", {});
+
+    let win = yield promiseOpenAndLoadWindow(windowOptions);
+    let tab = win.gBrowser.addTab();
+    yield promiseTabLoadEvent(tab, "about:mozilla");
+
+    let doc = tab.linkedBrowser.contentDocument;
+    let iframe = doc.createElement("iframe");
+    doc.body.appendChild(iframe);
+
+    ok(iframe.contentWindow, "iframe should have a window");
+    let gDownloadLastDir = new DownloadLastDir(iframe.contentWindow);
+    let cw = iframe.contentWindow;
+    let promiseIframeWindowGone = new Promise((resolve, reject) => {
+      Services.obs.addObserver(function obs(subject, topic) {
+        if (subject == cw) {
+          Services.obs.removeObserver(obs, topic);
+          resolve();
+        }
+      }, "dom-window-destroyed", false);
+    });
+    iframe.remove();
+    yield promiseIframeWindowGone;
+    cw = null;
+    ok(!iframe.contentWindow, "Managed to destroy iframe");
+
+    let someDir = "blah";
+    try {
+      someDir = yield new Promise((resolve, reject) => {
+        gDownloadLastDir.getFileAsync("http://www.mozilla.org/", function(dir) {
+          resolve(dir);
+        });
+      });
+    } catch (ex) {
+      ok(false, "Got an exception trying to get the directory where things should be saved.");
+      Cu.reportError(ex);
+    }
+    // NB: someDir can legitimately be null here when set, hence the 'blah' workaround:
+    isnot(someDir, "blah", "Should get a file even after the window was destroyed.");
+
+    try {
+      gDownloadLastDir.setFile("http://www.mozilla.org/", null);
+    } catch (ex) {
+      ok(false, "Got an exception trying to set the directory where things should be saved.");
+      Cu.reportError(ex);
+    }
+
+    yield promiseWindowClosed(win);
+    Services.prefs.clearUserPref(SAVE_PER_SITE_PREF);
+  };
+}
+
+add_task(test_deleted_iframe(false));
+add_task(test_deleted_iframe(false));
+add_task(test_deleted_iframe(true, {private: true}));
+add_task(test_deleted_iframe(true, {private: true}));
+
--- a/browser/components/downloads/test/browser/head.js
+++ b/browser/components/downloads/test/browser/head.js
@@ -26,16 +26,86 @@ let gTestTargetFile = FileUtils.getFile(
 gTestTargetFile.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE);
 registerCleanupFunction(function () {
   gTestTargetFile.remove(false);
 });
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Asynchronous support subroutines
 
+function promiseOpenAndLoadWindow(aOptions)
+{
+  return new Promise((resolve, reject) => {
+    let win = OpenBrowserWindow(aOptions);
+    win.addEventListener("load", function onLoad() {
+      win.removeEventListener("load", onLoad);
+      resolve(win);
+    });
+  });
+}
+
+/**
+ * Waits for a load (or custom) event to finish in a given tab. If provided
+ * load an uri into the tab.
+ *
+ * @param tab
+ *        The tab to load into.
+ * @param [optional] url
+ *        The url to load, or the current url.
+ * @param [optional] event
+ *        The load event type to wait for.  Defaults to "load".
+ * @return {Promise} resolved when the event is handled.
+ * @resolves to the received event
+ * @rejects if a valid load event is not received within a meaningful interval
+ */
+function promiseTabLoadEvent(tab, url, eventType="load")
+{
+  let deferred = Promise.defer();
+  info("Wait tab event: " + eventType);
+
+  function handle(event) {
+    if (event.originalTarget != tab.linkedBrowser.contentDocument ||
+        event.target.location.href == "about:blank" ||
+        (url && event.target.location.href != url)) {
+      info("Skipping spurious '" + eventType + "'' event" +
+           " for " + event.target.location.href);
+      return;
+    }
+    // Remove reference to tab from the cleanup function:
+    realCleanup = () => {};
+    tab.linkedBrowser.removeEventListener(eventType, handle, true);
+    info("Tab event received: " + eventType);
+    deferred.resolve(event);
+  }
+
+  // Juggle a bit to avoid leaks:
+  let realCleanup = () => tab.linkedBrowser.removeEventListener(eventType, handle, true);
+  registerCleanupFunction(() => realCleanup());
+
+  tab.linkedBrowser.addEventListener(eventType, handle, true, true);
+  if (url)
+    tab.linkedBrowser.loadURI(url);
+  return deferred.promise;
+}
+
+function promiseWindowClosed(win)
+{
+  let promise = new Promise((resolve, reject) => {
+    Services.obs.addObserver(function obs(subject, topic) {
+      if (subject == win) {
+        Services.obs.removeObserver(obs, topic);
+        resolve();
+      }
+    }, "domwindowclosed", false);
+  });
+  win.close();
+  return promise;
+}
+
+
 function promiseFocus()
 {
   let deferred = Promise.defer();
   waitForFocus(deferred.resolve);
   return deferred.promise;
 }
 
 function promisePanelOpened()
--- a/toolkit/mozapps/downloads/DownloadLastDir.jsm
+++ b/toolkit/mozapps/downloads/DownloadLastDir.jsm
@@ -25,16 +25,17 @@
  */
 
 const LAST_DIR_PREF = "browser.download.lastDir";
 const SAVE_PER_SITE_PREF = LAST_DIR_PREF + ".savePerSite";
 const nsIFile = Components.interfaces.nsIFile;
 
 this.EXPORTED_SYMBOLS = [ "DownloadLastDir" ];
 
+Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
 Components.utils.import("resource://gre/modules/Services.jsm");
 Components.utils.import("resource://gre/modules/PrivateBrowsingUtils.jsm");
 
 let observer = {
   QueryInterface: function (aIID) {
     if (aIID.equals(Components.interfaces.nsIObserver) ||
         aIID.equals(Components.interfaces.nsISupports) ||
         aIID.equals(Components.interfaces.nsISupportsWeakReference))
@@ -83,43 +84,48 @@ function isContentPrefEnabled() {
   catch (e) {
     return true;
   }
 }
 
 let gDownloadLastDirFile = readLastDirPref();
 
 this.DownloadLastDir = function DownloadLastDir(aWindow) {
-  this.window = aWindow;
+  let loadContext = aWindow.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
+                           .getInterface(Components.interfaces.nsIWebNavigation)
+                           .QueryInterface(Components.interfaces.nsILoadContext);
+  // Need this in case the real thing has gone away by the time we need it.
+  // We only care about the private browsing state. All the rest of the
+  // load context isn't of interest to the content pref service.
+  this.fakeContext = {
+    QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsILoadContext]),
+    usePrivateBrowsing: loadContext.usePrivateBrowsing
+  };
 }
 
 DownloadLastDir.prototype = {
   isPrivate: function DownloadLastDir_isPrivate() {
-    return PrivateBrowsingUtils.isWindowPrivate(this.window);
+    return this.fakeContext.usePrivateBrowsing;
   },
   // compat shims
   get file() this._getLastFile(),
   set file(val) { this.setFile(null, val); },
   cleanupPrivateFile: function () {
     gDownloadLastDirFile = null;
   },
   // This function is now deprecated as it uses the sync nsIContentPrefService
   // interface. New consumers should use the getFileAsync function.
   getFile: function (aURI) {
     let Deprecated = Components.utils.import("resource://gre/modules/Deprecated.jsm", {}).Deprecated;
     Deprecated.warning("DownloadLastDir.getFile is deprecated. Please use getFileAsync instead.",
                        "https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/DownloadLastDir.jsm",
                        Components.stack.caller);
 
     if (aURI && isContentPrefEnabled()) {
-      let loadContext = this.window
-                            .QueryInterface(Components.interfaces.nsIInterfaceRequestor)
-                            .getInterface(Components.interfaces.nsIWebNavigation)
-                            .QueryInterface(Components.interfaces.nsILoadContext);
-      let lastDir = Services.contentPrefs.getPref(aURI, LAST_DIR_PREF, loadContext);
+      let lastDir = Services.contentPrefs.getPref(aURI, LAST_DIR_PREF, this.fakeContext);
       if (lastDir) {
         var lastDirFile = Components.classes["@mozilla.org/file/local;1"]
                                     .createInstance(Components.interfaces.nsIFile);
         lastDirFile.initWithPath(lastDir);
         return lastDirFile;
       }
     }
     return this._getLastFile();
@@ -143,22 +149,18 @@ DownloadLastDir.prototype = {
       Services.tm.mainThread.dispatch(function() aCallback(plainPrefFile),
                                       Components.interfaces.nsIThread.DISPATCH_NORMAL);
       return;
     }
 
     let uri = aURI instanceof Components.interfaces.nsIURI ? aURI.spec : aURI;
     let cps2 = Components.classes["@mozilla.org/content-pref/service;1"]
                          .getService(Components.interfaces.nsIContentPrefService2);
-    let loadContext = this.window
-                          .QueryInterface(Components.interfaces.nsIInterfaceRequestor)
-                          .getInterface(Components.interfaces.nsIWebNavigation)
-                          .QueryInterface(Components.interfaces.nsILoadContext);
     let result = null;
-    cps2.getByDomainAndName(uri, LAST_DIR_PREF, loadContext, {
+    cps2.getByDomainAndName(uri, LAST_DIR_PREF, this.fakeContext, {
       handleResult: function(aResult) result = aResult,
       handleCompletion: function(aReason) {
         let file = plainPrefFile;
         if (aReason == Components.interfaces.nsIContentPrefCallback2.COMPLETE_OK &&
            result instanceof Components.interfaces.nsIContentPref) {
           file = Components.classes["@mozilla.org/file/local;1"]
                            .createInstance(Components.interfaces.nsIFile);
           file.initWithPath(result.value);
@@ -168,24 +170,20 @@ DownloadLastDir.prototype = {
     });
   },
 
   setFile: function (aURI, aFile) {
     if (aURI && isContentPrefEnabled()) {
       let uri = aURI instanceof Components.interfaces.nsIURI ? aURI.spec : aURI;
       let cps2 = Components.classes["@mozilla.org/content-pref/service;1"]
                            .getService(Components.interfaces.nsIContentPrefService2);
-      let loadContext = this.window
-                            .QueryInterface(Components.interfaces.nsIInterfaceRequestor)
-                            .getInterface(Components.interfaces.nsIWebNavigation)
-                            .QueryInterface(Components.interfaces.nsILoadContext);
       if (aFile instanceof Components.interfaces.nsIFile)
-        cps2.set(uri, LAST_DIR_PREF, aFile.path, loadContext);
+        cps2.set(uri, LAST_DIR_PREF, aFile.path, this.fakeContext);
       else
-        cps2.removeByDomainAndName(uri, LAST_DIR_PREF, loadContext);
+        cps2.removeByDomainAndName(uri, LAST_DIR_PREF, this.fakeContext);
     }
     if (this.isPrivate()) {
       if (aFile instanceof Components.interfaces.nsIFile)
         gDownloadLastDirFile = aFile.clone();
       else
         gDownloadLastDirFile = null;
     } else {
       if (aFile instanceof Components.interfaces.nsIFile)