[inspector] remove iterators and for...of loops from injected-script-source

src/inspector/injected-script-source.js

Index: src/inspector/injected-script-source.js
diff --git a/src/inspector/injected-script-source.js b/src/inspector/injected-script-source.js
index b52277a8ebec017a6deb3d3a590a8d66ae47f2ab..1968b3f11680210a88d2f81241e8d84ea9872fea 100644
--- a/src/inspector/injected-script-source.js
+++ b/src/inspector/injected-script-source.js
@@ -335,12 +335,10 @@ InjectedScript.prototype = {
             object = object.object;
         }
 
-        var descriptors = [];
-        var iter = this._propertyDescriptors(object, ownProperties, accessorPropertiesOnly, undefined);
         // Go over properties, wrap object values.
-        for (var descriptor of iter) {
-            if (subtype === "internal#scopeList" && descriptor.name === "length")
-                continue;
+        var descriptors = this._propertyDescriptors(object, ownProperties, accessorPropertiesOnly, undefined, false);
+        for (var i = 0; i < descriptors.length; ++i) {
+            var descriptor = descriptors[i];
             if ("get" in descriptor)
                 descriptor.get = this._wrapObject(descriptor.get, objectGroupName);
             if ("set" in descriptor)
@@ -353,7 +351,6 @@ InjectedScript.prototype = {
                 descriptor.enumerable = false;
             if ("symbol" in descriptor)
                 descriptor.symbol = this._wrapObject(descriptor.symbol, objectGroupName);
-            push(descriptors, descriptor);
         }
         return descriptors;
     },
@@ -377,60 +374,77 @@ InjectedScript.prototype = {
      * @param {!Object} object
      * @param {boolean=} ownProperties
      * @param {boolean=} accessorPropertiesOnly
-     * @param {?Array.<string>=} propertyNamesOnly
+     * @param {?Array<string>=} propertyNamesOnly
+     * @param {boolean=} forPreview
+     * @param {!function(): boolean=} isSpaceAvailable
+     * @param {!function(!Array<!Object>, !Object)=} customPush
+     * @return {!Array<!Object>}
      */
-    _propertyDescriptors: function*(object, ownProperties, accessorPropertiesOnly, propertyNamesOnly)
+    _propertyDescriptors: function(object, ownProperties, accessorPropertiesOnly, propertyNamesOnly, forPreview, isSpaceAvailable, customPush)
     {
+        var descriptors = [];
+        descriptors.__proto__ = null;
+        isSpaceAvailable = isSpaceAvailable || function() { return true; };
+        customPush = customPush || push;
+        if (!isSpaceAvailable())
+            return descriptors;
         var propertyProcessed = { __proto__: null };
+        var subtype = InjectedScriptHost.subtype(object);
 
         /**
-         * @param {?Object} o
-         * @param {!Iterable<string|symbol|number>|!Array<string|number|symbol>} properties
+         * @param {!Object} o
+         * @param {!Array<string|number|symbol>=} properties
+         * @param {number=} objectLength
          */
-        function* process(o, properties)
+        function process(o, properties, objectLength)
         {
-            for (var property of properties) {
+            // When properties is not provided, iterate over the object's indices.
+            var length = properties ? properties.length : objectLength;
+            for (var i = 0; i < length; ++i) {
+                if (!isSpaceAvailable())
+                    return;
+                var property = properties ? properties[i] : ("" + i);
+                if (propertyProcessed[property])
+                    continue;
+                propertyProcessed[property] = true;
                 var name;
                 if (isSymbol(property))
                     name = /** @type {string} */ (injectedScript._describe(property));
                 else
                     name = typeof property === "number" ? ("" + property) : /** @type {string} */(property);
 
-                if (propertyProcessed[property])
+                if (subtype === "internal#scopeList" && name === "length")
                     continue;
 
+                var descriptor;
                 try {
-                    propertyProcessed[property] = true;
-                    var descriptor = nullifyObjectProto(Object.getOwnPropertyDescriptor(o, property));
-                    if (descriptor) {
-                        if (accessorPropertiesOnly && !("get" in descriptor || "set" in descriptor))
-                            continue;
-                        if ("get" in descriptor && "set" in descriptor && name != "__proto__" && InjectedScriptHost.formatAccessorsAsProperties(object, descriptor.get) && !doesAttributeHaveObservableSideEffectOnGet(object, name)) {
-                            descriptor.value = object[property];
-                            descriptor.isOwn = true;
-                            delete descriptor.get;
-                            delete descriptor.set;
-                        }
-                    } else {
-                        // Not all bindings provide proper descriptors. Fall back to the writable, configurable property.
-                        if (accessorPropertiesOnly)
-                            continue;
-                        try {
-                            descriptor = { name: name, value: o[property], writable: false, configurable: false, enumerable: false, __proto__: null };
-                            if (o === object)
-                                descriptor.isOwn = true;
-                            yield descriptor;
-                        } catch (e) {
-                            // Silent catch.
-                        }
+                    descriptor = Object.getOwnPropertyDescriptor(o, property);
+                    var isAccessorProperty = descriptor && ("get" in descriptor || "set" in descriptor);
+                    if (accessorPropertiesOnly && !isAccessorProperty)
                         continue;
+                    if (descriptor && "get" in descriptor && "set" in descriptor && name !== "__proto__" &&
+                            InjectedScriptHost.formatAccessorsAsProperties(object, descriptor.get) &&
+                            !doesAttributeHaveObservableSideEffectOnGet(object, name)) {
+                        descriptor.value = object[property];
+                        descriptor.isOwn = true;
+                        delete descriptor.get;
+                        delete descriptor.set;
                     }
                 } catch (e) {
-                    if (accessorPropertiesOnly)
+                    if (accessorPropertiesOnly || forPreview)
                         continue;
-                    var descriptor = { __proto__: null };
-                    descriptor.value = e;
-                    descriptor.wasThrown = true;
+                    descriptor = { value: e, wasThrown: true };
+                }
+
+                // Not all bindings provide proper descriptors. Fall back to the non-configurable, non-enumerable,
+                // non-writable property.
+                if (!descriptor) {
+                    try {
+                        descriptor = { value: o[property], writable: false };
+                    } catch (e) {
+                        // Silent catch.
+                        continue;
+                    }
                 }
 
                 descriptor.name = name;
@@ -438,66 +452,81 @@ InjectedScript.prototype = {
                     descriptor.isOwn = true;
                 if (isSymbol(property))
                     descriptor.symbol = property;
-                yield descriptor;
+                if (forPreview) {
+                    // Ignore __proto__ property.
+                    if (name === "__proto__")
+                        continue;
+
+                    // Ignore length property of array.
+                    if ((subtype === "array" || subtype === "typedarray") && name === "length")
+                        continue;
+
+                    // Ignore size property of map, set.
+                    if ((subtype === "map" || subtype === "set") && name === "size")
+                        continue;
+
+                    // Never preview prototype properties.
+                    if (!descriptor.isOwn)
+                        continue;
+
+                    // Ignore computed properties unless they have getters.
+                    if (!("value" in descriptor) && !descriptor.get)
+                        continue;
+                }
+                customPush(descriptors, nullifyObjectProto(descriptor));

[kozy, 2017/02/23 23:27:15]

let's use regular push and pass callback to function:
bool propertyAdded(descriptor, moreAvailable:bool).
To calculate morePropertiesAavailable we can replace current for loop with while
loop and try to adjust index to next property after actual push and before
propertyAdded call.

[luoe, 2017/02/24 00:46:01]

After our offline discussion, I've removed isSpaceAvailable(), renamed the
callback to addPropertyIfNeeded(), which returns false if is no longer accepting
properties.  Also process() can return a boolean to let us break early when it
knows.

             }
         }
 
         if (propertyNamesOnly) {
             for (var i = 0; i < propertyNamesOnly.length; ++i) {
                 var name = propertyNamesOnly[i];
-                for (var o = object; this._isDefined(o); o = this._objectPrototype(o)) {
+                for (var o = object; this._isDefined(o); o = this._objectPrototype(/** @type {!Object} */ (o))) {
+                    o = /** @type {!Object} */ (o);
                     if (InjectedScriptHost.objectHasOwnProperty(o, name)) {
-                        for (var descriptor of process(o, [name]))
-                            yield descriptor;
+                        process(o, [name]);
+                        if (!isSpaceAvailable())
+                            return descriptors;
                         break;
                     }
-                    if (ownProperties)
+                    if (ownProperties || forPreview)
                         break;
                 }
             }
-            return;
-        }
-
-        /**
-         * @param {number} length
-         */
-        function* arrayIndexNames(length)
-        {
-            for (var i = 0; i < length; ++i)
-                yield "" + i;
-        }
-
-        var skipGetOwnPropertyNames;
-        try {
-            skipGetOwnPropertyNames = InjectedScriptHost.subtype(object) === "typedarray" && object.length > 500000;
-        } catch (e) {
+            return descriptors;
         }
 
-        for (var o = object; this._isDefined(o); o = this._objectPrototype(o)) {
+        for (var o = object; this._isDefined(o); o = this._objectPrototype(/** @type {!Object} */ (o))) {
+            o = /** @type {!Object} */ (o);
             if (InjectedScriptHost.subtype(o) === "proxy")
                 continue;
-            if (skipGetOwnPropertyNames && o === object) {
-                // Avoid OOM crashes from getting all own property names of a large TypedArray.
-                for (var descriptor of process(o, arrayIndexNames(o.length)))
-                    yield descriptor;
-            } else {
-                // First call Object.keys() to enforce ordering of the property descriptors.
-                for (var descriptor of process(o, Object.keys(/** @type {!Object} */ (o))))
-                    yield descriptor;
-                for (var descriptor of process(o, Object.getOwnPropertyNames(/** @type {!Object} */ (o))))
-                    yield descriptor;
-            }
-            if (Object.getOwnPropertySymbols) {
-                for (var descriptor of process(o, Object.getOwnPropertySymbols(/** @type {!Object} */ (o))))
-                    yield descriptor;
+
+            try {
+                var objectLength = o.length;
+                var skipGetOwnPropertyNames = subtype === "typedarray" && objectLength > 500000;
+                if (skipGetOwnPropertyNames && o === object) {
+                    process(o, undefined, objectLength);
+                } else {
+                    // First call Object.keys() to enforce ordering of the property descriptors.
+                    process(o, Object.keys(o));
+                    if (isSpaceAvailable())
+                        process(o, Object.getOwnPropertyNames(o));
+                }
+                if (isSpaceAvailable() && Object.getOwnPropertySymbols)
+                    process(o, Object.getOwnPropertySymbols(o));
+                if (ownProperties) {
+                    var proto = this._objectPrototype(o);
+                    if (isSpaceAvailable() && proto && !accessorPropertiesOnly) {
+                        var descriptor = { name: "__proto__", value: proto, writable: true, configurable: true, enumerable: false, isOwn: true, __proto__: null };
+                        customPush(descriptors, descriptor);
+                    }
+                }
+            } catch (e) {
             }
-            if (ownProperties) {
-                var proto = this._objectPrototype(o);
-                if (proto && !accessorPropertiesOnly)
-                    yield { name: "__proto__", value: proto, writable: true, configurable: true, enumerable: false, isOwn: true, __proto__: null };
+
+            if (ownProperties || forPreview)
                 break;
-            }
         }
+        return descriptors;
     },
 
     /**
@@ -863,7 +892,7 @@ InjectedScript.RemoteObject.prototype = {
     {
         var preview = this._createEmptyPreview();
         var firstLevelKeysCount = firstLevelKeys ? firstLevelKeys.length : 0;
-
+        var overflow = false;
         var propertiesThreshold = {
             properties: isTable ? 1000 : max(5, firstLevelKeysCount),
             indexes: isTable ? 1000 : max(100, firstLevelKeysCount),
@@ -871,11 +900,7 @@ InjectedScript.RemoteObject.prototype = {
         };
 
         try {
-            var descriptors = injectedScript._propertyDescriptors(object, undefined, undefined, firstLevelKeys);
-
-            this._appendPropertyDescriptors(preview, descriptors, propertiesThreshold, secondLevelKeys, isTable);
-            if (propertiesThreshold.indexes < 0 || propertiesThreshold.properties < 0)
-                return preview;
+            var descriptors = injectedScript._propertyDescriptors(object, undefined, undefined, firstLevelKeys, true, isSpaceAvailable, customPush);
 
             // Add internal properties to preview.
             var rawInternalProperties = InjectedScriptHost.getInternalProperties(object) || [];
@@ -886,64 +911,59 @@ InjectedScript.RemoteObject.prototype = {
                     entries = /** @type {!Array<*>} */(rawInternalProperties[i + 1]);
                     continue;
                 }
-                push(internalProperties, {
+                var internalPropertyDescriptor = {
                     name: rawInternalProperties[i],
                     value: rawInternalProperties[i + 1],
                     isOwn: true,
                     enumerable: true,
                     __proto__: null
-                });
+                };
+                customPush(descriptors, internalPropertyDescriptor);
             }
-            this._appendPropertyDescriptors(preview, internalProperties, propertiesThreshold, secondLevelKeys, isTable);
+            this._appendPropertyPreviewDescriptors(preview, descriptors, secondLevelKeys, isTable);
 
             if (this.subtype === "map" || this.subtype === "set" || this.subtype === "iterator")
                 this._appendEntriesPreview(entries, preview, skipEntriesPreview);
 
         } catch (e) {}
+        preview.overflow = preview.overflow || overflow;
 
         return preview;
+
+        /**
+         * @return {boolean}
+         */
+        function isSpaceAvailable() {
+            return propertiesThreshold.indexes >= 0 && propertiesThreshold.properties >= 0;
+        }
+
+        /**
+         * @param {!Array<!Object>} descriptors
+         * @param {!Object} descriptor
+         */
+        function customPush(descriptors, descriptor) {
+            if (toString(descriptor.name >>> 0) === descriptor.name)
+                propertiesThreshold.indexes--;
+            else
+                propertiesThreshold.properties--;
+            if (!isSpaceAvailable())
+                overflow = true;
+            else
+                push(descriptors, descriptor);
+        }
     },
 
     /**
      * @param {!RuntimeAgent.ObjectPreview} preview
      * @param {!Array.<*>|!Iterable.<*>} descriptors
-     * @param {!Object} propertiesThreshold
      * @param {?Array.<string>=} secondLevelKeys
      * @param {boolean=} isTable
      */
-    _appendPropertyDescriptors: function(preview, descriptors, propertiesThreshold, secondLevelKeys, isTable)
+    _appendPropertyPreviewDescriptors: function(preview, descriptors, secondLevelKeys, isTable)
     {
-        for (var descriptor of descriptors) {
-            if (propertiesThreshold.indexes < 0 || propertiesThreshold.properties < 0)
-                break;
-            if (!descriptor || descriptor.wasThrown)
-                continue;
-
+        for (var i = 0; i < descriptors.length; ++i) {
+            var descriptor = descriptors[i];
             var name = descriptor.name;
-
-            // Ignore __proto__ property.
-            if (name === "__proto__")
-                continue;
-
-            // Ignore length property of array.
-            if ((this.subtype === "array" || this.subtype === "typedarray") && name === "length")
-                continue;
-
-            // Ignore size property of map, set.
-            if ((this.subtype === "map" || this.subtype === "set") && name === "size")
-                continue;
-
-            // Never preview prototype properties.
-            if (!descriptor.isOwn)
-                continue;
-
-            // Ignore computed properties unless they have getters.
-            if (!("value" in descriptor)) {
-                if (descriptor.get)
-                    this._appendPropertyPreview(preview, { name: name, type: "accessor", __proto__: null }, propertiesThreshold);
-                continue;
-            }
-
             var value = descriptor.value;
             var type = typeof value;
 
@@ -951,9 +971,15 @@ InjectedScript.RemoteObject.prototype = {
             if (type === "undefined" && injectedScript._isHTMLAllCollection(value))
                 type = "object";
 
+            // Ignore computed properties unless they have getters.
+            if (descriptor.get && !("value" in descriptor)) {
+                push(preview.properties, { name: name, type: "accessor", __proto__: null });
+                continue;
+            }
+
             // Render own properties.
             if (value === null) {
-                this._appendPropertyPreview(preview, { name: name, type: "object", subtype: "null", value: "null", __proto__: null }, propertiesThreshold);
+                push(preview.properties, { name: name, type: "object", subtype: "null", value: "null", __proto__: null });
                 continue;
             }
 
@@ -961,7 +987,7 @@ InjectedScript.RemoteObject.prototype = {
             if (InjectedScript.primitiveTypes[type]) {
                 if (type === "string" && value.length > maxLength)
                     value = this._abbreviateString(value, maxLength, true);
-                this._appendPropertyPreview(preview, { name: name, type: type, value: toStringDescription(value), __proto__: null }, propertiesThreshold);
+                push(preview.properties, { name: name, type: type, value: toStringDescription(value), __proto__: null });
                 continue;
             }
 
@@ -981,24 +1007,6 @@ InjectedScript.RemoteObject.prototype = {
                     description = this._abbreviateString(/** @type {string} */ (injectedScript._describe(value)), maxLength, subtype === "regexp");
                 property.value = description;
             }
-            this._appendPropertyPreview(preview, property, propertiesThreshold);
-        }
-    },
-
-    /**
-     * @param {!RuntimeAgent.ObjectPreview} preview
-     * @param {!Object} property
-     * @param {!Object} propertiesThreshold
-     */
-    _appendPropertyPreview: function(preview, property, propertiesThreshold)
-    {
-        if (toString(property.name >>> 0) === property.name)
-            propertiesThreshold.indexes--;
-        else
-            propertiesThreshold.properties--;
-        if (propertiesThreshold.indexes < 0 || propertiesThreshold.properties < 0) {
-            preview.overflow = true;
-        } else {
             push(preview.properties, property);
         }
     },