Changeset 75884 in webkit for trunk/Source/JavaScriptCore/runtime/JSObject.cpp

Timestamp:

Jan 15, 2011, 3:39:36 PM (14 years ago)

Author:

[email protected]

Message:

2011-01-15 Oliver Hunt <​[email protected]>

Reviewed by Maciej Stachowiak.

Incorrect behavior changing attributes of an accessor
​https://bugs.webkit.org/show_bug.cgi?id=52515

defineProperty doesn't correctly handle changing attributes of an accessor
property. This is because we don't pass the full descriptor to the
putDescriptor helper function, which means we have insufficient information
to do the right thing. Once that's passed the correct behavior is relatively
simple to implement.

• runtime/JSObject.cpp: (JSC::putDescriptor): (JSC::JSObject::defineOwnProperty):

File:

• trunk/Source/JavaScriptCore/runtime/JSObject.cpp (modified) (5 diffs)

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -589 +589 @@
 }
 
-static bool putDescriptor(ExecState* exec, JSObject* target, const Identifier& propertyName, PropertyDescriptor& descriptor, unsigned attributes, JSValue oldValue)
+static bool putDescriptor(ExecState* exec, JSObject* target, const Identifier& propertyName, PropertyDescriptor& descriptor, unsigned attributes, const PropertyDescriptor& oldDescriptor)
 {
     if (descriptor.isGenericDescriptor() || descriptor.isDataDescriptor()) {
-        target->putWithAttributes(exec, propertyName, descriptor.value() ? descriptor.value() : oldValue, attributes & ~(Getter | Setter));
+        if (descriptor.isGenericDescriptor() && oldDescriptor.isAccessorDescriptor()) {
+            GetterSetter* accessor = new (exec) GetterSetter(exec);
+            if (oldDescriptor.getter()) {
+                attributes |= Getter;
+                accessor->setGetter(asObject(oldDescriptor.getter()));
+            }
+            if (oldDescriptor.setter()) {
+                attributes |= Setter;
+                accessor->setSetter(asObject(oldDescriptor.setter()));
+            }
+            target->putWithAttributes(exec, propertyName, accessor, attributes);
+            return true;
+        }
+        JSValue newValue = jsUndefined();
+        if (descriptor.value())
+            newValue = descriptor.value();
+        else if (oldDescriptor.value())
+            newValue = oldDescriptor.value();
+        target->putWithAttributes(exec, propertyName, newValue, attributes & ~(Getter | Setter));
         return true;
     }
@@ -609 +627 @@
     // If we have a new property we can just put it on normally
     PropertyDescriptor current;
-    if (!getOwnPropertyDescriptor(exec, propertyName, current))
-        return putDescriptor(exec, this, propertyName, descriptor, descriptor.attributes(), jsUndefined());
+    if (!getOwnPropertyDescriptor(exec, propertyName, current)) {
+        PropertyDescriptor oldDescriptor;
+        oldDescriptor.setValue(jsUndefined());
+        return putDescriptor(exec, this, propertyName, descriptor, descriptor.attributes(), oldDescriptor);
+    }
 
     if (descriptor.isEmpty())
@@ -636 +657 @@
         if (!current.attributesEqual(descriptor)) {
             deleteProperty(exec, propertyName);
-            putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current.value());
+            putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current);
         }
         return true;
@@ -649 +670 @@
         }
         deleteProperty(exec, propertyName);
-        return putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current.value() ? current.value() : jsUndefined());
+        return putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current);
     }
 
@@ -677 +698 @@
         }
         deleteProperty(exec, propertyName);
-        return putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current.value());
+        return putDescriptor(exec, this, propertyName, descriptor, current.attributesWithOverride(descriptor), current);
     }