Bug 875986, part 1 - use custom docShell flags instead of private-browsing for b/g thumbnails. r=adw
authorMark Hammond <mhammond@skippinet.com.au>
Mon, 09 Sep 2013 10:25:13 +1000
changeset 146131 b152bdd937ebdcd13b77bd4308e14ebf86093c05
parent 146130 e89edb607559cd9cd0ae32e5fb24f0791ca02c96
child 146132 6bee20c2e0c6e2f7ddcdd9bebc0c9b6d7a3d0895
push id25242
push useremorley@mozilla.com
push dateMon, 09 Sep 2013 12:13:52 +0000
treeherdermozilla-central@218d4334d29e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs875986
milestone26.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 875986, part 1 - use custom docShell flags instead of private-browsing for b/g thumbnails. r=adw
toolkit/components/thumbnails/BackgroundPageThumbs.jsm
toolkit/components/thumbnails/content/backgroundPageThumbsContent.js
toolkit/components/thumbnails/test/browser_thumbnails_background.js
--- a/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
+++ b/toolkit/components/thumbnails/BackgroundPageThumbs.jsm
@@ -10,27 +10,25 @@ const DEFAULT_CAPTURE_TIMEOUT = 30000; /
 const DESTROY_BROWSER_TIMEOUT = 60000; // ms
 const FRAME_SCRIPT_URL = "chrome://global/content/backgroundPageThumbsContent.js";
 
 const TELEMETRY_HISTOGRAM_ID_PREFIX = "FX_THUMBNAILS_BG_";
 
 // possible FX_THUMBNAILS_BG_CAPTURE_DONE_REASON telemetry values
 const TEL_CAPTURE_DONE_OK = 0;
 const TEL_CAPTURE_DONE_TIMEOUT = 1;
