Backed out changeset 8998e0bed6ff (bug 1392081)
authorSebastian Hengst <archaeopteryx@coole-files.de>
Tue, 05 Sep 2017 23:11:20 +0200
changeset 428541 db23967116ba47a873ea676192d40039a964b1aa
parent 428540 82ef4728fdf54703454e4a16200d20576e905017
child 428542 294c5bf9ffea5c0110c88656bad9f55db89da1f1
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1392081
milestone57.0a1
backs out8998e0bed6fff6d671db863bd8e85959320cbe39
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
Backed out changeset 8998e0bed6ff (bug 1392081)
browser/components/places/content/browserPlacesViews.js
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -112,24 +112,22 @@ PlacesViewBase.prototype = {
     return val;
   },
 
   /**
    * Gets the DOM node used for the given places node.
    *
    * @param aPlacesNode
    *        a places result node.
-   * @param aAllowMissing
-   *        whether the node may be missing
    * @throws if there is no DOM node set for aPlacesNode.
    */
   _getDOMNodeForPlacesNode:
-  function PVB__getDOMNodeForPlacesNode(aPlacesNode, aAllowMissing = false) {
+  function PVB__getDOMNodeForPlacesNode(aPlacesNode) {
     let node = this._domNodes.get(aPlacesNode, null);
-    if (!node && !aAllowMissing) {
+    if (!node) {
       throw new Error("No DOM node set for aPlacesNode.\nnode.type: " +
                       aPlacesNode.type + ". node.parent: " + aPlacesNode);
     }
     return node;
   },
 
   get controller() {
     return this._controller;
@@ -1035,39 +1033,20 @@ PlacesToolbar.prototype = {
 
     this._openedMenuButton = null;
     while (this._rootElt.hasChildNodes()) {
       this._rootElt.firstChild.remove();
     }
 
     let fragment = document.createDocumentFragment();
     let cc = this._resultNode.childCount;
-    if (cc > 0) {
-      // There could be a lot of nodes, but we only want to build the ones that
-      // are likely to be shown, not all of them. Then we'll lazily create the
-      // missing nodes when needed.
-      // We don't want to cause reflows at every node insertion to calculate
-      // a precise size, thus we guess a size from the first node.
-      let button = this._insertNewItem(this._resultNode.getChild(0),
-                                       this._rootElt);
-      requestAnimationFrame(() => {
-        // May have been destroyed in the meanwhile.
-        if (!this._resultNode || !this._rootElt)
-          return;
-        // We assume a button with just the icon will be more or less a square,
-        // then compensate the measurement error by considering a larger screen
-        // width. Moreover the window could be bigger than the screen.
-        let size = button.clientHeight;
-        let limit = Math.min(cc, parseInt((window.screen.width * 1.5) / size));
-        for (let i = 1; i < limit; ++i) {
-          this._insertNewItem(this._resultNode.getChild(i), fragment);
-        }
-        this._rootElt.appendChild(fragment);
-      });
+    for (let i = 0; i < cc; ++i) {
+      this._insertNewItem(this._resultNode.getChild(i), fragment);
     }
+    this._rootElt.appendChild(fragment);
 
     if (this._chevronPopup.hasAttribute("type")) {
       // Chevron has already been initialized, but since we are forcing
       // a rebuild of the toolbar, it has to be rebuilt.
       // Otherwise, it will be initialized when the toolbar overflows.
       this._chevronPopup.place = this.place;
     }
   },
@@ -1117,27 +1096,24 @@ PlacesToolbar.prototype = {
     button._placesNode = aChild;
     if (!this._domNodes.has(aChild))
       this._domNodes.set(aChild, button);
 
     if (aBefore)
       aInsertionNode.insertBefore(button, aBefore);
     else
       aInsertionNode.appendChild(button);
-    return button;
   },
 
   _updateChevronPopupNodesVisibility:
   function PT__updateChevronPopupNodesVisibility() {
-    // Note the toolbar by default builds less nodes than the chevron popup.
-    for (let toolbarNode = this._rootElt.firstChild,
-         node = this._chevronPopup._startMarker.nextSibling;
-         toolbarNode && node;
-         toolbarNode = toolbarNode.nextSibling, node = node.nextSibling) {
-      node.hidden = toolbarNode.style.visibility != "hidden";
+    for (let i = 0, node = this._chevronPopup._startMarker.nextSibling;
+         node != this._chevronPopup._endMarker;
+         i++, node = node.nextSibling) {
+      node.hidden = this._rootElt.childNodes[i].style.visibility != "hidden";
     }
   },
 
   _onChevronPopupShowing:
   function PT__onChevronPopupShowing(aEvent) {
     // Handle popupshowing only for the chevron popup, not for nested ones.
     if (aEvent.target != this._chevronPopup)
       return;
@@ -1248,63 +1224,48 @@ PlacesToolbar.prototype = {
       this._updateChevronTimer.cancel();
 
     this._updateChevronTimer = this._setTimer(100);
   },
 
   _updateChevronTimerCallback: function PT__updateChevronTimerCallback() {
     let scrollRect = this._rootElt.getBoundingClientRect();
     let childOverflowed = false;
-    for (let child of this._rootElt.childNodes) {
+    for (let i = 0; i < this._rootElt.childNodes.length; i++) {
+      let child = this._rootElt.childNodes[i];
       // Once a child overflows, all the next ones will.
       if (!childOverflowed) {
         let childRect = child.getBoundingClientRect();
         childOverflowed = this.isRTL ? (childRect.left < scrollRect.left)
                                      : (childRect.right > scrollRect.right);
+
       }
-
       if (childOverflowed) {
         child.removeAttribute("image");
         child.style.visibility = "hidden";
       } else {
         let icon = child._placesNode.icon;
         if (icon)
           child.setAttribute("image", icon);
         child.style.visibility = "visible";
       }
+
     }
 
     // We rebuild the chevron on popupShowing, so if it is open
     // we must update it.
     if (this._chevron.open)
       this._updateChevronPopupNodesVisibility();
   },
 
   nodeInserted:
   function PT_nodeInserted(aParentPlacesNode, aPlacesNode, aIndex) {
     let parentElt = this._getDOMNodeForPlacesNode(aParentPlacesNode);
     if (parentElt == this._rootElt) { // Node is on the toolbar.
       let children = this._rootElt.childNodes;
-      // Nothing to do if it's a never-visible node, but note it's possible
-      // we are appending.
-      if (aIndex > children.length)
-        return;
-
-      // Note that childCount is already accounting for the node being added,
-      // thus we must subtract one node from it.
-      if (this._resultNode.childCount - 1 > children.length) {
-        if (aIndex == children.length) {
-          // If we didn't build all the nodes and new node is being appended,
-          // we can skip it as well.
-          return;
-        }
-        // Keep the number of built nodes consistent.
-        this._rootElt.removeChild(this._rootElt.lastChild);
-      }
-
       let button = this._insertNewItem(aPlacesNode, this._rootElt,
                                        children[aIndex] || null);
       let prevSiblingOverflowed = aIndex > 0 && aIndex <= children.length &&
                                   children[aIndex - 1].style.visibility == "hidden";
       if (prevSiblingOverflowed) {
         button.style.visibility = "hidden";
       } else {
         let icon = aPlacesNode.icon;
@@ -1316,79 +1277,47 @@ PlacesToolbar.prototype = {
     }
 
     PlacesViewBase.prototype.nodeInserted.apply(this, arguments);
   },
 
   nodeRemoved:
   function PT_nodeRemoved(aParentPlacesNode, aPlacesNode, aIndex) {
     let parentElt = this._getDOMNodeForPlacesNode(aParentPlacesNode);
+    let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
+
+    // Here we need the <menu>.
+    if (elt.localName == "menupopup")
+      elt = elt.parentNode;
+
     if (parentElt == this._rootElt) { // Node is on the toolbar.
-      let elt = this._getDOMNodeForPlacesNode(aPlacesNode, true);
-      // Nothing to do if it's a never-visible node.
-      if (!elt)
-        return;
-
-      // Here we need the <menu>.
-      if (elt.localName == "menupopup")
-        elt = elt.parentNode;
-
       let overflowed = elt.style.visibility == "hidden";
       this._removeChild(elt);
-      if (this._resultNode.childCount > this._rootElt.childNodes.length) {
-        // A new node should be built to keep a coherent number of children.
-        this._insertNewItem(this._resultNode.getChild(this._rootElt.childNodes.length),
-                            this._rootElt);
-      }
       if (!overflowed)
         this.updateChevron();
       return;
     }
 
     PlacesViewBase.prototype.nodeRemoved.apply(this, arguments);
   },
 
   nodeMoved:
   function PT_nodeMoved(aPlacesNode,
                         aOldParentPlacesNode, aOldIndex,
                         aNewParentPlacesNode, aNewIndex) {
     let parentElt = this._getDOMNodeForPlacesNode(aNewParentPlacesNode);
     if (parentElt == this._rootElt) { // Node is on the toolbar.
-      // Do nothing if the node will never be visible.
-      let lastBuiltIndex = this._rootElt.childNodes.length - 1;
-      if (aOldIndex > lastBuiltIndex && aNewIndex > lastBuiltIndex + 1)
-        return;
-
-      let elt = this._getDOMNodeForPlacesNode(aPlacesNode, true);
-      if (elt) {
-        // Here we need the <menu>.
-        if (elt.localName == "menupopup")
-          elt = elt.parentNode;
-        this._removeChild(elt);
-      }
+      let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
 
-      if (aNewIndex > lastBuiltIndex + 1) {
-        if (this._resultNode.childCount > this._rootElt.childNodes.length) {
-          // If the element was built and becomes non built, another node should
-          // be built to keep a coherent number of children.
-          this._insertNewItem(this._resultNode.getChild(this._rootElt.childNodes.length),
-                              this._rootElt);
-        }
-        return;
-      }
+      // Here we need the <menu>.
+      if (elt.localName == "menupopup")
+        elt = elt.parentNode;
 
-      if (!elt) {
-        // The node has not been inserted yet, so we must create it.
-        elt = this._insertNewItem(aPlacesNode, this._rootElt, this._rootElt.childNodes[aNewIndex]);
-        let icon = aPlacesNode.icon;
-        if (icon)
-          elt.setAttribute("image", icon);
-      } else {
-        this._rootElt.insertBefore(elt, this._rootElt.childNodes[aNewIndex]);
-      }
+      this._removeChild(elt);
+      this._rootElt.insertBefore(elt, this._rootElt.childNodes[aNewIndex]);
 
       // The chevron view may get nodeMoved after the toolbar.  In such a case,
       // we should ensure (by manually swapping menuitems) that the actual nodes
       // are in the final position before updateChevron tries to updates their
       // visibility, or the chevron may go out of sync.
       // Luckily updateChevron runs on a timer, so, by the time it updates
       // nodes, the menu has already handled the notification.
 
@@ -1396,19 +1325,18 @@ PlacesToolbar.prototype = {
       return;
     }
 
     PlacesViewBase.prototype.nodeMoved.apply(this, arguments);
   },
 
   nodeAnnotationChanged:
   function PT_nodeAnnotationChanged(aPlacesNode, aAnno) {
-    let elt = this._getDOMNodeForPlacesNode(aPlacesNode, true);
-    // Nothing to do if it's a never-visible node.
-    if (!elt || elt == this._rootElt)
+    let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
+    if (elt == this._rootElt)
       return;
 
     // We're notified for the menupopup, not the containing toolbarbutton.
     if (elt.localName == "menupopup")
       elt = elt.parentNode;
 
     if (elt.parentNode == this._rootElt) { // Node is on the toolbar.
       // All livemarks have a feedURI, so use it as our indicator.
@@ -1423,40 +1351,37 @@ PlacesToolbar.prototype = {
       }
     } else {
       // Node is in a submenu.
       PlacesViewBase.prototype.nodeAnnotationChanged.apply(this, arguments);
     }
   },
 
   nodeTitleChanged: function PT_nodeTitleChanged(aPlacesNode, aNewTitle) {
-    let elt = this._getDOMNodeForPlacesNode(aPlacesNode, true);
+    let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
 
-    // Nothing to do if it's a never-visible node.
-    if (!elt || elt == this._rootElt)
+    // There's no UI representation for the root node, thus there's
+    // nothing to be done when the title changes.
+    if (elt == this._rootElt)
       return;
 
     PlacesViewBase.prototype.nodeTitleChanged.apply(this, arguments);
 
     // Here we need the <menu>.
     if (elt.localName == "menupopup")
       elt = elt.parentNode;
 
     if (elt.parentNode == this._rootElt) { // Node is on the toolbar.
       if (elt.style.visibility != "hidden")
         this.updateChevron();
     }
   },
 
   invalidateContainer: function PT_invalidateContainer(aPlacesNode) {
-    let elt = this._getDOMNodeForPlacesNode(aPlacesNode, true);
-    // Nothing to do if it's a never-visible node.
-    if (!elt)
-      return;
-
+    let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
     if (elt == this._rootElt) {
       // Container is the toolbar itself.
       this._rebuild();
       return;
     }
 
     PlacesViewBase.prototype.invalidateContainer.apply(this, arguments);
   },
@@ -1599,17 +1524,17 @@ PlacesToolbar.prototype = {
     timer.initWithCallback(this, aTime, timer.TYPE_ONE_SHOT);
     return timer;
   },
 
   notify: function PT_notify(aTimer) {
     if (aTimer == this._updateChevronTimer) {
       this._updateChevronTimer = null;
       BrowserUtils.promiseLayoutFlushed(document, "layout", () => {
-        window.requestAnimationFrame(this._updateChevronTimerCallback.bind(this));
+        window.requestAnimationFrame(this._updateChevronTimerCallback);
       });
     } else if (aTimer == this._ibTimer) {
       // * Timer to turn off indicator bar.
       this._dropIndicator.collapsed = true;
       this._ibTimer = null;
     } else if (aTimer == this._overFolder.openTimer) {
       // * Timer to open a menubutton that's being dragged over.
       // Set the autoopen attribute on the folder's menupopup so that