Bug 1375870 - _constructAfterChildren in preferences.xml should be called only once. r=mossop draft
authorNihanth Subramanya <nhnt11@gmail.com>
Fri, 23 Jun 2017 20:44:45 +0530
changeset 617787 4f761a66d97e5cc0ec97a12ce4050f9abb9ccaf6
parent 612966 0faada5c2f308f101ab7c54a87b3dce80b97d0e3
child 639885 b23c49cd923e08bdbce370de95d458db7fe6d785
push id71131
push usernhnt11@gmail.com
push dateFri, 28 Jul 2017 21:52:23 +0000
reviewersmossop
bugs1375870
milestone56.0a1
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">