Address review comments from Waldo (bug 672829 comment 24).
authorJason Orendorff <jorendorff@mozilla.com>
Thu, 21 Jul 2011 17:36:09 -0500
changeset 75224 501052c6263ba6d4b020dbd205df08a41251b9c0
parent 75223 1af365c2b3fc7341f23d43acbeee3f662ad41fb7
child 75225 46a7638aeb4d9132dc416bda95605347f0070857
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
bugs672829
milestone8.0a1
Address review comments from Waldo (bug 672829 comment 24).
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -2474,17 +2474,17 @@ DefineProperty(JSContext *cx, JSObject *
         if (obj->isProxy())
             return JSProxy::defineProperty(cx, obj, id, desc.pd);
         return Reject(cx, obj, JSMSG_OBJECT_NOT_EXTENSIBLE, throwError, rval);
     }
 
     return DefinePropertyOnObject(cx, obj, id, desc, throwError, rval);
 }
 
-}
+} /* namespace js */
 
 JSBool
 js_DefineOwnProperty(JSContext *cx, JSObject *obj, jsid id, const Value &descriptor, JSBool *bp)
 {
     AutoPropDescArrayRooter descs(cx);
     PropDesc *desc = descs.append();
     if (!desc || !desc->initialize(cx, descriptor))
         return false;
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -138,20 +138,37 @@ struct PropDesc {
     bool hasWritable : 1;
     bool hasEnumerable : 1;
     bool hasConfigurable : 1;
 
     friend class js::AutoPropDescArrayRooter;
 
     PropDesc();
 
-    /* 8.10.5 ToPropertyDescriptor(Obj) */
+    /*
+     * 8.10.5 ToPropertyDescriptor(Obj)
+     *
+     * If checkAccessors is false, skip steps 7.b and 8.b, which throw a
+     * TypeError if .get or .set is neither a callable object nor undefined.
+     *
+     * (DebuggerObject_defineProperty uses this: the .get and .set properties
+     * are expected to be Debugger.Object wrappers of functions, which are not
+     * themselves callable.)
+     */
     bool initialize(JSContext* cx, const js::Value &v, bool checkAccessors=true);
 
-    /* 8.10.4 FromPropertyDescriptor(Desc) */
+    /*
+     * 8.10.4 FromPropertyDescriptor(Desc)
+     *
+     * initFromPropertyDescriptor sets pd to undefined and populates all the
+     * other fields of this PropDesc from desc.
+     *
+     * makeObject populates pd based on the other fields of *this, creating a
+     * new property descriptor JSObject and defining properties on it.
+     */
     void initFromPropertyDescriptor(const PropertyDescriptor &desc);
     bool makeObject(JSContext *cx);
 
     /* 8.10.1 IsAccessorDescriptor(desc) */
     bool isAccessorDescriptor() const {
         return hasGet || hasSet;
     }
 
@@ -193,16 +210,21 @@ struct PropDesc {
 
     js::PropertyOp getter() const {
         return js::CastAsPropertyOp(getterObject());
     }
     js::StrictPropertyOp setter() const {
         return js::CastAsStrictPropertyOp(setterObject());
     }
 
+    /*
+     * 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.
+     */
     inline bool checkGetter(JSContext *cx);
     inline bool checkSetter(JSContext *cx);
 };
 
 typedef Vector<PropDesc, 1> PropDescArray;
 
 } /* namespace js */
 
@@ -1648,18 +1670,22 @@ DefineNativeProperty(JSContext *cx, JSOb
  * Specialized subroutine that allows caller to preset JSRESOLVE_* flags.
  */
 extern bool
 LookupPropertyWithFlags(JSContext *cx, JSObject *obj, jsid id, uintN flags,
                         JSObject **objp, JSProperty **propp);
 
 
 /*
- * Define a property as specified by the ES5 [[DefineOwnProperty]] algorithms,
- * sections 8.12.9 and (for arrays) 15.4.5.1.
+ * Call the [[DefineOwnProperty]] internal method of 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.
  */
 extern bool
 DefineProperty(JSContext *cx, JSObject *obj, const jsid &id, const PropDesc &desc, bool throwError,
                bool *rval);
 
 /*
  * Read property descriptors from props, as for Object.defineProperties. See
  * ES5 15.2.3.7 steps 3-5.
@@ -1761,17 +1787,17 @@ bool
 GetOwnPropertyDescriptor(JSContext *cx, JSObject *obj, jsid id, PropertyDescriptor *desc);
 
 bool
 GetOwnPropertyDescriptor(JSContext *cx, JSObject *obj, jsid id, Value *vp);
 
 bool
 NewPropertyDescriptorObject(JSContext *cx, const PropertyDescriptor *desc, Value *vp);
 
-}
+} /* namespace js */
 
 extern JSBool
 js_GetMethod(JSContext *cx, JSObject *obj, jsid id, uintN getHow, js::Value *vp);
 
 /*
  * Check whether it is OK to assign an undeclared property with name
  * propname of the global object in the current script on cx.  Reports
  * an error if one needs to be reported (in particular in all cases
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -283,16 +283,17 @@ inline js::Value
 JSObject::getReservedSlot(uintN index) const
 {
     return (index < numSlots()) ? getSlot(index) : js::UndefinedValue();
 }
 
 inline void
 JSObject::setReservedSlot(uintN index, const js::Value &v)
 {
+    JS_ASSERT(index < JSSLOT_FREE(getClass()));
     setSlot(index, v);
 }
 
 inline bool
 JSObject::canHaveMethodBarrier() const
 {
     return isObject() || isFunction() || isPrimitive() || isDate();
 }
@@ -1377,26 +1378,30 @@ DefineConstructorAndPrototype(JSContext 
         return false;
 
     global->setSlot(key, ObjectValue(*ctor));
     global->setSlot(key + JSProto_LIMIT, ObjectValue(*proto));
     global->setSlot(key + JSProto_LIMIT * 2, ObjectValue(*ctor));
     return true;
 }
 
-bool PropDesc::checkGetter(JSContext *cx) {
+bool
+PropDesc::checkGetter(JSContext *cx)
+{
     if (hasGet && !js_IsCallable(get) && !get.isUndefined()) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_GET_SET_FIELD,
                              js_getter_str);
         return false;
     }
     return true;
 }
 
-bool PropDesc::checkSetter(JSContext *cx) {
+bool
+PropDesc::checkSetter(JSContext *cx)
+{
     if (hasSet && !js_IsCallable(set) && !set.isUndefined()) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_GET_SET_FIELD,
                              js_setter_str);
         return false;
     }
     return true;
 }