Bug 1468355 - Optimize checking for site data usage for the site identity panel. r=Gijs
authorJohann Hofmann <jhofmann@mozilla.com>
Wed, 28 Aug 2019 21:45:30 +0000
changeset 490478 1d7d0461c58297298478a016aede70cefb78e691
parent 490477 ab1e0dacd90a8a729b064755a70c8af73618e634
child 490479 4dd3834214c99e2dcca6c73ca3da42805d9ab91c
push id36504
push userccoroiu@mozilla.com
push dateThu, 29 Aug 2019 04:08:39 +0000
treeherdermozilla-central@7004b8779a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1468355
milestone70.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 1468355 - Optimize checking for site data usage for the site identity panel. r=Gijs The idea here is that we avoid updating all site data in SiteDataManager.jsm just for checking a single host/origin and that we optimize performance by prioritizing the most common data type (cookies) and synchronous lookups (AppCache) and returning early if any data was found. We will still refresh the site data list for clearing once the user clicks on "Clear Site Data". Differential Revision: https://phabricator.services.mozilla.com/D42800
browser/base/content/browser-siteIdentity.js
browser/base/content/browser.js
browser/base/content/test/siteIdentity/browser_identityPopup_clearSiteData.js
browser/modules/SiteDataManager.jsm
--- a/browser/base/content/browser-siteIdentity.js
+++ b/browser/base/content/browser-siteIdentity.js
@@ -933,24 +933,18 @@ var gIdentityHandler = {
    * based on the specified mode, and the details of the SSL cert, where
    * applicable
    */
   refreshIdentityPopup() {
     // Update cookies and site data information and show the
     // "Clear Site Data" button if the site is storing local data.
     this._clearSiteDataFooter.hidden = true;
     if (this._uriHasHost) {
-      let host = this._uri.host;
-      SiteDataManager.updateSites().then(async () => {
-        let baseDomain = SiteDataManager.getBaseDomainFromHost(host);
-        let siteData = await SiteDataManager.getSites(baseDomain);
-
-        if (siteData && siteData.length) {
-          this._clearSiteDataFooter.hidden = false;
-        }
+      SiteDataManager.hasSiteData(this._uri.asciiHost).then(hasData => {
+        this._clearSiteDataFooter.hidden = !hasData;
       });
     }
 
     // Update "Learn More" for Mixed Content Blocking and Insecure Login Forms.
     let baseURL = Services.urlFormatter.formatURLPref("app.support.baseURL");
     this._identityPopupMixedContentLearnMore.forEach(e =>
       e.setAttribute("href", baseURL + "mixed-content")
     );
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -7960,48 +7960,23 @@ var OfflineApps = {
     // again.
     Services.perms.add(
       uri,
       "offline-app",
       Ci.nsIOfflineCacheUpdateService.ALLOW_NO_WARN
     );
   },
 
-  // XXX: duplicated in preferences/advanced.js
-  _getOfflineAppUsage(host, groups) {
-    let cacheService = Cc[
-      "@mozilla.org/network/application-cache-service;1"
-    ].getService(Ci.nsIApplicationCacheService);
-    if (!groups) {
-      try {
-        groups = cacheService.getGroups();
-      } catch (ex) {
-        return 0;
-      }
-    }
-
-    let usage = 0;
-    for (let group of groups) {
-      let uri = Services.io.newURI(group);
-      if (uri.asciiHost == host) {
-        let cache = cacheService.getActiveCache(group);
-        usage += cache.usage;
-      }
-    }
-
-    return usage;
-  },
-
   _usedMoreThanWarnQuota(uri) {
     // if the user has already allowed excessive usage, don't bother checking
     if (
       Services.perms.testExactPermission(uri, "offline-app") !=
       Ci.nsIOfflineCacheUpdateService.ALLOW_NO_WARN
     ) {
-      let usageBytes = this._getOfflineAppUsage(uri.asciiHost);
+      let usageBytes = SiteDataManager.getAppCacheUsageByHost(uri.asciiHost);
       let warnQuotaKB = Services.prefs.getIntPref("offline-apps.quota.warn");
       // The pref is in kb, the usage we get is in bytes, so multiply the quota
       // to compare correctly:
       if (usageBytes >= warnQuotaKB * 1024) {
         return true;
       }
     }
 
--- a/browser/base/content/test/siteIdentity/browser_identityPopup_clearSiteData.js
+++ b/browser/base/content/test/siteIdentity/browser_identityPopup_clearSiteData.js
@@ -1,75 +1,86 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 const TEST_ORIGIN = "https://example.com";
 const TEST_SUB_ORIGIN = "https://test1.example.com";
 const REMOVE_DIALOG_URL =
   "chrome://browser/content/preferences/siteDataRemoveSelected.xul";
 
+// Greek IDN for 'example.test'.
+const TEST_IDN_ORIGIN =
+  "https://\u03C0\u03B1\u03C1\u03AC\u03B4\u03B5\u03B9\u03B3\u03BC\u03B1.\u03B4\u03BF\u03BA\u03B9\u03BC\u03AE";
+const TEST_PUNY_ORIGIN = "https://xn--hxajbheg2az3al.xn--jxalpdlp/";
+const TEST_PUNY_SUB_ORIGIN = "https://sub1.xn--hxajbheg2az3al.xn--jxalpdlp/";
+
 ChromeUtils.defineModuleGetter(
   this,
   "SiteDataTestUtils",
   "resource://testing-common/SiteDataTestUtils.jsm"
 );
 
 add_task(async function setup() {
   let oldCanRecord = Services.telemetry.canRecordExtended;
   Services.telemetry.canRecordExtended = true;
 
   registerCleanupFunction(() => {
     Services.telemetry.canRecordExtended = oldCanRecord;
   });
 });
 
-async function testClearing(testQuota, testCookies) {
+async function testClearing(
+  testQuota,
+  testCookies,
+  testURI,
+  origin,
+  subOrigin
+) {
   // Add some test quota storage.
   if (testQuota) {
-    await SiteDataTestUtils.addToIndexedDB(TEST_ORIGIN);
-    await SiteDataTestUtils.addToIndexedDB(TEST_SUB_ORIGIN);
+    await SiteDataTestUtils.addToIndexedDB(origin);
+    await SiteDataTestUtils.addToIndexedDB(subOrigin);
   }
 
   // Add some test cookies.
   if (testCookies) {
-    SiteDataTestUtils.addToCookies(TEST_ORIGIN, "test1", "1");
-    SiteDataTestUtils.addToCookies(TEST_ORIGIN, "test2", "2");
-    SiteDataTestUtils.addToCookies(TEST_SUB_ORIGIN, "test3", "1");
+    SiteDataTestUtils.addToCookies(origin, "test1", "1");
+    SiteDataTestUtils.addToCookies(origin, "test2", "2");
+    SiteDataTestUtils.addToCookies(subOrigin, "test3", "1");
   }
 
-  await BrowserTestUtils.withNewTab(TEST_ORIGIN, async function(browser) {
+  await BrowserTestUtils.withNewTab(testURI, async function(browser) {
     // Verify we have added quota storage.
     if (testQuota) {
-      let usage = await SiteDataTestUtils.getQuotaUsage(TEST_ORIGIN);
+      let usage = await SiteDataTestUtils.getQuotaUsage(origin);
       Assert.greater(usage, 0, "Should have data for the base origin.");
 
-      usage = await SiteDataTestUtils.getQuotaUsage(TEST_SUB_ORIGIN);
+      usage = await SiteDataTestUtils.getQuotaUsage(subOrigin);
       Assert.greater(usage, 0, "Should have data for the sub origin.");
     }
 
     // Open the identity popup.
     let { gIdentityHandler } = gBrowser.ownerGlobal;
     let promisePanelOpen = BrowserTestUtils.waitForEvent(
       gIdentityHandler._identityPopup,
       "popupshown"
     );
-    let siteDataUpdated = TestUtils.topicObserved(
-      "sitedatamanager:sites-updated"
-    );
     gIdentityHandler._identityBox.click();
     await promisePanelOpen;
-    await siteDataUpdated;
 
     let clearFooter = document.getElementById(
       "identity-popup-clear-sitedata-footer"
     );
     let clearButton = document.getElementById(
       "identity-popup-clear-sitedata-button"
     );
-    ok(!clearFooter.hidden, "The clear data footer is not hidden.");
+    TestUtils.waitForCondition(
+      () => !clearFooter.hidden,
+      "The clear data footer is not hidden."
+    );
 
     let cookiesCleared;
     if (testCookies) {
       cookiesCleared = Promise.all([
         TestUtils.topicObserved(
           "cookie-changed",
           (subj, data) => data == "deleted" && subj.name == "test1"
         ),
@@ -82,17 +93,19 @@ async function testClearing(testQuota, t
           (subj, data) => data == "deleted" && subj.name == "test3"
         ),
       ]);
     }
 
     Services.telemetry.clearEvents();
 
     // Click the "Clear data" button.
-    siteDataUpdated = TestUtils.topicObserved("sitedatamanager:sites-updated");
+    let siteDataUpdated = TestUtils.topicObserved(
+      "sitedatamanager:sites-updated"
+    );
     let hideEvent = BrowserTestUtils.waitForEvent(
       gIdentityHandler._identityPopup,
       "popuphidden"
     );
     let removeDialogPromise = BrowserTestUtils.promiseAlertDialogOpen(
       "accept",
       REMOVE_DIALOG_URL
     );
@@ -111,64 +124,77 @@ async function testClearing(testQuota, t
     );
     is(buttonEvents.length, 1, "recorded telemetry for the button click");
 
     await siteDataUpdated;
 
     // Check that cookies were deleted.
     if (testCookies) {
       await cookiesCleared;
-      let uri = Services.io.newURI(TEST_ORIGIN);
+      let uri = Services.io.newURI(origin);
       is(
         Services.cookies.countCookiesFromHost(uri.host),
         0,
         "Cookies from the base domain should be cleared"
       );
-      uri = Services.io.newURI(TEST_SUB_ORIGIN);
+      uri = Services.io.newURI(subOrigin);
       is(
         Services.cookies.countCookiesFromHost(uri.host),
         0,
         "Cookies from the sub domain should be cleared"
       );
     }
 
     // Check that quota storage was deleted.
     if (testQuota) {
       await TestUtils.waitForCondition(async () => {
-        let usage = await SiteDataTestUtils.getQuotaUsage(TEST_ORIGIN);
+        let usage = await SiteDataTestUtils.getQuotaUsage(origin);
         return usage == 0;
       }, "Should have no data for the base origin.");
 
-      let usage = await SiteDataTestUtils.getQuotaUsage(TEST_SUB_ORIGIN);
+      let usage = await SiteDataTestUtils.getQuotaUsage(subOrigin);
       is(usage, 0, "Should have no data for the sub origin.");
     }
 
     // Open the site identity panel again to check that the button isn't shown anymore.
     promisePanelOpen = BrowserTestUtils.waitForEvent(
       gIdentityHandler._identityPopup,
       "popupshown"
     );
-    siteDataUpdated = TestUtils.topicObserved("sitedatamanager:sites-updated");
     gIdentityHandler._identityBox.click();
     await promisePanelOpen;
-    await siteDataUpdated;
+
+    // Wait for a second to see if the button is shown.
+    // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
+    await new Promise(c => setTimeout(c, 1000));
 
     ok(
       clearFooter.hidden,
       "The clear data footer is hidden after clearing data."
     );
   });
 }
 
 // Test removing quota managed storage.
 add_task(async function test_ClearSiteData() {
-  await testClearing(true, false);
+  await testClearing(true, false, TEST_ORIGIN, TEST_ORIGIN, TEST_SUB_ORIGIN);
 });
 
 // Test removing cookies.
 add_task(async function test_ClearCookies() {
-  await testClearing(false, true);
+  await testClearing(false, true, TEST_ORIGIN, TEST_ORIGIN, TEST_SUB_ORIGIN);
 });
 
 // Test removing both.
 add_task(async function test_ClearCookiesAndSiteData() {
-  await testClearing(true, true);
+  await testClearing(true, true, TEST_ORIGIN, TEST_ORIGIN, TEST_SUB_ORIGIN);
 });
+
+// Test IDN Domains
+add_task(async function test_IDN_ClearCookiesAndSiteData() {
+  await testClearing(
+    true,
+    true,
+    TEST_IDN_ORIGIN,
+    TEST_PUNY_ORIGIN,
+    TEST_PUNY_SUB_ORIGIN
+  );
+});
--- a/browser/modules/SiteDataManager.jsm
+++ b/browser/modules/SiteDataManager.jsm
@@ -19,18 +19,16 @@ XPCOMUtils.defineLazyGetter(this, "gStri
 
 XPCOMUtils.defineLazyGetter(this, "gBrandBundle", function() {
   return Services.strings.createBundle(
     "chrome://branding/locale/brand.properties"
   );
 });
 
 var SiteDataManager = {
-  _qms: Services.qms,
-
   _appCache: Cc["@mozilla.org/network/application-cache-service;1"].getService(
     Ci.nsIApplicationCacheService
   ),
 
   // A Map of sites and their disk usage according to Quota Manager and appcache
   // Key is host (group sites based on host across scheme, port, origin atttributes).
   // Value is one object holding:
   //   - principals: instances of nsIPrincipal (only when the site has
@@ -169,17 +167,17 @@ var SiteDataManager = {
             }
           }
         }
         resolve();
       };
       // XXX: The work of integrating localStorage into Quota Manager is in progress.
       //      After the bug 742822 and 1286798 landed, localStorage usage will be included.
       //      So currently only get indexedDB usage.
-      this._quotaUsageRequest = this._qms.getUsage(onUsageResult);
+      this._quotaUsageRequest = Services.qms.getUsage(onUsageResult);
     });
     return this._getQuotaUsagePromise;
   },
 
   _getAllCookies() {
     for (let cookie of Services.cookies.enumerator) {
       let site = this._getOrInsertSite(cookie.rawHost);
       site.cookies.push(cookie);
@@ -223,16 +221,104 @@ var SiteDataManager = {
       let site = this._getOrInsertSite(uri.host);
       if (!site.principals.some(p => p.origin == principal.origin)) {
         site.principals.push(principal);
       }
       site.appCacheList.push(cache);
     }
   },
 
+  /**
+   * Gets the current AppCache usage by host. This is using asciiHost to compare
+   * against the provided host.
+   *
+   * @param {String} the ascii host to check usage for
+   * @returns the usage in bytes
+   */
+  getAppCacheUsageByHost(host) {
+    let usage = 0;
+
+    let groups;
+    try {
+      groups = this._appCache.getGroups();
+    } catch (e) {
+      // NS_ERROR_NOT_AVAILABLE means that appCache is not initialized,
+      // which probably means the user has disabled it. Otherwise, log an
+      // error. Either way, there's nothing we can do here.
+      if (e.result != Cr.NS_ERROR_NOT_AVAILABLE) {
+        Cu.reportError(e);
+      }
+      return usage;
+    }
+
+    for (let group of groups) {
+      let uri = Services.io.newURI(group);
+      if (uri.asciiHost == host) {
+        let cache = this._appCache.getActiveCache(group);
+        usage += cache.usage;
+      }
+    }
+
+    return usage;
+  },
+
+  /**
+   * Checks if the site with the provided ASCII host is using any site data at all.
+   * This will check for:
+   *   - Cookies (incl. subdomains)
+   *   - AppCache
+   *   - Quota Usage
+   * in that order. This function is meant to be fast, and thus will
+   * end searching and return true once the first trace of site data is found.
+   *
+   * @param {String} the ASCII host to check
+   * @returns {Boolean} whether the site has any data associated with it
+   */
+  async hasSiteData(asciiHost) {
+    if (Services.cookies.countCookiesFromHost(asciiHost)) {
+      return true;
+    }
+
+    let appCacheUsage = this.getAppCacheUsageByHost(asciiHost);
+    if (appCacheUsage > 0) {
+      return true;
+    }
+
+    let hasQuota = await new Promise(resolve => {
+      Services.qms.getUsage(request => {
+        if (request.resultCode != Cr.NS_OK) {
+          resolve(false);
+          return;
+        }
+
+        for (let item of request.result) {
+          if (!item.persisted && item.usage <= 0) {
+            continue;
+          }
+
+          let principal = Services.scriptSecurityManager.createContentPrincipalFromOrigin(
+            item.origin
+          );
+          if (principal.URI.asciiHost == asciiHost) {
+            resolve(true);
+            return;
+          }
+        }
+
+        resolve(false);
+      });
+    });
+
+    if (hasQuota) {
+      return true;
+    }
+
+    return false;
+  },
+
   getTotalUsage() {
     return this._getQuotaUsagePromise.then(() => {
       let usage = 0;
       for (let site of this._sites.values()) {
         for (let cache of site.appCacheList) {
           usage += cache.usage;
         }
         usage += site.quotaUsage;