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

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

--HG--
extra : moz-landing-system : lando

browser/modules/BrowserUsageTelemetry.jsm

@@ -136,7 +136,7 @@ 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(),
 
@@ -235,11 +235,6 @@ let URICountListener = {
     // 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;
@@ -248,23 +243,22 @@ let URICountListener = {
       // 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);
   },
 
   /**
@@ -275,18 +269,18 @@ let URICountListener = {
   },
 
   /**
-   * 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,

browser/modules/PermissionUI.jsm

@@ -1041,7 +1041,7 @@ StorageAccessPermissionPrompt.prototype = {
     // 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);
   },
 

browser/modules/test/browser/browser_UsageTelemetry_uniqueOriginsVisitedInPast24Hours.js

@@ -8,41 +8,41 @@
 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();