Backed out changeset e4279ded250d (bug 997894)
authorEd Morley <emorley@mozilla.com>
Wed, 04 Jun 2014 18:49:31 +0100
changeset 206960 39e24f12b36b223612e5395ef98e1e4b3feeaccf
parent 206959 a82637fc77c9cab4bbef29c1e924e913874a3a37
child 206961 414d239d29212c78fdf9f5801f5bdba7b7024362
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)
bugs997894
milestone32.0a1
backs oute4279ded250df37ef3208ff33770927bd36e1584
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
Backed out changeset e4279ded250d (bug 997894)
js/src/jsobj.cpp
js/src/jsproxy.cpp
js/src/vm/Debugger.cpp
js/src/vm/ObjectImpl.cpp
js/src/vm/PropDesc.h
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -258,17 +258,18 @@ bool
 js::NewPropertyDescriptorObject(JSContext *cx, Handle<PropertyDescriptor> desc,
                                 MutableHandleValue vp)
 {
     if (!desc.object()) {
         vp.setUndefined();
         return true;
     }
 
-    Rooted<PropDesc> d(cx);
+    /* We have our own property, so start creating the descriptor. */
+    AutoPropDescRooter d(cx);
 
     d.initFromPropertyDescriptor(desc);
     if (!d.makeObject(cx))
         return false;
     vp.set(d.pd());
     return true;
 }
 
