Bug 1511818 - Keep track of unique domains with a valid hostname for the purposes of the automatic access grant heuristics of the Storage Access API r=mikedeboer a=RyanVM
☠☠ backed out by d2bd2bb0c329 ☠ ☠
authorEhsan Akhgari <ehsan@mozilla.com>
Wed, 12 Dec 2018 15:21:32 +0000
changeset 508996 2c85064c240cc9e3046fee8b781182a6291d3ee4
parent 508995 e72979e80a1c63f5fc1e78c4619123edf0295dfa
child 508997 d6a0b8b5bccaeb41eade51dcf2d84152f2f65fb2
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer, RyanVM
bugs1511818, 1509047
milestone65.0
Bug 1511818 - Keep track of unique domains with a valid hostname for the purposes of the automatic access grant heuristics of the Storage Access API r=mikedeboer a=RyanVM This is an attempt to fix a Talos regression caused by the cost of URL parsing. The approach I'm taking here is to modify the requirements of bug 1509047 part 2 slightly so that I can remove the expensive code which resulted in the Talos regression. The change in behaviour is that the automatic access grants for the Storage Access API will use the number of unique domains visited in the session as opposed to the number of unique origins visited. Differential Revision: https://phabricator.services.mozilla.com/D14156
browser/modules/BrowserUsageTelemetry.jsm
browser/modules/PermissionUI.jsm
browser/modules/test/browser/browser_UsageTelemetry_uniqueOriginsVisitedInPast24Hours.js
--- a/browser/modules/BrowserUsageTelemetry.jsm
+++ b/browser/modules/BrowserUsageTelemetry.jsm
@@ -131,17 +131,17 @@ function shouldRecordSearchCount(tabbrow
   return !PrivateBrowsingUtils.isWindowPrivate(tabbrowser.ownerGlobal) ||
          !Services.prefs.getBoolPref("browser.engagement.search_counts.pbm", false);
 }
 
 let URICountListener = {
   // A set containing the visited domains, see bug 1271310.
   _domainSet: new Set(),
   // A set containing the visited origins during the last 24 hours (similar to domains, but not quite the same)
-  _origin24hrSet: new Set(),
+  _domain24hrSet: new Set(),
   // A map to keep track of the URIs loaded from the restored tabs.
   _restoredURIsMap: new WeakMap(),
 
   isHttpURI(uri) {
     // Only consider http(s) schemas.
     return uri.schemeIs("http") || uri.schemeIs("https");
   },
 
@@ -230,68 +230,62 @@ let URICountListener = {
     }
 
     // Update the URI counts.
     Services.telemetry.scalarAdd(TOTAL_URI_COUNT_SCALAR_NAME, 1);
 
     // Update tab count
     BrowserUsageTelemetry._recordTabCount();
 
-    // We only want to count the unique domains up to MAX_UNIQUE_VISITED_DOMAINS.
-    if (this._domainSet.size == MAX_UNIQUE_VISITED_DOMAINS) {
-      return;
-    }
-
     // Unique domains should be aggregated by (eTLD + 1): x.test.com and y.test.com
     // are counted once as test.com.
     let baseDomain;
     try {
       // Even if only considering http(s) URIs, |getBaseDomain| could still throw
       // due to the URI containing invalid characters or the domain actually being
       // an ipv4 or ipv6 address.
       baseDomain = Services.eTLD.getBaseDomain(uri);
-      this._domainSet.add(baseDomain);
     } catch (e) {
-      baseDomain = uri.host;
+      return;
     }
 
-    // Record the origin, but with the base domain (eTLD + 1).
-    let baseDomainURI = uri.mutate()
-                           .setHost(baseDomain)
-                           .finalize();
-    this._origin24hrSet.add(baseDomainURI.prePath);
+    // We only want to count the unique domains up to MAX_UNIQUE_VISITED_DOMAINS.
+    if (this._domainSet.size < MAX_UNIQUE_VISITED_DOMAINS) {
+      this._domainSet.add(baseDomain);
+      Services.telemetry.scalarSet(UNIQUE_DOMAINS_COUNT_SCALAR_NAME, this._domainSet.size);
+    }
+
+    this._domain24hrSet.add(baseDomain);
     if (gRecentVisitedOriginsExpiry) {
       setTimeout(() => {
-        this._origin24hrSet.delete(baseDomainURI.prePath);
+        this._domain24hrSet.delete(baseDomain);
       }, gRecentVisitedOriginsExpiry * 1000);
     }
-
-    Services.telemetry.scalarSet(UNIQUE_DOMAINS_COUNT_SCALAR_NAME, this._domainSet.size);
   },
 
   /**
    * Reset the counts. This should be called when breaking a session in Telemetry.
    */
   reset() {
     this._domainSet.clear();
   },
 
   /**
-   * Returns the number of unique origins visited in this session during the
+   * Returns the number of unique domains visited in this session during the
    * last 24 hours.
    */
