Fix default bucketing (firefox-devtools#2897)

* Fix default bucketing

* tidy up

* tweaks

Author: jasonLaster

assets/module-manifest.json

@@ -33,11 +33,11 @@ export type ObjectInspectorItemContentsValue = {
   type: string
 };
 
-type ObjectInspectorItemContents = {
+export type ObjectInspectorItemContents = {
   value: ObjectInspectorItemContentsValue
 };
 
-type ObjectInspectorItem = {
+export type ObjectInspectorItem = {
   contents: Array<ObjectInspectorItem> & ObjectInspectorItemContents,
   name: string,
   path: string

src/components/shared/ObjectInspector.js

@@ -10,6 +10,57 @@ import "../App.css";
 import "../SecondaryPanes/Frames/Frames.css";
 import "devtools-launchpad/src/lib/themes/dark-theme.css";
 
+function createArrayPreview(name) {
+  return {
+    enumerable: true,
+    writerable: true,
+    configurable: true,
+    value: {
+      type: "object",
+      actor: `server2.conn45.child1/${name}`,
+      class: "Object",
+      ownPropertyLength: 2,
+      preview: {
+        kind: "ArrayLike",
+        ownProperties: {},
+        ownPropertiesLength: 0,
+        length: 1
+      }
+    }
+  };
+}
+
+function createObjectPreview(name) {
+  return {
+    enumerable: true,
+    writerable: true,
+    configurable: true,
+    value: {
+      type: "Object",
+      actor: `server2.conn45.child1/${name}`,
+      class: "Object",
+      ownPropertyLength: 2,
+      preview: {
+        kind: "object",
+        ownProperties: {},
+        ownPropertiesLength: 0,
+        length: 1
+      }
+    }
+  };
+}
+
+function createObjectGrip(id) {
+  return {
+    actor: `server2.conn45.child1/${id}`,
+    type: "object",
+    class: "Object",
+    ownProperties: {},
+    ownSymbols: {},
+    safeGetters: {}
+  };
+}
+
 const obj = {
   actor: "server2.conn45.child1/pausedobj81",
   type: "object",
@@ -141,5 +192,32 @@ options.forEach(option => {
         },
         option
       );
+    })
+    .add(`Object with window keys ${optionLabel}`, () => {
+      let grip = createObjectGrip("foo");
+      grip.ownProperties.arr = createArrayPreview("arr");
+      grip.ownProperties.location = createObjectPreview("location");
+      return PreviewFactory(
+        {
+          value: grip,
+          expression: "this",
+          loadedObjects: I.Map().set(grip.actor, grip)
+        },
+        option
+      );
+    })
+    .add(`Window Preview ${optionLabel}`, () => {
+      let grip = createObjectGrip("foo");
+      grip.class = "Window";
+      grip.ownProperties.arr = createArrayPreview("arr");
+      grip.ownProperties.location = createObjectPreview("location");
+      return PreviewFactory(
+        {
+          value: grip,
+          expression: "this",
+          loadedObjects: I.Map().set(grip.actor, grip)
+        },
+        option
+      );
     });
 });

src/components/stories/Preview.js

@@ -41,11 +41,11 @@ async function expandingProperties(ctx) {
 
   toggleNode(dbg, 4);
   await waitForDispatch(dbg, "LOAD_OBJECT_PROPERTIES");
-  is(getLabel(dbg, 5), "prototype");
+  is(getLabel(dbg, 5), "length");
 
   await stepOver(dbg);
   is(getLabel(dbg, 4), "foo()");
-  is(getLabel(dbg, 5), "prototype");
+  is(getLabel(dbg, 5), "Window");
 }
 
 async function changingScopes(ctx) {

src/test/integration/tests/scopes.js

@@ -23,7 +23,7 @@ add_task(function*() {
 
   toggleNode(dbg, 4);
   yield waitForDispatch(dbg, "LOAD_OBJECT_PROPERTIES");
-  is(getLabel(dbg, 5), "prototype");
+  is(getLabel(dbg, 5), "length");
 
   yield stepOver(dbg);
   is(getLabel(dbg, 4), "foo()");

src/test/mochitest/browser_dbg-scopes.js

@@ -111,13 +111,7 @@ function makeDefaultPropsBucket(props, parentPath, ownProperties) {
   const userProps = props.filter(name => !isDefault({ name }));
   const defaultProps = props.filter(name => isDefault({ name }));
 
-  let nodes = userProps.map(name =>
-    createNode(
-      maybeEscapePropertyName(name),
-      `${parentPath}/${name}`,
-      ownProperties[name]
-    )
-  );
+  let nodes = makeNodesForOwnProps(userProps, parentPath, ownProperties);
 
   if (defaultProps.length > 0) {
     const defaultNodes = defaultProps.map((name, index) =>
@@ -128,24 +122,34 @@ function makeDefaultPropsBucket(props, parentPath, ownProperties) {
       )
     );
     nodes.push(
-      createNode("[default properties]", `${parentPath}/default`, defaultNodes)
+      createNode(
+        "[default properties]",
+        `${parentPath}/##-default`,
+        defaultNodes
+      )
     );
   }
   return nodes;
 }
 
-/*
+function makeNodesForOwnProps(properties, parentPath, ownProperties) {
+  return properties.map(name =>
+    createNode(
+      maybeEscapePropertyName(name),
+      `${parentPath}/${name}`,
+      ownProperties[name]
+    )
+  );
+}
 
+/*
  * Ignore non-concrete values like getters and setters
  * for now by making sure we have a value.
 */
-function makeNodesForProperties(
-  objProps,
-  parentPath,
-  { bucketSize = 100 } = {}
-) {
+function makeNodesForProperties(objProps, parent, { bucketSize = 100 } = {}) {
   const { ownProperties, prototype, ownSymbols } = objProps;
-
+  const parentPath = parent.path;
+  const parentValue = parent.contents.value;
   const properties = sortProperties(Object.keys(ownProperties)).filter(name =>
     ownProperties[name].hasOwnProperty("value")
   );
@@ -160,8 +164,10 @@ function makeNodesForProperties(
       parentPath,
       ownProperties
     );
-  } else {
+  } else if (parentValue.class == "Window") {
     nodes = makeDefaultPropsBucket(properties, parentPath, ownProperties);
+  } else {
+    nodes = makeNodesForOwnProps(properties, parentPath, ownProperties);
   }
 
   for (let index in ownSymbols) {
@@ -230,7 +236,7 @@ function getChildren({ getObjectProperties, actors, item }) {
     return [];
   }
 
-  let children = makeNodesForProperties(loadedProps, item.path);
+  let children = makeNodesForProperties(loadedProps, item);
   if (isPromise(item)) {
     children.unshift(getPromiseProperties(item));
   }

src/utils/object-inspector.js

@@ -6,6 +6,11 @@ const {
   getPromiseProperties
 } = require("../object-inspector");
 
+const root = {
+  path: "root",
+  contents: { value: {} }
+};
+
 const objProperties = {
   ownProperties: {
     "0": {
@@ -26,13 +31,14 @@ const objProperties = {
 describe("object-inspector", () => {
   describe("makeNodesForProperties", () => {
     it("kitchen sink", () => {
-      const nodes = makeNodesForProperties(objProperties, "root");
+      // console.log(this.root);
+      const nodes = makeNodesForProperties(objProperties, root);
 
       const names = nodes.map(n => n.name);
-      expect(names).to.eql(["0", "[default properties]", "__proto__"]);
+      expect(names).to.eql(["0", "length", "__proto__"]);
 
       const paths = nodes.map(n => n.path);
-      expect(paths).to.eql(["root/0", "root/default", "root/__proto__"]);
+      expect(paths).to.eql(["root/0", "root/length", "root/__proto__"]);
     });
 
     it("excludes getters", () => {
@@ -46,7 +52,7 @@ describe("object-inspector", () => {
             class: "bla"
           }
         },
-        "root"
+        root
       );
 
       const names = nodes.map(n => n.name);
@@ -70,7 +76,7 @@ describe("object-inspector", () => {
             class: "bla"
           }
         },
-        "root"
+        root
       );
 
       const names = nodes.map(n => n.name);
@@ -95,7 +101,7 @@ describe("object-inspector", () => {
           },
           prototype: { value: {}, class: "bla" }
         },
-        "root"
+        root
       );
 
       const names = nodes.map(n => n.name);
@@ -105,13 +111,35 @@ describe("object-inspector", () => {
       expect(paths).to.eql(["root/bar", "root/__proto__"]);
     });
 
+    it("window object", () => {
+      const nodes = makeNodesForProperties(
+        {
+          ownProperties: {
+            bar: { value: {} },
+            location: { value: {} }
+          },
+          class: "Window"
+        },
+        {
+          path: "root",
+          contents: { value: { class: "Window" } }
+        }
+      );
+
+      const names = nodes.map(n => n.name);
+      const paths = nodes.map(n => n.path);
+
+      expect(names).to.eql(["bar", "[default properties]"]);
+      expect(paths).to.eql(["root/bar", "root/##-default"]);
+    });
+
     // For large arrays
     it("numerical buckets", () => {
       let objProps = { ownProperties: {}, prototype: { class: "Array" } };
       for (let i = 0; i < 331; i++) {
         objProps.ownProperties[i] = { value: {} };
       }
-      const nodes = makeNodesForProperties(objProps, "root");
+      const nodes = makeNodesForProperties(objProps, root);
 
       const names = nodes.map(n => n.name);
       const paths = nodes.map(n => n.path);
@@ -147,7 +175,7 @@ describe("object-inspector", () => {
             class: "WindowPrototype"
           }
         },
-        "root"
+        root
       );
 
       const names = nodes.map(n => n.name);

src/utils/tests/object-inspector.js

@@ -1444,8 +1444,8 @@ balanced-match@^0.4.0, balanced-match@^0.4.1, balanced-match@^0.4.2:
   resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-0.4.2.tgz#cb3f3e3c732dc0f01ee70b403f302e61d7709838"
 
 base62@^1.1.0:
-  version "1.1.2"
-  resolved "https://registry.yarnpkg.com/base62/-/base62-1.1.2.tgz#22ced6a49913565bc0b8d9a11563a465c084124c"
+  version "1.2.0"
+  resolved "https://registry.yarnpkg.com/base62/-/base62-1.2.0.tgz#31e7e560dc846c9f44c1a531df6514da35474157"
 
 base64-js@^1.0.2, base64-js@^1.1.2:
   version "1.2.0"
@@ -1660,8 +1660,8 @@ caniuse-api@^1.5.2:
     lodash.uniq "^4.5.0"
 
 caniuse-db@^1.0.30000187, caniuse-db@^1.0.30000529, caniuse-db@^1.0.30000634, caniuse-db@^1.0.30000639:
-  version "1.0.30000667"
-  resolved "https://registry.yarnpkg.com/caniuse-db/-/caniuse-db-1.0.30000667.tgz#fb6060dbf349c101df26f421442419802fc6dab1"
+  version "1.0.30000670"
+  resolved "https://registry.yarnpkg.com/caniuse-db/-/caniuse-db-1.0.30000670.tgz#90d33b79e3090e25829c311113c56d6b1788bf43"
 
 capture-stack-trace@^1.0.0:
   version "1.0.0"
@@ -2864,8 +2864,8 @@ es-to-primitive@^1.1.1:
     is-symbol "^1.0.1"
 
 es5-ext@^0.10.14, es5-ext@^0.10.9, es5-ext@~0.10.14:
-  version "0.10.16"
-  resolved "https://registry.yarnpkg.com/es5-ext/-/es5-ext-0.10.16.tgz#1ef1b04f3d09db6a5d630226d62202f2e425e45a"
+  version "0.10.18"
+  resolved "https://registry.yarnpkg.com/es5-ext/-/es5-ext-0.10.18.tgz#dc239d3dce4c22b9c939aa180878837a3c0b5c92"
   dependencies:
     es6-iterator "2"
     es6-symbol "~3.1"
@@ -2966,8 +2966,8 @@ eslint-plugin-babel@^3.3.0:
   resolved "https://registry.yarnpkg.com/eslint-plugin-babel/-/eslint-plugin-babel-3.3.0.tgz#2f494aedcf6f4aa4e75b9155980837bc1fbde193"
 
 eslint-plugin-flowtype@^2.20.0:
-  version "2.32.1"
-  resolved "https://registry.yarnpkg.com/eslint-plugin-flowtype/-/eslint-plugin-flowtype-2.32.1.tgz#bbee185dedf97e5f63ec975cdcddd199bd2a2501"
+  version "2.33.0"
+  resolved "https://registry.yarnpkg.com/eslint-plugin-flowtype/-/eslint-plugin-flowtype-2.33.0.tgz#b2783814ed2ddcf729953b8f65ff73c90cabee4b"
   dependencies:
     lodash "^4.15.0"
 
@@ -4971,8 +4971,8 @@ jsprim@^1.2.2:
     verror "1.3.6"
 
 jssha@^2.1.0:
-  version "2.2.0"
-  resolved "https://registry.yarnpkg.com/jssha/-/jssha-2.2.0.tgz#87dcf60821dc3bec593f3855bbebccd276aacc1c"
+  version "2.3.0"
+  resolved "https://registry.yarnpkg.com/jssha/-/jssha-2.3.0.tgz#bb375f545c59f3ffda7c1e80fd0b3c0867586ab8"
 
 jstransform@^11.0.3:
   version "11.0.3"
@@ -5591,8 +5591,8 @@ mobx@^2.3.4:
   resolved "https://registry.yarnpkg.com/mobx/-/mobx-2.7.0.tgz#cf3d82d18c0ca7f458d8f2a240817b3dc7e54a01"
 
 mocha@^3.1.2:
-  version "3.3.0"
-  resolved "https://registry.yarnpkg.com/mocha/-/mocha-3.3.0.tgz#d29b7428d3f52c82e2e65df1ecb7064e1aabbfb5"
+  version "3.4.1"
+  resolved "https://registry.yarnpkg.com/mocha/-/mocha-3.4.1.tgz#a3802b4aa381934cacb38de70cf771621da8f9af"
   dependencies:
     browser-stdout "1.3.0"
     commander "2.9.0"