Bug 521116: showing 'All bookmarks' from the awesomebar/autocomplete dropdown option can be very slow [r=mark.finkle]
authorVivien Nicolas <21@vingtetun.org>
Mon, 23 Nov 2009 11:44:34 -0500
changeset 65835 62d70dbe8e2eae3d4082a33aadc58c1dfe36a0bc
parent 65834 7d04e5829627cd99e10c7195a11f2f0b7201a750
child 65836 b4a6a533840a49d53d5e48da15593561c2cde7ac
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)
reviewersmark
bugs521116
Bug 521116: showing 'All bookmarks' from the awesomebar/autocomplete dropdown option can be very slow [r=mark.finkle]
mobile/chrome/content/bindings.xml
mobile/chrome/content/browser-ui.js
mobile/chrome/tests/browser_bookmarks.js
mobile/themes/hildon/browser.css
mobile/themes/wince/browser-high.css
mobile/themes/wince/browser-low.css
--- a/mobile/chrome/content/bindings.xml
+++ b/mobile/chrome/content/bindings.xml
@@ -342,50 +342,46 @@
         ]]>
       </handler>
     </handlers>
   </binding>
 
   <binding id="place-base">
     <content/>
     <implementation>
-      <constructor>
-        <![CDATA[
-          let itemId = this.getAttribute("itemid");
-          if (itemId)
-            this.init(itemId);
-        ]]>
-      </constructor>
-
-      <field name="_itemId">null</field>
       <field name="_uri">null</field>
       <field name="_control">null</field>
       <field name="_isEditing">false</field>
 
       <field name="_ioService" readonly="true">
         Components.classes["@mozilla.org/network/io-service;1"]
                   .getService(Components.interfaces.nsIIOService);
       </field>
-      <field name="_faviconService" readonly="true">
-        Components.classes["@mozilla.org/browser/favicon-service;1"]
-                  .getService(Components.interfaces.nsIFaviconService);
-      </field>
 
       <field name="_nameField">
         document.getAnonymousElementByAttribute(this, "anonid", "name");
       </field>
       <field name="_uriField">
         document.getAnonymousElementByAttribute(this, "anonid", "uri");
       </field>
       <field name="_tagsField">
         document.getAnonymousElementByAttribute(this, "anonid", "tags");
       </field>
-      <property name="itemId" onget="return this._itemId"/>
+      <property name="itemId" onget="return this.getAttribute('itemid');"/>
       <property name="type" onget="return this.getAttribute('type');"/>
 
+      <property name="uri">
+        <getter><![CDATA[
+          if (!this._uri && this.getAttribute("uri"))
+            this._uri = this._ioService.newURI(this.getAttribute("uri"), null, null);
+
+          return this._uri;
+        ]]></getter>
+      </property>
+
       <property name="name" onget="return this._nameField.value"
                             onset="this._nameField.value = val; return val;"/>
       <property name="spec" onget="return this._uriField.value"
                             onset="this._uriField.value = val; return val;"/>
       <property name="tags" onget="return this._tagsField.value"
                             onset="this._tagsField.value = val; return val;"/>
       <property name="tagsAsArray" readonly="true">
         <getter>
@@ -422,36 +418,21 @@
               }
               parent = parent.parentNode;
             }
             return null;
           ]]>
         </getter>
       </property>
 
