Bug 1356285 - micro-optimization in richlistbox selectedIndex setter. r=Enn draft
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 13 Apr 2017 19:01:48 +0200
changeset 562257 1c8e7500b77b3d0133ce7803e15de9360864d672
parent 561911 819a666afddc804b6099ee1b3cff3a0fdf35ec15
child 562927 f4b09a813cf4f1d6fa6d27e9b4ac886fcf673ecc
push id54008
push usermak77@bonardo.net
push dateThu, 13 Apr 2017 19:38:47 +0000
reviewersEnn
bugs1356285
milestone55.0a1
Bug 1356285 - micro-optimization in richlistbox selectedIndex setter. r=Enn MozReview-Commit-ID: 6fnhubnDUZ1
toolkit/content/widgets/listbox.xml
toolkit/content/widgets/richlistbox.xml
--- a/toolkit/content/widgets/listbox.xml
+++ b/toolkit/content/widgets/listbox.xml
@@ -98,17 +98,25 @@
           if (this.selectedItems.length > 0)
             return this.getIndexOfItem(this.selectedItems[0]);
           return -1;
         ]]>
         </getter>
         <setter>
         <![CDATA[
           if (val >= 0) {
-            this.selectItem(this.getItemAtIndex(val));
+            // This is a micro-optimization so that a call to getIndexOfItem or
+            // getItemAtIndex caused by _fireOnSelect (especially for derived
+            // widgets) won't loop the children.
+            this._selecting = {
+              item: this.getItemAtIndex(val),
+              index: val
+            };
+            this.selectItem(this._selecting.item);
+            delete this._selecting;
           } else {
             this.clearSelection();
             this.currentItem = null;
           }
         ]]>
         </setter>
       </property>
 
@@ -741,23 +749,31 @@
 
       <property name="itemCount" readonly="true"
                 onget="return this.listBoxObject.getRowCount()"/>
 
       <!-- ///////////////// nsIListBoxObject ///////////////// -->
       <method name="getIndexOfItem">
         <parameter name="item"/>
         <body>
-          return this.listBoxObject.getIndexOfItem(item);
+          <![CDATA[
+            if (this._selecting && this._selecting.item == item)
+              return this._selecting.index;
+            return this.listBoxObject.getIndexOfItem(item);
+          ]]>
         </body>
       </method>
       <method name="getItemAtIndex">
         <parameter name="index"/>
         <body>
-          return this.listBoxObject.getItemAtIndex(index);
+          <![CDATA[
+            if (this._selecting && this._selecting.index == index)
+              return this._selecting.item;
+            return this.listBoxObject.getItemAtIndex(index);
+          ]]>
         </body>
       </method>
       <method name="ensureIndexIsVisible">
         <parameter name="index"/>
         <body>
           return this.listBoxObject.ensureIndexIsVisible(index);
         </body>
       </method>
--- a/toolkit/content/widgets/richlistbox.xml
+++ b/toolkit/content/widgets/richlistbox.xml
@@ -170,26 +170,31 @@
 
       <method name="getIndexOfItem">
         <parameter name="aItem"/>
         <body>
           <![CDATA[
             // don't search the children, if we're looking for none of them
             if (aItem == null)
               return -1;
-
+            if (this._selecting && this._selecting.item == aItem)
+              return this._selecting.index;
             return this.children.indexOf(aItem);
           ]]>
         </body>
       </method>
 
       <method name="getItemAtIndex">
         <parameter name="aIndex"/>
         <body>
-          return this.children[aIndex] || null;
+          <![CDATA[
+            if (this._selecting && this._selecting.index == aIndex)
+              return this._selecting.item;
+            return this.children[aIndex] || null;
+          ]]>
         </body>
       </method>
 
       <method name="ensureIndexIsVisible">
         <parameter name="aIndex"/>
         <body>
           <![CDATA[
             // work around missing implementation in scrollBoxObject
@@ -310,26 +315,23 @@
           ]]>
         </body>
       </method>
 
     <!-- richlistbox specific -->
       <property name="children" readonly="true">
         <getter>
           <![CDATA[
-            var childNodes = [];
-            var isReverse = this.dir == "reverse" && this._mayReverse;
-            var child = isReverse ? this.lastChild : this.firstChild;
-            var prop = isReverse ? "previousSibling" : "nextSibling";
-            while (child) {
-              if (child instanceof Components.interfaces.nsIDOMXULSelectControlItemElement)
-                childNodes.push(child);
-              child = child[prop];
+            let iface = Components.interfaces.nsIDOMXULSelectControlItemElement;
+            let children = Array.from(this.childNodes)
+                                .filter(node => node instanceof iface);
+            if (this.dir == "reverse" && this._mayReverse) {
+              children.reverse();
             }
-            return childNodes;
+            return children;
           ]]>
         </getter>
       </property>
 
       <field name="_builderListener" readonly="true">
         <![CDATA[
           ({
             mOuter: this,