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
authorEhsan Akhgari <ehsan@mozilla.com>
Wed, 12 Dec 2018 15:21:32 +0000
changeset 509021 9052425ebdfefbc2cc92bb932f4d4eddeb0b6ed8
parent 509020 abea562511dcbf81391bec662d4bae27f12e174a
child 509022 52d5881f6318e003a2101cabb78d7cff583e9fe2
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);