Bug 1301794 - Allow sparse pseudo-arrays in console r=fitzgen
authorOriol <oriol-bugzilla@hotmail.com>
Mon, 12 Sep 2016 13:03:50 -0700
changeset 339294 35e1982426b2fa6b6cbd7320913497afb4701603
parent 339293 7826ad7df05fb73b79f808885ed4ddea9210da55
child 339295 395b05f7d02e3381bc891717e85c33a150ae0e40
push id10033
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:50:26 +0000
treeherdermozilla-aurora@5dddbefdf759 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1301794
milestone51.0a1
Bug 1301794 - Allow sparse pseudo-arrays in console r=fitzgen
devtools/client/webconsole/test/browser_webconsole_output_05.js
devtools/client/webconsole/test/browser_webconsole_output_06.js
devtools/server/actors/object.js
--- a/devtools/client/webconsole/test/browser_webconsole_output_05.js
+++ b/devtools/client/webconsole/test/browser_webconsole_output_05.js
@@ -140,20 +140,21 @@ var inputTests = [
     variablesViewLabel: "Promise"
   },
 
   // 14
   {
     input: "new Object({1: 'this\\nis\\nsupposed\\nto\\nbe\\na\\nvery" +
            "\\nlong\\nstring\\n,shown\\non\\na\\nsingle\\nline', " +
            "2: 'a shorter string', 3: 100})",
-    output: 'Object { 1: "this is supposed to be a very long ' + ELLIPSIS +
-            '", 2: "a shorter string", 3: 100 }',
+    output: '[ <1 empty slot>, "this is supposed to be a very long ' + ELLIPSIS +
+            '", "a shorter string", 100 ]',
     printOutput: "[object Object]",
-    inspectable: false,
+    inspectable: true,
+    variablesViewLabel: "Object[4]"
   },
 
   // 15
   {
     input: "new Proxy({a:1},[1,2,3])",
     output: 'Proxy { <target>: Object, <handler>: Array[3] }',
     printOutput: "[object Object]",
     inspectable: true,
--- a/devtools/client/webconsole/test/browser_webconsole_output_06.js
+++ b/devtools/client/webconsole/test/browser_webconsole_output_06.js
@@ -126,20 +126,20 @@ var inputTests = [
     printOutput: "[object Object]",
     inspectable: true,
     variablesViewLabel: "Object[2]",
   },
 
   // 14
   {
     input: '({0: "a", 42: "b"})',
-    output: 'Object { 0: "a", 42: "b" }',
+    output: '[ "a", <9 empty slots>, 33 more\u2026 ]',
     printOutput: "[object Object]",
     inspectable: true,
-    variablesViewLabel: "Object",
+    variablesViewLabel: "Object[43]",
   },
 
   // 15
   {
     input: '({0: "a", 1: "b", 2: "c", 3: "d", 4: "e", 5: "f", 6: "g", ' +
            '7: "h", 8: "i", 9: "j", 10: "k", 11: "l"})',
     output: 'Object [ "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", ' +
             "2 more\u2026 ]",
@@ -184,20 +184,20 @@ var inputTests = [
     printOutput: "[object Object]",
     inspectable: true,
     variablesViewLabel: "Object[0]",
   },
 
   // 20
   {
     input: '({length: 1})',
-    output: 'Object { length: 1 }',
+    output: '[ <1 empty slot> ]',
     printOutput: "[object Object]",
     inspectable: true,
-    variablesViewLabel: "Object",
+    variablesViewLabel: "Object[1]",
   },
 
   // 21
   {
     input: '({0: "a", 1: "b", length: 1})',
     output: 'Object { 1: "b", length: 1, 1 more\u2026 }',
     printOutput: "[object Object]",
     inspectable: true,
@@ -211,38 +211,38 @@ var inputTests = [
     printOutput: "[object Object]",
     inspectable: true,
     variablesViewLabel: "Object[2]",
   },
 
   // 23
   {
     input: '({0: "a", 1: "b", length: 3})',
-    output: 'Object { length: 3, 2 more\u2026 }',
+    output: '[ "a", "b", <1 empty slot> ]',
     printOutput: "[object Object]",
     inspectable: true,
-    variablesViewLabel: "Object",
+    variablesViewLabel: "Object[3]",
   },
 
   // 24
   {
     input: '({0: "a", 2: "b", length: 2})',
     output: 'Object { 2: "b", length: 2, 1 more\u2026 }',
     printOutput: "[object Object]",
     inspectable: true,
     variablesViewLabel: "Object",
   },
 
   // 25
   {
     input: '({0: "a", 2: "b", length: 3})',
-    output: 'Object { length: 3, 2 more\u2026 }',
+    output: '[ "a", <1 empty slot>, "b" ]',
     printOutput: "[object Object]",
     inspectable: true,
-    variablesViewLabel: "Object",
+    variablesViewLabel: "Object[3]",
   },
 
   // 26
   {
     input: '({0: "a", b: "b", length: 1})',
     output: 'Object { b: "b", length: 1, 1 more\u2026 }',
     printOutput: "[object Object]",
     inspectable: true,
@@ -252,16 +252,25 @@ var inputTests = [
   // 27
   {
     input: '({0: "a", b: "b", length: 2})',
     output: 'Object { b: "b", length: 2, 1 more\u2026 }',
     printOutput: "[object Object]",
     inspectable: true,
     variablesViewLabel: "Object",
   },
+
+  // 28
+  {
+    input: '({42: "a"})',
+    output: 'Object { 42: "a" }',
+    printOutput: "[object Object]",
+    inspectable: true,
+    variablesViewLabel: "Object",
+  },
 ];
 
 function test() {
   requestLongerTimeout(2);
   Task.spawn(function* () {
     let {tab} = yield loadTab(TEST_URI);
     let hud = yield openConsole(tab);
     return checkOutputForInputs(hud, inputTests);
--- a/devtools/server/actors/object.js
+++ b/devtools/server/actors/object.js
@@ -1788,65 +1788,76 @@ DebuggerServer.ObjectActorPreviewers.Obj
       lineNumber: hooks.createValueGrip(rawObj.lineNumber),
       columnNumber: hooks.createValueGrip(rawObj.columnNumber),
     };
 
     return true;
   },
 
   function PseudoArray({obj, hooks}, grip, rawObj) {
-    let length = 0;
+    let length;
 
     let keys = obj.getOwnPropertyNames();
     if (keys.length == 0) {
       return false;
     }
 
-    // Pseudo-arrays should only have array indices and, optionally, a "length" property.
-    // Since array indices are sorted first, check if the last property is "length".
-    if(keys[keys.length-1] === "length") {
-      keys.pop();
-      // The value of "length" should equal the number of other properties. If eventually
-      // we allow sparse pseudo-arrays, we should check whether it's a Uint32 instead.
-      if(rawObj.length !== keys.length) {
-        return false;
-      }
+    // If no item is going to be displayed in preview, better display as sparse object.
+    // The first key should contain the smallest integer index (if any).
+    if(keys[0] >= OBJECT_PREVIEW_MAX_ITEMS) {
+      return false;
     }
 
-    // Ensure that the keys are consecutive integers starting at "0". If eventually we
-    // allow sparse pseudo-arrays, we should check that they are array indices, that is:
-    // `(key >>> 0) + '' === key && key !== "4294967295"`.
-    // Checking the last property first allows us to avoid useless iterations when
-    // there is any property which is not an array index.
-    if(keys.length && keys[keys.length-1] !== keys.length - 1 + '') {
+    // Pseudo-arrays should only have array indices and, optionally, a "length" property.
+    // Since integer indices are sorted first, check if the last property is "length".
+    if(keys[keys.length-1] === "length") {
+      keys.pop();
+      length = DevToolsUtils.getProperty(obj, "length");
+    } else {
+      // Otherwise, let length be the (presumably) greatest array index plus 1.
+      length = +keys[keys.length-1] + 1;
+    }
+    // Check if length is a valid array length, i.e. is a Uint32 number.
+    if(typeof length !== "number" || length >>> 0 !== length) {
       return false;
     }
-    for (let key of keys) {
-      if (key !== (length++) + '') {
+
+    // Ensure all keys are increasing array indices smaller than length. The order is not
+    // guaranteed for exotic objects but, in most cases, big array indices and properties
+    // which are not integer indices should be at the end. Then, iterating backwards
+    // allows us to return earlier when the object is not completely a pseudo-array.
+    let prev = length;
+    for(let i = keys.length - 1; i >= 0; --i) {
+      let key = keys[i];
+      let numKey = key >>> 0; // ToUint32(key)
+      if (numKey + '' !== key || numKey >= prev) {
         return false;
       }
+      prev = numKey;
     }
 
     grip.preview = {
       kind: "ArrayLike",
       length: length,
     };
 
     // Avoid recursive object grips.
     if (hooks.getGripDepth() > 1) {
       return true;
     }
 
     let items = grip.preview.items = [];
+    let numItems = Math.min(OBJECT_PREVIEW_MAX_ITEMS, length);
 
-    let i = 0;
-    for (let key of keys) {
-      if (rawObj.hasOwnProperty(key) && i++ < OBJECT_PREVIEW_MAX_ITEMS) {
-        let value = makeDebuggeeValueIfNeeded(obj, rawObj[key]);
-        items.push(hooks.createValueGrip(value));
+    for (let i = 0; i < numItems; ++i) {
+      let desc = obj.getOwnPropertyDescriptor(i);
+      if (desc && 'value' in desc) {
+        items.push(hooks.createValueGrip(desc.value));
+      } else {
+        items.push(null);
       }
     }
 
     return true;
   },
 
   GenericObject,
 ];