Bug 1492668 - Store temporary site permissions by base domain to mitigate permission notification spam r=johannh
☠☠ backed out by 2b272977a1e5 ☠ ☠
authorPaul Zuehlcke <pzuhlcke@mozilla.com>
Tue, 09 Apr 2019 12:56:47 +0000
changeset 468551 1839a68aea956a9c69f305f1c6b493a20e72a2d7
parent 468550 3b8444793f558bd769d35515f4c0cc8f3ba52370
child 468552 9a6c6c9c81199563416ffe9795b76c382d736f41
push id35843
push usernbeleuzu@mozilla.com
push dateTue, 09 Apr 2019 22:08:13 +0000
treeherdermozilla-central@a31032a16330 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh
bugs1492668
milestone68.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 1492668 - Store temporary site permissions by base domain to mitigate permission notification spam r=johannh 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;