Backed out changeset 94f14d6b26d5 (bug 1125624)
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Wed, 17 Jun 2015 11:02:59 +0200
changeset 267414 0c43e4255e88976832574b1695ab3d9c19e77a94
parent 267413 ed587a5aaecf2afd6313a1c6aa230210811e7603
child 267415 65e6e21a47251b33d2b0a131530333707bde5ad9
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-esr52@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1125624
milestone41.0a1
backs out94f14d6b26d5e6c060e965c0982708e63d27db66
Backed out changeset 94f14d6b26d5 (bug 1125624)
dom/bindings/DOMJSProxyHandler.cpp
js/ipc/WrapperAnswer.cpp
js/src/builtin/Object.cpp
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
js/src/jsobj.cpp
js/src/jsobj.h
js/src/json.cpp
js/src/proxy/DirectProxyHandler.cpp
js/src/proxy/ScriptedDirectProxyHandler.cpp
js/src/vm/Debugger.cpp
js/src/vm/SelfHosting.cpp
js/src/vm/UnboxedObject.cpp
--- a/dom/bindings/DOMJSProxyHandler.cpp
+++ b/dom/bindings/DOMJSProxyHandler.cpp
@@ -176,22 +176,22 @@ DOMProxyHandler::defineProperty(JSContex
   if (desc.hasGetterObject() && desc.setter() == JS_StrictPropertyStub) {
     return result.failGetterOnly();
   }
 
   if (xpc::WrapperFactory::IsXrayWrapper(proxy)) {
     return result.succeed();
   }
 
-  JS::Rooted<JSObject*> expando(cx, EnsureExpandoObject(cx, proxy));
+  JSObject* expando = EnsureExpandoObject(cx, proxy);
   if (!expando) {
     return false;
   }
 
-  if (!JS_DefinePropertyById(cx, expando, id, desc, result)) {
+  if (!js::DefineOwnProperty(cx, expando, id, desc, result)) {
     return false;
   }
   *defined = true;
   return true;
 }
 
 bool
 DOMProxyHandler::set(JSContext *cx, Handle<JSObject*> proxy, Handle<jsid> id,
--- a/js/ipc/WrapperAnswer.cpp
+++ b/js/ipc/WrapperAnswer.cpp
@@ -184,17 +184,17 @@ WrapperAnswer::RecvDefineProperty(const 
     if (!fromJSIDVariant(cx, idVar, &id))
         return fail(jsapi, rs);
 
     Rooted<JSPropertyDescriptor> desc(cx);
     if (!toDescriptor(cx, descriptor, &desc))
         return fail(jsapi, rs);
 
     ObjectOpResult success;
-    if (!JS_DefinePropertyById(cx, obj, id, desc, success))
+    if (!js::DefineOwnProperty(cx, obj, id, desc, success))
         return fail(jsapi, rs);
     return ok(rs, success);
 }
 
 bool
 WrapperAnswer::RecvDelete(const ObjectId& objId, const JSIDVariant& idVar, ReturnStatus* rs)
 {
     AutoJSAPI jsapi;
--- a/js/src/builtin/Object.cpp
+++ b/js/src/builtin/Object.cpp
@@ -827,17 +827,17 @@ js::obj_defineProperty(JSContext* cx, un
         return false;
 
     // Steps 4-5.
     Rooted<PropertyDescriptor> desc(cx);
     if (!ToPropertyDescriptor(cx, args.get(2), true, &desc))
         return false;
 
     // Steps 6-8.
-    if (!DefineProperty(cx, obj, id, desc))
+    if (!StandardDefineProperty(cx, obj, id, desc))
         return false;
     args.rval().setObject(*obj);
     return true;
 }
 
 /* ES5 15.2.3.7: Object.defineProperties(O, Properties) */
 static bool
 obj_defineProperties(JSContext* cx, unsigned argc, Value* vp)
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -1196,16 +1196,33 @@ js::GetObjectMetadata(JSObject* obj)
 {
     ObjectWeakMap* map = obj->compartment()->objectMetadataTable;
     if (map)
         return map->lookup(obj);
     return nullptr;
 }
 
 JS_FRIEND_API(bool)
+js::DefineOwnProperty(JSContext* cx, JSObject* objArg, jsid idArg,
+                      JS::Handle<js::PropertyDescriptor> descriptor, ObjectOpResult& result)
+{
+    RootedObject obj(cx, objArg);
+    RootedId id(cx, idArg);
+    AssertHeapIsIdle(cx);
+    CHECK_REQUEST(cx);
+    assertSameCompartment(cx, obj, id, descriptor.value());
+    if (descriptor.hasGetterObject())
+        assertSameCompartment(cx, descriptor.getterObject());
+    if (descriptor.hasSetterObject())
+        assertSameCompartment(cx, descriptor.setterObject());
+
+    return StandardDefineProperty(cx, obj, id, descriptor, result);
+}
+
+JS_FRIEND_API(bool)
 js::ReportIsNotFunction(JSContext* cx, HandleValue v)
 {
     return ReportIsNotFunction(cx, v, -1);
 }
 
 JS_FRIEND_API(void)
 js::ReportErrorWithId(JSContext* cx, const char* msg, HandleId id)
 {
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -2807,16 +2807,20 @@ GetSavedFramePrincipals(JS::HandleObject
  * The savedFrame and cx do not need to be in the same compartment.
  */
 extern JS_FRIEND_API(JSObject*)
 GetFirstSubsumedSavedFrame(JSContext* cx, JS::HandleObject savedFrame);
 
 extern JS_FRIEND_API(bool)
 ReportIsNotFunction(JSContext* cx, JS::HandleValue v);
 
+extern JS_FRIEND_API(bool)
+DefineOwnProperty(JSContext* cx, JSObject* objArg, jsid idArg,
+                  JS::Handle<JSPropertyDescriptor> descriptor, JS::ObjectOpResult& result);
+
 extern JS_FRIEND_API(JSObject*)
 ConvertArgsToArray(JSContext* cx, const JS::CallArgs& args);
 
 } /* namespace js */
 
 extern JS_FRIEND_API(void)
 JS_StoreObjectPostBarrierCallback(JSContext* cx,
                                   void (*callback)(JSTracer* trc, JSObject* key, void* data),
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -269,17 +269,33 @@ js::Throw(JSContext* cx, JSObject* obj, 
     } else {
         MOZ_ASSERT(js_ErrorFormatString[errorNumber].argCount == 0);
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, errorNumber);
     }
     return false;
 }
 
 
-/*** PropertyDescriptor operations and DefineProperties ******************************************/
+/*** Standard-compliant property definition (used by Object.defineProperty) **********************/
+
+bool
+js::StandardDefineProperty(JSContext* cx, HandleObject obj, HandleId id,
+                           Handle<PropertyDescriptor> desc, ObjectOpResult& result)
+{
+    return DefineProperty(cx, obj, id, desc, result);
+}
+
+bool
+js::StandardDefineProperty(JSContext* cx, HandleObject obj, HandleId id,
+                           Handle<PropertyDescriptor> desc)
+{
+    ObjectOpResult success;
+    return DefineProperty(cx, obj, id, desc, success) &&
+           success.checkStrict(cx, obj, id);
+}
 
 bool
 CheckCallable(JSContext* cx, JSObject* obj, const char* fieldName)
 {
     if (obj && !obj->isCallable()) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_BAD_GET_SET_FIELD,
                              fieldName);
         return false;
@@ -466,24 +482,23 @@ bool
 js::DefineProperties(JSContext* cx, HandleObject obj, HandleObject props)
 {
     AutoIdVector ids(cx);
     AutoPropertyDescriptorVector descs(cx);
     if (!ReadPropertyDescriptors(cx, props, true, &ids, &descs))
         return false;
 
     for (size_t i = 0, len = ids.length(); i < len; i++) {
-        if (!DefineProperty(cx, obj, ids[i], descs[i]))
+        if (!StandardDefineProperty(cx, obj, ids[i], descs[i]))
             return false;
     }
 
     return true;
 }
 
-
 /*** Seal and freeze *****************************************************************************/
 
 static unsigned
 GetSealedOrFrozenAttributes(unsigned attrs, IntegrityLevel level)
 {
     /* Make all attributes permanent; if freezing, make data attributes read-only. */
     if (level == IntegrityLevel::Frozen && !(attrs & (JSPROP_GETTER | JSPROP_SETTER)))
         return JSPROP_PERMANENT | JSPROP_READONLY;
@@ -576,25 +591,25 @@ js::SetIntegrityLevel(JSContext* cx, Han
                 // 9.a.iii.1-2
                 if (currentDesc.isAccessorDescriptor())
                     desc.setAttributes(AllowConfigure | JSPROP_PERMANENT);
                 else
                     desc.setAttributes(AllowConfigureAndWritable | JSPROP_PERMANENT | JSPROP_READONLY);
             }
 
             // 8.a.i-ii. / 9.a.iii.3-4
-            if (!DefineProperty(cx, obj, id, desc))
+            if (!StandardDefineProperty(cx, obj, id, desc))
                 return false;
         }
     }
 
     // Ordinarily ArraySetLength handles this, but we're going behind its back
     // right now, so we must do this manually.  Neither the custom property
-    // tree mutations nor the DefineProperty call in the above code will do
-    // this for us.
+    // tree mutations nor the StandardDefineProperty call in the above code will
+    // do this for us.
     //
     // ArraySetLength also implements the capacity <= length invariant for
     // arrays with non-writable length.  We don't need to do anything special
     // for that, because capacity was zeroed out by preventExtensions.  (See
     // the assertion before the if-else above.)
     if (level == IntegrityLevel::Frozen && obj->is<ArrayObject>()) {
         if (!obj->as<ArrayObject>().maybeCopyElementsForWrite(cx))
             return false;
@@ -1090,17 +1105,17 @@ JS_CopyPropertyFrom(JSContext* cx, Handl
         desc.attributesRef() &= ~JSPROP_PERMANENT;
     }
 
     JSAutoCompartment ac(cx, target);
     RootedId wrappedId(cx, id);
     if (!cx->compartment()->wrap(cx, &desc))
         return false;
 
-    return DefineProperty(cx, target, wrappedId, desc);
+    return StandardDefineProperty(cx, target, wrappedId, desc);
 }
 
 JS_FRIEND_API(bool)
 JS_CopyPropertiesFrom(JSContext* cx, HandleObject target, HandleObject obj)
 {
     JSAutoCompartment ac(cx, obj);
 
     AutoIdVector props(cx);
@@ -2594,24 +2609,16 @@ js::GetOwnPropertyDescriptor(JSContext* 
     }
 
     desc.object().set(nobj);
     desc.assertComplete();
     return true;
 }
 
 bool
-js::DefineProperty(JSContext* cx, HandleObject obj, HandleId id, Handle<PropertyDescriptor> desc)
-{
-    ObjectOpResult result;
-    return DefineProperty(cx, obj, id, desc, result) &&
-           result.checkStrict(cx, obj, id);
-}
-
-bool
 js::DefineProperty(JSContext* cx, HandleObject obj, HandleId id, Handle<PropertyDescriptor> desc,
                    ObjectOpResult& result)
 {
     desc.assertValid();
     if (DefinePropertyOp op = obj->getOps()->defineProperty)
         return op(cx, obj, id, desc, result);
     return NativeDefineProperty(cx, obj.as<NativeObject>(), id, desc, result);
 }
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -744,17 +744,39 @@ PreventExtensions(JSContext* cx, HandleO
  *
  * If no such property exists on obj, return true with desc.object() set to
  * null.
  */
 extern bool
 GetOwnPropertyDescriptor(JSContext* cx, HandleObject obj, HandleId id,
                          MutableHandle<PropertyDescriptor> desc);
 
-/* ES6 [[DefineOwnProperty]]. Define a property on obj. */
+/*
+ * ES6 [[DefineOwnProperty]]. Define a property on obj.
+ *
+ * If obj is an array, this follows ES5 15.4.5.1.
+ * If obj is any other native object, this follows ES5 8.12.9.
+ * If obj is a proxy, this calls the proxy handler's defineProperty method.
+ * Otherwise, this reports an error and returns false.
+ *
+ * Both StandardDefineProperty functions hew close to the ES5 spec. Note that
+ * the DefineProperty functions do not enforce some invariants mandated by ES6.
+ */
+extern bool
+StandardDefineProperty(JSContext* cx, HandleObject obj, HandleId id,
+                       Handle<PropertyDescriptor> descriptor, ObjectOpResult& result);
+
+/*
+ * Same as above except without the ObjectOpResult out-parameter. Throws a
+ * TypeError on failure.
+ */
+extern bool
+StandardDefineProperty(JSContext* cx, HandleObject obj, HandleId id,
+                       Handle<PropertyDescriptor> desc);
+
 extern bool
 DefineProperty(JSContext* cx, HandleObject obj, HandleId id,
                Handle<PropertyDescriptor> desc, ObjectOpResult& result);
 
 extern bool
 DefineProperty(ExclusiveContext* cx, HandleObject obj, HandleId id, HandleValue value,
                JSGetterOp getter, JSSetterOp setter, unsigned attrs, ObjectOpResult& result);
 
@@ -766,19 +788,16 @@ extern bool
 DefineElement(ExclusiveContext* cx, HandleObject obj, uint32_t index, HandleValue value,
               JSGetterOp getter, JSSetterOp setter, unsigned attrs, ObjectOpResult& result);
 
 /*
  * When the 'result' out-param is omitted, the behavior is the same as above, except
  * that any failure results in a TypeError.
  */
 extern bool
-DefineProperty(JSContext* cx, HandleObject obj, HandleId id, Handle<PropertyDescriptor> desc);
-
-extern bool
 DefineProperty(ExclusiveContext* cx, HandleObject obj, HandleId id, HandleValue value,
                JSGetterOp getter = nullptr,
                JSSetterOp setter = nullptr,
                unsigned attrs = JSPROP_ENUMERATE);
 
 extern bool
 DefineProperty(ExclusiveContext* cx, HandleObject obj, PropertyName* name, HandleValue value,
                JSGetterOp getter = nullptr,
--- a/js/src/json.cpp
+++ b/js/src/json.cpp
@@ -714,17 +714,17 @@ Walk(JSContext* cx, HandleObject holder,
                 if (newElement.isUndefined()) {
                     /* Step 2a(iii)(2). The spec deliberately ignores strict failure. */
                     if (!DeleteProperty(cx, obj, id, ignored))
                         return false;
                 } else {
                     /* Step 2a(iii)(3). The spec deliberately ignores strict failure. */
                     Rooted<PropertyDescriptor> desc(cx);
                     desc.setDataDescriptor(newElement, JSPROP_ENUMERATE);
-                    if (!DefineProperty(cx, obj, id, desc, ignored))
+                    if (!StandardDefineProperty(cx, obj, id, desc, ignored))
                         return false;
                 }
             }
         } else {
             /* Step 2b(i). */
             AutoIdVector keys(cx);
             if (!GetPropertyKeys(cx, obj, JSITER_OWNONLY, &keys))
                 return false;
@@ -742,17 +742,17 @@ Walk(JSContext* cx, HandleObject holder,
                 if (newElement.isUndefined()) {
                     /* Step 2b(ii)(2). The spec deliberately ignores strict failure. */
                     if (!DeleteProperty(cx, obj, id, ignored))
                         return false;
                 } else {
                     /* Step 2b(ii)(3). The spec deliberately ignores strict failure. */
                     Rooted<PropertyDescriptor> desc(cx);
                     desc.setDataDescriptor(newElement, JSPROP_ENUMERATE);
-                    if (!DefineProperty(cx, obj, id, desc, ignored))
+                    if (!StandardDefineProperty(cx, obj, id, desc, ignored))
                         return false;
                 }
             }
         }
     }
 
     /* Step 3. */
     RootedString key(cx, IdToString(cx, name));
--- a/js/src/proxy/DirectProxyHandler.cpp
+++ b/js/src/proxy/DirectProxyHandler.cpp
@@ -34,17 +34,17 @@ DirectProxyHandler::getOwnPropertyDescri
 
 bool
 DirectProxyHandler::defineProperty(JSContext* cx, HandleObject proxy, HandleId id,
                                    Handle<PropertyDescriptor> desc,
                                    ObjectOpResult& result) const
 {
     assertEnteredPolicy(cx, proxy, id, SET);
     RootedObject target(cx, proxy->as<ProxyObject>().target());
-    return DefineProperty(cx, target, id, desc, result);
+    return StandardDefineProperty(cx, target, id, desc, result);
 }
 
 bool
 DirectProxyHandler::ownPropertyKeys(JSContext* cx, HandleObject proxy,
                                     AutoIdVector& props) const
 {
     assertEnteredPolicy(cx, proxy, JSID_VOID, ENUMERATE);
     RootedObject target(cx, proxy->as<ProxyObject>().target());
--- a/js/src/proxy/ScriptedDirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedDirectProxyHandler.cpp
@@ -543,17 +543,17 @@ ScriptedDirectProxyHandler::defineProper
 
     // steps 6-7
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().defineProperty, &trap))
         return false;
 
     // step 8
     if (trap.isUndefined())
-        return DefineProperty(cx, target, id, desc, result);
+        return StandardDefineProperty(cx, target, id, desc, result);
 
     // step 9
     RootedValue descObj(cx);
     if (!FromPropertyDescriptorToObject(cx, desc, &descObj))
         return false;
 
     // steps 10-11
     RootedValue propKey(cx);
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -6998,17 +6998,17 @@ DebuggerObject_defineProperty(JSContext*
 
     {
         Maybe<AutoCompartment> ac;
         ac.emplace(cx, obj);
         if (!cx->compartment()->wrap(cx, &desc))
             return false;
 
         ErrorCopier ec(ac);
-        if (!DefineProperty(cx, obj, id, desc))
+        if (!StandardDefineProperty(cx, obj, id, desc))
             return false;
     }
 
     args.rval().setUndefined();
     return true;
 }
 
 static bool
@@ -7041,17 +7041,17 @@ DebuggerObject_defineProperties(JSContex
         ac.emplace(cx, obj);
         for (size_t i = 0; i < n; i++) {
             if (!cx->compartment()->wrap(cx, descs[i]))
                 return false;
         }
 
         ErrorCopier ec(ac);
         for (size_t i = 0; i < n; i++) {
-            if (!DefineProperty(cx, obj, ids[i], descs[i]))
+            if (!StandardDefineProperty(cx, obj, ids[i], descs[i]))
                 return false;
         }
     }
 
     args.rval().setUndefined();
     return true;
 }
 
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -344,17 +344,17 @@ js::intrinsic_DefineDataProperty(JSConte
     } else {
         // If the fourth argument is unspecified, the attributes are for a
         // plain data property.
         attrs = JSPROP_ENUMERATE;
     }
 
     Rooted<PropertyDescriptor> desc(cx);
     desc.setDataDescriptor(value, attrs);
-    if (!DefineProperty(cx, obj, id, desc))
+    if (!StandardDefineProperty(cx, obj, id, desc))
         return false;
 
     args.rval().setUndefined();
     return true;
 }
 
 bool
 js::intrinsic_UnsafeSetReservedSlot(JSContext* cx, unsigned argc, Value* vp)
--- a/js/src/vm/UnboxedObject.cpp
+++ b/js/src/vm/UnboxedObject.cpp
@@ -604,17 +604,17 @@ UnboxedPlainObject::convertToNative(JSCo
         Rooted<UnboxedExpandoObject*> nexpando(cx, expando);
         RootedId id(cx);
         Rooted<PropertyDescriptor> desc(cx);
         for (size_t i = 0; i < ids.length(); i++) {
             id = ids[i];
             if (!GetOwnPropertyDescriptor(cx, nexpando, id, &desc))
                 return false;
             ObjectOpResult result;
-            if (!DefineProperty(cx, nobj, id, desc, result))
+            if (!StandardDefineProperty(cx, nobj, id, desc, result))
                 return false;
             MOZ_ASSERT(result.ok());
         }
     }
 
     return true;
 }