Bug 1602994 - Load site data into the remove dialog asynchronously. r=nhnt11,preferences-reviewers

We currently wait for all site data to be refreshed before showing the removal
dialog, in order to show the sites that will be affected from removal. This causes
some serious delay to the point of complete broken-ness on profiles with a lot of data.

Tests for this already exists and we're not adding any new functionality,
just optimizing the old behavior for better perceived performance.

Differential Revision: https://phabricator.services.mozilla.com/D93531

browser/base/content/browser-siteIdentity.js

@@ -437,22 +437,21 @@ var gIdentityHandler = {
 
     let host = this._uri.host;
 
-    // Site data could have changed while the identity popup was open,
-    // reload again to be sure.
-    await SiteDataManager.updateSites();
-
-    let baseDomain = SiteDataManager.getBaseDomainFromHost(host);
-    let siteData = await SiteDataManager.getSites(baseDomain);
-
     // Hide the popup before showing the removal prompt, to
     // avoid a pretty ugly transition. Also hide it even
     // if the update resulted in no site data, to keep the
     // illusion that clicking the button had an effect.
+    let hidden = new Promise(c => {
+      this._identityPopup.addEventListener("popuphidden", c, { once: true });
+    });
     PanelMultiView.hidePopup(this._identityPopup);
+    await hidden;
 
-    if (siteData && siteData.length) {
-      let hosts = siteData.map(site => site.host);
-      if (SiteDataManager.promptSiteDataRemoval(window, hosts)) {
+    let baseDomain = SiteDataManager.getBaseDomainFromHost(host);
+    if (SiteDataManager.promptSiteDataRemoval(window, null, baseDomain)) {
+      let siteData = await SiteDataManager.getSites(baseDomain);
+      if (siteData && siteData.length) {
+        let hosts = siteData.map(site => site.host);
         SiteDataManager.remove(hosts);
       }
     }

browser/components/preferences/dialogs/siteDataRemoveSelected.js

@@ -5,29 +5,87 @@
 
 "use strict";
 
+const { SiteDataManager } = ChromeUtils.import(
+  "resource:///modules/SiteDataManager.jsm"
+);
+
+/**
+ * This dialog will ask the user to confirm that they really want to delete
+ * all site data for a number of hosts. Displaying the hosts can be done in
+ * two different ways by passing options in the arguments object.
+ *
+ * - Passing a "baseDomain" will cause the dialog to search for all applicable
+ *   host names with that baseDomain using the SiteDataManager and populate the list
+ *   asynchronously. As a side effect, the SiteDataManager will update. Thus it is
+ *   safe to call SiteDataManager methods that require a manual updateSites call
+ *   after spawning this dialog. However, you need to ensure that the SiteDataManager
+ *   has finished updating.
+ *
+ * - Passing a "hosts" array allows you to manually specify the hosts to be displayed.
+ *   The SiteDataManager will not be updated by spawning this dialog in that case.
+ *
+ *   The two options are mutually exclusive. You must specify one.
+ **/
 let gSiteDataRemoveSelected = {
   init() {
-    let hosts = window.arguments[0].hosts;
-    hosts.sort();
-    let list = document.getElementById("removalList");
-    let fragment = document.createDocumentFragment();
-    for (let host of hosts) {
-      let listItem = document.createXULElement("richlistitem");
-      let label = document.createXULElement("label");
-      if (host) {
-        label.setAttribute("value", host);
-      } else {
-        document.l10n.setAttributes(label, "site-data-local-file-host");
-      }
-      listItem.appendChild(label);
-      fragment.appendChild(listItem);
-    }
-    list.appendChild(fragment);
     document.addEventListener("dialogaccept", function() {
       window.arguments[0].allowed = true;
     });
     document.addEventListener("dialogcancel", function() {
       window.arguments[0].allowed = false;
     });
+
+    let list = document.getElementById("removalList");
+
+    let baseDomain = window.arguments[0].baseDomain;
+    if (baseDomain) {
+      let hosts = new Set();
+      SiteDataManager.updateSites((host, site) => {
+        // Disregard local files.
+        if (!host) {
+          return;
+        }
+
+        if (site.baseDomain != baseDomain) {
+          return;
+        }
+
+        // Avoid listing duplicate hosts.
+        if (hosts.has(host)) {
+          return;
+        }
+        hosts.add(host);
+
+        let listItem = document.createXULElement("richlistitem");
+        let label = document.createXULElement("label");
+        label.setAttribute("value", host);
+        listItem.appendChild(label);
+        list.appendChild(listItem);
+      });
+      return;
+    }
+
+    let hosts = window.arguments[0].hosts;
+    if (hosts) {
+      hosts.sort();
+      let fragment = document.createDocumentFragment();
+      for (let host of hosts) {
+        let listItem = document.createXULElement("richlistitem");
+        let label = document.createXULElement("label");
+        if (host) {
+          label.setAttribute("value", host);
+        } else {
+          document.l10n.setAttributes(label, "site-data-local-file-host");
+        }
+        listItem.appendChild(label);
+        fragment.appendChild(listItem);
+      }
+      list.appendChild(fragment);
+      return;
+    }
+
+    throw new Error(
+      "Must specify either a hosts or baseDomain option in arguments."
+    );
   },
 };

browser/modules/SiteDataManager.jsm

@@ -46,13 +46,29 @@ var SiteDataManager = {
 
   _quotaUsageRequest: null,
 
-  async updateSites() {
+  /**
+   *  Retrieve the latest site data and store it in SiteDataManager.
+   *
+   *  Updating site data is a *very* expensive operation. This method exists so that
+   *  consumers can manually decide when to update, most methods on SiteDataManager
+   *  will not trigger updates automatically.
+   *
+   *  It is *highly discouraged* to await on this function to finish before showing UI.
+   *  Either trigger the update some time before the data is needed or use the
+   *  entryUpdatedCallback parameter to update the UI async.
+   *
+   * @param {entryUpdatedCallback} a function to be called whenever a site is added or
+   *        updated. This can be used to e.g. fill a UI that lists sites without
+   *        blocking on the entire update to finish.
+   * @returns a Promise that resolves when updating is done.
+   **/
+  async updateSites(entryUpdatedCallback) {
     Services.obs.notifyObservers(null, "sitedatamanager:updating-sites");
     // Clear old data and requests first
     this._sites.clear();
-    this._getAllCookies();
-    await this._getQuotaUsage();
-    this._updateAppCache();
+    this._getAllCookies(entryUpdatedCallback);
+    await this._getQuotaUsage(entryUpdatedCallback);
+    this._updateAppCache(entryUpdatedCallback);
     Services.obs.notifyObservers(null, "sitedatamanager:sites-updated");
   },
 
@@ -133,7 +149,7 @@ var SiteDataManager = {
     return this._getCacheSizePromise;
   },
 
-  _getQuotaUsage() {
+  _getQuotaUsage(entryUpdatedCallback) {
     this._cancelGetQuotaUsage();
     this._getQuotaUsagePromise = new Promise(resolve => {
       let onUsageResult = request => {
@@ -163,6 +179,9 @@ var SiteDataManager = {
               }
               site.principals.push(principal);
               site.quotaUsage += item.usage;
+              if (entryUpdatedCallback) {
+                entryUpdatedCallback(principal.host, site);
+              }
             }
           }
         }
@@ -176,9 +195,12 @@ var SiteDataManager = {
     return this._getQuotaUsagePromise;
   },
 
-  _getAllCookies() {
+  _getAllCookies(entryUpdatedCallback) {
     for (let cookie of Services.cookies.cookies) {
       let site = this._getOrInsertSite(cookie.rawHost);
+      if (entryUpdatedCallback) {
+        entryUpdatedCallback(cookie.rawHost, site);
+      }
       site.cookies.push(cookie);
       if (site.lastAccessed < cookie.lastAccessed) {
         site.lastAccessed = cookie.lastAccessed;
@@ -193,7 +215,7 @@ var SiteDataManager = {
     }
   },
 
-  _updateAppCache() {
+  _updateAppCache(entryUpdatedCallback) {
     let groups;
     try {
       groups = this._appCache.getGroups();
@@ -221,6 +243,9 @@ var SiteDataManager = {
         site.principals.push(principal);
       }
       site.appCacheList.push(cache);
+      if (entryUpdatedCallback) {
+        entryUpdatedCallback(principal.host, site);
+      }
     }
   },
 
@@ -501,11 +526,14 @@ var SiteDataManager = {
    *
    * @param {mozIDOMWindowProxy} a parent DOM window to host the dialog.
    * @param {Array} [optional] an array of host name strings that will be removed.
+   * @param {baseDomain} [optional] a baseDomain to use in the dialog when searching
+   *        for hosts to be removed. This will trigger a SiteDataManager update.
    * @returns a boolean whether the user confirmed the prompt.
    */
-  promptSiteDataRemoval(win, removals) {
-    if (removals) {
+  promptSiteDataRemoval(win, removals, baseDomain) {
+    if (baseDomain || removals) {
       let args = {
+        baseDomain,
         hosts: removals,
         allowed: false,
       };