Bug 1722561 - Avoid calling document.querySelectorAll repeatedly when…

… loading about:preferences, r=Gijs,preferences-reviewers. Differential Revision: https://phabricator.services.mozilla.com/D120995
iaxat · Jul 28, 2021 · a08b993 · a08b993
1 parent 87276e1
commit a08b993
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 35 deletions.
diff --git a/browser/components/preferences/experimental.js b/browser/components/preferences/experimental.js
@@ -163,6 +163,5 @@ var gExperimentalPane = {
       preference.setElementValue(checkbox);
     }
     this._featureGatesContainer.appendChild(frag);
-    Preferences.updateAllElements();
   },
 };
diff --git a/browser/components/preferences/preferences.js b/browser/components/preferences/preferences.js
@@ -86,6 +86,7 @@ function register_module(categoryName, categoryObject) {
   gCategoryInits.set(categoryName, {
     inited: false,
     async init() {
+      let startTime = performance.now();
       let template = document.getElementById("template-" + categoryName);
       if (template) {
         // Replace the template element with the nodes inside of it.
@@ -97,13 +98,18 @@ function register_module(categoryName, categoryObject) {
         template.replaceWith(frag);
         document.l10n.resumeObserving();
 
-        // Asks Preferences to update the attribute value of the entire
-        // document again (this can be simplified if we could seperate the
-        // preferences of each pane.)
-        Preferences.updateAllElements();
+        // We need to queue an update again because the previous update might
+        // have happened while we awaited on translateFragment.
+        Preferences.queueUpdateOfAllElements();
       }
+
       categoryObject.init();
       this.inited = true;
+      ChromeUtils.addProfilerMarker(
+        "Preferences",
+        { startTime },
+        categoryName + " init"
+      );
     },
   });
 }
@@ -113,6 +119,10 @@ document.addEventListener("DOMContentLoaded", init_all, { once: true });
 function init_all() {
   Preferences.forceEnableInstantApply();
 
+  // Asks Preferences to queue an update of the attribute values of
+  // the entire document.
+  Preferences.queueUpdateOfAllElements();
+
   register_module("paneGeneral", gMainPane);
   register_module("paneHome", gHomePane);
   register_module("paneSearch", gSearchPane);

diff --git a/browser/components/preferences/tests/browser_basic_rebuild_fonts_test.js b/browser/components/preferences/tests/browser_basic_rebuild_fonts_test.js
@@ -7,6 +7,10 @@ registerCleanupFunction(function() {
 add_task(async function() {
   await openPreferencesViaOpenPreferencesAPI("general", { leaveOpen: true });
   await gBrowser.contentWindow.gMainPane._selectDefaultLanguageGroupPromise;
+  await TestUtils.waitForCondition(
+    () => !gBrowser.contentWindow.Preferences.updateQueued
+  );
+
   let doc = gBrowser.contentDocument;
   let contentWindow = gBrowser.contentWindow;
   var langGroup = Services.prefs.getComplexValue(

diff --git a/toolkit/content/preferencesBindings.js b/toolkit/content/preferencesBindings.js
@@ -48,7 +48,9 @@ const Preferences = (window.Preferences = (function() {
       const pref = new Preference(prefInfo);
       this._all[pref.id] = pref;
       domContentLoadedPromise.then(() => {
-        pref.updateElements();
+        if (!this.updateQueued) {
+          pref.updateElements();
+        }
       });
       return pref;
     },
@@ -109,39 +111,36 @@ const Preferences = (window.Preferences = (function() {
       }
     },
 
-    onDOMContentLoaded() {
-      // Iterate elements with a "preference" attribute and log an error
-      // if there isn't a corresponding Preference object in order to catch
-      // any cases of elements referencing preferences that haven't (yet?)
-      // been registered.
-      //
-      // TODO: remove this code once we determine that there are no such
-      // elements (or resolve any bugs that cause this behavior).
-      //
-      const elements = getElementsByAttribute("preference");
-      for (const element of elements) {
-        const id = element.getAttribute("preference");
-        const pref = this.get(id);
-        if (!pref) {
-          console.error(`Missing preference for ID ${id}`);
-        }
-      }
-    },
-
     updateQueued: false,
 
-    updateAllElements() {
+    queueUpdateOfAllElements() {
       if (this.updateQueued) {
         return;
       }
 
       this.updateQueued = true;
 
-      Promise.resolve().then(() => {
-        const preferences = Preferences.getAll();
-        for (const preference of preferences) {
-          preference.updateElements();
+      Services.tm.dispatchToMainThread(() => {
+        let startTime = performance.now();
+
+        const elements = getElementsByAttribute("preference");
+        for (const element of elements) {
+          const id = element.getAttribute("preference");
+          let preference = this.get(id);
+          if (!preference) {
+            console.error(`Missing preference for ID ${id}`);
+            continue;
+          }
+
+          preference.setElementValue(element);
         }
+
+        ChromeUtils.addProfilerMarker(
+          "Preferences",
+          { startTime },
+          `updateAllElements: ${elements.length} preferences updated`
+        );
+
         this.updateQueued = false;
       });
     },
@@ -299,6 +298,9 @@ const Preferences = (window.Preferences = (function() {
 
     addSyncFromPrefListener(aElement, callback) {
       this._syncFromPrefListeners.set(aElement, callback);
+      if (this.updateQueued) {
+        return;
+      }
       // Make sure elements are updated correctly with the listener attached.
       let elementPref = aElement.getAttribute("preference");
       if (elementPref) {
@@ -311,6 +313,9 @@ const Preferences = (window.Preferences = (function() {
 
     addSyncToPrefListener(aElement, callback) {
       this._syncToPrefListeners.set(aElement, callback);
+      if (this.updateQueued) {
+        return;
+      }
       // Make sure elements are updated correctly with the listener attached.
       let elementPref = aElement.getAttribute("preference");
       if (elementPref) {
@@ -331,9 +336,6 @@ const Preferences = (window.Preferences = (function() {
   };
 
   Services.prefs.addObserver("", Preferences);
-  domContentLoadedPromise.then(result =>
-    Preferences.onDOMContentLoaded(result)
-  );
   window.addEventListener("change", Preferences);
   window.addEventListener("command", Preferences);
   window.addEventListener("dialogaccept", Preferences);
@@ -500,16 +502,22 @@ const Preferences = (window.Preferences = (function() {
     }
 
     updateElements() {
+      let startTime = performance.now();
+
       if (!this.id) {
         return;
       }
 
-      // This "change" event handler tracks changes made to preferences by
-      // sources other than the user in this window.
       const elements = getElementsByAttribute("preference", this.id);
       for (const element of elements) {
         this.setElementValue(element);
       }
+
+      ChromeUtils.addProfilerMarker(
+        "Preferences",
+        { startTime, captureStack: true },
+        `updateElements for ${this.id}`
+      );
     }
 
     onChange() {