Bug 1372592 - Should not display non-persistent-storage sites which stores 0 byte in Preferences Site Data section, r?jaws draft
authorFischer.json <fischer.json@gmail.com>
Thu, 24 Aug 2017 17:33:06 +0800
changeset 654734 040f10e48ccbda16e64a2215eea3c07c5d8ee8af
parent 654592 1b4c59eef820b46eb0037aca68f83a15088db45f
child 728642 624facd9b4e221274bb43acc507ccf335c973cb5
push id76659
push userbmo:fliu@mozilla.com
push dateTue, 29 Aug 2017 06:37:33 +0000
reviewersjaws
bugs1372592
milestone57.0a1
Bug 1372592 - Should not display non-persistent-storage sites which stores 0 byte in Preferences Site Data section, r?jaws MozReview-Commit-ID: BelcT0Lpmba
browser/components/preferences/SiteDataManager.jsm
browser/components/preferences/in-content-new/tests/browser.ini
browser/components/preferences/in-content-new/tests/browser_siteData2.js
browser/components/preferences/in-content-new/tests/browser_siteData3.js
browser/components/preferences/in-content/tests/browser.ini
browser/components/preferences/in-content/tests/browser_siteData.js
browser/components/preferences/in-content/tests/browser_siteData3.js
--- a/browser/components/preferences/SiteDataManager.jsm
+++ b/browser/components/preferences/SiteDataManager.jsm
@@ -43,16 +43,20 @@ this.SiteDataManager = {
   _getQuotaUsage() {
     // Clear old data and requests first
     this._sites.clear();
     this._cancelGetQuotaUsage();
     this._getQuotaUsagePromise = new Promise(resolve => {
       let onUsageResult = request => {
         let items = request.result;
         for (let item of items) {
+          if (!item.persisted && item.usage <= 0) {
+            // An non-persistent-storage site with 0 byte quota usage is redundant for us so skip it.
+            continue;
+          }
           let principal =
             Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(item.origin);
           let uri = principal.URI;
           if (uri.scheme == "http" || uri.scheme == "https") {
             let site = this._sites.get(uri.host);
             if (!site) {
               site = {
                 persisted: false,
@@ -90,31 +94,35 @@ this.SiteDataManager = {
       this._quotaUsageRequest.cancel();
       this._quotaUsageRequest = null;
     }
   },
 
   _updateAppCache() {
     let groups = this._appCache.getGroups();
     for (let group of groups) {
+      let cache = this._appCache.getActiveCache(group);
+      if (cache.usage <= 0) {
+        // A site with 0 byte appcache usage is redundant for us so skip it.
+        continue;
+      }
       let principal = Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(group);
       let uri = principal.URI;
       let site = this._sites.get(uri.host);
       if (!site) {
         site = {
           persisted: false,
           quotaUsage: 0,
           principals: [ principal ],
           appCacheList: [],
         };
         this._sites.set(uri.host, site);
       } else if (!site.principals.some(p => p.origin == principal.origin)) {
         site.principals.push(principal);
       }
-      let cache = this._appCache.getActiveCache(group);
       site.appCacheList.push(cache);
     }
   },
 
   getTotalUsage() {
     return this._getQuotaUsagePromise.then(() => {
       let usage = 0;
       for (let site of this._sites.values()) {
--- a/browser/components/preferences/in-content-new/tests/browser.ini
+++ b/browser/components/preferences/in-content-new/tests/browser.ini
@@ -61,16 +61,17 @@ skip-if = e10s
 [browser_privacypane_5.js]
 [browser_privacypane_8.js]
 [browser_sanitizeOnShutdown_prefLocked.js]
 [browser_searchsuggestions.js]
 [browser_security-1.js]
 [browser_security-2.js]
 [browser_siteData.js]
 [browser_siteData2.js]
+[browser_siteData3.js]
 [browser_site_login_exceptions.js]
 [browser_permissions_dialog.js]
 [browser_cookies_dialog.js]
 [browser_subdialogs.js]
 support-files =
   subdialog.xul
   subdialog2.xul
 [browser_telemetry.js]
--- a/browser/components/preferences/in-content-new/tests/browser_siteData2.js
+++ b/browser/components/preferences/in-content-new/tests/browser_siteData2.js
@@ -1,13 +1,11 @@
 "use strict";
 const { SiteDataManager } = Cu.import("resource:///modules/SiteDataManager.jsm", {});
-
 const REMOVE_DIALOG_URL = "chrome://browser/content/preferences/siteDataRemoveSelected.xul";
-const { DownloadUtils } = Cu.import("resource://gre/modules/DownloadUtils.jsm", {});
 
 function promiseSettingsDialogClose() {
   return new Promise(resolve => {
     let win = gBrowser.selectedBrowser.contentWindow;
     let dialogOverlay = win.gSubDialog._topDialog._overlay;
     let dialogWin = win.gSubDialog._topDialog._frame.contentWindow;
     dialogWin.addEventListener("unload", function unload() {
       if (dialogWin.document.documentURI === "chrome://browser/content/preferences/siteDataSettings.xul") {
@@ -311,77 +309,16 @@ add_task(async function() {
   await updatePromise;
   await openSiteDataSettingsDialog();
   assertSitesListed(doc, fakeHosts.filter(host => !host.includes("xyz")));
 
   mockSiteDataManager.unregister();
   await BrowserTestUtils.removeTab(gBrowser.selectedTab);
 });
 
-// Test grouping and listing sites across scheme, port and origin attributes by host
-add_task(async function() {
-  await SpecialPowers.pushPrefEnv({set: [["browser.storageManager.enabled", true]]});
-  const quotaUsage = 1024;
-  mockSiteDataManager.register(SiteDataManager);
-  mockSiteDataManager.fakeSites = [
-    {
-      usage: quotaUsage,
-      principal: Services.scriptSecurityManager
-                         .createCodebasePrincipalFromOrigin("https://account.xyz.com^userContextId=1"),
-      persisted: true
-    },
-    {
-      usage: quotaUsage,
-      principal: Services.scriptSecurityManager
-                         .createCodebasePrincipalFromOrigin("https://account.xyz.com"),
-      persisted: false
-    },
-    {
-      usage: quotaUsage,
-      principal: Services.scriptSecurityManager
-                         .createCodebasePrincipalFromOrigin("https://account.xyz.com:123"),
-      persisted: false
-    },
-    {
-      usage: quotaUsage,
-      principal: Services.scriptSecurityManager
-                         .createCodebasePrincipalFromOrigin("http://account.xyz.com"),
-      persisted: false
-    },
-  ];
-
-  let updatedPromise = promiseSiteDataManagerSitesUpdated();
-  await openPreferencesViaOpenPreferencesAPI("privacy", { leaveOpen: true });
-  await updatedPromise;
-  await openSiteDataSettingsDialog();
-  let win = gBrowser.selectedBrowser.contentWindow;
-  let dialogFrame = win.gSubDialog._topDialog._frame;
-  let frameDoc = dialogFrame.contentDocument;
-
-  let siteItems = frameDoc.getElementsByTagName("richlistitem");
-  is(siteItems.length, 1, "Should group sites across scheme, port and origin attributes");
-
-  let expected = "account.xyz.com";
-  let host = siteItems[0].getAttribute("host");
-  is(host, expected, "Should group and list sites by host");
-
-  let prefStrBundle = frameDoc.getElementById("bundlePreferences");
-  expected = prefStrBundle.getFormattedString("siteUsage",
-    DownloadUtils.convertByteUnits(quotaUsage * mockSiteDataManager.fakeSites.length));
-  let usage = siteItems[0].getAttribute("usage");
-  is(usage, expected, "Should sum up usages across scheme, port and origin attributes");
-
-  expected = prefStrBundle.getString("persistent");
-  let status = siteItems[0].getAttribute("status");
-  is(status, expected, "Should mark persisted status across scheme, port and origin attributes");
-
-  mockSiteDataManager.unregister();
-  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
-});
-
 // Test dynamically clearing all site data
 add_task(async function() {
   await SpecialPowers.pushPrefEnv({set: [["browser.storageManager.enabled", true]]});
   mockSiteDataManager.register(SiteDataManager);
   mockSiteDataManager.fakeSites = [
     {
       usage: 1024,
       principal: Services.scriptSecurityManager
new file mode 100644
--- /dev/null
+++ b/browser/components/preferences/in-content-new/tests/browser_siteData3.js
@@ -0,0 +1,107 @@
+"use strict";
+const { SiteDataManager } = Cu.import("resource:///modules/SiteDataManager.jsm", {});
+const { DownloadUtils } = Cu.import("resource://gre/modules/DownloadUtils.jsm", {});
+
+// Test not displaying sites which store 0 byte and don't have persistent storage.
+add_task(async function() {
+  await SpecialPowers.pushPrefEnv({set: [["browser.storageManager.enabled", true]]});
+  mockSiteDataManager.register(SiteDataManager);
+  mockSiteDataManager.fakeSites = [
+    {
+      usage: 0,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("https://account.xyz.com"),
+      persisted: true
+    },
+    {
+      usage: 0,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("https://shopping.xyz.com"),
+      persisted: false
+    },
+    {
+      usage: 1024,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("http://cinema.bar.com"),
+      persisted: true
+    },
+    {
+      usage: 1024,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("http://email.bar.com"),
+      persisted: false
+    },
+  ];
+  let fakeHosts = mockSiteDataManager.fakeSites.map(site => site.principal.URI.host);
+
+  let updatePromise = promiseSiteDataManagerSitesUpdated();
+  let doc = gBrowser.selectedBrowser.contentDocument;
+  await openPreferencesViaOpenPreferencesAPI("privacy", { leaveOpen: true });
+  await updatePromise;
+  await openSiteDataSettingsDialog();
+  assertSitesListed(doc, fakeHosts.filter(host => host != "shopping.xyz.com"));
+
+  mockSiteDataManager.unregister();
+  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
+});
+
+// Test grouping and listing sites across scheme, port and origin attributes by host
+add_task(async function() {
+  await SpecialPowers.pushPrefEnv({set: [["browser.storageManager.enabled", true]]});
+  const quotaUsage = 1024;
+  mockSiteDataManager.register(SiteDataManager);
+  mockSiteDataManager.fakeSites = [
+    {
+      usage: quotaUsage,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("https://account.xyz.com^userContextId=1"),
+      persisted: true
+    },
+    {
+      usage: quotaUsage,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("https://account.xyz.com"),
+      persisted: false
+    },
+    {
+      usage: quotaUsage,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("https://account.xyz.com:123"),
+      persisted: false
+    },
+    {
+      usage: quotaUsage,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("http://account.xyz.com"),
+      persisted: false
+    },
+  ];
+
+  let updatedPromise = promiseSiteDataManagerSitesUpdated();
+  await openPreferencesViaOpenPreferencesAPI("privacy", { leaveOpen: true });
+  await updatedPromise;
+  await openSiteDataSettingsDialog();
+  let win = gBrowser.selectedBrowser.contentWindow;
+  let dialogFrame = win.gSubDialog._topDialog._frame;
+  let frameDoc = dialogFrame.contentDocument;
+
+  let siteItems = frameDoc.getElementsByTagName("richlistitem");
+  is(siteItems.length, 1, "Should group sites across scheme, port and origin attributes");
+
+  let expected = "account.xyz.com";
+  let host = siteItems[0].getAttribute("host");
+  is(host, expected, "Should group and list sites by host");
+
+  let prefStrBundle = frameDoc.getElementById("bundlePreferences");
+  expected = prefStrBundle.getFormattedString("siteUsage",
+    DownloadUtils.convertByteUnits(quotaUsage * mockSiteDataManager.fakeSites.length));
+  let usage = siteItems[0].getAttribute("usage");
+  is(usage, expected, "Should sum up usages across scheme, port and origin attributes");
+
+  expected = prefStrBundle.getString("persistent");
+  let status = siteItems[0].getAttribute("status");
+  is(status, expected, "Should mark persisted status across scheme, port and origin attributes");
+
+  mockSiteDataManager.unregister();
+  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
+});
--- a/browser/components/preferences/in-content/tests/browser.ini
+++ b/browser/components/preferences/in-content/tests/browser.ini
@@ -43,16 +43,17 @@ skip-if = e10s
 [browser_privacypane_4.js]
 [browser_privacypane_5.js]
 [browser_privacypane_8.js]
 [browser_sanitizeOnShutdown_prefLocked.js]
 [browser_searchsuggestions.js]
 [browser_security.js]
 [browser_siteData.js]
 [browser_siteData2.js]
+[browser_siteData3.js]
 [browser_site_login_exceptions.js]
 [browser_cookies_dialog.js]
 [browser_subdialogs.js]
 support-files =
   subdialog.xul
   subdialog2.xul
 [browser_telemetry.js]
 # Skip this test on Android as FHR and Telemetry are separate systems there.
--- a/browser/components/preferences/in-content/tests/browser_siteData.js
+++ b/browser/components/preferences/in-content/tests/browser_siteData.js
@@ -423,66 +423,8 @@ add_task(async function() {
       }
     }
   }
 
   function findSiteByHost(host) {
     return mockSiteDataManager.fakeSites.find(site => site.principal.URI.host == host);
   }
 });
-
-// Test search on the host column
-add_task(async function() {
-  await SpecialPowers.pushPrefEnv({set: [["browser.storageManager.enabled", true]]});
-  mockSiteDataManager.register();
-  mockSiteDataManager.fakeSites = [
-    {
-      usage: 1024,
-      principal: Services.scriptSecurityManager
-                         .createCodebasePrincipalFromOrigin("https://account.xyz.com"),
-      persisted: true
-    },
-    {
-      usage: 1024,
-      principal: Services.scriptSecurityManager
-                         .createCodebasePrincipalFromOrigin("https://shopping.xyz.com"),
-      persisted: false
-    },
-    {
-      usage: 1024,
-      principal: Services.scriptSecurityManager
-                         .createCodebasePrincipalFromOrigin("http://cinema.bar.com"),
-      persisted: true
-    },
-    {
-      usage: 1024,
-      principal: Services.scriptSecurityManager
-                         .createCodebasePrincipalFromOrigin("http://email.bar.com"),
-      persisted: false
-    },
-  ];
-  let fakeHosts = mockSiteDataManager.fakeSites.map(site => site.principal.URI.host);
-
-  let updatePromise = promiseSitesUpdated();
-  await openPreferencesViaOpenPreferencesAPI("advanced", "networkTab", { leaveOpen: true });
-  await updatePromise;
-  await openSettingsDialog();
-
-  let win = gBrowser.selectedBrowser.contentWindow;
-  let doc = gBrowser.selectedBrowser.contentDocument;
-  let frameDoc = win.gSubDialog._topDialog._frame.contentDocument;
-  let searchBox = frameDoc.getElementById("searchBox");
-
-  searchBox.value = "xyz";
-  searchBox.doCommand();
-  assertSitesListed(doc, fakeHosts.filter(host => host.includes("xyz")));
-
-  searchBox.value = "bar";
-  searchBox.doCommand();
-  assertSitesListed(doc, fakeHosts.filter(host => host.includes("bar")));
-
-  searchBox.value = "";
-  searchBox.doCommand();
-  assertSitesListed(doc, fakeHosts);
-
-  mockSiteDataManager.unregister();
-  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
-});
new file mode 100644
--- /dev/null
+++ b/browser/components/preferences/in-content/tests/browser_siteData3.js
@@ -0,0 +1,102 @@
+"use strict";
+
+// Test search on the host column
+add_task(async function() {
+  await SpecialPowers.pushPrefEnv({set: [["browser.storageManager.enabled", true]]});
+  mockSiteDataManager.register();
+  mockSiteDataManager.fakeSites = [
+    {
+      usage: 1024,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("https://account.xyz.com"),
+      persisted: true
+    },
+    {
+      usage: 1024,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("https://shopping.xyz.com"),
+      persisted: false
+    },
+    {
+      usage: 1024,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("http://cinema.bar.com"),
+      persisted: true
+    },
+    {
+      usage: 1024,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("http://email.bar.com"),
+      persisted: false
+    },
+  ];
+  let fakeHosts = mockSiteDataManager.fakeSites.map(site => site.principal.URI.host);
+
+  let updatePromise = promiseSitesUpdated();
+  await openPreferencesViaOpenPreferencesAPI("advanced", "networkTab", { leaveOpen: true });
+  await updatePromise;
+  await openSettingsDialog();
+
+  let win = gBrowser.selectedBrowser.contentWindow;
+  let doc = gBrowser.selectedBrowser.contentDocument;
+  let frameDoc = win.gSubDialog._topDialog._frame.contentDocument;
+  let searchBox = frameDoc.getElementById("searchBox");
+
+  searchBox.value = "xyz";
+  searchBox.doCommand();
+  assertSitesListed(doc, fakeHosts.filter(host => host.includes("xyz")));
+
+  searchBox.value = "bar";
+  searchBox.doCommand();
+  assertSitesListed(doc, fakeHosts.filter(host => host.includes("bar")));
+
+  searchBox.value = "";
+  searchBox.doCommand();
+  assertSitesListed(doc, fakeHosts);
+
+  mockSiteDataManager.unregister();
+  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
+});
+
+// Test not displaying sites which store 0 byte and don't have persistent storage.
+add_task(async function() {
+  await SpecialPowers.pushPrefEnv({set: [["browser.storageManager.enabled", true]]});
+  mockSiteDataManager.register();
+  mockSiteDataManager.fakeSites = [
+    {
+      usage: 0,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("https://account.xyz.com"),
+      persisted: true
+    },
+    {
+      usage: 0,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("https://shopping.xyz.com"),
+      persisted: false
+    },
+    {
+      usage: 1024,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("http://cinema.bar.com"),
+      persisted: true
+    },
+    {
+      usage: 1024,
+      principal: Services.scriptSecurityManager
+                         .createCodebasePrincipalFromOrigin("http://email.bar.com"),
+      persisted: false
+    },
+  ];
+  let fakeHosts = mockSiteDataManager.fakeSites.map(site => site.principal.URI.host);
+
+  let updatePromise = promiseSitesUpdated();
+  let doc = gBrowser.selectedBrowser.contentDocument;
+  await openPreferencesViaOpenPreferencesAPI("advanced", "networkTab", { leaveOpen: true });
+  await updatePromise;
+  await openSettingsDialog();
+  assertSitesListed(doc, fakeHosts.filter(host => host != "shopping.xyz.com"));
+
+  mockSiteDataManager.unregister();
+  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
+});