Bug 1576318 Part 2 - Use replay-friendly methods when previewing objects and getting the contents of container objects, r=loganfsmyth.

Depends on D43318 Differential Revision: https://phabricator.services.mozilla.com/D43319 --HG-- extra : moz-landing-system : lando
2019-08-30 17:06:05 +00:00 · 2019-08-30 17:06:05 +00:00 · fe92dccb84
--- a/devtools/server/actors/object.js
+++ b/devtools/server/actors/object.js
@ -430,13 +430,6 @@ const proto = {
      return safeGetterValues;
    }

-    // Do not search for safe getters while replaying. While this would be nice
-    // to support, it involves a lot of back-and-forth between processes and
-    // would be better to do entirely in the replaying process.
-    if (isReplaying) {
-      return safeGetterValues;
-    }
-
    // Most objects don't have any safe getters but inherit some from their
    // prototype. Avoid calling getOwnPropertyNames on objects that may have
    // many properties like Array, strings or js objects. That to avoid
--- a/devtools/server/actors/object/previewers.js
+++ b/devtools/server/actors/object/previewers.js
@ -137,10 +137,7 @@ const previewers = {

  RegExp: [
    function({ obj, hooks }, grip) {
-      const str = DevToolsUtils.callPropertyOnObject(obj, "toString");
-      if (typeof str != "string") {
-        return false;
-      }
+      const str = ObjectUtils.getRegExpString(obj);

      grip.displayString = hooks.createValueGrip(str);
      return true;
@ -149,7 +146,7 @@ const previewers = {

  Date: [
    function({ obj, hooks }, grip) {
-      const time = DevToolsUtils.callPropertyOnObject(obj, "getTime");
+      const time = ObjectUtils.getDateTime(obj);
      if (typeof time != "number") {
        return false;
      }
@ -212,10 +209,7 @@ const previewers = {

  Set: [
    function(objectActor, grip) {
-      const size = DevToolsUtils.getProperty(objectActor.obj, "size");
-      if (typeof size != "number") {
-        return false;
-      }
+      const size = ObjectUtils.getContainerSize(objectActor.obj);

      grip.preview = {
        kind: "ArrayLike",
@ -228,7 +222,7 @@ const previewers = {
      }

      const items = (grip.preview.items = []);
-      for (const item of PropertyIterators.enumSetEntries(objectActor)) {
+      for (const item of PropertyIterators.enumSetEntries(objectActor, /* forPreview */ true)) {
        items.push(item);
        if (items.length == OBJECT_PREVIEW_MAX_ITEMS) {
          break;
@ -241,7 +235,7 @@ const previewers = {

  WeakSet: [
    function(objectActor, grip) {
-      const enumEntries = PropertyIterators.enumWeakSetEntries(objectActor);
+      const enumEntries = PropertyIterators.enumWeakSetEntries(objectActor, /* forPreview */ true);

      grip.preview = {
        kind: "ArrayLike",
@ -267,10 +261,7 @@ const previewers = {

  Map: [
    function(objectActor, grip) {
-      const size = DevToolsUtils.getProperty(objectActor.obj, "size");
-      if (typeof size != "number") {
-        return false;
-      }
+      const size = ObjectUtils.getContainerSize(objectActor.obj);

      grip.preview = {
        kind: "MapLike",
@ -282,7 +273,7 @@ const previewers = {
      }

      const entries = (grip.preview.entries = []);
-      for (const entry of PropertyIterators.enumMapEntries(objectActor)) {
+      for (const entry of PropertyIterators.enumMapEntries(objectActor, /* forPreview */ true)) {
        entries.push(entry);
        if (entries.length == OBJECT_PREVIEW_MAX_ITEMS) {
          break;
@ -295,7 +286,7 @@ const previewers = {

  WeakMap: [
    function(objectActor, grip) {
-      const enumEntries = PropertyIterators.enumWeakMapEntries(objectActor);
+      const enumEntries = PropertyIterators.enumWeakMapEntries(objectActor, /* forPreview */ true);

      grip.preview = {
        kind: "MapLike",
@ -521,7 +512,10 @@ function GenericObject(
    }
  }

-  if (i < OBJECT_PREVIEW_MAX_ITEMS) {
+  // Do not search for safe getters when generating previews while replaying.
+  // This involves a lot of back-and-forth communication which we don't want to
+  // incur while previewing objects.
+  if (i < OBJECT_PREVIEW_MAX_ITEMS && !isReplaying) {
    preview.safeGetterValues = objectActor._findSafeGetterValues(
      Object.keys(preview.ownProperties),
      OBJECT_PREVIEW_MAX_ITEMS - i
@ -548,24 +542,14 @@ previewers.Object = [
      return true;
    }

-    const raw = obj.unsafeDereference();
-
-    // The raw object will be null/unavailable when interacting with a
-    // replaying execution, and Cu is unavailable in workers. In either case we
-    // do not need to worry about xrays.
-    if (raw && !isWorker) {
-      const global = Cu.getGlobalForObject(DebuggerServer);
-      const classProto = global[obj.class].prototype;
-      // The Xray machinery for TypedArrays denies indexed access on the grounds
-      // that it's slow, and advises callers to do a structured clone instead.
-      const safeView = Cu.cloneInto(
-        classProto.subarray.call(raw, 0, OBJECT_PREVIEW_MAX_ITEMS),
-        global
-      );
-      const items = (grip.preview.items = []);
-      for (let i = 0; i < safeView.length; i++) {
-        items.push(safeView[i]);
+    const previewLength = Math.min(OBJECT_PREVIEW_MAX_ITEMS, grip.preview.length);
+    grip.preview.items = [];
+    for (let i = 0; i < previewLength; i++) {
+      const desc = obj.getOwnPropertyDescriptor(i);
+      if (!desc) {
+        break;
      }
+      grip.preview.items.push(desc.value);
    }

    return true;
@ -580,21 +564,11 @@ previewers.Object = [
      case "SyntaxError":
      case "TypeError":
      case "URIError":
-        const name = DevToolsUtils.getProperty(obj, "name");
-        const msg = DevToolsUtils.getProperty(obj, "message");
-        const stack = DevToolsUtils.getProperty(obj, "stack");
-        const fileName = DevToolsUtils.getProperty(obj, "fileName");
-        const lineNumber = DevToolsUtils.getProperty(obj, "lineNumber");
-        const columnNumber = DevToolsUtils.getProperty(obj, "columnNumber");
-        grip.preview = {
-          kind: "Error",
-          name: hooks.createValueGrip(name),
-          message: hooks.createValueGrip(msg),
-          stack: hooks.createValueGrip(stack),
-          fileName: hooks.createValueGrip(fileName),
-          lineNumber: hooks.createValueGrip(lineNumber),
-          columnNumber: hooks.createValueGrip(columnNumber),
-        };
+        grip.preview = { kind: "Error" };
+        const properties = ObjectUtils.getErrorProperties(obj);
+        Object.keys(properties).forEach(p => {
+          grip.preview[p] = hooks.createValueGrip(properties[p]);
+        });
        return true;
      default:
        return false;
--- a/devtools/server/actors/object/property-iterator.js
+++ b/devtools/server/actors/object/property-iterator.js
@ -256,7 +256,11 @@ function enumObjectProperties(objectActor, options) {
  };
 }

-function enumMapEntries(objectActor) {
+function getMapEntries(obj, forPreview) {
+  if (isReplaying) {
+    return obj.containerContents(forPreview);
+  }
+
  // Iterating over a Map via .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,
@ -269,29 +273,32 @@ function enumMapEntries(objectActor) {
  // Even then though, we might want to continue waiving Xrays here for the
  // same reason we do so for Arrays above - this filtering behavior is likely
  // to be more confusing than beneficial in the case of Object previews.
-  const raw = objectActor.obj.unsafeDereference();
-  const iterator = objectActor.obj.makeDebuggeeValue(
+  const raw = obj.unsafeDereference();
+  const iterator = obj.makeDebuggeeValue(
    waiveXrays(Map.prototype.keys.call(raw))
  );
-  const keys = [...DevToolsUtils.makeDebuggeeIterator(iterator)].map(k =>
-    waiveXrays(ObjectUtils.unwrapDebuggeeValue(k))
-  );
-  const getValue = key => Map.prototype.get.call(raw, key);
+  return [...DevToolsUtils.makeDebuggeeIterator(iterator)].map(k => {
+    const key = waiveXrays(ObjectUtils.unwrapDebuggeeValue(k))
+    const value = Map.prototype.get.call(raw, key);
+    return [key, value];
+  });
+}
+
+function enumMapEntries(objectActor, forPreview = false) {
+  const entries = getMapEntries(objectActor.obj, forPreview);

  return {
    [Symbol.iterator]: function*() {
-      for (const key of keys) {
-        const value = getValue(key);
+      for (const [key, value] of entries) {
        yield [key, value].map(val => gripFromEntry(objectActor, val));
      }
    },
-    size: keys.length,
+    size: entries.length,
    propertyName(index) {
      return index;
    },
    propertyDescription(index) {
-      const key = keys[index];
-      const val = getValue(key);
+      const [key, val] = entries[index];
      return {
        enumerable: true,
        value: {
@ -344,7 +351,11 @@ function enumStorageEntries(objectActor) {
  };
 }

-function enumWeakMapEntries(objectActor) {
+function getWeakMapEntries(obj, forPreview) {
+  if (isReplaying) {
+    return obj.containerContents(forPreview);
+  }
+
  // 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
@ -355,27 +366,27 @@ function enumWeakMapEntries(objectActor) {
  // This code is designed to handle untrusted objects, so we can safely
  // waive Xrays on the iterable, and relying on the Debugger machinery to
  // make sure we handle the resulting objects carefully.
-  const raw = objectActor.obj.unsafeDereference();
+  const raw = obj.unsafeDereference();
  const keys = waiveXrays(ChromeUtils.nondeterministicGetWeakMapKeys(raw));

-  const values = [];
-  for (const k of keys) {
-    values.push(WeakMap.prototype.get.call(raw, k));
-  }
+  return keys.map(k => [k, WeakMap.prototype.get.call(raw, k)]);
+}
+
+function enumWeakMapEntries(objectActor, forPreview = false) {
+  const entries = getWeakMapEntries(objectActor.obj, forPreview);

  return {
    [Symbol.iterator]: function*() {
-      for (let i = 0; i < keys.length; i++) {
-        yield [keys[i], values[i]].map(val => gripFromEntry(objectActor, val));
+      for (let i = 0; i < entries.length; i++) {
+        yield entries[i].map(val => gripFromEntry(objectActor, val));
      }
    },
-    size: keys.length,
+    size: entries.length,
    propertyName(index) {
      return index;
    },
    propertyDescription(index) {
-      const key = keys[index];
-      const val = values[index];
+      const [key, val] = entries[index];
      return {
        enumerable: true,
        value: {
@ -390,7 +401,11 @@ function enumWeakMapEntries(objectActor) {
  };
 }

-function enumSetEntries(objectActor) {
+function getSetValues(obj, forPreview) {
+  if (isReplaying) {
+    return obj.containerContents(forPreview);
+  }
+
  // We currently lack XrayWrappers for Set, 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
@ -401,11 +416,15 @@ function enumSetEntries(objectActor) {
  // This code is designed to handle untrusted objects, so we can safely
  // waive Xrays on the iterable, and relying on the Debugger machinery to
  // make sure we handle the resulting objects carefully.
-  const raw = objectActor.obj.unsafeDereference();
-  const iterator = objectActor.obj.makeDebuggeeValue(
+  const raw = obj.unsafeDereference();
+  const iterator = obj.makeDebuggeeValue(
    waiveXrays(Set.prototype.values.call(raw))
  );
-  const values = [...DevToolsUtils.makeDebuggeeIterator(iterator)].map(v =>
+  return [...DevToolsUtils.makeDebuggeeIterator(iterator)];
+}
+
+function enumSetEntries(objectActor, forPreview = false) {
+  const values = getSetValues(objectActor.obj, forPreview).map(v =>
    waiveXrays(ObjectUtils.unwrapDebuggeeValue(v))
  );

@ -429,7 +448,11 @@ function enumSetEntries(objectActor) {
  };
 }

-function enumWeakSetEntries(objectActor) {
+function getWeakSetEntries(obj, forPreview) {
+  if (isReplaying) {
+    return obj.containerContents(forPreview);
+  }
+
  // We currently lack XrayWrappers for WeakSet, 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
@ -440,8 +463,12 @@ function enumWeakSetEntries(objectActor) {
  // This code is designed to handle untrusted objects, so we can safely
  // waive Xrays on the iterable, and relying on the Debugger machinery to
  // make sure we handle the resulting objects carefully.
-  const raw = objectActor.obj.unsafeDereference();
-  const keys = waiveXrays(ChromeUtils.nondeterministicGetWeakSetKeys(raw));
+  const raw = obj.unsafeDereference();
+  return waiveXrays(ChromeUtils.nondeterministicGetWeakSetKeys(raw));
+}
+
+function enumWeakSetEntries(objectActor, forPreview = false) {
+  const keys = getWeakSetEntries(objectActor.obj, forPreview);

  return {
    [Symbol.iterator]: function*() {
--- a/devtools/server/actors/object/utils.js
+++ b/devtools/server/actors/object/utils.js
@ -203,6 +203,12 @@ function getArrayLength(object) {
    return DevToolsUtils.getProperty(object, "length");
  }

+  // When replaying, we use a special API to get typed array lengths. We can't
+  // invoke getters on the proxy returned by unsafeDereference().
+  if (isReplaying) {
+    return object.getTypedArrayLength();
+  }
+
  // For typed arrays, `DevToolsUtils.getProperty` is not reliable because the `length`
  // getter could be shadowed by an own property, and `getOwnPropertyNames` is
  // unnecessarily slow. Obtain the `length` getter safely and call it manually.
@ -211,6 +217,25 @@ function getArrayLength(object) {
  return getter.call(object.unsafeDereference());
 }

+/**
+ * Returns the number of elements in a Set or Map.
+ *
+ * @param object Debugger.Object
+ *        The debuggee object of the Set or Map.
+ * @return Number
+ */
+function getContainerSize(object) {
+  if (object.class != "Set" && object.class != "Map") {
+    throw new Error(`Expected a set/map, got a ${object.class}`);
+  }
+
+  if (isReplaying) {
+    return object.getContainerSize();
+  }
+
+  return DevToolsUtils.getProperty(object, "size");
+}
+
 /**
 * Returns true if the parameter is suitable to be an array index.
 *
@ -254,6 +279,41 @@ function getStorageLength(object) {
  return DevToolsUtils.getProperty(object, "length");
 }

+// Get the string representation of a Debugger.Object for a RegExp.
+function getRegExpString(object) {
+  if (isReplaying) {
+    return object.getRegExpString();
+  }
+
+  return DevToolsUtils.callPropertyOnObject(object, "toString");
+}
+
+// Get the time associated with a Debugger.Object for a Date.
+function getDateTime(object) {
+  if (isReplaying) {
+    return object.getDateTime();
+  }
+
+  return DevToolsUtils.callPropertyOnObject(object, "getTime");
+}
+
+// Get the properties of a Debugger.Object for an Error which are needed to
+// preview the object.
+function getErrorProperties(object) {
+  if (isReplaying) {
+    return object.getErrorProperties();
+  }
+
+  return {
+    name: DevToolsUtils.getProperty(object, "name"),
+    message: DevToolsUtils.getProperty(object, "message"),
+    stack: DevToolsUtils.getProperty(object, "stack"),
+    fileName: DevToolsUtils.getProperty(object, "fileName"),
+    lineNumber: DevToolsUtils.getProperty(object, "lineNumber"),
+    columnNumber: DevToolsUtils.getProperty(object, "columnNumber"),
+  };
+}
+
 module.exports = {
  getPromiseState,
  makeDebuggeeValueIfNeeded,
@ -265,5 +325,9 @@ module.exports = {
  isStorage,
  getArrayLength,
  getStorageLength,
+  getContainerSize,
  isArrayIndex,
+  getRegExpString,
+  getDateTime,
+  getErrorProperties,
 };