Bug 1002737 - Implement PropDesc::wrapInto as JSCompartment::wrap. (r=jorendorff)

js/src/jscompartment.cpp

@@ -495,6 +495,41 @@ JSCompartment::wrap(JSContext *cx, AutoIdVector &props)
     return true;
 }
 
+bool
+JSCompartment::wrap(JSContext *cx, MutableHandle<PropDesc> desc)
+{
+    if (desc.isUndefined())
+        return true;
+
+    JSCompartment *comp = cx->compartment();
+
+    if (desc.hasValue()) {
+        RootedValue value(cx, desc.value());
+        if (!comp->wrap(cx, &value))
+            return false;
+        desc.setValue(value);
+    }
+    if (desc.hasGet()) {
+        RootedValue get(cx, desc.getterValue());
+        if (!comp->wrap(cx, &get))
+            return false;
+        desc.setGetter(get);
+    }
+    if (desc.hasSet()) {
+        RootedValue set(cx, desc.setterValue());
+        if (!comp->wrap(cx, &set))
+            return false;
+        desc.setSetter(set);
+    }
+    if (desc.descriptorValue().isObject()) {
+        RootedObject descObj(cx, &desc.descriptorValue().toObject());
+        if (!comp->wrap(cx, &descObj))
+            return false;
+        desc.setDescriptorObject(descObj);
+    }
+    return true;
+}
+
 /*
  * This method marks pointers that cross compartment boundaries. It should be
  * called only for per-compartment GCs, since full GCs naturally follow pointers

js/src/jscompartment.h

@@ -318,6 +318,7 @@ struct JSCompartment
     bool wrap(JSContext *cx, js::StrictPropertyOp *op);
     bool wrap(JSContext *cx, JS::MutableHandle<js::PropertyDescriptor> desc);
     bool wrap(JSContext *cx, js::AutoIdVector &props);
+    bool wrap(JSContext *cx, JS::MutableHandle<js::PropDesc> desc);
 
     bool putWrapper(JSContext *cx, const js::CrossCompartmentKey& wrapped, const js::Value& wrapper);
 

js/src/vm/Debugger.cpp

@@ -5325,16 +5325,18 @@ DebuggerObject_defineProperty(JSContext *cx, unsigned argc, Value *vp)
         return false;
 
     {
-        RootedId wrappedId(cx);
-
         Maybe<AutoCompartment> ac;
         ac.construct(cx, obj);
-        if (!desc.get().wrapInto(cx, obj, id, wrappedId.address(), desc.address()))
+        if (!cx->compartment()->wrapId(cx, id.address()))
+            return false;
+        if (!cx->compartment()->wrap(cx, &desc))
+            return false;
+        if (!desc.makeObject(cx))
             return false;
 
         ErrorCopier ec(ac, dbg->toJSObject());
         bool dummy;
-        if (!DefineProperty(cx, obj, wrappedId, desc, true, &dummy))
+        if (!DefineProperty(cx, obj, id, desc, true, &dummy))
             return false;
     }
 
@@ -5376,10 +5378,14 @@ DebuggerObject_defineProperties(JSContext *cx, unsigned argc, Value *vp)
         Maybe<AutoCompartment> ac;
         ac.construct(cx, obj);
         for (size_t i = 0; i < n; i++) {
-            if (!rewrappedIds.append(JSID_VOID) || !rewrappedDescs.append(PropDesc()))
+            if (!rewrappedIds.append(ids[i]) || !rewrappedDescs.append(unwrappedDescs[i]))
                 return false;
-            if (!unwrappedDescs[i].get().wrapInto(cx, obj, ids[i], rewrappedIds[i].address(),
-                                                  rewrappedDescs[i].address()))
+            if (!cx->compartment()->wrapId(cx, rewrappedIds[i].address()))
+                return false;
+            if (!cx->compartment()->wrap(cx, rewrappedDescs[i]))
+                return false;
+            if (rewrappedDescs[i].descriptorValue().isUndefined() &&
+                !rewrappedDescs[i].makeObject(cx))
             {
                 return false;
             }

js/src/vm/ObjectImpl.cpp

@@ -67,37 +67,6 @@ PropDesc::checkSetter(JSContext *cx)
     return true;
 }
 
-/*
- * Rewrap *idp and the fields of *desc for the current compartment.  Also:
- * defining a property on a proxy requires pd_ to contain a descriptor object,
- * so reconstitute desc->pd_ if needed.
- */
-bool
-PropDesc::wrapInto(JSContext *cx, HandleObject obj, const jsid &id, jsid *wrappedId,
-                   PropDesc *desc) const
-{
-    MOZ_ASSERT(!isUndefined());
-
-    JSCompartment *comp = cx->compartment();
-
-    *wrappedId = id;
-    if (!comp->wrapId(cx, wrappedId))
-        return false;
-
-    *desc = *this;
-    RootedValue value(cx, desc->value_);
-    RootedValue get(cx, desc->get_);
-    RootedValue set(cx, desc->set_);
-
-    if (!comp->wrap(cx, &value) || !comp->wrap(cx, &get) || !comp->wrap(cx, &set))
-        return false;
-
-    desc->value_ = value;
-    desc->get_ = get;
-    desc->set_ = set;
-    return !obj->is<ProxyObject>() || desc->makeObject(cx);
-}
-
 static const ObjectElements emptyElementsHeader(0, 0);
 
 /* Objects with no elements share one empty set of elements. */

js/src/vm/PropDesc.h

@@ -141,7 +141,8 @@ struct PropDesc {
         MOZ_ASSERT(!isUndefined());
         return descObj_ ? ObjectValue(*descObj_) : UndefinedValue();
     }
-    void clearDescriptorObject() { descObj_ = nullptr; }
+    void setDescriptorObject(JSObject *obj) { descObj_ = obj; }
+    void clearDescriptorObject() { setDescriptorObject(nullptr); }
 
     uint8_t attributes() const { MOZ_ASSERT(!isUndefined()); return attrs; }
 
@@ -239,9 +240,6 @@ struct PropDesc {
      */
     bool checkGetter(JSContext *cx);
     bool checkSetter(JSContext *cx);
-
-    bool wrapInto(JSContext *cx, HandleObject obj, const jsid &id, jsid *wrappedId,
-                  PropDesc *wrappedDesc) const;
 };
 
 } /* namespace js */
@@ -282,10 +280,6 @@ class PropDescOperations
 
     JSPropertyOp getter() const { return desc()->getter(); }
     JSStrictPropertyOp setter() const { return desc()->setter(); }
-
-    // We choose not to expose the debugger-specific parts of PropDesc, both
-    // because they are not really general use, but also because they are a
-    // pain to expose.
 };
 
 template <typename Outer>
@@ -323,6 +317,8 @@ class MutablePropDescOperations : public PropDescOperations<Outer>
     }
 
     void setUndefined() { desc()->setUndefined(); }
+
+    void setDescriptorObject(JSObject *obj) { desc()->setDescriptorObject(obj); }
     void clearDescriptorObject() { desc()->clearDescriptorObject(); }
 };