Bug 1002737 - Implement PropDesc::wrapInto as JSCompartment::wrap. (r=jorendorff)
authorEric Faust <efaustbmo@gmail.com>
Tue, 03 Jun 2014 12:37:44 -0700
changeset 207303 1374cc1b03c0c5efbffee8b8557af26597c9c30c
parent 207302 a98a39b58a54faf6e343c947e7a5dce23d3b6a72
child 207304 4db799c3cee9b520d1b99d7175924ddfce9c3674
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [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
@@ -5337,26 +5337,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
@@ -5388,20 +5390,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
@@ -137,17 +137,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());
     }
 
@@ -235,19 +236,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
@@ -278,20 +276,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:
@@ -319,16 +313,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 <>