From 115bc7f78f8e2724241f7228564fce8703f1d4de Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Fri, 16 Jan 2015 11:50:32 +0000 Subject: [PATCH] Bug 1115248 - let DownloadLastDir work even if the window it depends on goes away, r=felipe --HG-- extra : rebase_source : 79decdb5e95dc3b31cd97d806705f16444eb4e31 --- .../downloads/test/browser/browser.ini | 4 ++ .../browser_iframe_gone_mid_download.js | 62 ++++++++++++++++ .../components/downloads/test/browser/head.js | 70 +++++++++++++++++++ toolkit/mozapps/downloads/DownloadLastDir.jsm | 34 +++++---- 4 files changed, 152 insertions(+), 18 deletions(-) create mode 100644 browser/components/downloads/test/browser/browser_iframe_gone_mid_download.js diff --git a/browser/components/downloads/test/browser/browser.ini b/browser/components/downloads/test/browser/browser.ini index c7f1fd16005a..d9cb4e58c2df 100644 --- a/browser/components/downloads/test/browser/browser.ini +++ b/browser/components/downloads/test/browser/browser.ini @@ -8,3 +8,7 @@ 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 + diff --git a/browser/components/downloads/test/browser/browser_iframe_gone_mid_download.js b/browser/components/downloads/test/browser/browser_iframe_gone_mid_download.js new file mode 100644 index 000000000000..ebdd4f9af84e --- /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})); + diff --git a/browser/components/downloads/test/browser/head.js b/browser/components/downloads/test/browser/head.js index 6566e693a022..7c4ffe5a342c 100644 --- a/browser/components/downloads/test/browser/head.js +++ b/browser/components/downloads/test/browser/head.js @@ -31,6 +31,76 @@ registerCleanupFunction(function () { //////////////////////////////////////////////////////////////////////////////// //// 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(); diff --git a/toolkit/mozapps/downloads/DownloadLastDir.jsm b/toolkit/mozapps/downloads/DownloadLastDir.jsm index add8beefbff2..89d1af8742b7 100644 --- a/toolkit/mozapps/downloads/DownloadLastDir.jsm +++ b/toolkit/mozapps/downloads/DownloadLastDir.jsm @@ -30,6 +30,7 @@ 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"); @@ -88,12 +89,21 @@ function isContentPrefEnabled() { 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(), @@ -110,11 +120,7 @@ DownloadLastDir.prototype = { 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); @@ -148,12 +154,8 @@ DownloadLastDir.prototype = { 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; @@ -173,14 +175,10 @@ DownloadLastDir.prototype = { 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)