Bug 1375870 - _constructAfterChildren in preferences.xml should be called only once. r=mossop
authorNihanth Subramanya <nhnt11@gmail.com>
Fri, 23 Jun 2017 20:44:45 +0530
changeset 423296 6d87add067739ed60262f9246d05606b61ca3025
parent 423295 58c1de79d494b91b1d70de5ae18833c8f297c3b7
child 423297 3ba4ec951216ff5ccdebd1062f8f5c4a61e0ae32
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmossop
bugs1375870
milestone56.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 1375870 - _constructAfterChildren in preferences.xml should be called only once. r=mossop MozReview-Commit-ID: IXywqnrAMkf
toolkit/content/widgets/preferences.xml
--- a/toolkit/content/widgets/preferences.xml
+++ b/toolkit/content/widgets/preferences.xml
@@ -28,29 +28,29 @@
 #   </prefwindow>
 #
 
   <binding id="preferences">
     <implementation implements="nsIObserver">
       <method name="_constructAfterChildren">
       <body>
       <![CDATA[
-      // This method will be called after each one of the child
+      // This method will be called after the last of the child
       // <preference> elements is constructed. Its purpose is to propagate
-      // the values to the associated form elements
+      // the values to the associated form elements. Sometimes the code for
+      // some <preference> initializers depend on other <preference> elements
+      // being initialized so we wait and call updateElements on all of them
+      // once the last one has been constructed. See bugs 997570 and 992185.
 
       var elements = this.getElementsByTagName("preference");
       for (let element of elements) {
-        if (!element._constructed) {
-          return;
-        }
-      }
-      for (let element of elements) {
         element.updateElements();
       }
+
+      this._constructAfterChildrenCalled = true;
       ]]>
       </body>
       </method>
       <method name="observe">
         <parameter name="aSubject"/>
         <parameter name="aTopic"/>
         <parameter name="aData"/>
         <body>
@@ -107,25 +107,42 @@
         <getter>
           <![CDATA[
             var doc = document.documentElement;
             return this.type == "child" ? doc.instantApply
                                         : doc.instantApply || this.rootBranch.getBoolPref("browser.preferences.instantApply");
           ]]>
         </getter>
       </property>
+
+      <!-- We want to call _constructAfterChildren after all child
+           <preference> elements have been constructed. To do this, we get
+           and store the node list of all child <preference> elements in the
+           constructor, and maintain a count which is incremented in the
+           constructor of <preference>. _constructAfterChildren is called
+           when the count matches the length of the list. -->
+      <field name="_constructedChildrenCount">0</field>
+      <field name="_preferenceChildren">null</field>
+      <!-- Some <preference> elements are added dynamically after
+           _constructAfterChildren has already been called - we want to
+           avoid looping over all of them again in this case so we remember
+           if we already called it. -->
+      <field name="_constructAfterChildrenCalled">false</field>
+      <constructor>
+      <![CDATA[
+        this._preferenceChildren = this.getElementsByTagName("preference");
+      ]]>
+      </constructor>
     </implementation>
   </binding>
 
   <binding id="preference">
     <implementation>
       <constructor>
       <![CDATA[
-        this._constructed = true;
-
         // if the element has been inserted without the name attribute set,
         // we have nothing to do here
         if (!this.name)
           return;
 
         this.preferences.rootBranchInternal
             .addObserver(this.name, this.preferences);
         // In non-instant apply mode, we must try and use the last saved state
@@ -148,19 +165,31 @@
             for (var l = 0; (l < parentPrefs.length && !preference); ++l) {
               if (parentPrefs[l].localName == "preference")
                 preference = parentPrefs[l];
             }
           }
 
           // Don't use the value setter here, we don't want updateElements to be prematurely fired.
           this._value = preference ? preference.value : this.valueFromPreferences;
-        } else
+        } else {
           this._value = this.valueFromPreferences;
-        this.preferences._constructAfterChildren();
+        }
+        if (this.preferences._constructAfterChildrenCalled) {
+          // This <preference> was added after _constructAfterChildren() was already called.
+          // We can directly call updateElements().
+          this.updateElements();
+          return;
+        }
+        this.preferences._constructedChildrenCount++;
+        if (this.preferences._constructedChildrenCount ==
+            this.preferences._preferenceChildren.length) {
+          // This is the last <preference>, time to updateElements() on all of them.
+          this.preferences._constructAfterChildren();
+        }
       ]]>
       </constructor>
       <destructor>
         this.preferences.rootBranchInternal
             .removeObserver(this.name, this.preferences);
       </destructor>
       <field name="_constructed">false</field>
       <property name="instantApply">