Bug 522066: awesomebar unresponsive when attempting to load many bookmarks, so only display 25 items at a time, and add additional items as they are scrolled to, r=mfinkle
authorGavin Sharp <gavin@gavinsharp.com>
Wed, 09 Dec 2009 16:14:40 -0800
changeset 65923 2a35c61d84398fa031eace94cfc9d31afa7e7e09
parent 65922 57859a75796eccc1b76f821411ee094320ba0732
child 65924 034e9221488ebee1aa1ba13f40e84e4a11f451bd
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmfinkle
bugs522066
Bug 522066: awesomebar unresponsive when attempting to load many bookmarks, so only display 25 items at a time, and add additional items as they are scrolled to, r=mfinkle
mobile/chrome/content/bindings.xml
mobile/chrome/content/browser-ui.js
--- a/mobile/chrome/content/bindings.xml
+++ b/mobile/chrome/content/bindings.xml
@@ -681,17 +681,23 @@
       <field name="_manageUI">false</field>
       <field name="_parents">
         document.getAnonymousElementByAttribute(this, "anonid", "parent-items");
       </field>
       <field name="_children">
         document.getAnonymousElementByAttribute(this, "anonid", "child-items");
       </field>
 
-      <property name="scrollBoxObject" readonly="true" onget="return this._children.scrollBoxObject;"/>
+      <field name="scrollBoxObject">this._children.scrollBoxObject</field>
+      <!-- This won't be updated when the window size changes, but that's OK,
+           since we don't need an exact value - we just use it to get an
+           approximate scroll position for deciding whether to append additional
+           items (see batchSize/_insertItems()).
+        -->
+      <field name="_childrenHeight">this.scrollBoxObject.height;</field>
 
       <property name="items" readonly="true" onget="return this._children.childNodes"/>
 
       <field name="mobileRoot"><![CDATA[
         PlacesUtils.annotations.getItemsWithAnnotation("mobile/bookmarksRoot", {})[0];
       ]]></field>
 
       <property name="isRootFolder" readonly="true">
@@ -818,16 +824,24 @@
               }
             }
             rootNode.containerOpen = false;
             return items;
           ]]>
         </body>
       </method>
 
+      <!-- Number of elements to add to the list initially. If there are more
+           than this many bookmarks to display, only add them to the list once
+           the user has scrolled towards them. This is a performance
+           optimization to avoid locking up while attempting to append hundreds
+           of bookmarks to our richlistbox.
+        -->
+      <field name="batchSize">25</field>
+
       <method name="openFolder">
         <parameter name="aRootFolder"/>
         <body>
           <![CDATA[
             aRootFolder = aRootFolder || this.mobileRoot;
 
             this._activeItem = null;
 
@@ -860,27 +874,105 @@
             } while (folderId != PlacesUtils.bookmarks.placesRoot)
 
             let children = this._children;
             while (children.firstChild)
               children.removeChild(children.firstChild);
 
             children.scrollBoxObject.scrollTo(0, 0);
 
-            let fragment = document.createDocumentFragment();
-            let childItems = (aRootFolder == this._fakeDesktopFolderId) ? this._desktopChildren
-                                                                        : this._getChildren(aRootFolder);
+            this._childItems = (aRootFolder == this._fakeDesktopFolderId) ? this._desktopChildren.concat()
+                                                                          : this._getChildren(aRootFolder);
 
             if (aRootFolder == this.mobileRoot && !this.isDesktopFolderEmpty())
-              fragment.appendChild(this.createItem(this._desktopFolder));
-            
-            let count = childItems.length;
+              this._childItems.unshift(this._desktopFolder);
+
+            this._insertItems();
+          ]]>
+        </body>
+      </method>
+
+      <method name="close">
+        <body>
+          <![CDATA[
+            // Clear out references to child items, and remove event listener
+            // if needed
+            this._childItems = null;
+            if (this._scrollListenerAdded) {
+              this._children.removeEventListener("scroll", this._scrollListener, false);
+              this._scrollListenerAdded = false;
+            }
+          ]]>
+        </body>        
+      </method>
+
+      <method name="_insertItems">
+        <body>
+          <![CDATA[
+            let items = this._childItems.splice(0, this.batchSize);
+
+            if (!items.length)
+              return; // no items to insert
+
+            let children = this._children;
+            let itemsRemaining = this._childItems.length > 0;
+            if (itemsRemaining && !this._scrollListenerAdded) {
+              // We're not going to insert all items, so add a scroll listener
+              // to know when to add them.
+              this._children.addEventListener("scroll", this._scrollListener, false);
+              this._scrollListenerAdded = true;
+            }
+
+            if (!itemsRemaining && this._scrollListenerAdded) {
+              // Can get rid of the scroll listener now that all items are added.
+              this._children.removeEventListener("scroll", this._scrollListener, false);
+              this._scrollListenerAdded = false;
+            }
+
+            let fragment = document.createDocumentFragment();            
+            let count = items.length;
             for (let i=0; i<count; i++)
-              fragment.appendChild(this.createItem(childItems[i]));
+              fragment.appendChild(this.createItem(items[i]));
             children.appendChild(fragment);
+
+            // make sure we recalculate the scrollHeight of the children
+            this._childrenScrollHeight = -1;
+          ]]>
+        </body>
+      </method>
+
+      <field name="_scrollListener"><![CDATA[
+        ({
+          self: this,
+          handleEvent: function () {
+            this.self._checkForInsert();
+          }
+        })
+      ]]></field>
+
+      <method name="_checkForInsert">
+        <body>
+          <![CDATA[
+            if (this._childrenScrollHeight == -1) {
+              // Need to force a reflow to get the right value here since we may
+              // have just added children.
+              Browser.forceChromeReflow();
+              let scrollheight = {};
+              this.scrollBoxObject.getScrolledSize({}, scrollheight);
+              this._childrenScrollHeight = scrollheight.value;
+            }
+
+            let y = {};
+            this.scrollBoxObject.getPosition({}, y);
+            let scrollRatio = (y.value + this._childrenHeight) / this._childrenScrollHeight;
+
+            // If we're scrolled 80% to the bottom of the list, append the next
+            // set of items
+            if (scrollRatio > 0.8)
+              this._insertItems();
           ]]>
         </body>
       </method>
 
       <method name="createItem">
         <parameter name="aItem"/>
         <body>
           <![CDATA[
--- a/mobile/chrome/content/browser-ui.js
+++ b/mobile/chrome/content/browser-ui.js
@@ -1012,16 +1012,18 @@ var BookmarkList = {
     
     this._manageButton = document.getElementById("tool-bookmarks-manage");
     this._manageButton.disabled = (this._bookmarks.items.length == 0);
   },
 
   close: function() {
     BrowserUI.updateStar();
     
+    this._bookmarks.close();
+
     if (this._bookmarks.manageUI)
       this.toggleManage();
     this._bookmarks.blur();
     this._bookmarks.removeEventListener("BookmarkRemove", this, true);
 
     this._panel.hidden = true;
     BrowserUI.popDialog();
   },