cr/ui/list.js: Support rows with variable heights.

chrome/browser/resources/shared/js/cr/ui/list.js

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // require: array_data_model.js
 // require: list_selection_model.js
 // require: list_selection_controller.js
 // require: list_item.js
 
 /**
@@ skipping 98 matching lines @@
      * is needed. Note that lead item is allowed to have a different height, to
      * accommodate lists where a single item at a time can be expanded to show
      * more detail.
      * @type {{height: number, marginVertical: number, width: number,
      *     marginHorizontal: number}}
      * @private
      */
     measured_: undefined,
 
     /**
      * The height of the lead item, which is allowed to have a different height

[arv (Not doing code reviews), 2011/11/21 20:38:01]

Is there some way we can simplify this so that we do not need a special lead
item height and let the cases that used that use non fixed height rows instead?

[yoshiki, 2011/11/28 11:03:31]

Done.

      * than other list items to accommodate lists where a single item at a time
      * can be expanded to show more detail. It is explicitly set by client code
      * when the height of the lead item is changed with {@code set
      * leadItemHeight}, and presumed equal to {@code itemHeight_} otherwise.
+     * This value will be ignored when {@code fixedHeight_} is false.
      * @type {number}
      * @private
      */
     leadItemHeight_: 0,
 
     /**
      * Whether or not the list is autoexpanding. If true, the list resizes
      * its height to accomadate all children.
      * @type {boolean}
      * @private
      */
     autoExpands_: false,
 
     /**
+     * Cached list items.
+     * @type {Array<cr.ui.ListItem>}
+     * @private
+     */
+    cachedItems_: [],

[arv (Not doing code reviews), 2011/11/21 20:38:01]

These two arrays needs to go on the instance of they will be shared between all
instances.

[yoshiki, 2011/11/28 11:03:31]

Done.

+    /**
+     * Cached sized of list items.
+     * @type {Array<cr.ui.ListItem>}
+     * @private
+     */
+    cachedItemSizes_: [],
+
+    /**
+     * Whether or not the list view has a blank space below the last row.
+     * @type {boolean}
+     * @private
+     */
+    remainSpace_: true,
+
+    /**
      * Function used to create grid items.
      * @type {function(): !ListItem}
      * @private
      */
     itemConstructor_: cr.ui.ListItem,
 
     /**
      * Function used to create grid items.
      * @type {function(): !ListItem}
      */
@@ skipping 22 matching lines @@
           this.boundHandleDataModelChange_ =
               this.handleDataModelChange_.bind(this);
         }
 
         if (this.dataModel_) {
           this.dataModel_.removeEventListener(
               'permuted',
               this.boundHandleDataModelPermuted_);
           this.dataModel_.removeEventListener('change',
                                               this.boundHandleDataModelChange_);
+          this.dataModel_.removeEventListener('splice',
+                                              this.boundHandleDataModelChange_);
         }
 
         this.dataModel_ = dataModel;
 
         this.cachedItems_ = {};
         this.selectionModel.clear();
         if (dataModel)
           this.selectionModel.adjustLength(dataModel.length);
 
         if (this.dataModel_) {
           this.dataModel_.addEventListener(
               'permuted',
               this.boundHandleDataModelPermuted_);
           this.dataModel_.addEventListener('change',
                                            this.boundHandleDataModelChange_);
+          this.dataModel_.addEventListener('splice',
+                                           this.boundHandleDataModelChange_);
         }
 
         this.redraw();
       }
     },
 
     get dataModel() {
       return this.dataModel_;
     },
 
