Simplify HeapSnapshotFilteredOrderedIterator implementation

Source/devtools/front_end/HeapSnapshot.js

Index: Source/devtools/front_end/HeapSnapshot.js
diff --git a/Source/devtools/front_end/HeapSnapshot.js b/Source/devtools/front_end/HeapSnapshot.js
index 12b186b808ba0a92f771c70ff81ec941b48b62a4..1d380c86457152d831233c4ff54e8f40e6f0b406 100644
--- a/Source/devtools/front_end/HeapSnapshot.js
+++ b/Source/devtools/front_end/HeapSnapshot.js
@@ -654,6 +654,118 @@ WebInspector.HeapSnapshotNodeIterator.prototype = {
 
 
 /**
+ * @constructor
+ */
+WebInspector.HeapSnapshotIndexRangeIterator = function(iterator, indexes)

[alph, 2014/03/20 09:58:04]

annotate plz

[yurys, 2014/03/20 11:47:42]

Done.

+{
+    this._iterator = iterator;
+    this._indexes = indexes;
+    this._position = 0;
+    this._forcedIndex = -1;
+}
+
+WebInspector.HeapSnapshotIndexRangeIterator.prototype = {
+    /**
+     * @return {boolean}
+     */
+    hasNext: function()
+    {
+        return this._position < this._indexes.length

[alph, 2014/03/20 09:58:04]

; missing

[yurys, 2014/03/20 11:47:42]

Done.

+    },
+
+    /**
+     * @return {number}
+     */
+    index: function()

[loislo, 2014/03/20 09:58:26]

Can we just use a free function for converting index into an item.
because it looks strange when we stick the iterator to a particular index and
don't allow it to move next.

[yurys, 2014/03/20 11:47:42]

We will need to pass it as a separate object as it needs a context (snapshot and
item's class). I'll address this in a separate patch.

+    {
+        if (this._forcedIndex !== -1)
+            return this._forcedIndex;
+        return this._indexes[this._position];
+    },
+
+    /**
+     * @param {number} newIndex
+     */
+    setIndex: function(newIndex)
+    {
+        this._forcedIndex = newIndex;
+    },
+
+    /**
+     * @return {!WebInspector.HeapSnapshotNode}
+     */
+    item: function()
+    {
+        this._iterator.setIndex(this.index());
+        return this._iterator.item();
+    },
+
+    next: function()
+    {
+        ++this._position;
+    }
+}
+
+
+/**
+ * @constructor
+ */
+WebInspector.HeapSnapshotFilteredIterator = function(iterator, filter)
+{
+    this._iterator = iterator;
+    this._filter = filter;
+    this._skipFilteredItems();
+}
+
+WebInspector.HeapSnapshotFilteredIterator.prototype = {
+    /**
+     * @return {boolean}
+     */
+    hasNext: function()
+    {
+        return this._iterator.hasNext();
+    },
+
+    /**
+     * @return {number}
+     */
+    index: function()
+    {
+        return this._iterator.index();
+    },
+
+    /**
+     * @param {number} newIndex
+     */
+    setIndex: function(newIndex)
+    {
+        this._iterator.setIndex(newIndex);
+    },
+
+    /**
+     * @return {!WebInspector.HeapSnapshotNode}
+     */
+    item: function()
+    {
+        return this._iterator.item();
+    },
+
+    next: function()
+    {
+        this._iterator.next();
+        this._skipFilteredItems();
+    },
+
+    _skipFilteredItems: function()
+    {
+        while (this._iterator.hasNext() && !this._filter(this._iterator.item())) {
+            this._iterator.next();
+        }
+    }
+}
+
+
+/**
  * @param {!WebInspector.HeapSnapshotWorkerDispatcher=} dispatcher
  * @constructor
  */
@@ -1892,16 +2004,13 @@ WebInspector.HeapSnapshot.prototype = {
 
 /**
  * @constructor
- * @param {(!Array.<number>|!Uint32Array)=} unfilteredIterationOrder
  */
-WebInspector.HeapSnapshotFilteredOrderedIterator = function(iterator, filter, unfilteredIterationOrder)
+WebInspector.HeapSnapshotFilteredOrderedIterator = function(iterator)

[alph, 2014/03/20 09:58:04]

I'd suggest drop "Filtered" from the class name.

[yurys, 2014/03/20 11:47:42]

Done.

 {
-    this._filter = filter;
     this._iterator = iterator;
-    this._unfilteredIterationOrder = unfilteredIterationOrder;
-    /** @type {?Array.<number>|?Uint32Array} */
+    this._isEmpty = !iterator.hasNext();
+    /** @type {?Array.<number>} */
     this._iterationOrder = null;
-    this._position = 0;
     this._currentComparator = null;
     this._sortedPrefixLength = 0;
     this._sortedSuffixLength = 0;
@@ -1912,43 +2021,9 @@ WebInspector.HeapSnapshotFilteredOrderedIterator.prototype = {
     {
         if (this._iterationOrder)
             return;
-        if (this._unfilteredIterationOrder && !this._filter) {
-            this._iterationOrder = this._unfilteredIterationOrder;
-            this._unfilteredIterationOrder = null;
-            return;
-        }
         this._iterationOrder = [];
-        var iterator = this._iterator;
-        if (!this._unfilteredIterationOrder && !this._filter) {
-            for (iterator.rewind(); iterator.hasNext(); iterator.next())
-                this._iterationOrder.push(iterator.index());
-        } else if (!this._unfilteredIterationOrder) {
-            for (iterator.rewind(); iterator.hasNext(); iterator.next()) {
-                if (this._filter(iterator.item()))
-                    this._iterationOrder.push(iterator.index());
-            }
-        } else {
-            var order = this._unfilteredIterationOrder;
-            for (var i = 0, l = order.length; i < l; ++i) {
-                iterator.setIndex(order[i]);
-                if (this._filter(iterator.item()))
-                    this._iterationOrder.push(iterator.index());
-            }
-            this._unfilteredIterationOrder = null;
-        }
-    },
-
-    rewind: function()
-    {
-        this._position = 0;
-    },
-
-    /**
-     * @return {boolean}
-     */
-    hasNext: function()
-    {
-        return this._position < this._iterationOrder.length;
+        for (var iterator = this._iterator; iterator.hasNext(); iterator.next())
+            this._iterationOrder.push(iterator.index());
     },
 
     /**
@@ -1956,50 +2031,7 @@ WebInspector.HeapSnapshotFilteredOrderedIterator.prototype = {
      */
     isEmpty: function()
     {
-        if (this._iterationOrder)
-            return !this._iterationOrder.length;
-        if (this._unfilteredIterationOrder && !this._filter)
-            return !this._unfilteredIterationOrder.length;
-        var iterator = this._iterator;
-        if (!this._unfilteredIterationOrder && !this._filter) {
-            iterator.rewind();
-            return !iterator.hasNext();
-        } else if (!this._unfilteredIterationOrder) {
-            for (iterator.rewind(); iterator.hasNext(); iterator.next())
-                if (this._filter(iterator.item()))
-                    return false;
-        } else {
-            var order = this._unfilteredIterationOrder;
-            for (var i = 0, l = order.length; i < l; ++i) {
-                iterator.setIndex(order[i]);
-                if (this._filter(iterator.item()))
-                    return false;
-            }
-        }
-        return true;
-    },
-
-    /**
-     * @return {*}
-     */
-    item: function()
-    {
-        this._iterator.setIndex(this._iterationOrder[this._position]);
-        return this._iterator.item();
-    },
-
-    /**
-     * @type {number}
-     */
-    get length()
-    {
-        this._createIterationOrder();
-        return this._iterationOrder.length;
-    },
-
-    next: function()
-    {
-        ++this._position;
+        return this._isEmpty;
     },
 
     /**
@@ -2021,14 +2053,15 @@ WebInspector.HeapSnapshotFilteredOrderedIterator.prototype = {
             if (end >= this._iterationOrder.length - this._sortedSuffixLength)
                 this._sortedSuffixLength = this._iterationOrder.length - begin;
         }
-
-        this._position = begin;
-        var startPosition = this._position;
+        var position = begin;
         var count = end - begin;
         var result = new Array(count);
-        for (var i = 0 ; i < count && this.hasNext(); ++i, this.next())
-            result[i] = this.item().serialize();
-        return new WebInspector.HeapSnapshotCommon.ItemsRange(startPosition, this._position, this._iterationOrder.length, result);
+        var iterator = this._iterator;
+        for (var i = 0 ; i < count; ++i) {
+            iterator.setIndex(this._iterationOrder[position++]);
+            result[i] = iterator.item().serialize();

[alph, 2014/03/20 09:58:04]

Consider joining setIndex+item into itemByIndex

[yurys, 2014/03/20 11:47:42]

Let me address this in a separate patch.

+        }
+        return new WebInspector.HeapSnapshotCommon.ItemsRange(begin, end, this._iterationOrder.length, result);
     },
 
     sortAndRewind: function(comparator)
@@ -2036,7 +2069,6 @@ WebInspector.HeapSnapshotFilteredOrderedIterator.prototype = {
         this._currentComparator = comparator;
         this._sortedPrefixLength = 0;
         this._sortedSuffixLength = 0;
-        this.rewind();
     }
 }
 
@@ -2047,7 +2079,9 @@ WebInspector.HeapSnapshotFilteredOrderedIterator.prototype = {
 WebInspector.HeapSnapshotEdgesProvider = function(snapshot, filter, edgesIter)
 {
     this.snapshot = snapshot;
-    WebInspector.HeapSnapshotFilteredOrderedIterator.call(this, edgesIter, filter);
+    if (filter)
+        edgesIter = new WebInspector.HeapSnapshotFilteredIterator(edgesIter, filter);
+    WebInspector.HeapSnapshotFilteredOrderedIterator.call(this, edgesIter);
 }
 
 WebInspector.HeapSnapshotEdgesProvider.prototype = {
@@ -2139,12 +2173,16 @@ WebInspector.HeapSnapshotEdgesProvider.prototype = {
 /**
  * @constructor
  * @extends {WebInspector.HeapSnapshotFilteredOrderedIterator}
- * @param {(!Array.<number>|!Uint32Array)=} nodeIndexes
+ * @param {?function(!WebInspector.HeapSnapshotNode):boolean} filter
+ * @param {(!Array.<number>|!Uint32Array)} nodeIndexes
  */
 WebInspector.HeapSnapshotNodesProvider = function(snapshot, filter, nodeIndexes)
 {
     this.snapshot = snapshot;
-    WebInspector.HeapSnapshotFilteredOrderedIterator.call(this, snapshot._allNodes(), filter, nodeIndexes);
+    var it = new WebInspector.HeapSnapshotIndexRangeIterator(snapshot._allNodes(), nodeIndexes);
+    if (filter)
+        it = new WebInspector.HeapSnapshotFilteredIterator(it, filter);
+    WebInspector.HeapSnapshotFilteredOrderedIterator.call(this, it);
 }
 
 WebInspector.HeapSnapshotNodesProvider.prototype = {