diff --git a/toolkit/components/thumbnails/BackgroundPageThumbs.jsm b/toolkit/components/thumbnails/BackgroundPageThumbs.jsm index fcfc3973925..ff0d27e0ec7 100644 --- a/toolkit/components/thumbnails/BackgroundPageThumbs.jsm +++ b/toolkit/components/thumbnails/BackgroundPageThumbs.jsm @@ -38,17 +38,6 @@ const BackgroundPageThumbs = { * the queue and started. Defaults to 30000 (30 seconds). */ capture: function (url, options={}) { - if (isPrivateBrowsingActive()) { - // There's only one, global private-browsing state shared by all private - // windows and the thumbnail browser. Just as if you log into a site in - // one private window you're logged in in all private windows, you're also - // logged in in the thumbnail browser. A crude way to avoid capturing - // sites in this situation is to refuse to capture at all when any private - // windows are open. See bug 870179. - if (options.onDone) - Services.tm.mainThread.dispatch(options.onDone.bind(options, url), 0); - return; - } this._captureQueue = this._captureQueue || []; this._capturesByURL = this._capturesByURL || new Map(); // We want to avoid duplicate captures for the same URL. If there is an @@ -239,11 +228,28 @@ Capture.prototype = { * @param messageManager The nsIMessageSender of the thumbnail browser. */ start: function (messageManager) { - let timeout = typeof(this.options.timeout) == "number" ? this.options.timeout : + // The thumbnail browser uses private browsing mode and therefore shares + // browsing state with private windows. To avoid capturing sites that the + // user is logged into in private browsing windows, (1) observe window + // openings, and if a private window is opened during capture, discard the + // capture when it finishes, and (2) don't start the capture at all if a + // private window is open already. + Services.ww.registerNotification(this); + if (isPrivateBrowsingActive()) { + // Captures should always finish asyncly. + schedule(() => this._done(null)); + return; + } + + // timeout timer + let timeout = typeof(this.options.timeout) == "number" ? + this.options.timeout : DEFAULT_CAPTURE_TIMEOUT; this._timeoutTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); - this._timeoutTimer.initWithCallback(this, timeout, Ci.nsITimer.TYPE_ONE_SHOT); + this._timeoutTimer.initWithCallback(this, timeout, + Ci.nsITimer.TYPE_ONE_SHOT); + // didCapture registration this._msgMan = messageManager; this._msgMan.sendAsyncMessage("BackgroundPageThumbs:capture", { id: this.id, url: this.url }); @@ -268,6 +274,7 @@ Capture.prototype = { delete this._msgMan; } delete this.captureCallback; + Services.ww.unregisterNotification(this); }, // Called when the didCapture message is received. @@ -283,6 +290,13 @@ Capture.prototype = { this._done(null); }, + // Called when the window watcher notifies us. + observe: function (subj, topic, data) { + if (topic == "domwindowopened" && + PrivateBrowsingUtils.isWindowPrivate(subj)) + this._privateWinOpenedDuringCapture = true; + }, + _done: function (data) { // Note that _done will be called only once, by either receiveMessage or // notify, since it calls destroy, which cancels the timeout timer and @@ -302,7 +316,7 @@ Capture.prototype = { } }.bind(this); - if (!data) { + if (!data || this._privateWinOpenedDuringCapture) { callOnDones(); return; } @@ -343,3 +357,7 @@ function isPrivateBrowsingActive() { return true; return false; } + +function schedule(callback) { + Services.tm.mainThread.dispatch(callback, Ci.nsIThread.DISPATCH_NORMAL); +} diff --git a/toolkit/components/thumbnails/test/browser_thumbnails_background.js b/toolkit/components/thumbnails/test/browser_thumbnails_background.js index 8657033f012..689b0c404b3 100644 --- a/toolkit/components/thumbnails/test/browser_thumbnails_background.js +++ b/toolkit/components/thumbnails/test/browser_thumbnails_background.js @@ -164,6 +164,42 @@ let tests = [ win.close(); }, + function openPrivateWindowDuringCapture() { + let url = "http://example.com/"; + let file = fileForURL(url); + ok(!file.exists(), "Thumbnail file should not already exist."); + + let deferred = imports.Promise.defer(); + + let waitCount = 0; + function maybeFinish() { + if (++waitCount == 2) + deferred.resolve(); + } + + imports.BackgroundPageThumbs.capture(url, { + onDone: function (capturedURL) { + is(capturedURL, url, "Captured URL should be URL passed to capture."); + ok(!file.exists(), + "Thumbnail file should not exist because a private window " + + "was opened during the capture."); + maybeFinish(); + }, + }); + + // Opening the private window at this point relies on a couple of + // implementation details: (1) The capture will start immediately and + // synchronously (since at this point in the test, the service is + // initialized and its queue is empty), and (2) when it starts the capture + // registers with the window watcher. + openPrivateWindow().then(function (win) { + win.close(); + maybeFinish(); + }); + + yield deferred.promise; + }, + function noCookies() { // Visit the test page in the browser and tell it to set a cookie. let url = testPageURL({ setGreenCookie: true }); @@ -263,7 +299,7 @@ let tests = [ imports.BackgroundPageThumbs.capture(url, {onDone: doneCallback}); imports.BackgroundPageThumbs.capture(url, {onDone: doneCallback}); yield deferred.promise; - } + }, ]; function capture(url, options) {