Bug 1522244 - Fix previewing in worker threads for several builtin classes, r=lsmyth.
authorBrian Hackett <bhackett1024@gmail.com>
Sun, 03 Mar 2019 07:05:07 -1000
changeset 520065 faddaf06f46c8452a6c3b2358940d2c23fa137b2
parent 520064 6889cdacf015d4674a88cf195364c8783d7cad90
child 520066 0d261741c46150c46144dd5ef382d57e40e45279
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsmyth
bugs1522244
milestone67.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 1522244 - Fix previewing in worker threads for several builtin classes, r=lsmyth.
devtools/server/actors/object/previewers.js
devtools/server/actors/object/property-iterator.js
devtools/server/actors/object/utils.js
devtools/server/tests/unit/test_objectgrips-20.js
devtools/shared/DevToolsUtils.js
dom/workers/RuntimeService.cpp
--- a/devtools/server/actors/object/previewers.js
+++ b/devtools/server/actors/object/previewers.js
@@ -124,34 +124,35 @@ const previewers = {
     if (hooks.getGripDepth() > 1) {
       return true;
     }
 
     const raw = obj.unsafeDereference();
     const items = grip.preview.items = [];
 
     for (let i = 0; i < length; ++i) {
-      if (raw) {
+      if (raw && !isWorker) {
         // Array Xrays filter out various possibly-unsafe properties (like
         // functions, and claim that the value is undefined instead. This
         // is generally the right thing for privileged code accessing untrusted
         // objects, but quite confusing for Object previews. So we manually
         // override this protection by waiving Xrays on the array, and re-applying
         // Xrays on any indexed value props that we pull off of it.
         const desc = Object.getOwnPropertyDescriptor(Cu.waiveXrays(raw), i);
         if (desc && !desc.get && !desc.set) {
           let value = Cu.unwaiveXrays(desc.value);
           value = ObjectUtils.makeDebuggeeValueIfNeeded(obj, value);
           items.push(hooks.createValueGrip(value));
         } else {
           items.push(null);
         }
       } else {
-        // When recording/replaying we don't have a raw object, but also don't
-        // need to deal with Xrays into the debuggee compartment.
+        // Workers do not have access to Cu, and when recording/replaying we
+        // don't have a raw object. In either case we do not need to deal with
+        // xray wrappers.
         const value = DevToolsUtils.getProperty(obj, i);
         items.push(hooks.createValueGrip(value));
       }
 
       if (items.length == OBJECT_PREVIEW_MAX_ITEMS) {
         break;
       }
     }
@@ -452,18 +453,19 @@ previewers.Object = [
 
     if (hooks.getGripDepth() > 1) {
       return true;
     }
 
     const raw = obj.unsafeDereference();
 
     // The raw object will be null/unavailable when interacting with a
-    // replaying execution.
-    if (raw) {
+    // 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++) {
--- a/devtools/server/actors/object/property-iterator.js
+++ b/devtools/server/actors/object/property-iterator.js
@@ -108,18 +108,21 @@ const PropertyIteratorActor  = protocol.
     return this.slice({ start: 0, count: this.iterator.size });
   },
 });
 
 /**
  * Helper function to create a grip from a Map/Set entry
  */
 function gripFromEntry({ obj, hooks }, entry) {
+  if (!isWorker) {
+    entry = Cu.unwaiveXrays(entry);
+  }
   return hooks.createValueGrip(
-    ObjectUtils.makeDebuggeeValueIfNeeded(obj, Cu.unwaiveXrays(entry)));
+    ObjectUtils.makeDebuggeeValueIfNeeded(obj, entry));
 }
 
 function enumArrayProperties(objectActor, options) {
   return {
     size: ObjectUtils.getArrayLength(objectActor.obj),
     propertyName(index) {
       return index;
     },
@@ -246,33 +249,47 @@ function enumMapEntries(objectActor) {
   // 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();
+  let keys, getValue;
+  if (isWorker) {
+    const keysIterator = DevToolsUtils.callPropertyOnObject(objectActor.obj, "keys");
+    keys = [...DevToolsUtils.makeDebuggeeIterator(keysIterator)];
+    const valuesIterator = DevToolsUtils.callPropertyOnObject(objectActor.obj, "values");
+    const values = [...DevToolsUtils.makeDebuggeeIterator(valuesIterator)];
+    const map = new Map();
+    for (let i = 0; i < keys.length; i++) {
+      map.set(keys[i], values[i]);
+    }
+    getValue = key => map.get(key);
+  } else {
+    const raw = objectActor.obj.unsafeDereference();
+    keys = [...Cu.waiveXrays(Map.prototype.keys.call(raw))];
+    getValue = key => Map.prototype.get.call(raw, key);
+  }
 
-  const keys = [...Cu.waiveXrays(Map.prototype.keys.call(raw))];
   return {
     [Symbol.iterator]: function* () {
       for (const key of keys) {
-        const value = Map.prototype.get.call(raw, key);
+        const value = getValue(key);
         yield [ key, value ].map(val => gripFromEntry(objectActor, val));
       }
     },
     size: keys.length,
     propertyName(index) {
       return index;
     },
     propertyDescription(index) {
       const key = keys[index];
-      const val = Map.prototype.get.call(raw, key);
+      const val = getValue(key);
       return {
         enumerable: true,
         value: {
           type: "mapEntry",
           preview: {
             key: gripFromEntry(objectActor, key),
             value: gripFromEntry(objectActor, val),
           },
@@ -369,18 +386,24 @@ function enumSetEntries(objectActor) {
   // 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 values = [...Cu.waiveXrays(Set.prototype.values.call(raw))];
+  let values;
+  if (isWorker) {
+    const iterator = DevToolsUtils.callPropertyOnObject(objectActor.obj, "values");
+    values = [...DevToolsUtils.makeDebuggeeIterator(iterator)];
+  } else {
+    const raw = objectActor.obj.unsafeDereference();
+    values = [...Cu.waiveXrays(Set.prototype.values.call(raw))];
+  }
 
   return {
     [Symbol.iterator]: function* () {
       for (const item of values) {
         yield gripFromEntry(objectActor, item);
       }
     },
     size: values.length,
--- a/devtools/server/actors/object/utils.js
+++ b/devtools/server/actors/object/utils.js
@@ -166,16 +166,22 @@ function getArrayLength(object) {
   // Real arrays have a reliable `length` own property.
   if (object.class === "Array") {
     return DevToolsUtils.getProperty(object, "length");
   }
 
   // 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.
+  if (isWorker) {
+    // Workers can't wrap debugger values into debuggees, so do the calculations
+    // in the debuggee itself.
+    const getter = object.proto.proto.getOwnPropertyDescriptor("length").get;
+    return getter.call(object).return;
+  }
   const typedProto = Object.getPrototypeOf(Uint8Array.prototype);
   const getter = Object.getOwnPropertyDescriptor(typedProto, "length").get;
   return getter.call(object.unsafeDereference());
 }
 
 /**
  * Returns true if the parameter is suitable to be an array index.
  *
--- a/devtools/server/tests/unit/test_objectgrips-20.js
+++ b/devtools/server/tests/unit/test_objectgrips-20.js
@@ -140,17 +140,30 @@ add_task(threadClientTest(async ({ threa
     expectedNonIndexedProperties: [
       ["foo", "bar"],
       ["bar", "foo"],
       ["length", 2],
       ["buffer", DO_NOT_CHECK_VALUE],
       ["byteLength", 2],
       ["byteOffset", 0],
     ],
-  }, {
+  }];
+
+  for (const test of testCases) {
+    await test_object_grip(debuggee, client, threadClient, test);
+  }
+}));
+
+// These tests are not yet supported in workers.
+add_task(threadClientTest(async ({ threadClient, debuggee, client }) => {
+  debuggee.eval(function stopMe(arg1) {
+    debugger;
+  }.toString());
+
+  const testCases = [{
     evaledObject: `(() => {
       x = new Int8Array([1, 2]);
       Object.defineProperty(x, 'length', {value: 0});
       return x;
     })()`,
     expectedIndexedProperties: [["0", 1], ["1", 2]],
     expectedNonIndexedProperties: [
       ["length", 0],
@@ -166,17 +179,17 @@ add_task(threadClientTest(async ({ threa
     })()`,
     expectedIndexedProperties: [["0", 1], ["1", 2]],
     expectedNonIndexedProperties: [],
   }];
 
   for (const test of testCases) {
     await test_object_grip(debuggee, client, threadClient, test);
   }
-}));
+}, { doNotRunWorker: true }));
 
 async function test_object_grip(debuggee, dbgClient, threadClient, testData = {}) {
   const {
     evaledObject,
     expectedIndexedProperties,
     expectedNonIndexedProperties,
   } = testData;
 
--- a/devtools/shared/DevToolsUtils.js
+++ b/devtools/shared/DevToolsUtils.js
@@ -793,8 +793,22 @@ function callPropertyOnObject(object, na
   }
   if ("throw" in result) {
     throw result.throw;
   }
   return result.return;
 }
 
 exports.callPropertyOnObject = callPropertyOnObject;
+
+// Convert a Debugger.Object wrapping an iterator into an iterator in the
+// debugger's realm.
+function* makeDebuggeeIterator(object) {
+  while (true) {
+    const nextValue = callPropertyOnObject(object, "next");
+    if (exports.getProperty(nextValue, "done")) {
+      break;
+    }
+    yield exports.getProperty(nextValue, "value");
+  }
+}
+
+exports.makeDebuggeeIterator = makeDebuggeeIterator;
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -817,17 +817,18 @@ static bool PreserveWrapper(JSContext* c
 
   return mozilla::dom::TryPreserveWrapper(obj);
 }
 
 JSObject* Wrap(JSContext* cx, JS::HandleObject existing, JS::HandleObject obj) {
   JSObject* targetGlobal = JS::CurrentGlobalOrNull(cx);
   if (!IsWorkerDebuggerGlobal(targetGlobal) &&
       !IsWorkerDebuggerSandbox(targetGlobal)) {
-    MOZ_CRASH("There should be no edges from the debuggee to the debugger.");
+    JS_ReportErrorASCII(cx, "There should be no edges from the debuggee to the debugger.");
+    return nullptr;
   }
 
   // Note: the JS engine unwraps CCWs before calling this callback.
   JSObject* originGlobal = JS::GetNonCCWObjectGlobal(obj);
 
   const js::Wrapper* wrapper = nullptr;
   if (IsWorkerDebuggerGlobal(originGlobal) ||
       IsWorkerDebuggerSandbox(originGlobal)) {