Bug 1454103 - Correct display of local and sessionStorage in DevTools r=nchevobbe
authorMike Ratcliffe <mratcliffe@mozilla.com>
Sat, 14 Apr 2018 00:29:59 +0100
changeset 416481 b860b811cbb04a993b1c12af0fe72477c79c9978
parent 416480 e6f674c99538d39323e0260693af26e4a9c969a5
child 416482 4dbde138d6305db19fc4b35800fbdaba2fa9d2cf
push id63488
push usermratcliffe@mozilla.com
push dateWed, 02 May 2018 08:32:43 +0000
treeherderautoland@b860b811cbb0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe
bugs1454103
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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)
         };