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

src/inspector/injected-script-source.js

 /*
  * Copyright (C) 2007 Apple Inc.  All rights reserved.
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 196 matching lines @@
 InjectedScript.closureTypes["local"] = "Local";
 InjectedScript.closureTypes["closure"] = "Closure";
 InjectedScript.closureTypes["catch"] = "Catch";
 InjectedScript.closureTypes["block"] = "Block";
 InjectedScript.closureTypes["script"] = "Script";
 InjectedScript.closureTypes["with"] = "With Block";
 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
      * @return {boolean}
      */
     isPrimitiveValue: function(object)
     {
         // FIXME(33716): typeof document.all is always 'undefined'.
         return InjectedScript.primitiveTypes[typeof object] && !this._isHTMLAllCollection(object);
     },
@@ skipping 101 matching lines @@
     getProperties: function(object, objectGroupName, ownProperties, accessorPropertiesOnly, generatePreview)
     {
         var subtype = this._subtype(object);
         if (subtype === "internal#scope") {
             // Internally, scope contains object with scope variables and additional information like type,
             // we use additional information for preview and would like to report variables as scope
             // properties.
             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)
                 descriptor.set = this._wrapObject(descriptor.set, objectGroupName);
             if ("value" in descriptor)
                 descriptor.value = this._wrapObject(descriptor.value, objectGroupName, false, generatePreview);
             if (!("configurable" in descriptor))
                 descriptor.configurable = false;
             if (!("enumerable" in descriptor))
                 descriptor.enumerable = false;
             if ("symbol" in descriptor)
                 descriptor.symbol = this._wrapObject(descriptor.symbol, objectGroupName);
-            push(descriptors, descriptor);
         }
         return descriptors;
     },
 
     /**
      * @param {!Object} object
      * @return {?Object}
      */
     _objectPrototype: function(object)
     {
         if (InjectedScriptHost.subtype(object) === "proxy")
             return null;
         try {
             return Object.getPrototypeOf(object);
         } catch (e) {
             return null;
         }
     },
 
     /**
      * @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));
                 else
                     name = typeof property === "number" ? ("" + property) : /** @type {string} */(property);
 
                 if (subtype === "internal#scopeList" && name === "length")
                     continue;
 
                 var descriptor;
                 try {
                     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;
                     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;
                 if (o === object)
                     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;
         try {
             skipGetOwnPropertyNames = subtype === "typedarray" && object.length > 500000;
         } 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;
+                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 (Object.getOwnPropertySymbols) {
-                for (var descriptor of process(o, Object.getOwnPropertySymbols(o)))
-                    yield descriptor;
-            }
+            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 };
+                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;
     },
 
     /**
      * @param {string|undefined} objectGroupName
      * @param {*} jsonMLObject
      * @throws {string} error message
      */
     _substituteObjectTagsInCustomPreview: function(objectGroupName, jsonMLObject)
     {
         var maxCustomPreviewRecursionDepth = 20;
@@ skipping 258 matching lines @@
             this.preview = this._generatePreview(object, undefined, columnNames, isTable, skipEntriesPreview);
     }
 
     if (injectedScript._customObjectFormatterEnabled) {
         var customPreview = this._customPreview(object, objectGroupName, customObjectConfig);
         if (customPreview)
             this.customPreview = customPreview;
     }
 }
 
+/**
+ * @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 = {
 
     /**
      * @param {*} object
      * @param {string=} objectGroupName
      * @param {*=} customObjectConfig
      * @return {?RuntimeAgent.CustomPreview}
      */
     _customPreview: function(object, objectGroupName, customObjectConfig)
     {
@@ skipping 67 matching lines @@
      * @param {?Array.<string>=} firstLevelKeys
      * @param {?Array.<string>=} secondLevelKeys
      * @param {boolean=} isTable
      * @param {boolean=} skipEntriesPreview
      * @return {!RuntimeAgent.ObjectPreview} preview
      */
     _generatePreview: function(object, firstLevelKeys, secondLevelKeys, isTable, skipEntriesPreview)
     {
         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) || [];
             var internalProperties = [];
             var entries = null;
             for (var i = 0; i < rawInternalProperties.length; i += 2) {
                 if (rawInternalProperties[i] === "[[Entries]]") {
                     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;
     },
 
     /**
      * @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;
 
             // Special-case HTMLAll.
             if (type === "undefined" && injectedScript._isHTMLAllCollection(value))
                 type = "object";
 
-            // Render own properties.
-            if (value === null) {
-                this._appendPropertyPreview(preview, { name: name, type: "object", subtype: "null", value: "null", __proto__: null }, propertiesThreshold);
+            // 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) {
+                push(preview.properties, { name: name, type: "object", subtype: "null", value: "null", __proto__: null });
+                continue;
+            }
+
             var maxLength = 100;
             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;
             }
 
             var property = { name: name, type: type, __proto__: null };
             var subtype = injectedScript._subtype(value);
             if (subtype)
                 property.subtype = subtype;
 
             if (secondLevelKeys === null || secondLevelKeys) {
                 var subPreview = this._generatePreview(value, secondLevelKeys || undefined, undefined, isTable);
                 property.valuePreview = subPreview;
                 if (subPreview.overflow)
                     preview.overflow = true;
             } else {
                 var description = "";
                 if (type !== "function")
                     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);
         }
     },
 
     /**
      * @param {?Array<*>} entries
      * @param {!RuntimeAgent.ObjectPreview} preview
      * @param {boolean=} skipEntriesPreview
      */
     _appendEntriesPreview: function(entries, preview, skipEntriesPreview)
@@ skipping 50 matching lines @@
             return string.substr(0, leftHalf) + "\u2026" + string.substr(string.length - rightHalf, rightHalf);
         }
         return string.substr(0, maxLength) + "\u2026";
     },
 
     __proto__: null
 }
 
 return injectedScript;
 })