Bug 1524200 - Sanitizer.jsm should retrieve the list of nsIPrincipal objects with site data only when needed, r=johannh
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 26 Feb 2019 17:27:52 +0000
changeset 461216 b7ea35510cf18afcb86d64c050172db079912ad3
parent 461215 8e6506978820a5ece5c7f18a6e9c14024517b109
child 461217 601742636bd1a82241698e5cb86683b0a20140d7
push id79048
push useramarchesini@mozilla.com
push dateTue, 26 Feb 2019 18:58:07 +0000
treeherderautoland@b7ea35510cf1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh
bugs1524200
milestone67.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 1524200 - Sanitizer.jsm should retrieve the list of nsIPrincipal objects with site data only when needed, r=johannh Differential Revision: https://phabricator.services.mozilla.com/D20995
browser/modules/Sanitizer.jsm
--- a/browser/modules/Sanitizer.jsm
+++ b/browser/modules/Sanitizer.jsm
@@ -659,29 +659,102 @@ async function sanitizeInternal(items, a
   if (!progress.isShutdown)
     removePendingSanitization(uid);
   progress = {};
   if (seenError) {
     throw new Error("Error sanitizing");
   }
 }
 
+// This is an helper that retrieves the principals with site data just once
+// and only when needed.
+class PrincipalsCollector {
+  constructor() {
+    this.principals = null;
+  }
+
+  async getAllPrincipals(progress) {
+    if (this.principals == null) {
+      // Here is the list of principals with site data.
+      progress.advancement = "get-principals";
+      this.principals = await this.getAllPrincipalsInternal(progress);
+    }
+
+    return this.principals;
+  }
+
+  async getAllPrincipalsInternal(progress) {
+    progress.step = "principals-quota-manager";
+    let principals = await new Promise(resolve => {
+      quotaManagerService.getUsage(request => {
+        progress.step = "principals-quota-manager-getUsage";
+        if (request.resultCode != Cr.NS_OK) {
+          // We are probably shutting down. We don't want to propagate the
+          // error, rejecting the promise.
+          resolve([]);
+          return;
+        }
+
+        let list = [];
+        for (let item of request.result) {
+          let principal = Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(item.origin);
+          let uri = principal.URI;
+          if (isSupportedURI(uri)) {
+            list.push(principal);
+          }
+        }
+
+        progress.step = "principals-quota-manager-completed";
+        resolve(list);
+      });
+    }).catch(ex => {
+      Cu.reportError("QuotaManagerService promise failed: " + ex);
+      return [];
+    });
+
+    progress.step = "principals-service-workers";
+    let serviceWorkers = serviceWorkerManager.getAllRegistrations();
+    for (let i = 0; i < serviceWorkers.length; i++) {
+      let sw = serviceWorkers.queryElementAt(i, Ci.nsIServiceWorkerRegistrationInfo);
+      // We don't need to check the scheme. SW are just exposed to http/https URLs.
+      principals.push(sw.principal);
+    }
+
+    // Let's take the list of unique hosts+OA from cookies.
+    progress.step = "principals-cookies";
+    let enumerator = Services.cookies.enumerator;
+    let hosts = new Set();
+    for (let cookie of enumerator) {
+      hosts.add(cookie.rawHost + ChromeUtils.originAttributesToSuffix(cookie.originAttributes));
+    }
+
+    progress.step = "principals-host-cookie";
+    hosts.forEach(host => {
+      // Cookies and permissions are handled by origin/host. Doesn't matter if we
+      // use http: or https: schema here.
+      principals.push(
+        Services.scriptSecurityManager.createCodebasePrincipalFromOrigin("https://" + host));
+    });
+
+    progress.step = "total-principals:" + principals.length;
+    return principals;
+  }
+}
+
 async function sanitizeOnShutdown(progress) {
   log("Sanitizing on shutdown");
 
   if (Sanitizer.shouldSanitizeOnShutdown) {
     // Need to sanitize upon shutdown
     progress.advancement = "shutdown-cleaner";
     let itemsToClear = getItemsToClearFromPrefBranch(Sanitizer.PREF_SHUTDOWN_BRANCH);
     await Sanitizer.sanitize(itemsToClear, { progress });
   }
 
-  // Here is the list of principals with site data.
-  progress.advancement = "get-principals";
-  let principals = await getAllPrincipals(progress);
+  let principalsCollector = new PrincipalsCollector();
 
   // Clear out QuotaManager storage for principals that have been marked as
   // session only.  The cookie service has special logic that avoids writing
   // such cookies to disk, but QuotaManager always touches disk, so we need to
   // wipe the data on shutdown (or startup if we failed to wipe it at
   // shutdown).  (Note that some session cookies do survive Firefox restarts
   // because the Session Store that remembers your tabs between sessions takes
   // on the responsibility for persisting them through restarts.)
@@ -699,16 +772,18 @@ async function sanitizeOnShutdown(progre
   // ACCEPT_NORMALLY need to be wiped.  Second, the set of origins that have
   // the permission explicitly set to ACCEPT_SESSION need to be wiped.  There
   // are also other ways to think about and accomplish this, but this is what
   // the logic below currently does!
   if (Services.prefs.getIntPref(PREF_COOKIE_LIFETIME,
                                 Ci.nsICookieService.ACCEPT_NORMALLY) == Ci.nsICookieService.ACCEPT_SESSION) {
     log("Session-only configuration detected");
     progress.advancement = "session-only";
+
+    let principals = await principalsCollector.getAllPrincipals(progress);
     await maybeSanitizeSessionPrincipals(progress, principals);
     return;
   }
 
   progress.advancement = "session-permission";
 
   // Let's see if we have to forget some particular site.
   for (let permission of Services.perms.enumerator) {
@@ -720,16 +795,17 @@ async function sanitizeOnShutdown(progre
     // We consider just permissions set for http, https and file URLs.
     if (!isSupportedURI(permission.principal.URI)) {
       continue;
     }
 
     log("Custom session cookie permission detected for: " + permission.principal.URI.spec);
 
     // We use just the URI here, because permissions ignore OriginAttributes.
+    let principals = await principalsCollector.getAllPrincipals(progress);
     let selectedPrincipals = extractMatchingPrincipals(principals, permission.principal.URI);
     await maybeSanitizeSessionPrincipals(progress, selectedPrincipals);
   }
 
   if (Sanitizer.shouldSanitizeNewTabContainer) {
     progress.advancement = "newtab-segregation";
     sanitizeNewTabSegregation();
     removePendingSanitization("newtab-container");
@@ -740,71 +816,16 @@ async function sanitizeOnShutdown(progre
     // sanitizing again on startup.
     removePendingSanitization("shutdown");
     Services.prefs.savePrefFile(null);
   }
 
   progress.advancement = "done";
 }
 
-// Retrieve the list of nsIPrincipals with site data.
-async function getAllPrincipals(progress) {
-  progress.step = "principals-quota-manager";
-  let principals = await new Promise(resolve => {
-    quotaManagerService.getUsage(request => {
-      progress.step = "principals-quota-manager-getUsage";
-      if (request.resultCode != Cr.NS_OK) {
-        // We are probably shutting down. We don't want to propagate the
-        // error, rejecting the promise.
-        resolve([]);
-        return;
-      }
-
-      let list = [];
-      for (let item of request.result) {
-        let principal = Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(item.origin);
-        let uri = principal.URI;
-        if (isSupportedURI(uri)) {
-          list.push(principal);
-        }
-      }
-
-      progress.step = "principals-quota-manager-completed";
-      resolve(list);
-    });
-  }).catch(() => []);
-
-  progress.step = "principals-service-workers";
-  let serviceWorkers = serviceWorkerManager.getAllRegistrations();
-  for (let i = 0; i < serviceWorkers.length; i++) {
-    let sw = serviceWorkers.queryElementAt(i, Ci.nsIServiceWorkerRegistrationInfo);
-    // We don't need to check the scheme. SW are just exposed to http/https URLs.
-    principals.push(sw.principal);
-  }
-
-  // Let's take the list of unique hosts+OA from cookies.
-  progress.step = "principals-cookies";
-  let enumerator = Services.cookies.enumerator;
-  let hosts = new Set();
-  for (let cookie of enumerator) {
-    hosts.add(cookie.rawHost + ChromeUtils.originAttributesToSuffix(cookie.originAttributes));
-  }
-
-  progress.step = "principals-host-cookie";
-  hosts.forEach(host => {
-    // Cookies and permissions are handled by origin/host. Doesn't matter if we
-    // use http: or https: schema here.
-    principals.push(
-      Services.scriptSecurityManager.createCodebasePrincipalFromOrigin("https://" + host));
-  });
-
-  progress.step = "total-principals:" + principals.length;
-  return principals;
-}
-
 // Extracts the principals matching matchUri as root domain.
 function extractMatchingPrincipals(principals, matchUri) {
   return principals.filter(principal => {
     return Services.eTLD.hasRootDomain(matchUri.host, principal.URI.host);
   });
 }
 
 // This method receives a list of principals and it checks if some of them or