@@ skipping 37 matching lines @@
       return this.autoExpands_;
     },
     set autoExpands(autoExpands) {
       if (this.autoExpands_ == autoExpands)
         return;
       this.autoExpands_ = autoExpands;
       this.redraw();
     },
 
     /**
+     * Whether or not the rows on list have various heights.
+     * @type {boolean}
+     */
+    get fixedHeight() {
+      return this.fixedHeight;

[arv (Not doing code reviews), 2011/11/21 20:38:01]

iloop

I assume you meant this.fixedHeight_

[yoshiki, 2011/11/28 11:03:31]

Done.

+    },
+    set fixedHeight(fixedHeight) {
+      if (this.fixedHeight_ == fixedHeight)
+        return;
+      this.fixedHeight_ = fixedHeight;
+      this.redraw();
+    },
+
+    /**
      * Convenience alias for selectionModel.selectedItem
      * @type {cr.ui.ListItem}
      */
     get selectedItem() {
       var dataModel = this.dataModel;
       if (dataModel) {
         var index = this.selectionModel.selectedIndex;
         if (index != -1)
           return dataModel.item(index);
       }
       return null;
     },
     set selectedItem(selectedItem) {
       var dataModel = this.dataModel;
       if (dataModel) {
         var index = this.dataModel.indexOf(selectedItem);
         this.selectionModel.selectedIndex = index;
       }
     },
 
     /**
      * The height of the lead item.
      * If set to 0, resets to the same height as other items.
      * @type {number}
      */
     get leadItemHeight() {
-      return this.leadItemHeight_ || this.getItemHeight_();
+      return this.leadItemHeight_ || this.getDefaultItemHeight_();
     },
     set leadItemHeight(height) {
       if (height) {
-        var size = this.getItemSize_();
+        var size = this.getDefaultItemSize_();
         this.leadItemHeight_ = Math.max(0, height + size.marginVertical);
       } else {
         this.leadItemHeight_ = 0;
       }
     },
 
     /**
      * Convenience alias for selectionModel.selectedItems
      * @type {!Array<cr.ui.ListItem>}
      */
@@ skipping 63 matching lines @@
       this.addEventListener('blur', this.handleElementBlur_, true);
       this.addEventListener('scroll', this.handleScroll.bind(this));
       this.setAttribute('role', 'listbox');
 
       // Make list focusable
       if (!this.hasAttribute('tabindex'))
         this.tabIndex = 0;
     },
 
     /**
-     * @return {number} The height of an item, measuring it if necessary.
+     * @return {number} The height of default item, measuring it if necessary.
      * @private
      */