-const TEL_CAPTURE_DONE_PB_BEFORE_START = 2;
-const TEL_CAPTURE_DONE_PB_AFTER_START = 3;
+// 2 and 3 were used when we had special handling for private-browsing.
 
 const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 const HTML_NS = "http://www.w3.org/1999/xhtml";
 
 const { classes: Cc, interfaces: Ci, utils: Cu } = Components;
 
 Cu.import("resource://gre/modules/PageThumbs.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://gre/modules/PrivateBrowsingUtils.jsm");
 
 const BackgroundPageThumbs = {
 
   /**
    * Asynchronously captures a thumbnail of the given URL.
    *
    * The page is loaded anonymously, and plug-ins are disabled.
    *
@@ -83,42 +81,30 @@ const BackgroundPageThumbs = {
       // Already fully initialized.
       return true;
     if (this._startedParentWinInit)
       // Already started initializing.
       return false;
 
     this._startedParentWinInit = true;
 
-    PrivateBrowsingUtils.whenHiddenPrivateWindowReady(function (parentWin) {
-      parentWin.addEventListener("unload", function (event) {
-        if (event.target == parentWin.document)
-          this._destroy();
-      }.bind(this), true);
-
-      if (canHostBrowser(parentWin)) {
-        this._parentWin = parentWin;
-        this._processCaptureQueue();
-        return;
-      }
-
-      // Otherwise, create an html:iframe, stick it in the parent document, and
-      // use it to host the browser.  about:blank will not have the system
-      // principal, so it can't host, but a document with a chrome URI will.
-      let iframe = parentWin.document.createElementNS(HTML_NS, "iframe");
-      iframe.setAttribute("src", "chrome://global/content/mozilla.xhtml");
-      let onLoad = function onLoadFn() {
-        iframe.removeEventListener("load", onLoad, true);
-        this._parentWin = iframe.contentWindow;
-        this._processCaptureQueue();
-      }.bind(this);
-      iframe.addEventListener("load", onLoad, true);
-      parentWin.document.documentElement.appendChild(iframe);
-      this._hostIframe = iframe;
-    }.bind(this));
+    // Create an html:iframe, stick it in the parent document, and
+    // use it to host the browser.  about:blank will not have the system
+    // principal, so it can't host, but a document with a chrome URI will.
+    let hostWindow = Services.appShell.hiddenDOMWindow;
+    let iframe = hostWindow.document.createElementNS(HTML_NS, "iframe");
+    iframe.setAttribute("src", "chrome://global/content/mozilla.xhtml");
+    let onLoad = function onLoadFn() {
+      iframe.removeEventListener("load", onLoad, true);
+      this._parentWin = iframe.contentWindow;
+      this._processCaptureQueue();
+    }.bind(this);
+    iframe.addEventListener("load", onLoad, true);
+    hostWindow.document.documentElement.appendChild(iframe);
+    this._hostIframe = iframe;
 
     return false;
   },
 
   /**
    * Destroys the service.  Queued and pending captures will never complete, and
    * their consumer callbacks will never be called.
    */
@@ -139,17 +125,16 @@ const BackgroundPageThumbs = {
    */
   _ensureBrowser: function () {
     if (this._thumbBrowser)
       return;
 
     let browser = this._parentWin.document.createElementNS(XUL_NS, "browser");
     browser.setAttribute("type", "content");
     browser.setAttribute("remote", "true");
-    browser.setAttribute("privatebrowsing", "true");
 
     // Size the browser.  Make its aspect ratio the same as the canvases' that
     // the thumbnails are drawn into; the canvases' aspect ratio is the same as
     // the screen's, so use that.  Aim for a size in the ballpark of 1024x768.
     let [swidth, sheight] = [{}, {}];
     Cc["@mozilla.org/gfx/screenmanager;1"].
       getService(Ci.nsIScreenManager).
       primaryScreen.
@@ -245,30 +230,16 @@ Capture.prototype = {
    * Sends a message to the content script to start the capture.
    *
    * @param messageManager  The nsIMessageSender of the thumbnail browser.
    */
   start: function (messageManager) {
     this.startDate = new Date();
     tel("CAPTURE_QUEUE_TIME_MS", this.startDate - this.creationDate);
 
-    // 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()) {
-      tel("CAPTURE_DONE_REASON", TEL_CAPTURE_DONE_PB_BEFORE_START);
-      // 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);
 
@@ -312,23 +283,16 @@ Capture.prototype = {
   },
 
   // Called when the timeout timer fires.
   notify: function () {
     tel("CAPTURE_DONE_REASON", TEL_CAPTURE_DONE_TIMEOUT);
     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
     // removes the didCapture message listener.
 
     this.captureCallback(this);
     this.destroy();
 
@@ -345,62 +309,29 @@ Capture.prototype = {
           callback.call(this.options, this.url);
         }
         catch (err) {
           Cu.reportError(err);
         }
       }
     }.bind(this);
 
-    if (!data || this._privateWinOpenedDuringCapture) {
-      if (this._privateWinOpenedDuringCapture)
-        tel("CAPTURE_DONE_REASON", TEL_CAPTURE_DONE_PB_AFTER_START);
+    if (!data) {
       callOnDones();
       return;
     }
+
     PageThumbs._store(this.url, data.finalURL, data.imageData, data.wasErrorResponse)
               .then(callOnDones);
   },
 };
 
 Capture.nextID = 0;
 
 /**
- * Returns true if the given window is suitable for hosting our xul:browser.
- *
- * @param win  The window.
- * @return     True if the window can host the browser, false otherwise.
- */
-function canHostBrowser(win) {
-  // The host document needs to have the system principal since, like all code
-  // intended to be used in chrome, the browser binding does lots of things that
-  // assume it has it.  The document must also allow XUL children.  So check for
-  // both the system principal and the "allowXULXBL" permission.  (It turns out
-  // that allowXULXBL is satisfied by the system principal alone, making that
-  // check not strictly necessary, but it's here for robustness.)
-  let principal = win.document.nodePrincipal;
-  if (!Services.scriptSecurityManager.isSystemPrincipal(principal))
-    return false;
-  let permResult = Services.perms.testPermissionFromPrincipal(principal,
-                                                              "allowXULXBL");
-  return permResult == Ci.nsIPermissionManager.ALLOW_ACTION;
-}
-
-/**
- * Returns true if there are any private windows.
- */
-function isPrivateBrowsingActive() {
-  let wins = Services.ww.getWindowEnumerator();
-  while (wins.hasMoreElements())
-    if (PrivateBrowsingUtils.isWindowPrivate(wins.getNext()))
-      return true;
-  return false;
-}
-
-/**
  * Adds a value to one of this module's telemetry histograms.
  *
  * @param histogramID  This is prefixed with this module's ID.
  * @param value        The value to add.
  */
 function tel(histogramID, value) {
   let id = TELEMETRY_HISTOGRAM_ID_PREFIX + histogramID;
   Services.telemetry.getHistogramById(id).add(value);
--- a/toolkit/components/thumbnails/content/backgroundPageThumbsContent.js
+++ b/toolkit/components/thumbnails/content/backgroundPageThumbsContent.js
@@ -24,16 +24,21 @@ const backgroundPageThumbsContent = {
     // etc - so set it to the lowest priority available.
     this._webNav.QueryInterface(Ci.nsIDocumentLoader).
       loadGroup.QueryInterface(Ci.nsISupportsPriority).
       priority = Ci.nsISupportsPriority.PRIORITY_LOWEST;
 
     docShell.allowMedia = false;
     docShell.allowPlugins = false;
     docShell.allowContentRetargeting = false;
+    let defaultFlags = Ci.nsIRequest.LOAD_ANONYMOUS |
+                       Ci.nsIRequest.LOAD_BYPASS_CACHE |
+                       Ci.nsIRequest.INHIBIT_CACHING |
+                       Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_HISTORY;
+    docShell.defaultLoadFlags = defaultFlags;
 
     addMessageListener("BackgroundPageThumbs:capture",
                        this._onCapture.bind(this));
     docShell.
       QueryInterface(Ci.nsIInterfaceRequestor).
       getInterface(Ci.nsIWebProgress).
       addProgressListener(this, Ci.nsIWebProgress.NOTIFY_STATE_WINDOW);
   },
--- a/toolkit/components/thumbnails/test/browser_thumbnails_background.js
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_background.js
@@ -17,17 +17,20 @@ function test() {
   spawnNextTest();
 }
 
 function spawnNextTest() {
   if (!tests.length) {
     finish();
     return;
   }
-  imports.Task.spawn(tests.shift()).then(spawnNextTest, function onError(err) {
+
+  let nextTest = tests.shift();
+  info("starting sub-test " + nextTest.name);
+  imports.Task.spawn(nextTest).then(spawnNextTest, function onError(err) {
     ok(false, err);
     spawnNextTest();
   });
 }
 
 let tests = [
 
   function basic() {
@@ -153,18 +156,19 @@ let tests = [
   function privateBrowsingActive() {
     let url = "http://example.com/";
     let file = fileForURL(url);
     ok(!file.exists(), "Thumbnail file should not already exist.");
 
     let win = yield openPrivateWindow();
     let capturedURL = yield capture(url);
     is(capturedURL, url, "Captured URL should be URL passed to capture.");
-    ok(!file.exists(),
-       "Thumbnail file should not exist because a private window is open.");
+    ok(file.exists(),
+       "Thumbnail file should be created even when a private window is open.");
+    file.remove(false);
 
     win.close();
   },
 
   function openPrivateWindowDuringCapture() {
     let url = "http://example.com/";
     let file = fileForURL(url);
     ok(!file.exists(), "Thumbnail file should not already exist.");
@@ -175,19 +179,20 @@ let tests = [
     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 " +
+        ok(file.exists(),
+           "Thumbnail file should be created even though a private window " +
            "was opened during the capture.");
+        file.remove(false);
         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