@@ -1056,37 +1057,41 @@ js::DefineProperty(JSContext *cx, Handle
 
     return DefinePropertyOnObject(cx, obj, id, desc, throwError, rval);
 }
 
 bool
 js::DefineOwnProperty(JSContext *cx, HandleObject obj, HandleId id, HandleValue descriptor,
                       bool *bp)
 {
-    Rooted<PropDesc> desc(cx);
-    if (!desc.initialize(cx, descriptor))
+    AutoPropDescArrayRooter descs(cx);
+    PropDesc *desc = descs.append();
+    if (!desc || !desc->initialize(cx, descriptor))
         return false;
 
     bool rval;
-    if (!DefineProperty(cx, obj, id, desc, true, &rval))
+    if (!DefineProperty(cx, obj, id, *desc, true, &rval))
         return false;
     *bp = !!rval;
     return true;
 }
 
 bool
 js::DefineOwnProperty(JSContext *cx, HandleObject obj, HandleId id,
                       Handle<PropertyDescriptor> descriptor, bool *bp)
 {
-    Rooted<PropDesc> desc(cx);
-
-    desc.initFromPropertyDescriptor(descriptor);
+    AutoPropDescArrayRooter descs(cx);
+    PropDesc *desc = descs.append();
+    if (!desc)
+        return false;
+
+    desc->initFromPropertyDescriptor(descriptor);
 
     bool rval;
-    if (!DefineProperty(cx, obj, id, desc, true, &rval))
+    if (!DefineProperty(cx, obj, id, *desc, true, &rval))
         return false;
     *bp = !!rval;
     return true;
 }
 
 
 bool
 js::ReadPropertyDescriptors(JSContext *cx, HandleObject props, bool checkAccessors,
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -673,26 +673,27 @@ Trap2(JSContext *cx, HandleObject handle
     argv[1].set(v);
     return Trap(cx, handler, fval, 2, argv.begin(), rval);
 }
 
 static bool
 ParsePropertyDescriptorObject(JSContext *cx, HandleObject obj, const Value &v,
                               MutableHandle<PropertyDescriptor> desc, bool complete = false)
 {
-    Rooted<PropDesc> d(cx);
-    if (!d.initialize(cx, v))
+    AutoPropDescArrayRooter descs(cx);
+    PropDesc *d = descs.append();
+    if (!d || !d->initialize(cx, v))
         return false;
     if (complete)
-        d.complete();
+        d->complete();
     desc.object().set(obj);
-    desc.value().set(d.hasValue() ? d.value() : UndefinedValue());
-    desc.setAttributes(d.attributes());
-    desc.setGetter(d.getter());
-    desc.setSetter(d.setter());
+    desc.value().set(d->hasValue() ? d->value() : UndefinedValue());
+    desc.setAttributes(d->attributes());
+    desc.setGetter(d->getter());
+    desc.setSetter(d->setter());
     return true;
 }
 
 static bool
 IndicatePropertyNotFound(MutableHandle<PropertyDescriptor> desc)
 {
     desc.object().set(nullptr);
     return true;
@@ -1095,28 +1096,28 @@ class ScriptedDirectProxyHandler : publi
     static ScriptedDirectProxyHandler singleton;
 };
 
 // This variable exists solely to provide a unique address for use as an identifier.
 static const char sScriptedDirectProxyHandlerFamily = 0;
 
 // Aux.2 FromGenericPropertyDescriptor(Desc)
 static bool
-FromGenericPropertyDescriptor(JSContext *cx, MutableHandle<PropDesc> desc, MutableHandleValue rval)
+FromGenericPropertyDescriptor(JSContext *cx, PropDesc *desc, MutableHandleValue rval)
 {
     // Aux.2 step 1
-    if (desc.isUndefined()) {
+    if (desc->isUndefined()) {
         rval.setUndefined();
         return true;
     }
 
     // steps 3-9
-    if (!desc.makeObject(cx))
+    if (!desc->makeObject(cx))
         return false;
-    rval.set(desc.pd());
+    rval.set(desc->pd());
     return true;
 }
 
 /*
  * Aux.3 NormalizePropertyDescriptor(Attributes)
  *
  * NOTE: to minimize code duplication, the code for this function is shared with
  * that for Aux.4 NormalizeAndCompletePropertyDescriptor (see below). The
@@ -1125,34 +1126,35 @@ FromGenericPropertyDescriptor(JSContext 
 static bool
 NormalizePropertyDescriptor(JSContext *cx, MutableHandleValue vp, bool complete = false)
 {
     // Aux.4 step 1
     if (complete && vp.isUndefined())
         return true;
 
     // Aux.3 steps 1-2 / Aux.4 steps 2-3
-    Rooted<PropDesc> desc(cx);
-    if (!desc.initialize(cx, vp.get()))
+    AutoPropDescArrayRooter descs(cx);
+    PropDesc *desc = descs.append();
+    if (!desc || !desc->initialize(cx, vp.get()))
         return false;
     if (complete)
-        desc.complete();
+        desc->complete();
     JS_ASSERT(vp.isObject()); // due to desc->initialize
     RootedObject attributes(cx, &vp.toObject());
 
     /*
      * Aux.3 step 3 / Aux.4 step 4
      *
      * NOTE: Aux.4 step 4 actually specifies FromPropertyDescriptor here.
      * However, the way FromPropertyDescriptor is implemented (PropDesc::
      * makeObject) is actually closer to FromGenericPropertyDescriptor,
      * and is in fact used to implement the latter, so we might as well call it
      * directly.
      */
-    if (!FromGenericPropertyDescriptor(cx, &desc, vp))
+    if (!FromGenericPropertyDescriptor(cx, desc, vp))
         return false;
     if (vp.isUndefined())
         return true;
     RootedObject descObj(cx, &vp.toObject());
 
     // Aux.3 steps 4-5 / Aux.4 steps 5-6
     AutoIdVector props(cx);
     if (!GetPropertyNames(cx, attributes, 0, &props))
@@ -1196,114 +1198,114 @@ IsDataDescriptor(const PropertyDescripto
 static inline bool
 IsAccessorDescriptor(const PropertyDescriptor &desc)
 {
     return desc.obj && desc.attrs & (JSPROP_GETTER | JSPROP_SETTER);
 }
 
 // Aux.5 ValidateProperty(O, P, Desc)
 static bool
-ValidateProperty(JSContext *cx, HandleObject obj, HandleId id, Handle<PropDesc> desc, bool *bp)
+ValidateProperty(JSContext *cx, HandleObject obj, HandleId id, PropDesc *desc, bool *bp)
 {
     // step 1
     Rooted<PropertyDescriptor> current(cx);
     if (!GetOwnPropertyDescriptor(cx, obj, id, &current))
         return false;
 
     /*
      * steps 2-4 are redundant since ValidateProperty is never called unless
      * target.[[HasOwn]](P) is true
      */
     JS_ASSERT(current.object());
 
     // step 5
-    if (!desc.hasValue() && !desc.hasWritable() && !desc.hasGet() && !desc.hasSet() &&
-        !desc.hasEnumerable() && !desc.hasConfigurable())
+    if (!desc->hasValue() && !desc->hasWritable() && !desc->hasGet() && !desc->hasSet() &&
+        !desc->hasEnumerable() && !desc->hasConfigurable())
     {
         *bp = true;
         return true;
     }
 
     // step 6
-    if ((!desc.hasWritable() || desc.writable() == !current.isReadonly()) &&
-        (!desc.hasGet() || desc.getter() == current.getter()) &&
-        (!desc.hasSet() || desc.setter() == current.setter()) &&
-        (!desc.hasEnumerable() || desc.enumerable() == current.isEnumerable()) &&
-        (!desc.hasConfigurable() || desc.configurable() == !current.isPermanent()))
+    if ((!desc->hasWritable() || desc->writable() == !current.isReadonly()) &&
+        (!desc->hasGet() || desc->getter() == current.getter()) &&
+        (!desc->hasSet() || desc->setter() == current.setter()) &&
+        (!desc->hasEnumerable() || desc->enumerable() == current.isEnumerable()) &&
+        (!desc->hasConfigurable() || desc->configurable() == !current.isPermanent()))
     {
-        if (!desc.hasValue()) {
+        if (!desc->hasValue()) {
             *bp = true;
             return true;
         }
         bool same = false;
-        if (!SameValue(cx, desc.value(), current.value(), &same))
+        if (!SameValue(cx, desc->value(), current.value(), &same))
             return false;
         if (same) {
             *bp = true;
             return true;
         }
     }
 
     // step 7
     if (current.isPermanent()) {
-        if (desc.hasConfigurable() && desc.configurable()) {
+        if (desc->hasConfigurable() && desc->configurable()) {
             *bp = false;
             return true;
         }
 
-        if (desc.hasEnumerable() &&
-            desc.enumerable() != current.isEnumerable())
+        if (desc->hasEnumerable() &&
+            desc->enumerable() != current.isEnumerable())
         {
             *bp = false;
             return true;
         }
     }
 
     // step 8
-    if (desc.isGenericDescriptor()) {
+    if (desc->isGenericDescriptor()) {
         *bp = true;
         return true;
     }
 
     // step 9
-    if (IsDataDescriptor(current) != desc.isDataDescriptor()) {
+    if (IsDataDescriptor(current) != desc->isDataDescriptor()) {
         *bp = !current.isPermanent();
         return true;
     }
 
     // step 10
     if (IsDataDescriptor(current)) {
-        JS_ASSERT(desc.isDataDescriptor()); // by step 9
+        JS_ASSERT(desc->isDataDescriptor()); // by step 9
         if (current.isPermanent() && current.isReadonly()) {
-            if (desc.hasWritable() && desc.writable()) {
+            if (desc->hasWritable() && desc->writable()) {
                 *bp = false;
                 return true;
             }
 
-            if (desc.hasValue()) {
+            if (desc->hasValue()) {
                 bool same;
-                if (!SameValue(cx, desc.value(), current.value(), &same))
+                if (!SameValue(cx, desc->value(), current.value(), &same))
                     return false;
                 if (!same) {
                     *bp = false;
                     return true;
                 }
             }
         }
 
         *bp = true;
         return true;
     }
 
     // steps 11-12
     JS_ASSERT(IsAccessorDescriptor(current)); // by step 10
-    JS_ASSERT(desc.isAccessorDescriptor()); // by step 9
+    JS_ASSERT(desc->isAccessorDescriptor()); // by step 9
     *bp = (!current.isPermanent() ||
-           ((!desc.hasSet() || desc.setter() == current.setter()) &&
-            (!desc.hasGet() || desc.getter() == current.getter())));
+           ((!desc->hasSet() || desc->setter() == current.setter()) &&
+            (!desc->hasGet() || desc->getter() == current.getter())));
     return true;
 }
 
 // Aux.6 IsSealed(O, P)
 static bool
 IsSealed(JSContext* cx, HandleObject obj, HandleId id, bool *bp)
 {
     // step 1
@@ -1422,34 +1424,35 @@ TrapGetOwnProperty(JSContext *cx, Handle
     bool extensible;
     if (!JSObject::isExtensible(cx, target, &extensible))
         return false;
     if (!extensible && !isFixed) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NEW);
         return false;
     }
 
-    Rooted<PropDesc> desc(cx);
-    if (!desc.initialize(cx, trapResult))
+    AutoPropDescArrayRooter descs(cx);
+    PropDesc *desc = descs.append();
+    if (!desc || !desc->initialize(cx, trapResult))
         return false;
 
     /* step 10 */
     if (isFixed) {
         bool valid;
         if (!ValidateProperty(cx, target, id, desc, &valid))
             return false;
 
         if (!valid) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_INVALID);
             return false;
         }
     }
 
     // step 11
-    if (!desc.configurable() && !isFixed) {
+    if (!desc->configurable() && !isFixed) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NE_AS_NC);
         return false;
     }
 
     // step 12
     rval.set(trapResult);
     return true;
 }
@@ -1505,31 +1508,32 @@ TrapDefineOwnProperty(JSContext *cx, Han
         bool extensible;
         if (!JSObject::isExtensible(cx, target, &extensible))
             return false;
         if (!extensible && !isFixed) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_NEW);
             return false;
         }
 
-        Rooted<PropDesc> desc(cx);
-        if (!desc.initialize(cx, normalizedDesc))
+        AutoPropDescArrayRooter descs(cx);
+        PropDesc *desc = descs.append();
+        if (!desc || !desc->initialize(cx, normalizedDesc))
             return false;
 
         if (isFixed) {
             bool valid;
             if (!ValidateProperty(cx, target, id, desc, &valid))
                 return false;
             if (!valid) {
                 JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_INVALID);
                 return false;
             }
         }
 
-        if (!desc.configurable() && !isFixed) {
+        if (!desc->configurable() && !isFixed) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_NE_AS_NC);
             return false;
         }
 
         vp.set(BooleanValue(true));
         return true;
     }
 