-    getItemHeight_: function() {
-      return this.getItemSize_().height;
+    getDefaultItemHeight_: function() {
+      return this.getDefaultItemSize_().height;
+    },
+
+    getItemHeightByIndex_: function(index) {

[arv (Not doing code reviews), 2011/11/21 20:38:01]

missing jsdoc

[yoshiki, 2011/11/28 11:03:31]

Done.

+      if (this.cachedItemSizes_[index])
+        return this.cachedItemSizes_[index].height;
+
+      var item = this.getListItemByIndex(index);
+      return item ? this.getItemSize_(item).height :
+                    this.getDefaultItemHeight_();
     },
 
     /**
-     * @return {number} The width of an item, measuring it if necessary.
+     * @return {number} The width of default item, measuring it if necessary.
      * @private
      */
-    getItemWidth_: function() {
-      return this.getItemSize_().width;
+    getDefaultItemWidth_: function() {
+      return this.getDefaultItemSize_().width;
     },
 
     /**
      * @return {{height: number, width: number}} The height and width
-     *     of an item, measuring it if necessary.
+     *     of default item, measuring it if necessary.
      * @private
      */
-    getItemSize_: function() {
+    getDefaultItemSize_: function() {
       if (!this.measured_ || !this.measured_.height) {
         this.measured_ = measureItem(this);
       }
       return this.measured_;
     },
 
     /**
+     * @return {{height: number, width: number}} The height and width
+     *     of an item, measuring it if necessary.
+     * @private
+     */
+    getItemSize_: function(item) {
+      if (this.cachedItemSizes_[item.listIndex])
+        return this.cachedItemSizes_[item.listIndex];
+
+      var size = measureItem(this, item);
+      if (!isNaN(size.height) && !isNaN(size.weight))
+        this.cachedItemSizes_[item.listIndex] = size;
+
+      return size;
+    },
+
+    /**
      * Callback for the double click event.
      * @param {Event} e The mouse event object.
      * @private
      */
     handleDoubleClick_: function(e) {
       if (this.disabled)
         return;
 
       var target = this.getListItemAncestor(e.target);
       if (target)
@@ skipping 191 matching lines @@
       sm.adjustLength(e.newLength);
       sm.adjustToReordering(e.permutation);
 
       this.endBatchUpdates();
 
       if (sm.leadIndex != -1)
         this.scrollIndexIntoView(sm.leadIndex);
     },
 
     handleDataModelChange_: function(e) {
-      if (e.index >= this.firstIndex_ && e.index < this.lastIndex_) {
+      if (e.index >= this.firstIndex_ &&
+          (e.index < this.lastIndex_ || this.remainSpace_)) {

[arv (Not doing code reviews), 2011/11/21 20:38:01]

rename to remainingSpace_?

[yoshiki, 2011/11/28 11:03:31]

Done.

         if (this.cachedItems_[e.index])
           delete this.cachedItems_[e.index];
         this.redraw();
       }
     },
 
     /**
      * @param {number} index The index of the item.
-     * @return {number} The top position of the item inside the list, not taking
-     *     into account lead item. May vary in the case of multiple columns.
+     * @return {number} The top position of the item inside the list.
      */
     getItemTop: function(index) {
-      return index * this.getItemHeight_();
+      if (this.fixedHeight_) {
+        var itemHeight = this.getDefaultItemHeight_();
+        var top = index * itemHeight;
+        if (this.selectionModel.leadIndex > -1 &&
+            this.selectionModel.leadIndex < index) {
+          top += this.leadItemHeight - itemHeight;
+        }
+        return top;
+      } else {
+        var top = 0;
+        for (var i = 0; i < index; i++)

[arv (Not doing code reviews), 2011/11/21 20:38:01]

Only skip {} for if/else

[yoshiki, 2011/11/28 11:03:31]

Done.

+          top += this.getItemHeightByIndex_(i);
+        return top;
+      }
     },
 
     /**
      * @param {number} index The index of the item.
      * @return {number} The row of the item. May vary in the case
      *     of multiple columns.
      */
     getItemRow: function(index) {
       return index;
     },
 
     /**
      * @param {number} row The row.
      * @return {number} The index of the first item in the row.
      */
     getFirstItemInRow: function(row) {
       return row;
     },
 
     /**
      * Ensures that a given index is inside the viewport.
      * @param {number} index The index of the item to scroll into view.
      * @return {boolean} Whether any scrolling was needed.
      */
     scrollIndexIntoView: function(index) {
       var dataModel = this.dataModel;
       if (!dataModel || index < 0 || index >= dataModel.length)
         return false;
 
-      var itemHeight = this.getItemHeight_();
+      var itemHeight = this.getItemHeightByIndex_(index);
       var scrollTop = this.scrollTop;
       var top = this.getItemTop(index);
-      var leadIndex = this.selectionModel.leadIndex;
+      var clientHeight = this.clientHeight;
 
-      // Adjust for the lead item if it is above the given index.
-      if (leadIndex > -1 && leadIndex < index)
-        top += this.leadItemHeight - itemHeight;
-      else if (leadIndex == index)
-        itemHeight = this.leadItemHeight;
+      var self = this;
+      // Function to adjust the tops of viewport and row.
+      var scrollToAdjustTopFunc = function() {

[arv (Not doing code reviews), 2011/11/21 20:38:01]

function scrollToAdjustTop() {

[yoshiki, 2011/11/28 11:03:31]

Done.

+          self.scrollTop = top;
+          return true;
+      };
+      // Function to adjust the bottoms of viewport and row.
+      var scrollToAdjustBottomFunc = function () {

[arv (Not doing code reviews), 2011/11/21 20:38:01]

function scrollToAdjustBottom() {

}

[yoshiki, 2011/11/28 11:03:31]

Done.

+          var cs = getComputedStyle(self);
+          var paddingY = parseInt(cs.paddingTop, 10) +
+                         parseInt(cs.paddingBottom, 10);
 
-      if (top < scrollTop) {
-        this.scrollTop = top;
-        return true;
+          if (top + itemHeight > scrollTop + clientHeight - paddingY) {
+            self.scrollTop = top + itemHeight - clientHeight + paddingY;
+            return true;
+          }
+          return false;
+      };
+
+      if (itemHeight <= clientHeight) {
+        if (top < scrollTop)
+          return scrollToAdjustTopFunc(index);
+        else ((scrollTop + clientHeight) < (top + itemHeight))

[arv (Not doing code reviews), 2011/11/21 20:38:01]

Syntax error

[yoshiki, 2011/11/28 11:03:31]

Done.

+          return scrollToAdjustBottomFunc(index);
       } else {
-        var clientHeight = this.clientHeight;
-        var cs = getComputedStyle(this);
-        var paddingY = parseInt(cs.paddingTop, 10) +
-                       parseInt(cs.paddingBottom, 10);
-
-        if (top + itemHeight > scrollTop + clientHeight - paddingY) {
-          this.scrollTop = top + itemHeight - clientHeight + paddingY;
-          return true;
-        }
+        if (scrollTop < top)
+          return scrollToAdjustTopFunc(index);
+        else if ((top + itemHeight) < (scrollTop + clientHeight))

[arv (Not doing code reviews), 2011/11/21 20:38:01]

no else after return

[arv (Not doing code reviews), 2011/11/21 20:38:01]

Too many parentheses

if (top + itemHeight < scrollTop + clientHeight)

[yoshiki, 2011/11/28 11:03:31]

Done.

[yoshiki, 2011/11/28 11:03:31]

Done.

+          return scrollToAdjustBottomFunc(index);
       }
-
       return false;
     },
 
     /**
      * @return {!ClientRect} The rect to use for the context menu.
      */
     getRectForContextMenu: function() {
       // TODO(arv): Add trait support so we can share more code between trees
       // and lists.
       var index = this.selectionModel.selectedIndex;
@@ skipping 65 matching lines @@
 
     /**
      * Return the heights (in pixels) of the top of the given item index within
      * the list, and the height of the given item itself, accounting for the
      * possibility that the lead item may be a different height.
      * @param {number} index The index to find the top height of.
      * @return {{top: number, height: number}} The heights for the given index.
      * @private
      */
     getHeightsForIndex_: function(index) {
-      var itemHeight = this.getItemHeight_();
+      var itemHeight = this.getItemHeightByIndex_(index);
       var top = this.getItemTop(index);
-      if (this.selectionModel.leadIndex > -1 &&
-          this.selectionModel.leadIndex < index) {
-        top += this.leadItemHeight - itemHeight;
-      } else if (this.selectionModel.leadIndex == index) {
-        itemHeight = this.leadItemHeight;
-      }
       return {top: top, height: itemHeight};
     },
 
     /**
      * Find the index of the list item containing the given y offset (measured
      * in pixels from the top) within the list. In the case of multiple columns,
      * returns the first index in the row.
      * @param {number} offset The y offset in pixels to get the index of.
-     * @return {number} The index of the list item.
+     * @return {number} The index of the list item. Returns the list size if
+     *     given offset exceeds the height of list.
      * @private
      */
     getIndexForListOffset_: function(offset) {
-      var itemHeight = this.getItemHeight_();
-      var leadIndex = this.selectionModel.leadIndex;
-      var leadItemHeight = this.leadItemHeight;
-      if (leadIndex < 0 || leadItemHeight == itemHeight) {
-        // Simple case: no lead item or lead item height is not different.
+      var itemHeight = this.getDefaultItemHeight_();
+      if (itemHeight == 0)

[arv (Not doing code reviews), 2011/11/21 20:38:01]

if (!itemHeight)

[yoshiki, 2011/11/28 11:03:31]

Done.

+          return this.dataModel.length;

[arv (Not doing code reviews), 2011/11/21 20:38:01]

fix indentation

[yoshiki, 2011/11/28 11:03:31]

Done.

+
+      if (this.fixedHeight_) {
+        var leadIndex = this.selectionModel.leadIndex;
+        var leadItemHeight = this.leadItemHeight;
+        if (leadIndex < 0 || leadItemHeight == itemHeight) {
+          // Simple case: no lead item or lead item height is not different.
+          return this.getFirstItemInRow(Math.floor(offset / itemHeight));
+        }
+        var leadTop = this.getItemTop(leadIndex);
+        // If the given offset is above the lead item, it's also simple.
+        if (offset < leadTop)
+          return this.getFirstItemInRow(Math.floor(offset / itemHeight));
+        // If the lead item contains the given offset, we just return its index.
+        if (offset < leadTop + leadItemHeight)

[arv (Not doing code reviews), 2011/11/21 20:38:01]

please add some newlines somewhere... this could use some grouping

[yoshiki, 2011/11/28 11:03:31]

Done.

+          return this.getFirstItemInRow(this.getItemRow(leadIndex));
+        // The given offset must be below the lead item. Adjust and recalculate.
+        offset -= leadItemHeight - itemHeight;
         return this.getFirstItemInRow(Math.floor(offset / itemHeight));
+      } else {
+        // If offset exceeds the height of list.
+        var lastHeight =  0;
+        if (this.dataModel.length) {
+          var h = this.getHeightsForIndex_(this.dataModel.length - 1);
+          lastHeight = h.top + h.height;
+        }
+        if (lastHeight < offset)
+          return this.dataModel.length;
+
+        // Estimates index.
+        var estimatedIndex = Math.min(Math.floor(offset / itemHeight),
+                                       this.dataModel.length - 1);

[arv (Not doing code reviews), 2011/11/21 20:38:01]

fix indentation

[yoshiki, 2011/11/28 11:03:31]

Done.

+        var isIncrementing = this.getItemTop(estimatedIndex) < offset;
+
+        // Searchs the correct index.
+        do {
+          var heights = this.getHeightsForIndex_(estimatedIndex);
+          var top = heights.top;
+          var height = heights.height;
+
+          if (top <= offset && offset <= (top + height))
+            break;
+
+          isIncrementing ? ++estimatedIndex: --estimatedIndex;
+        } while (0 < estimatedIndex && estimatedIndex < this.dataModel.length)
+
+        return estimatedIndex;
       }
-      var leadTop = this.getItemTop(leadIndex);
-      // If the given offset is above the lead item, it's also simple.
-      if (offset < leadTop)
-        return this.getFirstItemInRow(Math.floor(offset / itemHeight));
-      // If the lead item contains the given offset, we just return its index.
-      if (offset < leadTop + leadItemHeight)
-        return this.getFirstItemInRow(this.getItemRow(leadIndex));
-      // The given offset must be below the lead item. Adjust and recalculate.
-      offset -= leadItemHeight - itemHeight;
-      return this.getFirstItemInRow(Math.floor(offset / itemHeight));
     },
 
     /**
      * Return the number of items that occupy the range of heights between the
      * top of the start item and the end offset.
      * @param {number} startIndex The index of the first visible item.
      * @param {number} endOffset The y offset in pixels of the end of the list.
      * @return {number} The number of list items visible.
      * @private
      */
     countItemsInRange_: function(startIndex, endOffset) {
       var endIndex = this.getIndexForListOffset_(endOffset);
       return endIndex - startIndex + 1;
     },
 
     /**
-     * Calculates the number of items fitting in viewport given the index of
-     * first item and heights.
-     * @param {number} itemHeight The height of the item.
-     * @param {number} firstIndex Index of the first item in viewport.
+     * Calculates the number of items fitting in the given viewport.
      * @param {number} scrollTop The scroll top position.
-     * @return {number} The number of items in view port.
+     * @param {number} clientHeight The height of viewport.
+     * @return {{first: number, length: number, last: number}} The index of
+     *     first item in view port, The number of items, The item past the last.
      */
-    getItemsInViewPort: function(itemHeight, firstIndex, scrollTop) {
-      // This is a bit tricky. We take the minimum of the available items to
-      // show and the number we want to show, so as not to go off the end of the
-      // list. For the number we want to show, we take the maximum of the number
-      // that would fit without a differently-sized lead item, and with one. We
-      // do this so that if the size of the lead item changes without a scroll
-      // event to trigger redrawing the list, we won't end up with empty space.
-      var clientHeight = this.clientHeight;
-      return this.autoExpands_ ? this.dataModel.length : Math.min(
-          this.dataModel.length - firstIndex,
-          Math.max(
-              Math.ceil(clientHeight / itemHeight) + 1,
-              this.countItemsInRange_(firstIndex, scrollTop + clientHeight)));
+    getItemsInViewPort: function(scrollTop, clientHeight) {
+      if (this.autoExpands_) {
+        return {first: 0,

[arv (Not doing code reviews), 2011/11/21 20:38:01]

return {
  first: ...
};

[yoshiki, 2011/11/28 11:03:31]

Done.

+                length: this.dataModel.length,
+                last: this.dataModel.length};
+      } else {
+        var firstIndex = this.getIndexForListOffset_(scrollTop);
+        var lastIndex = this.getIndexForListOffset_(scrollTop + clientHeight);
+
+        return {first: firstIndex,
+                length: lastIndex - firstIndex + 1,
+                last: lastIndex + 1};
+      }
     },
 
     /**
      * Adds items to the list and {@code newCachedItems}.
      * @param {number} firstIndex The index of first item, inclusively.
      * @param {number} lastIndex The index of last item, exclusively.
      * @param {Object.<string, ListItem>} cachedItems Old items cache.
      * @param {Object.<string, ListItem>} newCachedItems New items cache.
      */
     addItems: function(firstIndex, lastIndex, cachedItems, newCachedItems) {
       var listItem;
       var dataModel = this.dataModel;
 
       window.l = this;
       for (var y = firstIndex; y < lastIndex; y++) {
         var dataItem = dataModel.item(y);
         listItem = cachedItems[y] || this.createItem(dataItem);
         listItem.listIndex = y;
         this.appendChild(listItem);
         newCachedItems[y] = listItem;
+        this.cachedItemSizes_[y] = measureItem(this, listItem);

[arv (Not doing code reviews), 2011/11/21 20:38:01]

This is bad. it will regress performance since it will make a reflow for every
iteration.

To solve this you need to add all the elements first before any measuring is
done.

[yoshiki, 2011/11/28 11:03:31]

Done.

       }
     },
 
     /**
      * Returns the height of after filler in the list.
      * @param {number} lastIndex The index of item past the last in viewport.
      * @param {number} itemHeight The height of the item.
      * @return {number} The height of after filler.
      */
-    getAfterFillerHeight: function(lastIndex, itemHeight) {
-      return (this.dataModel.length - lastIndex) * itemHeight;
+    getAfterFillerHeight: function(lastIndex) {
+      if (this.fixedHeight_) {
+        var itemHeight = this.getDefaultItemHeight_();
+        var afterFillerHeight =
+          (this.dataModel.length - lastIndex) * itemHeight;
+
+        var sm = this.selectionModel;
+        var leadIndex = sm.leadIndex;
+        if (leadIndex >= lastIndex)
+          afterFillerHeight += this.leadItemHeight - itemHeight;
+
+        return afterFillerHeight;
+      } else {
+        var height = 0;
+        for (var i = lastIndex; i < this.dataModel.length; i++) {
+          var item = this.getListItemByIndex(i);
+          if (item)
+            height += this.getItemSize_(item).height;
+        }
+        return height;
+      }
     },
 
     /**
      * Redraws the viewport.
      */
     redraw: function() {
       if (this.batchCount_ != 0)
         return;
 
       var dataModel = this.dataModel;
       if (!dataModel) {
+        this.cachedItems_ = {};
+        this.firstIndex_ = 0;
+        this.lastIndex_ = 0;
+        this.remainSpace_ = true;
         this.textContent = '';
         return;
       }
 
-      var scrollTop = this.scrollTop;
-      var clientHeight = this.clientHeight;
-
-      var itemHeight = this.getItemHeight_();
-
       // We cache the list items since creating the DOM nodes is the most
       // expensive part of redrawing.
       var cachedItems = this.cachedItems_ || {};
       var newCachedItems = {};
 
-      var desiredScrollHeight = this.getHeightsForIndex_(dataModel.length).top;
+      var autoExpands = this.autoExpands_;
+      var scrollTop = this.scrollTop;
+      var clientHeight = this.clientHeight;
 
-      var autoExpands = this.autoExpands_;
-      var firstIndex = autoExpands ? 0 : this.getIndexForListOffset_(scrollTop);
-      var itemsInViewPort = this.getItemsInViewPort(itemHeight, firstIndex,
-          scrollTop);
-      var lastIndex = firstIndex + itemsInViewPort;
+      var lastItemHeights = this.getHeightsForIndex_(dataModel.length - 1);
+      var desiredScrollHeight = lastItemHeights.top + lastItemHeights.height;
 
+      var itemsInViewPort = this.getItemsInViewPort(scrollTop, clientHeight);
+      // Draws the hidden rows just above/below the viewport to prevent
+      // flashing in scroll.
+      var firstIndex = Math.max(0, itemsInViewPort.first - 1);
+      var lastIndex = Math.min(itemsInViewPort.last + 1, dataModel.length);
+
+      var beforeFillerHeight =
+          this.autoExpands ? 0 : this.getItemTop(firstIndex);
+      var afterFillerHeight =
+          this.autoExpands ? 0 : this.getAfterFillerHeight(lastIndex);
+
+      // Clear list and Adds elements on list.
       this.textContent = '';
 
-      this.beforeFiller_.style.height =
-          this.getHeightsForIndex_(firstIndex).top + 'px';
+      this.beforeFiller_.style.height = beforeFillerHeight + 'px';
       this.appendChild(this.beforeFiller_);
 
+      this.addItems(firstIndex, lastIndex, cachedItems, newCachedItems);

[arv (Not doing code reviews), 2011/11/21 20:38:01]

Make sure there are no measuring done in here.

Basically, you need to ensure that you do not interleave measuring with mutating
the dom (appendChild, element.style.foo assignment etc). You should be able to
see this in the inspector.

[yoshiki, 2011/11/28 11:03:31]

I added the function ensureAllItemSizesInCache() and call it in advance before
redrawing. I checked that the dom is not mutated after that before addItems().

On 2011/11/21 20:38:01, arv wrote:

+
+      this.afterFiller_.style.height = afterFillerHeight + 'px';
+      this.appendChild(this.afterFiller_);
+
       var sm = this.selectionModel;
       var leadIndex = sm.leadIndex;
 
-      this.addItems(firstIndex, lastIndex, cachedItems, newCachedItems);
-
-      var afterFillerHeight = this.getAfterFillerHeight(lastIndex, itemHeight);
-      if (leadIndex >= lastIndex)
-        afterFillerHeight += this.leadItemHeight - itemHeight;
-      this.afterFiller_.style.height = afterFillerHeight + 'px';
-      this.appendChild(this.afterFiller_);
-
       // We don't set the lead or selected properties until after adding all
       // items, in case they force relayout in response to these events.
       var listItem = null;
-      if (newCachedItems[leadIndex])
+      if (leadIndex != -1 && newCachedItems[leadIndex])
         newCachedItems[leadIndex].lead = true;
       for (var y = firstIndex; y < lastIndex; y++) {
         if (sm.getIndexSelected(y))
           newCachedItems[y].selected = true;
         else if (y != leadIndex)
           listItem = newCachedItems[y];
       }
 
       this.scrollTop = scrollTop;
 
       this.firstIndex_ = firstIndex;
       this.lastIndex_ = lastIndex;
 
+      this.remainSpace_ = itemsInViewPort.last > dataModel.length;
       this.cachedItems_ = newCachedItems;
 
       // Measure again in case the item height has changed due to a page zoom.
       //
       // The measure above is only done the first time but this measure is done
       // after every redraw. It is done in a timeout so it will not trigger
       // a reflow (which made the redraw speed 3 times slower on my system).
       // By using a timeout the measuring will happen later when there is no
       // need for a reflow.
-      if (listItem) {
+      if (listItem && this.fixedHeight_) {
         var list = this;
         window.setTimeout(function() {
           if (listItem.parentNode == list) {
             list.measured_ = measureItem(list, listItem);
           }
         });
       }
     },
 
     /**
      * Invalidates list by removing cached items.
      */
     invalidate: function() {
       this.cachedItems_ = {};
     },
 
     /**
      * Redraws a single item.
      * @param {number} index The row index to redraw.
      */
     redrawItem: function(index) {
-      if (index >= this.firstIndex_ && index < this.lastIndex_) {
+      if (index >= this.firstIndex_ &&
+          (index < this.lastIndex_ || this.remainSpace_)) {
         delete this.cachedItems_[index];
         this.redraw();
       }
     },
 
     /**
      * Called when a list item is activated, currently only by a double click
      * event.
      * @param {number} index The index of the activated item.
      */
     activateItemAtIndex: function(index) {
     },
   };
 
   cr.defineProperty(List, 'disabled', cr.PropertyKind.BOOL_ATTR);
 
   /**
    * Whether the list or one of its descendents has focus. This is necessary
    * because list items can contain controls that can be focused, and for some
    * purposes (e.g., styling), the list can still be conceptually focused at
    * that point even though it doesn't actually have the page focus.
    */
   cr.defineProperty(List, 'hasElementFocus', cr.PropertyKind.BOOL_ATTR);
 
   return {
     List: List
   }
 });