Bug 1524200 - Optimize the comparison of principals in Sanitizer.jsm, r=johannh
authorAndrea Marchesini <amarchesini@mozilla.com>
Thu, 21 Feb 2019 11:28:55 +0000
changeset 460239 25397a6f8c4f5f96c0f669921a178f15aa4b05b3
parent 460238 5ff556e3158671af8f7cd450ec9f6929128a5171
child 460240 d28d0498bd7b1cbacdb9077ac3ea456041dbe722
push id35588
push usernbeleuzu@mozilla.com
push dateThu, 21 Feb 2019 15:59:59 +0000
treeherdermozilla-central@7fba2ab9df84 [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 - Optimize the comparison of principals in Sanitizer.jsm, r=johannh Differential Revision: https://phabricator.services.mozilla.com/D20331
browser/modules/Sanitizer.jsm
--- a/browser/modules/Sanitizer.jsm
+++ b/browser/modules/Sanitizer.jsm
@@ -669,16 +669,20 @@ async function sanitizeOnShutdown(progre
 
   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);
+
   // 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.)
   //
@@ -695,18 +699,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 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) {
     if (permission.type != "cookie" ||
         permission.capability != Ci.nsICookiePermission.ACCESS_SESSION) {
@@ -716,18 +720,18 @@ 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 getAllPrincipals(progress, permission.principal.URI);
-    await maybeSanitizeSessionPrincipals(progress, principals);
+    let selectedPrincipals = extractMatchingPrincipals(principals, permission.principal.URI);
+    await maybeSanitizeSessionPrincipals(progress, selectedPrincipals);
   }
 
   if (Sanitizer.shouldSanitizeNewTabContainer) {
     progress.advancement = "newtab-segregation";
     sanitizeNewTabSegregation();
     removePendingSanitization("newtab-container");
   }
 
@@ -736,79 +740,75 @@ 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. If matchUri is not null,
-// it returns only the principals matching that URI, ignoring the
-// OriginAttributes.
-async function getAllPrincipals(progress, matchUri = null) {
+// 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 => {
       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)) {
-          continue;
-        }
-
-        if (!matchUri || Services.eTLD.hasRootDomain(matchUri.host, uri.host)) {
+        if (isSupportedURI(uri)) {
           list.push(principal);
         }
       }
       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);
-    let uri = sw.principal.URI;
     // We don't need to check the scheme. SW are just exposed to http/https URLs.
-    if (!matchUri || Services.eTLD.hasRootDomain(matchUri.host, uri.host)) {
-      principals.push(sw.principal);
-    }
+    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) {
-    if (!matchUri || Services.eTLD.hasRootDomain(matchUri.host, cookie.rawHost)) {
-      hosts.add(cookie.rawHost + ChromeUtils.originAttributesToSuffix(cookie.originAttributes));
-    }
+    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
 // some of their sub-domain need to be sanitize.
 async function maybeSanitizeSessionPrincipals(progress, principals) {
   log("Sanitizing " + principals.length + " principals");
 
   let promises = [];
 
   principals.forEach(principal => {