@@ -1744,20 +1748,21 @@ ScriptedDirectProxyHandler::getOwnProper
     return ParsePropertyDescriptorObject(cx, proxy, v, desc, true);
 }
 
 bool
 ScriptedDirectProxyHandler::defineProperty(JSContext *cx, HandleObject proxy, HandleId id,
                                            MutableHandle<PropertyDescriptor> desc)
 {
     // step 1
-    Rooted<PropDesc> d(cx);
-    d.initFromPropertyDescriptor(desc);
+    AutoPropDescArrayRooter descs(cx);
+    PropDesc *d = descs.append();
+    d->initFromPropertyDescriptor(desc);
     RootedValue v(cx);
-    if (!FromGenericPropertyDescriptor(cx, &d, &v))
+    if (!FromGenericPropertyDescriptor(cx, d, &v))
         return false;
 
     // step 2
     return TrapDefineOwnProperty(cx, proxy, id, &v);
 }
 
 bool
 ScriptedDirectProxyHandler::getOwnPropertyNames(JSContext *cx, HandleObject proxy,
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -5252,37 +5252,44 @@ DebuggerObject_defineProperty(JSContext 
 {
     THIS_DEBUGOBJECT_OWNER_REFERENT(cx, argc, vp, "defineProperty", args, dbg, obj);
     REQUIRE_ARGC("Debugger.Object.defineProperty", 2);
 
     RootedId id(cx);
     if (!ValueToId<CanGC>(cx, args[0], &id))
         return false;
 
-    Rooted<PropDesc> desc(cx);
-    if (!desc.initialize(cx, args[1], false))
+    AutoPropDescArrayRooter descs(cx);
+    if (!descs.reserve(3)) // desc, unwrappedDesc, rewrappedDesc
+        return false;
+    PropDesc *desc = descs.append();
+    if (!desc || !desc->initialize(cx, args[1], false))
         return false;
-    desc.clearPd();
-
-    if (!desc.get().unwrapDebuggerObjectsInto(cx, dbg, obj, desc.address()))
+    desc->clearPd();
+
+    PropDesc *unwrappedDesc = descs.append();
+    if (!unwrappedDesc || !desc->unwrapDebuggerObjectsInto(cx, dbg, obj, unwrappedDesc))
         return false;
-    if (!desc.checkGetter(cx) || !desc.checkSetter(cx))
+    if (!unwrappedDesc->checkGetter(cx) || !unwrappedDesc->checkSetter(cx))
         return false;
 
     {
+        PropDesc *rewrappedDesc = descs.append();
+        if (!rewrappedDesc)
+            return false;
         RootedId wrappedId(cx);
 
         Maybe<AutoCompartment> ac;
         ac.construct(cx, obj);
-        if (!desc.get().wrapInto(cx, obj, id, wrappedId.address(), desc.address()))
+        if (!unwrappedDesc->wrapInto(cx, obj, id, wrappedId.address(), rewrappedDesc))
             return false;
 
         ErrorCopier ec(ac, dbg->toJSObject());
         bool dummy;
-        if (!DefineProperty(cx, obj, wrappedId, desc, true, &dummy))
+        if (!DefineProperty(cx, obj, wrappedId, *rewrappedDesc, true, &dummy))
             return false;
     }
 
     args.rval().setUndefined();
     return true;
 }
 
 static bool
--- a/js/src/vm/ObjectImpl.cpp
+++ b/js/src/vm/ObjectImpl.cpp
@@ -375,15 +375,24 @@ js::ObjectImpl::markChildren(JSTracer *t
 
     if (shape_->isNative()) {
         MarkObjectSlots(trc, obj, 0, obj->slotSpan());
         gc::MarkArraySlots(trc, obj->getDenseInitializedLength(), obj->getDenseElements(), "objectElements");
     }
 }
 
 void
+AutoPropDescRooter::trace(JSTracer *trc)
+{
+    gc::MarkValueRoot(trc, &propDesc.pd_, "AutoPropDescRooter pd");
+    gc::MarkValueRoot(trc, &propDesc.value_, "AutoPropDescRooter value");
+    gc::MarkValueRoot(trc, &propDesc.get_, "AutoPropDescRooter get");
+    gc::MarkValueRoot(trc, &propDesc.set_, "AutoPropDescRooter set");
+}
+
+void
 PropDesc::trace(JSTracer *trc)
 {
     gc::MarkValueRoot(trc, &pd_, "PropDesc pd");
     gc::MarkValueRoot(trc, &value_, "PropDesc value");
     gc::MarkValueRoot(trc, &get_, "PropDesc get");
     gc::MarkValueRoot(trc, &set_, "PropDesc set");
 }
--- a/js/src/vm/PropDesc.h
+++ b/js/src/vm/PropDesc.h
@@ -61,16 +61,17 @@ struct PropDesc {
         attrs(0),
         hasGet_(false), hasSet_(false),
         hasValue_(true), hasWritable_(false), hasEnumerable_(false), hasConfigurable_(false),
         isUndefined_(false)
     {
     }
 
   public:
+    friend class AutoPropDescRooter;
     friend void JS::AutoGCRooter::trace(JSTracer *trc);
     friend struct GCMethods<PropDesc>;
 
     void trace(JSTracer *trc);
 
     enum Enumerability { Enumerable = true, NonEnumerable = false };
     enum Configurability { Configurable = true, NonConfigurable = false };
     enum Writability { Writable = true, NonWritable = false };
@@ -226,16 +227,74 @@ struct PropDesc {
 
     bool unwrapDebuggerObjectsInto(JSContext *cx, Debugger *dbg, HandleObject obj,
                                    PropDesc *unwrapped) const;
 
     bool wrapInto(JSContext *cx, HandleObject obj, const jsid &id, jsid *wrappedId,
                   PropDesc *wrappedDesc) const;
 };
 
+class AutoPropDescRooter : private JS::CustomAutoRooter
+{
+  public:
+    explicit AutoPropDescRooter(JSContext *cx
+                                MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+      : CustomAutoRooter(cx)
+    {
+        MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    }
+
+    PropDesc& getPropDesc() { return propDesc; }
+
+    void initFromPropertyDescriptor(Handle<JSPropertyDescriptor> desc) {
+        propDesc.initFromPropertyDescriptor(desc);
+    }
+
+    bool makeObject(JSContext *cx) {
+        return propDesc.makeObject(cx);
+    }
+
+    void setUndefined() { propDesc.setUndefined(); }
+    bool isUndefined() const { return propDesc.isUndefined(); }
+
+    bool hasGet() const { return propDesc.hasGet(); }
+    bool hasSet() const { return propDesc.hasSet(); }
+    bool hasValue() const { return propDesc.hasValue(); }
+    bool hasWritable() const { return propDesc.hasWritable(); }
+    bool hasEnumerable() const { return propDesc.hasEnumerable(); }
+    bool hasConfigurable() const { return propDesc.hasConfigurable(); }
+
+    Value pd() const { return propDesc.pd(); }
+    void clearPd() { propDesc.clearPd(); }
+
+    uint8_t attributes() const { return propDesc.attributes(); }
+
+    bool isAccessorDescriptor() const { return propDesc.isAccessorDescriptor(); }
+    bool isDataDescriptor() const { return propDesc.isDataDescriptor(); }
+    bool isGenericDescriptor() const { return propDesc.isGenericDescriptor(); }
+    bool configurable() const { return propDesc.configurable(); }
+    bool enumerable() const { return propDesc.enumerable(); }
+    bool writable() const { return propDesc.writable(); }
+
+    HandleValue value() const { return propDesc.value(); }
+    JSObject *getterObject() const { return propDesc.getterObject(); }
+    JSObject *setterObject() const { return propDesc.setterObject(); }
+    HandleValue getterValue() const { return propDesc.getterValue(); }
+    HandleValue setterValue() const { return propDesc.setterValue(); }
+
+    JSPropertyOp getter() const { return propDesc.getter(); }
+    JSStrictPropertyOp setter() const { return propDesc.setter(); }
+
+  private:
+    virtual void trace(JSTracer *trc);
+
+    PropDesc propDesc;
+    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+};
+
 } /* namespace js */
 
 namespace JS {
 
 template <typename Outer>
 class PropDescOperations
 {
     const js::PropDesc * desc() const { return static_cast<const Outer*>(this)->extract(); }