Bug 1576318 Part 2 - Use replay-friendly methods when previewing objects and getting the contents of container objects, r=loganfsmyth.
authorBrian Hackett <bhackett1024@gmail.com>
Fri, 30 Aug 2019 17:06:05 +0000
changeset 551410 19ab89cb9468e44c27385c5dae3959f6f46d7391
parent 551409 52a541822f4286e87c70e68133540bb1b8dcc38f
child 551411 607ed403709d8706e9328c438d16cb90add0ebd3
push id11865
push userbtara@mozilla.com
push dateMon, 02 Sep 2019 08:54:37 +0000
treeherdermozilla-beta@37f59c4671b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersloganfsmyth
bugs1576318
milestone70.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 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
devtools/server/actors/object.js
devtools/server/actors/object/previewers.js
devtools/server/actors/object/property-iterator.js
devtools/server/actors/object/utils.js
--- a/devtools/server/actors/object.js
+++ b/devtools/server/actors/object.js
@@ -425,23 +425,16 @@ const proto = {
     let level = 0,
       i = 0;
 
     // Do not search safe getters in unsafe objects.
     if (!DevToolsUtils.isSafeDebuggerObject(obj)) {
       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
     // freezing firefox when doing so.
     if (isArray(this.obj) || ["Object", "String"].includes(this.obj.class)) {
       obj = obj.proto;
       level++;
     }
--- a/devtools/server/actors/object/previewers.js
+++ b/devtools/server/actors/object/previewers.js
@@ -132,29 +132,26 @@ const previewers = {
       }
 
       return true;
     },
   ],
 
   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;
     },
   ],
 
   Date: [
     function({ obj, hooks }, grip) {
-      const time = DevToolsUtils.callPropertyOnObject(obj, "getTime");
+      const time = ObjectUtils.getDateTime(obj);
       if (typeof time != "number") {
         return false;
       }
 
       grip.preview = {
         timestamp: hooks.createValueGrip(time),
       };
       return true;
@@ -207,46 +204,43 @@ const previewers = {
       }
 
       return true;
     },
   ],
 
   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",
         length: size,
       };
 
       // Avoid recursive object grips.
       if (objectActor.hooks.getGripDepth() > 1) {
         return true;
       }
 
       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;
         }
       }
 
       return true;
     },
   ],
 
   WeakSet: [
     function(objectActor, grip) {
-      const enumEntries = PropertyIterators.enumWeakSetEntries(objectActor);
+      const enumEntries = PropertyIterators.enumWeakSetEntries(objectActor, /* forPreview */ true);
 
       grip.preview = {
         kind: "ArrayLike",
         length: enumEntries.size,
       };
 
       // Avoid recursive object grips.
       if (objectActor.hooks.getGripDepth() > 1) {
@@ -262,45 +256,42 @@ const previewers = {
       }
 
       return true;
     },
   ],
 
   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",
         size: size,
       };
 
       if (objectActor.hooks.getGripDepth() > 1) {
         return true;
       }
 
       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;
         }
       }
 
       return true;
     },
   ],
 
   WeakMap: [
     function(objectActor, grip) {
-      const enumEntries = PropertyIterators.enumWeakMapEntries(objectActor);
+      const enumEntries = PropertyIterators.enumWeakMapEntries(objectActor, /* forPreview */ true);
 
       grip.preview = {
         kind: "MapLike",
         size: enumEntries.size,
       };
 
       if (objectActor.hooks.getGripDepth() > 1) {
         return true;
@@ -516,17 +507,20 @@ function GenericObject(
       )
     );
 
     if (++i == OBJECT_PREVIEW_MAX_ITEMS) {
       break;
     }
   }
 
-  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
     );
   }
 
   return true;
 }
@@ -543,63 +537,43 @@ previewers.Object = [
       kind: "ArrayLike",
       length: ObjectUtils.getArrayLength(obj),
     };
 
     if (hooks.getGripDepth() > 1) {
       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;
   },
 
   function Error({ obj, hooks }, grip) {
     switch (obj.class) {
       case "Error":
       case "EvalError":
       case "RangeError":
       case "ReferenceError":
       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;
     }
   },
 
   function CSSMediaRule({ obj, hooks }, grip, rawObj) {
     if (isWorker || !rawObj || obj.class != "CSSMediaRule") {
--- a/devtools/server/actors/object/property-iterator.js
+++ b/devtools/server/actors/object/property-iterator.js
@@ -251,52 +251,59 @@ function enumObjectProperties(objectActo
         desc.getterValue = getterValue;
         desc.getterPrototypeLevel = getterPrototypeLevel;
       }
       return desc;
     },
   };
 }
 
-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,
   // so we get Opaque wrappers for them. And even though we have Xrays to
   // Arrays, the semantics often deny access to the entires based on the
   // nature of the values. So we need waive Xrays for the iterator object
   // and the tupes, and then re-apply them on the underlying values until
   // we fix bug 1023984.
   //
   // 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: {
           type: "mapEntry",
           preview: {
             key: gripFromEntry(objectActor, key),
             value: gripFromEntry(objectActor, val),
           },
@@ -339,78 +346,90 @@ function enumStorageEntries(objectActor)
             value: gripFromEntry(objectActor, val),
           },
         },
       };
     },
   };
 }
 
-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
   // 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.
   //
   // 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: {
           type: "mapEntry",
           preview: {
             key: gripFromEntry(objectActor, key),
             value: gripFromEntry(objectActor, val),
           },
         },
       };
     },
   };
 }
 
-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
   // 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.
   //
   // 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))
   );
 
   return {
     [Symbol.iterator]: function*() {
       for (const item of values) {
         yield gripFromEntry(objectActor, item);
       }
@@ -424,29 +443,37 @@ function enumSetEntries(objectActor) {
       return {
         enumerable: true,
         value: gripFromEntry(objectActor, val),
       };
     },
   };
 }
 
-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
   // 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.
   //
   // 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*() {
       for (const item of keys) {
         yield gripFromEntry(objectActor, item);
       }
     },
     size: keys.length,
--- a/devtools/server/actors/object/utils.js
+++ b/devtools/server/actors/object/utils.js
@@ -198,25 +198,50 @@ function getArrayLength(object) {
     throw new Error("Expected an array, got a " + object.class);
   }
 
   // Real arrays have a reliable `length` own property.
   if (object.class === "Array") {
     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.
   const typedProto = Object.getPrototypeOf(Uint8Array.prototype);
   const getter = Object.getOwnPropertyDescriptor(typedProto, "length").get;
   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.
  *
  * @param str String
  * @return Boolean
  */
 function isArrayIndex(str) {
   // Transform the parameter to a 32-bit unsigned integer.
   const num = str >>> 0;
@@ -249,21 +274,60 @@ function isStorage(object) {
  */
 function getStorageLength(object) {
   if (!isStorage(object)) {
     throw new Error("Expected a storage object, got a " + object.class);
   }
   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,
   unwrapDebuggeeValue,
   createValueGrip,
   stringIsLong,
   isTypedArray,
   isArray,
   isStorage,
   getArrayLength,
   getStorageLength,
+  getContainerSize,
   isArrayIndex,
+  getRegExpString,
+  getDateTime,
+  getErrorProperties,
 };