Bug 1492668 - Store temporary site permissions by base domain to mitigate permission notification spam r=johannh
authorPaul Zuehlcke <pzuhlcke@mozilla.com>
Tue, 09 Apr 2019 12:56:47 +0000
changeset 523245 a725a9315f5d84022d54bc2bcdc88190eb05abb0
parent 523244 53da81e19127fb55bcbc00c86bd910751c5b56cd
child 523246 1694fdeb8a6ad706eb77001076bb6f0ba95a577a
push id11115
push useraiakab@mozilla.com
push dateFri, 19 Apr 2019 16:14:22 +0000
treeherdermozilla-beta@86f6ec90b34b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh
bugs1492668
milestone67.0
Bug 1492668 - Store temporary site permissions by base domain to mitigate permission notification spam r=johannh a=pascalc Differential Revision: https://phabricator.services.mozilla.com/D26704
browser/modules/SitePermissions.jsm
browser/modules/test/browser/browser_SitePermissions_tab_urls.js
--- a/browser/modules/SitePermissions.jsm
+++ b/browser/modules/SitePermissions.jsm
@@ -19,95 +19,111 @@ var gStringBundle =
  * This uses a WeakMap to key browsers, so that entries are
  * automatically cleared once the browser stops existing
  * (once there are no other references to the browser object);
  */
 const TemporaryPermissions = {
   // This is a three level deep map with the following structure:
   //
   // Browser => {
-  //   <prePath>: {
+  //   <baseDomain>: {
   //     <permissionID>: {Number} <timeStamp>
   //   }
   // }
   //
   // Only the top level browser elements are stored via WeakMap. The WeakMap
-  // value is an object with URI prePaths as keys. The keys of that object
+  // value is an object with URI baseDomains as keys. The keys of that object
   // are ids that identify permissions that were set for the specific URI.
   // The final value is an object containing the timestamp of when the permission
   // was set (in order to invalidate after a certain amount of time has passed).
   _stateByBrowser: new WeakMap(),
 
   // Private helper method that bundles some shared behavior for
   // get() and getAll(), e.g. deleting permissions when they have expired.
-  _get(entry, prePath, id, permission) {
+  _get(entry, baseDomain, id, permission) {
     if (permission == null || permission.timeStamp == null) {
-      delete entry[prePath][id];
+      delete entry[baseDomain][id];
       return null;
     }
     if (permission.timeStamp + SitePermissions.temporaryPermissionExpireTime < Date.now()) {
-      delete entry[prePath][id];
+      delete entry[baseDomain][id];
       return null;
     }
     return {id, state: permission.state, scope: SitePermissions.SCOPE_TEMPORARY};
   },
 
+  // Extract baseDomain from uri. Fallback to hostname on conversion error.
+  _uriToBaseDomain(uri) {
+    try {
+      return Services.eTLD.getBaseDomain(uri);
+    } catch (error) {
+      if (error.result !== Cr.NS_ERROR_HOST_IS_IP_ADDRESS &&
+          error.result !== Cr.NS_ERROR_INSUFFICIENT_DOMAIN_LEVELS) {
+        throw error;
+      }
+      return uri.host;
+    }
+  },
+
   // Sets a new permission for the specified browser.
   set(browser, id, state) {
     if (!browser) {
       return;
     }
     if (!this._stateByBrowser.has(browser)) {
       this._stateByBrowser.set(browser, {});
     }
     let entry = this._stateByBrowser.get(browser);
-    let prePath = browser.currentURI.prePath;
-    if (!entry[prePath]) {
-      entry[prePath] = {};
+    let baseDomain = this._uriToBaseDomain(browser.currentURI);
+    if (!entry[baseDomain]) {
+      entry[baseDomain] = {};
     }
-    entry[prePath][id] = {timeStamp: Date.now(), state};
+    entry[baseDomain][id] = {timeStamp: Date.now(), state};
   },
 
   // Removes a permission with the specified id for the specified browser.
   remove(browser, id) {
-    if (!browser) {
+    if (!browser || !this._stateByBrowser.has(browser)) {
       return;
     }
     let entry = this._stateByBrowser.get(browser);
-    let prePath = browser.currentURI.prePath;
-    if (entry && entry[prePath]) {
-      delete entry[prePath][id];
+    let baseDomain = this._uriToBaseDomain(browser.currentURI);
+    if (entry[baseDomain]) {
+      delete entry[baseDomain][id];
     }
   },
 
   // Gets a permission with the specified id for the specified browser.
   get(browser, id) {
-    if (!browser || !browser.currentURI) {
+    if (!browser || !browser.currentURI || !this._stateByBrowser.has(browser)) {
       return null;
     }
     let entry = this._stateByBrowser.get(browser);
-    let prePath = browser.currentURI.prePath;
-    if (entry && entry[prePath]) {
-      let permission = entry[prePath][id];
-      return this._get(entry, prePath, id, permission);
+    let baseDomain = this._uriToBaseDomain(browser.currentURI);
+    if (entry[baseDomain]) {
+      let permission = entry[baseDomain][id];
+      return this._get(entry, baseDomain, id, permission);
     }
     return null;
   },
 
   // Gets all permissions for the specified browser.
   // Note that only permissions that apply to the current URI
   // of the passed browser element will be returned.
   getAll(browser) {
     let permissions = [];
+    if (!this._stateByBrowser.has(browser)) {
+      return permissions;
+    }
     let entry = this._stateByBrowser.get(browser);
-    let prePath = browser.currentURI.prePath;
-    if (entry && entry[prePath]) {
-      let timeStamps = entry[prePath];
+    let baseDomain = this._uriToBaseDomain(browser.currentURI);
+    if (entry[baseDomain]) {
+      let timeStamps = entry[baseDomain];
       for (let id of Object.keys(timeStamps)) {
-        let permission = this._get(entry, prePath, id, timeStamps[id]);
+        let permission = this._get(entry, baseDomain, id, timeStamps[id]);
         // _get() returns null when the permission has expired.
         if (permission) {
           permissions.push(permission);
         }
       }
     }
     return permissions;
   },
--- a/browser/modules/test/browser/browser_SitePermissions_tab_urls.js
+++ b/browser/modules/test/browser/browser_SitePermissions_tab_urls.js
@@ -16,25 +16,25 @@ add_task(async function testTemporaryPer
   SpecialPowers.pushPrefEnv({set: [
         ["network.http.phishy-userpass-length", 2048],
   ]});
 
   // This usually takes about 60 seconds on 32bit Linux debug,
   // due to the combinatory nature of the test that is hard to fix.
   requestLongerTimeout(2);
 
-
   let same = [ newURI("https://example.com"),
                newURI("https://example.com/sub/path"),
-               newURI("https://example.com:443") ];
+               newURI("https://example.com:443"),
+               newURI("https://test1.example.com"),
+               newURI("https://name:password@example.com"),
+               newURI("http://example.com") ];
   let different = [ newURI("https://example.com"),
-                    newURI("https://name:password@example.com"),
-                    newURI("https://test1.example.com"),
-                    newURI("http://example.com"),
-                    newURI("http://example.org") ];
+                    newURI("http://example.org"),
+                    newURI("http://example.net") ];
 
   let id = "microphone";
 
   await BrowserTestUtils.withNewTab("about:blank", async function(browser) {
     for (let uri of same) {
         let loaded = BrowserTestUtils.browserLoaded(browser, false, uri.spec);
         BrowserTestUtils.loadURI(browser, uri.spec);
         await loaded;