Bug 1454103 - Correct display of local and sessionStorage in DevTools r?nchevobbe draft
authorMike Ratcliffe <mratcliffe@mozilla.com>
Sat, 14 Apr 2018 00:29:59 +0100
changeset 789084 c1d646d44619326b97e0ecea9d7bced6c707dfa2
parent 788735 63a0e2f626febb98d87d2543955ab99a653654ff
push id108164
push userbmo:mratcliffe@mozilla.com
push dateFri, 27 Apr 2018 14:55:31 +0000
reviewersnchevobbe
bugs1454103
milestone61.0a1
Bug 1454103 - Correct display of local and sessionStorage in DevTools r?nchevobbe Take into account node offsets in browser_webconsole_object_inspector_local_session_storage.js. MozReview-Commit-ID: 73waFejjsF0
devtools/client/shared/widgets/VariablesViewController.jsm
devtools/client/webconsole/test/mochitest/browser.ini
devtools/client/webconsole/test/mochitest/browser_webconsole_object_inspector_local_session_storage.js
devtools/client/webconsole/test/mochitest/test-local-session-storage.html
devtools/server/actors/object.js
devtools/server/actors/object/previewers.js
devtools/server/actors/object/property-iterator.js
devtools/server/actors/object/utils.js
devtools/shared/client/object-client.js
--- a/devtools/client/shared/widgets/VariablesViewController.jsm
+++ b/devtools/client/shared/widgets/VariablesViewController.jsm
@@ -611,17 +611,17 @@ VariablesViewController.prototype = {
     if (aSource.type === "property-iterator") {
       return this._populateFromPropertyIterator(aTarget, aSource);
     }
 
     if (aSource.type === "entries-list") {
       return this._populateFromEntries(aTarget, aSource);
     }
 
-    if (aSource.type === "mapEntry") {
+    if (aSource.type === "mapEntry" || aSource.type === "storageEntry") {
       aTarget.addItems({
         key: { value: aSource.preview.key },
         value: { value: aSource.preview.value }
       }, {
         callback: this.addExpander
       });
 
       return promise.resolve();
--- a/devtools/client/webconsole/test/mochitest/browser.ini
+++ b/devtools/client/webconsole/test/mochitest/browser.ini
@@ -98,16 +98,17 @@ support-files =
   test-ineffective-iframe-sandbox-warning4.html
   test-ineffective-iframe-sandbox-warning5.html
   test-insecure-frame.html
   test-insecure-passwords-about-blank-web-console-warning.html
   test-insecure-passwords-web-console-warning.html
   test-inspect-cross-domain-objects-frame.html
   test-inspect-cross-domain-objects-top.html
   test-jsterm-dollar.html
+  test-local-session-storage.html
   test-location-debugger-link-console-log.js
   test-location-debugger-link-errors.js
   test-location-debugger-link.html
   test-location-styleeditor-link-1.css
   test-location-styleeditor-link-2.css
   test-location-styleeditor-link.html
   test-message-categories-canvas-css.html
   test-message-categories-canvas-css.js
@@ -309,16 +310,17 @@ skip-if = (os == 'linux') || (os == 'win
 [browser_webconsole_network_reset_filter.js]
 [browser_webconsole_nodes_highlight.js]
 [browser_webconsole_nodes_select.js]
 [browser_webconsole_object_in_sidebar.js]
 [browser_webconsole_object_in_sidebar_keyboard_nav.js]
 [browser_webconsole_object_inspector.js]
 [browser_webconsole_object_inspector_entries.js]
 [browser_webconsole_object_inspector_key_sorting.js]
+[browser_webconsole_object_inspector_local_session_storage.js]
 [browser_webconsole_object_inspector_while_debugging_and_inspecting.js]
 [browser_webconsole_observer_notifications.js]
 [browser_webconsole_optimized_out_vars.js]
 [browser_webconsole_output_copy.js]
 subsuite = clipboard
 [browser_webconsole_output_copy_newlines.js]
 subsuite = clipboard
 [browser_webconsole_output_order.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/mochitest/browser_webconsole_object_inspector_local_session_storage.js
@@ -0,0 +1,88 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Check expanding/collapsing local and session storage in the console.
+const TEST_URI = "http://example.com/browser/devtools/client/webconsole/" +
+                 "test/mochitest/test-local-session-storage.html";
+
+add_task(async function() {
+  const hud = await openNewTabAndConsole(TEST_URI);
+  const messages = await logMessages(hud);
+  const objectInspectors = messages.map(node => node.querySelector(".tree"));
+
+  is(objectInspectors.length, 2, "There is the expected number of object inspectors");
+
+  await checkValues(objectInspectors[0], "localStorage");
+  await checkValues(objectInspectors[1], "sessionStorage");
+});
+
+async function logMessages(hud) {
+  await ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
+    content.console.log("localStorage", content.localStorage);
+  });
+  const localStorageMsg = await waitFor(() => findMessage(hud, "localStorage"));
+
+  await ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
+    content.console.log("sessionStorage", content.sessionStorage);
+  });
+  const sessionStorageMsg = await waitFor(() => findMessage(hud, "sessionStorage"));
+
+  return [localStorageMsg, sessionStorageMsg];
+}
+
+async function checkValues(oi, storageType) {
+  info(`Expanding the ${storageType} object`);
+  let onMapOiMutation = waitForNodeMutation(oi, {
+    childList: true
+  });
+
+  oi.querySelector(".arrow").click();
+  await onMapOiMutation;
+
+  ok(oi.querySelector(".arrow").classList.contains("expanded"),
+    "The arrow of the node has the expected class after clicking on it");
+
+  let nodes = oi.querySelectorAll(".node");
+  // There are 4 nodes: the root, size, entries and the proto.
+  is(nodes.length, 5, "There is the expected number of nodes in the tree");
+
+  info("Expanding the <entries> leaf of the map");
+  let entriesNode = nodes[3];
+  is(entriesNode.textContent, "<entries>", "There is the expected <entries> node");
+  onMapOiMutation = waitForNodeMutation(oi, {
+    childList: true
+  });
+
+  entriesNode.querySelector(".arrow").click();
+  await onMapOiMutation;
+
+  nodes = oi.querySelectorAll(".node");
+  // There are now 7 nodes, the 5 original ones, and the 2 entries.
+  is(nodes.length, 7, "There is the expected number of nodes in the tree");
+
+  const title = nodes[0].querySelector(".objectTitle").textContent;
+  const length = [...nodes[2].querySelectorAll(".object-label,.objectBox")]
+                             .map(node => node.textContent);
+  const key2 = [...nodes[4].querySelectorAll(".object-label,.nodeName,.objectBox-string")]
+                           .map(node => node.textContent);
+  const key = [...nodes[5].querySelectorAll(".object-label,.nodeName,.objectBox-string")]
+                          .map(node => node.textContent);
+
+  is(title, "Storage", `${storageType} object has the expected title`);
+  is(length[0], "length", `${storageType} length property name is correct`);
+  is(length[1], "2", `${storageType} length property value is correct`);
+  is(key2[0], "0", `1st entry of ${storageType} entry has the correct index`);
+  is(key2[1], "key2", `1st entry of ${storageType} entry has the correct key`);
+
+  const firstValue = storageType === "localStorage" ? `"value2"` : `"value4"`;
+  is(key2[2], firstValue, `1st entry of ${storageType} entry has the correct value`);
+  is(key[0], "1", `2nd entry of ${storageType} entry has the correct index`);
+  is(key[1], "key", `2nd entry of ${storageType} entry has the correct key`);
+
+  const secondValue = storageType === "localStorage" ? `"value1"` : `"value3"`;
+  is(key[2], secondValue, `2nd entry of ${storageType} entry has the correct value`);
+}
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/mochitest/test-local-session-storage.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <meta charset="utf-8" />
+  <title>localStorage and sessionStorage Test</title>
+
+  <script>
+    "use strict";
+
+    /* eslint-disable no-unused-vars */
+
+    function init() {
+      // We use the value key to ensure we cover an issue with iterating through
+      // storage entries... please leave this key as "key"
+      localStorage.setItem("key", "value1");
+      localStorage.setItem("key2", "value2");
+
+      // We use the value key to ensure we cover an issue with iterating through
+      // storage entries... please leave this key as "key"
+      sessionStorage.setItem("key", "value3");
+      sessionStorage.setItem("key2", "value4");
+    }
+  </script>
+</head>
+<body onload="init()">
+  <h1>localStorage and sessionStorage Test</h1>
+</body>
+</html>
--- a/devtools/server/actors/object.js
+++ b/devtools/server/actors/object.js
@@ -14,17 +14,19 @@ const { assert } = DevToolsUtils;
 loader.lazyRequireGetter(this, "PropertyIteratorActor", "devtools/server/actors/object/property-iterator", true);
 loader.lazyRequireGetter(this, "SymbolIteratorActor", "devtools/server/actors/object/symbol-iterator", true);
 loader.lazyRequireGetter(this, "previewers", "devtools/server/actors/object/previewers");
 loader.lazyRequireGetter(this, "stringify", "devtools/server/actors/object/stringifiers");
 
 const {
   getArrayLength,
   getPromiseState,
+  getStorageLength,
   isArray,
+  isStorage,
   isTypedArray,
 } = require("devtools/server/actors/object/utils");
 /**
  * Creates an actor for the specified object.
  *
  * @param obj Debugger.Object
  *        The debuggee object.
  * @param hooks Object
@@ -126,16 +128,18 @@ ObjectActor.prototype = {
       g.promiseState = this._createPromiseState();
     }
 
     // FF40+: Allow to know how many properties an object has to lazily display them
     // when there is a bunch.
     if (isTypedArray(g)) {
       // Bug 1348761: getOwnPropertyNames is unnecessary slow on TypedArrays
       g.ownPropertyLength = getArrayLength(this.obj);
+    } else if (isStorage(g)) {
+      g.ownPropertyLength = getStorageLength(this.obj);
     } else {
       try {
         g.ownPropertyLength = this.obj.getOwnPropertyNames().length;
       } catch (err) {
         // The above can throw when the debuggee does not subsume the object's
         // compartment, or for some WrappedNatives like Cu.Sandbox.
       }
     }
@@ -553,16 +557,28 @@ ObjectActor.prototype = {
       return {
         configurable: false,
         writable: false,
         enumerable: false,
         value: e.name
       };
     }
 
+    if (isStorage(this.obj)) {
+      if (name === "length") {
+        return undefined;
+      }
+      return {
+        configurable: true,
+        writable: true,
+        enumerable: true,
+        value: name
+      };
+    }
+
     if (!desc || onlyEnumerable && !desc.enumerable) {
       return undefined;
     }
 
     let retval = {
       configurable: desc.configurable,
       enumerable: desc.enumerable
     };
--- a/devtools/server/actors/object/previewers.js
+++ b/devtools/server/actors/object/previewers.js
@@ -356,17 +356,27 @@ function GenericObject(objectActor, grip
   let i = 0, names = [], symbols = [];
   let preview = grip.preview = {
     kind: "Object",
     ownProperties: Object.create(null),
     ownSymbols: [],
   };
 
   try {
-    names = obj.getOwnPropertyNames();
+    if (ObjectUtils.isStorage(obj)) {
+      // local and session storage cannot be iterated over using
+      // Object.getOwnPropertyNames() because it skips keys that are duplicated
+      // on the prototype e.g. "key", "getKeys" so we need to gather the real
+      // keys using the storage.key() function.
+      for (let j = 0; j < rawObj.length; j++) {
+        names.push(rawObj.key(j));
+      }
+    } else {
+      names = obj.getOwnPropertyNames();
+    }
     symbols = obj.getOwnPropertySymbols();
   } catch (ex) {
     // Calling getOwnPropertyNames() on some wrapped native prototypes is not
     // allowed: "cannot modify properties of a WrappedNative". See bug 952093.
   }
   preview.ownPropertiesLength = names.length;
   preview.ownSymbolsLength = symbols.length;
 
--- a/devtools/server/actors/object/property-iterator.js
+++ b/devtools/server/actors/object/property-iterator.js
@@ -53,16 +53,18 @@ const PropertyIteratorActor  = protocol.
       if (cls == "Map") {
         this.iterator = enumMapEntries(objectActor);
       } else if (cls == "WeakMap") {
         this.iterator = enumWeakMapEntries(objectActor);
       } else if (cls == "Set") {
         this.iterator = enumSetEntries(objectActor);
       } else if (cls == "WeakSet") {
         this.iterator = enumWeakSetEntries(objectActor);
+      } else if (cls == "Storage") {
+        this.iterator = enumStorageEntries(objectActor);
       } else {
         throw new Error("Unsupported class to enumerate entries from: " + cls);
       }
     } else if (
       ObjectUtils.isArray(objectActor.obj)
       && options.ignoreNonIndexedProperties
       && !options.query
     ) {
@@ -275,16 +277,54 @@ function enumMapEntries(objectActor) {
             value: gripFromEntry(objectActor, val)
           }
         }
       };
     }
   };
 }
 
+function enumStorageEntries(objectActor) {
+  // Iterating over local / sessionStorage entries goes through various
+  // intermediate objects - an Iterator object, then a 2-element Array object,
+  // then the actual values we care about. We don't have Xrays to Iterator
+  // objects, so we get Opaque wrappers for them.
+  let raw = objectActor.obj.unsafeDereference();
+  let keys = [];
+  for (let i = 0; i < raw.length; i++) {
+    keys.push(raw.key(i));
+  }
+  return {
+    [Symbol.iterator]: function* () {
+      for (let key of keys) {
+        let value = raw.getItem(key);
+        yield [ key, value ].map(val => gripFromEntry(objectActor, val));
+      }
+    },
+    size: keys.length,
+    propertyName(index) {
+      return index;
+    },
+    propertyDescription(index) {
+      let key = keys[index];
+      let val = raw.getItem(key);
+      return {
+        enumerable: true,
+        value: {
+          type: "storageEntry",
+          preview: {
+            key: gripFromEntry(objectActor, key),
+            value: gripFromEntry(objectActor, val)
+          }
+        }
+      };
+    }
+  };
+}
+
 function enumWeakMapEntries(objectActor) {
   // We currently lack XrayWrappers for WeakMap, so when we iterate over
   // the values, the temporary iterator objects get created in the target
   // compartment. However, we _do_ have Xrays to Object now, so we end up
   // Xraying those temporary objects, and filtering access to |it.value|
   // based on whether or not it's Xrayable and/or callable, which breaks
   // the for/of iteration.
   //
--- a/devtools/server/actors/object/utils.js
+++ b/devtools/server/actors/object/utils.js
@@ -148,17 +148,17 @@ function isTypedArray(object) {
  */
 function isArray(object) {
   return isTypedArray(object) || object.class === "Array";
 }
 
 /**
  * Returns the length of an array (or typed array).
  *
- * @param obj Debugger.Object
+ * @param object Debugger.Object
  *        The debuggee object of the array.
  * @return Number
  * @throws if the object is not an array.
  */
 function getArrayLength(object) {
   if (!isArray(object)) {
     throw new Error("Expected an array, got a " + object.class);
   }
@@ -186,18 +186,46 @@ function isArrayIndex(str) {
   // Transform the parameter to a 32-bit unsigned integer.
   let num = str >>> 0;
   // Check that the parameter is a canonical Uint32 index.
   return num + "" === str &&
     // Array indices cannot attain the maximum Uint32 value.
     num != -1 >>> 0;
 }
 
+/**
+ * Returns true if a debuggee object is a local or sessionStorage object.
+ *
+ * @param object Debugger.Object
+ *        The debuggee object to test.
+ * @return Boolean
+ */
+function isStorage(object) {
+  return object.class === "Storage";
+}
+
+/**
+ * Returns the length of a local or sessionStorage object.
+ *
+ * @param object Debugger.Object
+ *        The debuggee object of the array.
+ * @return Number
+ * @throws if the object is not a local or sessionStorage object.
+ */
+function getStorageLength(object) {
+  if (!isStorage(object)) {
+    throw new Error("Expected a storage object, got a " + object.class);
+  }
+  return DevToolsUtils.getProperty(object, "length");
+}
+
 module.exports = {
   getPromiseState,
   makeDebuggeeValueIfNeeded,
   createValueGrip,
   stringIsLong,
   isTypedArray,
   isArray,
+  isStorage,
   getArrayLength,
+  getStorageLength,
   isArrayIndex,
 };
--- a/devtools/shared/client/object-client.js
+++ b/devtools/shared/client/object-client.js
@@ -126,18 +126,18 @@ ObjectClient.prototype = {
    * Map/Set-like object.
    *
    * @param onResponse function Called with the request's response.
    */
   enumEntries: DebuggerClient.requester({
     type: "enumEntries"
   }, {
     before: function(packet) {
-      if (!["Map", "WeakMap", "Set", "WeakSet"].includes(this._grip.class)) {
-        throw new Error("enumEntries is only valid for Map/Set-like grips.");
+      if (!["Map", "WeakMap", "Set", "WeakSet", "Storage"].includes(this._grip.class)) {
+        throw new Error("enumEntries is only valid for Map/Set/Storage-like grips.");
       }
       return packet;
     },
     after: function(response) {
       if (response.iterator) {
         return {
           iterator: new PropertyIteratorClient(this._client, response.iterator)
         };