Bug 1138499, part 2 - Strengthen assertComplete() to require that both [[Get]] and [[Set]] be present on accessor properties. r=Waldo.

--HG--
extra : rebase_source : d9351de6bd09ed6e45e8b829b84f6225a13bb6c5

js/src/jsapi.h

@@ -2618,6 +2618,7 @@ class PropertyDescriptorOperations
                                      JSPROP_SHARED |
                                      JSPROP_REDEFINE_NONCONFIGURABLE |
                                      SHADOWABLE)) == 0);
+        MOZ_ASSERT_IF(isAccessorDescriptor(), has(JSPROP_GETTER) && has(JSPROP_SETTER));
 #endif
     }
 

js/src/jsarray.cpp

@@ -699,8 +699,9 @@ js::ArraySetLength(JSContext* cx, Handle<ArrayObject*> arr, HandleId id,
     // the long run, with accessors replacing them both internally and at the
     // API level, just run with this.
     RootedShape lengthShape(cx, arr->lookup(cx, id));
-    if (!NativeObject::changeProperty(cx, arr, lengthShape, attrs,
-                                      JSPROP_PERMANENT | JSPROP_READONLY | JSPROP_SHARED,
+    if (!NativeObject::changeProperty(cx, arr, lengthShape,
+                                      attrs | JSPROP_PERMANENT | JSPROP_SHARED |
+                                      (lengthShape->attributes() & JSPROP_READONLY),
                                       array_length_getter, array_length_setter))
     {
         return false;

js/src/vm/NativeObject-inl.h

@@ -28,13 +28,6 @@ NativeObject::fixedData(size_t nslots) const
     return reinterpret_cast<uint8_t*>(&fixedSlots()[nslots]);
 }
 
-/* static */ inline bool
-NativeObject::changePropertyAttributes(JSContext* cx, HandleNativeObject obj,
-                                       HandleShape shape, unsigned attrs)
-{
-    return !!changeProperty(cx, obj, shape, attrs, 0, shape->getter(), shape->setter());
-}
-
 inline void
 NativeObject::removeLastProperty(ExclusiveContext* cx)
 {

js/src/vm/NativeObject.cpp

@@ -1358,8 +1358,8 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId
                 if (!CheckAccessorRedefinition(cx, obj, shape, getter, setter, id, attrs))
                     return false;
                 attrs = ApplyOrDefaultAttributes(attrs, shape);
-                shape = NativeObject::changeProperty(cx, obj, shape, attrs,
-                                                     JSPROP_GETTER | JSPROP_SETTER,
+                shape = NativeObject::changeProperty(cx, obj, shape,
+                                                     attrs | JSPROP_GETTER | JSPROP_SETTER,
                                                      (attrs & JSPROP_GETTER)
                                                      ? getter
                                                      : shape->getter(),
@@ -1371,6 +1371,12 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId
                 shouldDefine = false;
             }
         }
+
+        // Either we are converting a data property to an accessor property, or
+        // creating a new accessor property; either way [[Get]] and [[Set]]
+        // must both be filled in.
+        if (shouldDefine)
+            attrs |= JSPROP_GETTER | JSPROP_SETTER;
     } else if (desc.hasValue()) {
         // If we did a normal lookup here, it would cause resolve hook recursion in
         // the following case. Suppose the first script we run in a lazy global is
@@ -1402,9 +1408,7 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId
             }
         }
     } else {
-        // We have been asked merely to update some attributes. If the
-        // property already exists and it's a data property, we can just
-        // call JSObject::changeProperty.
+        // We have been asked merely to update JSPROP_PERMANENT and/or JSPROP_ENUMERATE.
         if (!NativeLookupOwnProperty<CanGC>(cx, obj, id, &shape))
             return false;
 

js/src/vm/NativeObject.h

@@ -691,12 +691,8 @@ class NativeObject : public JSObject
 
     /* Change the given property into a sibling with the same id in this scope. */
     static Shape*
-    changeProperty(ExclusiveContext* cx, HandleNativeObject obj,
-                   HandleShape shape, unsigned attrs, unsigned mask,
-                   JSGetterOp getter, JSSetterOp setter);
-
-    static inline bool changePropertyAttributes(JSContext* cx, HandleNativeObject obj,
-                                                HandleShape shape, unsigned attrs);
+    changeProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleShape shape,
+                   unsigned attrs, JSGetterOp getter, JSSetterOp setter);
 
     /* Remove the property named by id from this object. */
     bool removeProperty(ExclusiveContext* cx, jsid id);

js/src/vm/Shape.cpp

@@ -866,13 +866,11 @@ NativeObject::putProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId
 
 /* static */ Shape*
 NativeObject::changeProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleShape shape,
-                             unsigned attrs, unsigned mask, GetterOp getter, SetterOp setter)
+                             unsigned attrs, GetterOp getter, SetterOp setter)
 {
     MOZ_ASSERT(obj->containsPure(shape));
     MOZ_ASSERT(getter != JS_PropertyStub);
     MOZ_ASSERT(setter != JS_StrictPropertyStub);
-
-    attrs |= shape->attrs & mask;
     MOZ_ASSERT_IF(attrs & (JSPROP_GETTER | JSPROP_SETTER), attrs & JSPROP_SHARED);
 
     /* Allow only shared (slotless) => unshared (slotful) transition. */