-  get uniqueOriginsVisitedInPast24Hours() {
-    return this._origin24hrSet.size;
+  get uniqueDomainsVisitedInPast24Hours() {
+    return this._domain24hrSet.size;
   },
 
   /**
-   * Resets the number of unique origins visited in this session.
+   * Resets the number of unique domains visited in this session.
    */
-  resetUniqueOriginsVisitedInPast24Hours() {
-    this._origin24hrSet.clear();
+  resetUniqueDomainsVisitedInPast24Hours() {
+    this._domain24hrSet.clear();
   },
 
   QueryInterface: ChromeUtils.generateQI([Ci.nsIWebProgressListener,
                                           Ci.nsISupportsWeakReference]),
 };
 
 let urlbarListener = {
 
--- a/browser/modules/PermissionUI.jsm
+++ b/browser/modules/PermissionUI.jsm
@@ -1036,17 +1036,17 @@ StorageAccessPermissionPrompt.prototype 
     return this.request.topLevelPrincipal;
   },
 
   get maxConcurrentAutomaticGrants() {
     // one percent of the number of top-levels origins visited in the current
     // session (but not to exceed 24 hours), or the value of the
     // dom.storage_access.max_concurrent_auto_grants preference, whichever is
     // higher.
-    return Math.max(Math.max(Math.floor(URICountListener.uniqueOriginsVisitedInPast24Hours / 100),
+    return Math.max(Math.max(Math.floor(URICountListener.uniqueDomainsVisitedInPast24Hours / 100),
                              this._maxConcurrentAutoGrants), 0);
   },
 
   getOriginsThirdPartyHasAccessTo(thirdPartyOrigin) {
     let prefix = `3rdPartyStorage^${thirdPartyOrigin}`;
     let perms = Services.perms.getAllWithTypePrefix(prefix);
     let origins = new Set();
     while (perms.length) {
--- a/browser/modules/test/browser/browser_UsageTelemetry_uniqueOriginsVisitedInPast24Hours.js
+++ b/browser/modules/test/browser/browser_UsageTelemetry_uniqueOriginsVisitedInPast24Hours.js
@@ -3,51 +3,51 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 ChromeUtils.defineModuleGetter(this, "URICountListener",
                                "resource:///modules/BrowserUsageTelemetry.jsm");
 
-add_task(async function test_uniqueOriginsVisitedInPast24Hours() {
+add_task(async function test_uniqueDomainsVisitedInPast24Hours() {
   registerCleanupFunction(async () => {
     info("Cleaning up");
-    URICountListener.resetUniqueOriginsVisitedInPast24Hours();
+    URICountListener.resetUniqueDomainsVisitedInPast24Hours();
   });
 
-  URICountListener.resetUniqueOriginsVisitedInPast24Hours();
-  let startingCount = URICountListener.uniqueOriginsVisitedInPast24Hours;
-  is(startingCount, 0, "We should have no origins recorded in the history right after resetting");
+  URICountListener.resetUniqueDomainsVisitedInPast24Hours();
+  let startingCount = URICountListener.uniqueDomainsVisitedInPast24Hours;
+  is(startingCount, 0, "We should have no domains recorded in the history right after resetting");
 
   // Add a new window and then some tabs in it.
   let win = await BrowserTestUtils.openNewBrowserWindow();
   await BrowserTestUtils.openNewForegroundTab(win.gBrowser, "http://example.com");
 
   await BrowserTestUtils.openNewForegroundTab(win.gBrowser, "http://test1.example.com");
-  is(URICountListener.uniqueOriginsVisitedInPast24Hours, startingCount + 1,
+  is(URICountListener.uniqueDomainsVisitedInPast24Hours, startingCount + 1,
      "test1.example.com should only count as a unique visit if example.com wasn't visited before");
 
-  // http://www.exämple.test
-  await BrowserTestUtils.openNewForegroundTab(win.gBrowser, "http://xn--exmple-cua.test");
-  is(URICountListener.uniqueOriginsVisitedInPast24Hours, startingCount + 2,
-     "www.exämple.test should count as a unique visit");
+  await BrowserTestUtils.openNewForegroundTab(win.gBrowser, "http://127.0.0.1");
+  is(URICountListener.uniqueDomainsVisitedInPast24Hours, startingCount + 1,
+     "127.0.0.1 should not count as a unique visit");
 
   // Set the expiry time to 1 second
   await SpecialPowers.pushPrefEnv({set: [["browser.engagement.recent_visited_origins.expiry", 1]]});
 
-  await BrowserTestUtils.openNewForegroundTab(win.gBrowser, "http://127.0.0.1");
-  is(URICountListener.uniqueOriginsVisitedInPast24Hours, startingCount + 3,
-     "127.0.0.1 should count as a unique visit");
+  // http://www.exämple.test
+  await BrowserTestUtils.openNewForegroundTab(win.gBrowser, "http://xn--exmple-cua.test");
+  is(URICountListener.uniqueDomainsVisitedInPast24Hours, startingCount + 2,
+     "www.exämple.test should count as a unique visit");
 
-  let countBefore = URICountListener.uniqueOriginsVisitedInPast24Hours;
+  let countBefore = URICountListener.uniqueDomainsVisitedInPast24Hours;
 
   await new Promise(resolve => {
     setTimeout(_ => {
-      let countAfter = URICountListener.uniqueOriginsVisitedInPast24Hours;
+      let countAfter = URICountListener.uniqueDomainsVisitedInPast24Hours;
       is(countAfter, countBefore - 1,
          "The expiry should work correctly");
       resolve();
     }, 1100);
   });
 
   BrowserTestUtils.removeTab(win.gBrowser.selectedTab);
   BrowserTestUtils.removeTab(win.gBrowser.selectedTab);