-      <method name="init">
-        <parameter name="aItemId"/>
-        <body>
-          <![CDATA[
-            this._itemId = aItemId;
-            if (!this._itemId)
-              return;
-
-            if (this.hasAttribute("uri") && this.type != "folder") {
-              this._uri = this._ioService.newURI(this.getAttribute("uri"), null, null);
-              this.setAttribute("src", this._faviconService.getFaviconImageForPage(this._uri).spec);
-            }
-          ]]>
-        </body>
-      </method>
       <method name="startEditing">
         <parameter name="autoSelect"/>
         <body>
           <![CDATA[
-            if (!this._itemId)
+            if (!this.itemId)
               return;
 
             this._isEditing = true;
             if (this.control) {
               this.setAttribute("selected", "true");
               this.control.scrollBoxObject.ensureElementIsVisible(this);
               this.control.activeItem = this;
             }
@@ -488,72 +469,72 @@
             this.dispatchEvent(event);
           ]]>
         </body>
       </method>
       <method name="save">
         <body>
           <![CDATA[
             // Update the tags
-            if (this._uri && this.type != "folder") {
-              let currentTags = PlacesUtils.tagging.getTagsForURI(this._uri, {});
+            if (this.uri && this.type != "folder") {
+              let currentTags = PlacesUtils.tagging.getTagsForURI(this.uri, {});
               let tags = this.tagsAsArray;
               if (tags.length > 0 || currentTags.length > 0) {
                 let tagsToRemove = [];
                 let tagsToAdd = [];
                 for (let i = 0; i < currentTags.length; i++) {
                   if (tags.indexOf(currentTags[i]) == -1)
                     tagsToRemove.push(currentTags[i]);
                 }
                 for (let i = 0; i < tags.length; i++) {
                   if (currentTags.indexOf(tags[i]) == -1)
                     tagsToAdd.push(tags[i]);
                 }
 
                 if (tagsToAdd.length > 0)
-                  PlacesUtils.tagging.tagURI(this._uri, tagsToAdd);
+                  PlacesUtils.tagging.tagURI(this.uri, tagsToAdd);
                 if (tagsToRemove.length > 0)
-                  PlacesUtils.tagging.untagURI(this._uri, tagsToRemove);
+                  PlacesUtils.tagging.untagURI(this.uri, tagsToRemove);
               }
               this.setAttribute('tags', this.tags);
 
               // If the URI was updated change it in the bookmark, but don't
               // allow a blank URI. Revert to previous URI if blank.
               let spec = this.spec;
-              if (spec && this._uri.spec != spec) {
+              if (spec && this.uri.spec != spec) {
                 try {
                   this._uri = this._ioService.newURI(spec, null, null);
-                  PlacesUtils.bookmarks.changeBookmarkURI(this._itemId, this._uri);
+                  PlacesUtils.bookmarks.changeBookmarkURI(this.itemId, this.uri);
                   this.setAttribute('uri', this.spec);
                 }
                 catch (e) { }
               }
-              if (spec != this._uri.spec)
-                spec = this.spec = this._uri.spec;
+              if (spec != this.uri.spec)
+                spec = this.spec = this.uri.spec;
             }
 
             // Update the name and use the URI if name is blank
             this.name = this.name || spec;
             this.setAttribute('title', this.name);
-            PlacesUtils.bookmarks.setItemTitle(this._itemId, this.name);
+            PlacesUtils.bookmarks.setItemTitle(this.itemId, this.name);
           ]]>
         </body>
       </method>
       <method name="remove">
         <body>
           <![CDATA[
-            PlacesUtils.bookmarks.removeItem(this._itemId);
+            PlacesUtils.bookmarks.removeItem(this.itemId);
 
             // If this was the last bookmark (excluding tag-items and livemark
             // children, see getMostRecentBookmarkForURI) for the bookmark's url,
             // remove the url from tag containers as well.
-            if (this._uri && this.type != "folder") {
-              if (PlacesUtils.getMostRecentBookmarkForURI(this._uri) == -1) {
-                var tags = PlacesUtils.tagging.getTagsForURI(this._uri, {});
-                PlacesUtils.tagging.untagURI(this._uri, tags);
+            if (this.uri && this.type != "folder") {
+              if (PlacesUtils.getMostRecentBookmarkForURI(this.uri) == -1) {
+                var tags = PlacesUtils.tagging.getTagsForURI(this.uri, {});
+                PlacesUtils.tagging.untagURI(this.uri, tags);
               }
             }
 
             this.stopEditing(false);
 
             let event = document.createEvent("Events");
             event.initEvent("BookmarkRemove", true, false);
             this.dispatchEvent(event);
@@ -802,21 +783,22 @@
             } 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 = this._getChildren(aRootFolder);
-            for (let i=0; i<childItems.length; i++) {
-              let node = childItems[i];
-              children.appendChild(this.createItem(node));
-            }
+            let count = childItems.length;
+            for (let i=0; i<count; i++)
+              fragment.appendChild(this.createItem(childItems[i]));
+            children.appendChild(fragment);
           ]]>
         </body>
       </method>
 
       <method name="createItem">
         <parameter name="aItem"/>
         <body>
           <![CDATA[
@@ -825,19 +807,21 @@
             let child = document.createElementNS(XULNS, "placeitem");
             child.setAttribute("itemid", aItem.itemId);
             child.setAttribute("class", "bookmark-item");
             child.setAttribute("title", aItem.title);
             child.setAttribute("uri", aItem.uri);
             child.setAttribute("tags", aItem.tags);
             if (aItem.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER)
               child.setAttribute("type", "folder");
+            else
+              child.setAttribute("src", aItem.icon);
 
             // XXX make a <handler>
-            var self = this;
+            let self = this;
             child.addEventListener("click", function(e) { self._fireOpen(e, child); }, false);
 
             return child;
           ]]>
         </body>
       </method>
 
       <method name="removeItem">
--- a/mobile/chrome/content/browser-ui.js
+++ b/mobile/chrome/content/browser-ui.js
@@ -879,40 +879,39 @@ var BookmarkHelper = {
 
     let itemId = PlacesUtils.getMostRecentBookmarkForURI(aURI);
     if (itemId == -1)
       return;
 
     let title = PlacesUtils.bookmarks.getItemTitle(itemId);
     let tags = PlacesUtils.tagging.getTagsForURI(aURI, {});
 
-    this._editor = document.createElement("placeitem");
+    const XULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
+    this._editor = document.createElementNS(XULNS, "placeitem");
     this._editor.setAttribute("id", "bookmark-item");
     this._editor.setAttribute("flex", "1");
     this._editor.setAttribute("type", "bookmark");
     this._editor.setAttribute("ui", "manage");
     this._editor.setAttribute("title", title);
     this._editor.setAttribute("uri", aURI.spec);
+    this._editor.setAttribute("itemid", itemId);
     this._editor.setAttribute("tags", tags.join(", "));
     this._editor.setAttribute("onclose", "BookmarkHelper.hide()");
     document.getElementById("bookmark-form").appendChild(this._editor);
 
     let toolbar = document.getElementById("toolbar-main");
     let top = toolbar.top + toolbar.boxObject.height;
 
     this._panel = document.getElementById("bookmark-container");
     this._panel.top = (top < 0 ? 0 : top);
     this._panel.hidden = false;
     BrowserUI.pushPopup(this, this._panel);
 
     let self = this;
-    setTimeout(function() {
-      self._editor.init(itemId);
-      self._editor.startEditing();
-    }, 0);
+    Util.executeSoon(function() { self._editor.startEditing(); });
   },
 
   save: function BH_save() {
     this._editor.stopEditing(true);
   },
   
   hide: function BH_hide() {
     BrowserUI.updateStar();
--- a/mobile/chrome/tests/browser_bookmarks.js
+++ b/mobile/chrome/tests/browser_bookmarks.js
@@ -213,17 +213,17 @@ gTests.push({
     BookmarkList.toggleManage();
 
     waitFor(gCurrentTest.onBookmarksReady, function() { return document.getElementById("bookmark-items").manageUI == true; });
   },
   
   onBookmarksReady: function() {
     var bookmarkitems = document.getElementById("bookmark-items");
     gCurrentTest.bookmarkitem = document.getAnonymousElementByAttribute(bookmarkitems, "uri", testURL_02);
-    EventUtils.synthesizeMouse(gCurrentTest.bookmarkitem, gCurrentTest.bookmarkitem.clientWidth / 2, gCurrentTest.bookmarkitem.clientHeight / 2, {});
+    gCurrentTest.bookmarkitem.click();
 
     waitFor(gCurrentTest.onEditorReady, function() { return gCurrentTest.bookmarkitem.isEditing == true; });
   },
   
   onEditorReady: function() {
     var removebutton = document.getAnonymousElementByAttribute(gCurrentTest.bookmarkitem, "anonid", "remove-button");
     removebutton.click();
     
--- a/mobile/themes/hildon/browser.css
+++ b/mobile/themes/hildon/browser.css
@@ -594,16 +594,20 @@ placelist placeitem:active,
 .bookmark-item-label > image,
 placeitem > .bookmark-manage > image {
   width: 32px;
   height: 32px;
   max-height: 32px;
   margin: 0mm 2.5mm 0mm 1mm;
 }
 
+placeitem[src=""] .bookmark-item-label > image {
+  list-style-image: url(chrome://mozapps/skin/places/defaultFavicon.png);
+}
+
 .autocomplete-item-label > label,
 .bookmark-item-label > label {
   -moz-margin-start: 1px;
 }
 
 .autocomplete-item-label[favorite="true"] {
   padding-right: 30px;
   background: url(images/star-24.png) no-repeat 100%;
--- a/mobile/themes/wince/browser-high.css
+++ b/mobile/themes/wince/browser-high.css
@@ -269,16 +269,20 @@ placeitem[type="folder"] > .bookmark-man
 .autocomplete-item-label > image,
 .bookmark-item-label > image,
 placeitem > .bookmark-manage > image {
   width: 32px;
   height: 32px;
   max-height: 32px;
 }
 
+placeitem[src=""] .bookmark-item-label > image {
+  list-style-image: url(chrome://mozapps/skin/places/defaultFavicon.png);
+}
+
 .autocomplete-item-url,
 .bookmark-item-url {
   margin-left: 34px;
 }
 
 autocompleteresult.allbookmarks > .autocomplete-item-image {
   width: 44px;
   height: 30px;
--- a/mobile/themes/wince/browser-low.css
+++ b/mobile/themes/wince/browser-low.css
@@ -268,16 +268,20 @@ placeitem[type="folder"] > .bookmark-man
 .autocomplete-item-label > image,
 .bookmark-item-label > image,
 placeitem > .bookmark-manage > image {
   width: 24px;
   height: 24px;
   max-height: 24px;
 }
 
+placeitem[src=""] .bookmark-item-label > image {
+  list-style-image: url(chrome://mozapps/skin/places/defaultFavicon.png);
+}
+
 .autocomplete-item-url,
 .bookmark-item-url {
   margin-left: 26px;
 }
 
 autocompleteresult.allbookmarks > .autocomplete-item-image {
   width: 35px;
   height: 24px;