Bug 1002737 - Implement PropDesc::wrapInto as JSCompartment::wrap. (r=jorendorff)
☠☠ backed out by b922ed24938f ☠ ☠
authorEric Faust <efaustbmo@gmail.com>
Tue, 03 Jun 2014 12:37:44 -0700
changeset 205658 8a63bad8faedc052e7011c943dd89647ca858454
parent 205657 5afce70dad1fb3411d40fb5043aa02df9bb0a3ef
child 205659 ec411f0ce167f6b6ad38909b4d9bb4b0e0b00f3b
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1002737
milestone32.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 1002737 - Implement PropDesc::wrapInto as JSCompartment::wrap. (r=jorendorff)
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/vm/Debugger.cpp
js/src/vm/ObjectImpl.cpp
js/src/vm/PropDesc.h
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -490,16 +490,51 @@ JSCompartment::wrap(JSContext *cx, AutoI
     int length = props.length();
     for (size_t n = 0; n < size_t(length); ++n) {
         if (!wrapId(cx, &vector[n]))
             return false;
     }
     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
  * across compartments.
  */
 void
 JSCompartment::markCrossCompartmentWrappers(JSTracer *trc)
 {
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -313,16 +313,17 @@ struct JSCompartment
     bool wrap(JSContext *cx, js::HeapPtrString *strp);
     bool wrap(JSContext *cx, JS::MutableHandleObject obj,
               JS::HandleObject existingArg = js::NullPtr());
     bool wrapId(JSContext *cx, jsid *idp);
     bool wrap(JSContext *cx, js::PropertyOp *op);
     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::WrapperMap::Ptr lookupWrapper(const js::Value& wrapped) {
         return crossCompartmentWrappers.lookup(js::CrossCompartmentKey(wrapped));
     }
 
     void removeWrapper(js::WrapperMap::Ptr p) {
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -5320,26 +5320,28 @@ DebuggerObject_defineProperty(JSContext 
     desc.clearDescriptorObject();
 
     if (!dbg->unwrapPropDescInto(cx, obj, desc, &desc))
         return false;
     if (!desc.checkGetter(cx) || !desc.checkSetter(cx))
         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;
     }
 
     args.rval().setUndefined();
     return true;
 }
 
 static bool
@@ -5371,20 +5373,24 @@ DebuggerObject_defineProperties(JSContex
 
     {
         AutoIdVector rewrappedIds(cx);
         AutoPropDescVector rewrappedDescs(cx);
 
         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 (!cx->compartment()->wrapId(cx, rewrappedIds[i].address()))
                 return false;
-            if (!unwrappedDescs[i].get().wrapInto(cx, obj, ids[i], rewrappedIds[i].address(),
-                                                  rewrappedDescs[i].address()))
+            if (!cx->compartment()->wrap(cx, rewrappedDescs[i]))
+                return false;
+            if (rewrappedDescs[i].descriptorValue().isUndefined() &&
+                !rewrappedDescs[i].makeObject(cx))
             {
                 return false;
             }
         }
 
         ErrorCopier ec(ac, dbg->toJSObject());
         for (size_t i = 0; i < n; i++) {
             bool dummy;
--- a/js/src/vm/ObjectImpl.cpp
+++ b/js/src/vm/ObjectImpl.cpp
@@ -62,47 +62,16 @@ PropDesc::checkSetter(JSContext *cx)
             JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_BAD_GET_SET_FIELD,
                                  js_setter_str);
             return false;
         }
     }
     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. */
 HeapSlot *const js::emptyObjectElements =
     reinterpret_cast<HeapSlot *>(uintptr_t(&emptyElementsHeader) + sizeof(ObjectElements));
 
 #ifdef DEBUG
 
--- a/js/src/vm/PropDesc.h
+++ b/js/src/vm/PropDesc.h
@@ -136,17 +136,18 @@ struct PropDesc {
     bool hasWritable() const { MOZ_ASSERT(!isUndefined()); return hasWritable_; }
     bool hasEnumerable() const { MOZ_ASSERT(!isUndefined()); return hasEnumerable_; }
     bool hasConfigurable() const { MOZ_ASSERT(!isUndefined()); return hasConfigurable_; }
 
     Value descriptorValue() const {
         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; }
 
     /* 8.10.1 IsAccessorDescriptor(desc) */
     bool isAccessorDescriptor() const {
         return !isUndefined() && (hasGet() || hasSet());
     }
 
@@ -234,19 +235,16 @@ struct PropDesc {
 
     /*
      * Throw a TypeError if a getter/setter is present and is neither callable
      * nor undefined. These methods do exactly the type checks that are skipped
      * by passing false as the checkAccessors parameter of initialize.
      */
     bool checkGetter(JSContext *cx);
     bool checkSetter(JSContext *cx);
-
-    bool wrapInto(JSContext *cx, HandleObject obj, const jsid &id, jsid *wrappedId,
-                  PropDesc *wrappedDesc) const;
 };
 
 } /* namespace js */
 
 namespace JS {
 
 template <typename Outer>
 class PropDescOperations
@@ -277,20 +275,16 @@ class PropDescOperations
     HandleValue value() const { return desc()->value(); }
     JSObject *getterObject() const { return desc()->getterObject(); }
     JSObject *setterObject() const { return desc()->setterObject(); }
     HandleValue getterValue() const { return desc()->getterValue(); }
     HandleValue setterValue() const { return desc()->setterValue(); }
 
     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>
 class MutablePropDescOperations : public PropDescOperations<Outer>
 {
     js::PropDesc * desc() { return static_cast<Outer*>(this)->extractMutable(); }
 
   public:
@@ -318,16 +312,18 @@ class MutablePropDescOperations : public
     void setGetter(const Value &getter) {
         desc()->setGetter(getter);
     }
     void setSetter(const Value &setter) {
         desc()->setSetter(setter);
     }
 
     void setUndefined() { desc()->setUndefined(); }
+
+    void setDescriptorObject(JSObject *obj) { desc()->setDescriptorObject(obj); }
     void clearDescriptorObject() { desc()->clearDescriptorObject(); }
 };
 
 } /* namespace JS */
 
 namespace js {
 
 template <>