[inspector] send internal properties first, remove unnecessary props

Currently, injected script source adds natural object properties before
internal properties. This can result in important ones such as
"[[PrimitiveValue]]" being left out. This CL
- makes sure internal properties are always added to preview
- removes unused "[[Iterator*]]" properties from preview
- boxed strings (e.g. new String("foo")) will not send unnecessary
properties 0:"f", 1:"o", 2:"o" if the [[PrimitiveValue]] is sent.

Bug: chromium:567265
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Icd5c7410351f371055277ce471226cc6fb5a861f
Reviewed-on: https://chromium-review.googlesource.com/634584Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Erik Luo <luoe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47622}

src/inspector/injected-script-source.js

@@ -879,19 +879,25 @@ InjectedScript.RemoteObject.prototype = {
             __proto__: null
         };
         var subtype = this.subtype;
+        var primitiveString;
 
         try {
-            var descriptors = injectedScript._propertyDescriptors(object, addPropertyIfNeeded, false /* ownProperties */, undefined /* accessorPropertiesOnly */, firstLevelKeys);
+            var descriptors = [];
+            InjectedScriptHost.nullifyPrototype(descriptors);
 
             // Add internal properties to preview.
             var rawInternalProperties = InjectedScriptHost.getInternalProperties(object) || [];
             var internalProperties = [];
+            InjectedScriptHost.nullifyPrototype(rawInternalProperties);
+            InjectedScriptHost.nullifyPrototype(internalProperties);
             var entries = null;
             for (var i = 0; i < rawInternalProperties.length; i += 2) {
                 if (rawInternalProperties[i] === "[[Entries]]") {
                     entries = /** @type {!Array<*>} */(rawInternalProperties[i + 1]);
                     continue;
                 }
+                if (rawInternalProperties[i] === "[[PrimitiveValue]]" && typeof rawInternalProperties[i + 1] === 'string')
+                    primitiveString = rawInternalProperties[i + 1];
                 var internalPropertyDescriptor = {
                     name: rawInternalProperties[i],
                     value: rawInternalProperties[i + 1],
@@ -899,9 +905,12 @@ InjectedScript.RemoteObject.prototype = {
                     enumerable: true,
                     __proto__: null
                 };
-                if (!addPropertyIfNeeded(descriptors, internalPropertyDescriptor))
-                    break;
+                push(descriptors, internalPropertyDescriptor);
             }
+            var naturalDescriptors = injectedScript._propertyDescriptors(object, addPropertyIfNeeded, false /* ownProperties */, undefined /* accessorPropertiesOnly */, firstLevelKeys);
+            for (var i = 0; i < naturalDescriptors.length; i++)
+                push(descriptors, naturalDescriptors[i]);
+
             this._appendPropertyPreviewDescriptors(preview, descriptors, secondLevelKeys, isTable);
 
             if (subtype === "map" || subtype === "set" || subtype === "weakmap" || subtype === "weakset" || subtype === "iterator")
@@ -940,6 +949,10 @@ InjectedScript.RemoteObject.prototype = {
             if (!("value" in descriptor) && !descriptor.get)
                 return true;
 
+            // Ignore index properties when there is a primitive string.
+            if (primitiveString && primitiveString[descriptor.name] === descriptor.value)
+                return true;
+
             if (toString(descriptor.name >>> 0) === descriptor.name)
                 propertiesThreshold.indexes--;
             else

src/inspector/v8-injected-script-host.cc

@@ -248,13 +248,9 @@ void V8InjectedScriptHost::getInternalPropertiesCallback(
     allowedProperties.insert(String16("[[PromiseValue]]"));
   } else if (info[0]->IsGeneratorObject()) {
     allowedProperties.insert(String16("[[GeneratorStatus]]"));
-  } else if (info[0]->IsMapIterator() || info[0]->IsSetIterator()) {
-    allowedProperties.insert(String16("[[IteratorHasMore]]"));
-    allowedProperties.insert(String16("[[IteratorIndex]]"));
-    allowedProperties.insert(String16("[[IteratorKind]]"));
-    allowedProperties.insert(String16("[[Entries]]"));
   } else if (info[0]->IsMap() || info[0]->IsWeakMap() || info[0]->IsSet() ||
-             info[0]->IsWeakSet()) {
+             info[0]->IsWeakSet() || info[0]->IsMapIterator() ||
+             info[0]->IsSetIterator()) {
     allowedProperties.insert(String16("[[Entries]]"));
   }
   if (!allowedProperties.size()) return;

test/inspector/debugger/object-preview-internal-properties-expected.txt

@@ -145,21 +145,6 @@ expression: new WeakSet([{}])
 
 Running test: iteratorObject
 expression: (new Map([[1,2]])).entries()
-{
-    name : [[IteratorHasMore]]
-    type : boolean
-    value : true
-}
-{
-    name : [[IteratorIndex]]
-    type : number
-    value : 0
-}
-{
-    name : [[IteratorKind]]
-    type : string
-    value : entries
-}
 [[Entries]]:
 [
     [0] : {
@@ -181,21 +166,6 @@ expression: (new Map([[1,2]])).entries()
 ]
 
 expression: (new Set([[1,2]])).entries()
-{
-    name : [[IteratorHasMore]]
-    type : boolean
-    value : true
-}
-{
-    name : [[IteratorIndex]]
-    type : number
-    value : 0
-}
-{
-    name : [[IteratorKind]]
-    type : string
-    value : entries
-}
 [[Entries]]:
 [
     [0] : {
@@ -237,8 +207,28 @@ Running test: noPreviewForFunctionObject
 
 Running test: otherObjects
 expression: [1,2,3]
+{
+    name : 0
+    type : number
+    value : 1
+}
+{
+    name : 1
+    type : number
+    value : 2
+}
+{
+    name : 2
+    type : number
+    value : 3
+}
 
 expression: /123/
+{
+    name : lastIndex
+    type : number
+    value : 0
+}
 
 expression: ({})
 

test/inspector/debugger/object-preview-internal-properties.js

@@ -95,10 +95,8 @@ function dumpInternalPropertiesAndEntries(message)
     InspectorTest.logMessage(message);
     return;
   }
-  for (var property of properties) {
-    if (property.name.startsWith("[["))
-      InspectorTest.logMessage(property);
-  }
+  for (var property of properties)
+    InspectorTest.logMessage(property);
   if (entries) {
     InspectorTest.log("[[Entries]]:");
     InspectorTest.logMessage(entries);