[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 d702e1a41795fc71b95d29325e5bdaf175f1cede..858d5c2944777313434fe071b09abd20de8f8234 100644
--- a/src/inspector/injected-script-source.js
+++ b/src/inspector/injected-script-source.js
@@ -214,6 +214,47 @@ InjectedScript.closureTypes["global"] = "Global";
 InjectedScript.closureTypes["eval"] = "Eval";
 InjectedScript.closureTypes["module"] = "Module";
 
+/**
+ * @interface
+ */
+InjectedScript.AdderWithThreshold = function () {};
+InjectedScript.AdderWithThreshold.prototype = {
+    /**
+     * @param {!Array<!Object>} descriptors
+     * @param {!Object} descriptor
+     */
+    add: function (descriptors, descriptor) {},
+
+    /**
+     * @return {boolean}
+     */
+    isSpaceAvailable: function () {}
+};
+
+/**
+ * @implements {InjectedScript.AdderWithThreshold}
+ * @constructor
+ */
+InjectedScript.defaultPropertyAdder = function () {};
+InjectedScript.defaultPropertyAdder.prototype = {

[kozy, 2017/02/21 19:01:55]

We try to avoid using any complex JS construction in injected script to make it
more reliable. Could we avoid prototype inheritance and new call? We can pass
isSpaceAvailable as a function to this._propertyDescriptors instead.

[luoe, 2017/02/21 21:53:51]

Yes, I think a customPush() will also need to be passed into
_propertyDescriptors() so that we can remember whether an overflow occurred.

+    /**
+     * @override
+     * @param {!Array<!Object>} descriptors
+     * @param {!Object} descriptor
+     */
+    add: function (descriptors, descriptor) {
+        push(descriptors, descriptor);
+    },
+
+    /**
+     * @override
+     * @return {boolean}
+     */
+    isSpaceAvailable: function () {
+        return true;
+    }
+};
+
 InjectedScript.prototype = {
     /**
      * @param {*} object
@@ -335,10 +376,11 @@ 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) {
+        var adder = new InjectedScript.defaultPropertyAdder();
+        var descriptors = this._propertyDescriptors(object, ownProperties, accessorPropertiesOnly, undefined, adder, 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)
@@ -351,7 +393,6 @@ InjectedScript.prototype = {
                 descriptor.enumerable = false;
             if ("symbol" in descriptor)
                 descriptor.symbol = this._wrapObject(descriptor.symbol, objectGroupName);
-            push(descriptors, descriptor);
         }
         return descriptors;
     },
@@ -375,24 +416,34 @@ InjectedScript.prototype = {
      * @param {!Object} object
      * @param {boolean=} ownProperties
      * @param {boolean=} accessorPropertiesOnly
-     * @param {?Array.<string>=} propertyNamesOnly
+     * @param {?Array<string>=} propertyNamesOnly
+     * @param {!InjectedScript.AdderWithThreshold=} adder
+     * @param {boolean=} forPreview
+     * @return {!Array}
      */
-    _propertyDescriptors: function*(object, ownProperties, accessorPropertiesOnly, propertyNamesOnly)
+    _propertyDescriptors: function(object, ownProperties, accessorPropertiesOnly, propertyNamesOnly, adder, forPreview)
     {
+        var descriptors = [];

[kozy, 2017/02/21 19:01:55]

descriptors.__proto__ = null;
and then we will be sure that we don't use Array.prototype.push but use our cool
push function().

[luoe, 2017/02/21 21:53:51]

Done.

+        if (!adder.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 {!Iterable<string|symbol|number>|!Array<string|number|symbol>=} properties
          */
-        function* process(o, properties)
+        function process(o, properties)
         {
-            for (var property of properties) {
+            // When properties is not provided, iterate over the object's indices.
+            var length = properties ? properties.length : (o.length || 0);

[kozy, 2017/02/21 19:01:55]

o.length can throw.

[luoe, 2017/02/21 21:53:51]

I'll pass in objectLength

+            for (var i = 0; i < length; ++i) {
+                if (!adder.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));
@@ -417,7 +468,7 @@ InjectedScript.prototype = {
                         delete descriptor.set;
                     }
                 } catch (e) {
-                    if (accessorPropertiesOnly)
+                    if (accessorPropertiesOnly || forPreview)
                         continue;
                     descriptor = { value: e, wasThrown: true };
                 }
@@ -438,33 +489,47 @@ InjectedScript.prototype = {
                     descriptor.isOwn = true;
                 if (isSymbol(property))
                     descriptor.symbol = property;
-                yield nullifyObjectProto(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;
+                }
+                adder.add(descriptors, nullifyObjectProto(descriptor));
             }
         }
 
         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(/** @type {!Object} */ (o), [name]))
-                            yield descriptor;
+                        process(o, [name]);
+                        if (!adder.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;
+            return descriptors;
         }
 
         var skipGetOwnPropertyNames;
@@ -473,32 +538,31 @@ InjectedScript.prototype = {
         } catch (e) {
         }
 
-        for (var o = object; this._isDefined(o); o = this._objectPrototype(o)) {
-            /** @type {!Object} */ (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;
+                process(o, undefined);
             } else {
                 // First call Object.keys() to enforce ordering of the property descriptors.
-                for (var descriptor of process(o, Object.keys(o)))
-                    yield descriptor;
-                for (var descriptor of process(o, Object.getOwnPropertyNames(o)))
-                    yield descriptor;
-            }
-            if (Object.getOwnPropertySymbols) {
-                for (var descriptor of process(o, Object.getOwnPropertySymbols(o)))
-                    yield descriptor;
+                process(o, Object.keys(o));

[kozy, 2017/02/21 19:01:55]

All Object.* can throw, can we wrap it with try catch?

[luoe, 2017/02/21 21:53:51]

Done.

+                if (adder.isSpaceAvailable())
+                    process(o, Object.getOwnPropertyNames(o));
             }
+            if (adder.isSpaceAvailable() && Object.getOwnPropertySymbols)
+                process(o, Object.getOwnPropertySymbols(o));
             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 };
-                break;
+                if (adder.isSpaceAvailable() && proto && !accessorPropertiesOnly) {
+                    var descriptor = { name: "__proto__", value: proto, writable: true, configurable: true, enumerable: false, isOwn: true, __proto__: null };
+                    adder.add(descriptors, descriptor);
+                }
             }
+            if (ownProperties || forPreview)
+                break;
         }
+        return descriptors;
     },
 
     /**
@@ -777,6 +841,54 @@ InjectedScript.RemoteObject = function(object, objectGroupName, doNotBind, force
     }
 }
 
+/**
+ * @implements {InjectedScript.AdderWithThreshold}
+ * @constructor
+ * @param {boolean} isTable
+ * @param {number} firstLevelKeysCount
+ */
+InjectedScript.PreviewPropertyAdder = function (isTable, firstLevelKeysCount) {
+    this._propertiesThreshold = {
+        properties: isTable ? 1000 : max(5, firstLevelKeysCount),
+        indexes: isTable ? 1000 : max(100, firstLevelKeysCount),
+        __proto__: null
+    };
+    this._overflow = false;
+};
+
+InjectedScript.PreviewPropertyAdder.prototype = {
+    /**
+     * @override
+     * @param {!Array<!Object>} descriptors
+     * @param {!Object} descriptor
+     */
+    add: function (descriptors, descriptor) {
+        if (toString(descriptor.name >>> 0) === descriptor.name)
+            this._propertiesThreshold.indexes--;
+        else
+            this._propertiesThreshold.properties--;
+        if (!this.isSpaceAvailable())
+            this._overflow = true;
+        else
+            push(descriptors, descriptor);
+    },
+
+    /**
+     * @override
+     * @return {boolean}
+     */
+    isSpaceAvailable: function () {
+        return this._propertiesThreshold.indexes >= 0 && this._propertiesThreshold.properties >= 0;
+    },
+
+    /**
+     * @return {boolean}
+     */
+    overflows: function() {
+        return this._overflow;
+    }
+};
+
 InjectedScript.RemoteObject.prototype = {
 
     /**
@@ -864,19 +976,9 @@ InjectedScript.RemoteObject.prototype = {
     {
         var preview = this._createEmptyPreview();
         var firstLevelKeysCount = firstLevelKeys ? firstLevelKeys.length : 0;
-
-        var propertiesThreshold = {
-            properties: isTable ? 1000 : max(5, firstLevelKeysCount),
-            indexes: isTable ? 1000 : max(100, firstLevelKeysCount),
-            __proto__: null
-        };
-
+        var adder = new InjectedScript.PreviewPropertyAdder(!!isTable, firstLevelKeysCount);
         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, adder, true);
 
             // Add internal properties to preview.
             var rawInternalProperties = InjectedScriptHost.getInternalProperties(object) || [];
@@ -887,20 +989,22 @@ 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
-                });
+                };
+                adder.add(descriptors, internalPropertyDescriptor);

[kozy, 2017/02/21 19:01:55]

Our push safer.

[luoe, 2017/02/21 21:53:51]

Replaced with a customPush() that calls push() internally.

             }
-            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 || adder.overflows();
 
         return preview;
     },
@@ -908,43 +1012,14 @@ InjectedScript.RemoteObject.prototype = {
     /**
      * @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;
 
@@ -952,9 +1027,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;
             }
 
@@ -962,7 +1043,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;
             }
 
@@ -982,24 +1063,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);
         }
     },