Title: [104836] trunk
Revision
104836
Author
barraclo...@apple.com
Date
2012-01-12 10:38:24 -0800 (Thu, 12 Jan 2012)

Log Message

Allow accessor get/set property to be set to undefined
https://bugs.webkit.org/show_bug.cgi?id=76148

Reviewed by Oliver Hunt.

Source/_javascript_Core: 

AccessorDescriptor properties may have their get & set properties defined to reference a function
(Callable object) or be set to undefined. Valid PropertyDescriptors created by toPropertyDescriptor
(defined from JS code via Object.defineProperty, etc) have get and set properties that are in one of
three states (1) nonexistent, (2) set to undefined, or (3) a function (any Callable object).

On the PropertyDescriptor object these three states are represneted by JSValue(), jsUndefined(), and
any JSObject* (with a constraint that this must be callable).

Logically the get/set property of an accessor descriptor on an object might be in any of the three
states above, but in practice there is no way to distinguish between the first two states. As such
we stor the get/set values in property storage in a JSObject* field, with 0 indicating absent or
undefined. When unboxing to a PropertyDescriptor, map this back to a JS undefined value.

* runtime/GetterSetter.h:
(JSC::GetterSetter::setGetter):
(JSC::GetterSetter::setSetter):
    - Allow the getter/setter to be cleared.
* runtime/JSArray.cpp:
(JSC::JSArray::putDescriptor):
    - Changed to call getterObject/setterObject.
(JSC::JSArray::defineOwnNumericProperty):
    - Added ASSERT.
* runtime/JSObject.cpp:
(JSC::putDescriptor):
(JSC::JSObject::defineOwnProperty):
    - Changed to call getterObject/setterObject.
* runtime/ObjectConstructor.cpp:
(JSC::objectConstructorGetOwnPropertyDescriptor):
    - getter/setter values read from properties on object are never missing, they will now be set as undefined by 'setDescriptor'.
(JSC::toPropertyDescriptor):
    - Do not translate undefined->empty, this loses an important distinction between a get/set property being absent, or being explicitly set to undefined.
* runtime/PropertyDescriptor.cpp:
(JSC::PropertyDescriptor::getterObject):
(JSC::PropertyDescriptor::setterObject):
    - Accessors to convert the get/set property to an object pointer, converting undefined to 0.
(JSC::PropertyDescriptor::setDescriptor):
(JSC::PropertyDescriptor::setAccessorDescriptor):
    - Translate a getter/setter internally represented at 0 to undefined, indicating that it is present.
* runtime/PropertyDescriptor.h:
    - Declare getterObject/setterObject.

LayoutTests: 

* fast/js/Object-defineProperty-expected.txt:
* fast/js/script-tests/Object-defineProperty.js:
    - Update a couple of inaccurate tests (it is invalid for a property to have
      both a get: and value: field; AccessorDescritor properties do not have a
      writable property). Add more test cases.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (104835 => 104836)


--- trunk/LayoutTests/ChangeLog	2012-01-12 18:20:05 UTC (rev 104835)
+++ trunk/LayoutTests/ChangeLog	2012-01-12 18:38:24 UTC (rev 104836)
@@ -1,3 +1,16 @@
+2012-01-11  Gavin Barraclough  <barraclo...@apple.com>
+
+        Allow accessor get/set property to be set to undefined
+        https://bugs.webkit.org/show_bug.cgi?id=76148
+
+        Reviewed by Oliver Hunt.
+
+        * fast/js/Object-defineProperty-expected.txt:
+        * fast/js/script-tests/Object-defineProperty.js:
+            - Update a couple of inaccurate tests (it is invalid for a property to have
+              both a get: and value: field; AccessorDescritor properties do not have a
+              writable property). Add more test cases.
+
 2012-01-12  Pavel Podivilov  <podivi...@chromium.org>
 
         Web Inspector: [JSC] //@ sourceURL is not respected.

Modified: trunk/LayoutTests/fast/js/Object-defineProperty-expected.txt (104835 => 104836)


--- trunk/LayoutTests/fast/js/Object-defineProperty-expected.txt	2012-01-12 18:20:05 UTC (rev 104835)
+++ trunk/LayoutTests/fast/js/Object-defineProperty-expected.txt	2012-01-12 18:38:24 UTC (rev 104836)
@@ -5,7 +5,7 @@
 
 PASS JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {value:1}), 'foo')) is JSON.stringify({value: 1, writable: false, enumerable: false, configurable: false})
 PASS JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {}), 'foo')) is JSON.stringify({writable: false, enumerable: false, configurable: false})
-PASS JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {get:undefined}), 'foo')) is JSON.stringify({writable: true, enumerable: false, configurable: false})
+PASS JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {get:undefined}), 'foo')) is JSON.stringify({enumerable: false, configurable: false})
 PASS JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {value:1, writable: false}), 'foo')) is JSON.stringify({value: 1, writable: false, enumerable: false, configurable: false})
 PASS JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {value:1, writable: true}), 'foo')) is JSON.stringify({value: 1, writable: true, enumerable: false, configurable: false})
 PASS JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {value:1, enumerable: false}), 'foo')) is JSON.stringify({value: 1, writable: false, enumerable: false, configurable: false})
@@ -22,7 +22,7 @@
 PASS Object.defineProperty('foo') threw exception TypeError: Properties can only be defined on Objects..
 PASS Object.defineProperty({}) threw exception TypeError: Property description must be an object..
 PASS Object.defineProperty({}, 'foo') threw exception TypeError: Property description must be an object..
-PASS Object.defineProperty({}, 'foo', {get:undefined, value:true}).foo is true
+PASS Object.defineProperty({}, 'foo', {get:undefined, value:true}).foo threw exception TypeError: Invalid property.  'value' present on property with getter or setter..
 PASS Object.defineProperty({get foo() { return true; } }, 'foo', {configurable:false}).foo is true
 PASS Object.defineProperty(createUnconfigurableProperty({}, 'foo'), 'foo', {configurable: true}) threw exception TypeError: Attempting to configurable attribute of unconfigurable property..
 PASS Object.defineProperty(createUnconfigurableProperty({}, 'foo'), 'foo', {writable: true}) threw exception TypeError: Attempting to change writable attribute of unconfigurable property..
@@ -58,6 +58,26 @@
 PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1}), 'foo', {set: setter1}) threw exception TypeError: Attempting to change access mechanism for an unconfigurable property..
 PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true}), 'foo', {get: getter1}).foo threw exception called getter1.
 PASS Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true}), 'foo', {set: setter1}).foo='test' threw exception called setter1.
+PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo is 42
+PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo = 42; o.result; is 42
+PASS var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(x){this.result = x;}}); o.foo is undefined
+PASS var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(x){this.result = x;}}); o.foo = 42; o.result; is 42
+PASS var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo is undefined
+PASS var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo = 42; o.result; is 42
+PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo is 42
+PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
+PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo is 42
+PASS var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
+PASS var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo is undefined
+PASS var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
+PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo is 13
+PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo = 42; o.result; is 42
+PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:undefined}); o.foo is undefined
+PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:undefined}); o.foo = 42; o.result; is 42
+PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo is 42
+PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo = 42; o.result; is 13
+PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo is 42
+PASS var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/js/script-tests/Object-defineProperty.js (104835 => 104836)


--- trunk/LayoutTests/fast/js/script-tests/Object-defineProperty.js	2012-01-12 18:20:05 UTC (rev 104835)
+++ trunk/LayoutTests/fast/js/script-tests/Object-defineProperty.js	2012-01-12 18:38:24 UTC (rev 104836)
@@ -5,7 +5,7 @@
 shouldBe("JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {}), 'foo'))",
          "JSON.stringify({writable: false, enumerable: false, configurable: false})");
 shouldBe("JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {get:undefined}), 'foo'))",
-         "JSON.stringify({writable: true, enumerable: false, configurable: false})");
+         "JSON.stringify({enumerable: false, configurable: false})");
 shouldBe("JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {value:1, writable: false}), 'foo'))",
          "JSON.stringify({value: 1, writable: false, enumerable: false, configurable: false})");
 shouldBe("JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty({}, 'foo', {value:1, writable: true}), 'foo'))",
@@ -30,7 +30,7 @@
 shouldThrow("Object.defineProperty('foo')");
 shouldThrow("Object.defineProperty({})");
 shouldThrow("Object.defineProperty({}, 'foo')");
-shouldBeTrue("Object.defineProperty({}, 'foo', {get:undefined, value:true}).foo");
+shouldThrow("Object.defineProperty({}, 'foo', {get:undefined, value:true}).foo");
 shouldBeTrue("Object.defineProperty({get foo() { return true; } }, 'foo', {configurable:false}).foo");
 
 function createUnconfigurableProperty(o, prop, writable, enumerable) {
@@ -80,3 +80,27 @@
 shouldThrow("Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1}), 'foo', {set: setter1})");
 shouldThrow("Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true}), 'foo', {get: getter1}).foo", "'called getter1'");
 shouldThrow("Object.defineProperty(Object.defineProperty({}, 'foo', {value: 1, configurable: true}), 'foo', {set: setter1}).foo='test'", "'called setter1'");
+
+// Test an object with a getter setter.
+// Either accessor may be omitted or replaced with undefined, or both may be replaced with undefined.
+shouldBe("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo", '42')
+shouldBe("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}); o.foo = 42; o.result;", '42');
+shouldBe("var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(x){this.result = x;}}); o.foo", 'undefined')
+shouldBe("var o = Object.defineProperty({}, 'foo', {get:undefined, set:function(x){this.result = x;}}); o.foo = 42; o.result;", '42');
+shouldBe("var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo", 'undefined')
+shouldBe("var o = Object.defineProperty({}, 'foo', {set:function(x){this.result = x;}}); o.foo = 42; o.result;", '42');
+shouldBe("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo", '42')
+shouldThrow("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}, set:undefined}); o.foo = 42; o.result;");
+shouldBe("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo", '42')
+shouldThrow("var o = Object.defineProperty({}, 'foo', {get:function(){return 42;}}); o.foo = 42; o.result;");
+shouldBe("var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo", 'undefined')
+shouldThrow("var o = Object.defineProperty({}, 'foo', {get:undefined, set:undefined}); o.foo = 42; o.result;");
+// Test replacing or removing either the getter or setter individually.
+shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo", '13')
+shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:function(){return 13;}}); o.foo = 42; o.result;", '42')
+shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:undefined}); o.foo", 'undefined')
+shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {get:undefined}); o.foo = 42; o.result;", '42')
+shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo", '42')
+shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:function(){this.result = 13;}}); o.foo = 42; o.result;", '13')
+shouldBe("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo", '42')
+shouldThrow("var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo = 42; o.result;")

Modified: trunk/Source/_javascript_Core/ChangeLog (104835 => 104836)


--- trunk/Source/_javascript_Core/ChangeLog	2012-01-12 18:20:05 UTC (rev 104835)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-01-12 18:38:24 UTC (rev 104836)
@@ -1,3 +1,51 @@
+2012-01-11  Gavin Barraclough  <barraclo...@apple.com>
+
+        Allow accessor get/set property to be set to undefined
+        https://bugs.webkit.org/show_bug.cgi?id=76148
+
+        Reviewed by Oliver Hunt.
+
+        AccessorDescriptor properties may have their get & set properties defined to reference a function
+        (Callable object) or be set to undefined. Valid PropertyDescriptors created by toPropertyDescriptor
+        (defined from JS code via Object.defineProperty, etc) have get and set properties that are in one of
+        three states (1) nonexistent, (2) set to undefined, or (3) a function (any Callable object).
+
+        On the PropertyDescriptor object these three states are represneted by JSValue(), jsUndefined(), and
+        any JSObject* (with a constraint that this must be callable).
+
+        Logically the get/set property of an accessor descriptor on an object might be in any of the three
+        states above, but in practice there is no way to distinguish between the first two states. As such
+        we stor the get/set values in property storage in a JSObject* field, with 0 indicating absent or
+        undefined. When unboxing to a PropertyDescriptor, map this back to a JS undefined value.
+
+        * runtime/GetterSetter.h:
+        (JSC::GetterSetter::setGetter):
+        (JSC::GetterSetter::setSetter):
+            - Allow the getter/setter to be cleared.
+        * runtime/JSArray.cpp:
+        (JSC::JSArray::putDescriptor):
+            - Changed to call getterObject/setterObject.
+        (JSC::JSArray::defineOwnNumericProperty):
+            - Added ASSERT.
+        * runtime/JSObject.cpp:
+        (JSC::putDescriptor):
+        (JSC::JSObject::defineOwnProperty):
+            - Changed to call getterObject/setterObject.
+        * runtime/ObjectConstructor.cpp:
+        (JSC::objectConstructorGetOwnPropertyDescriptor):
+            - getter/setter values read from properties on object are never missing, they will now be set as undefined by 'setDescriptor'.
+        (JSC::toPropertyDescriptor):
+            - Do not translate undefined->empty, this loses an important distinction between a get/set property being absent, or being explicitly set to undefined.
+        * runtime/PropertyDescriptor.cpp:
+        (JSC::PropertyDescriptor::getterObject):
+        (JSC::PropertyDescriptor::setterObject):
+            - Accessors to convert the get/set property to an object pointer, converting undefined to 0.
+        (JSC::PropertyDescriptor::setDescriptor):
+        (JSC::PropertyDescriptor::setAccessorDescriptor):
+            - Translate a getter/setter internally represented at 0 to undefined, indicating that it is present.
+        * runtime/PropertyDescriptor.h:
+            - Declare getterObject/setterObject.
+
 2012-01-12  Zeno Albisser  <z...@webkit.org>
 
         [Qt][WK2][Mac] Conflict of MacTypes.h defining a Fixed type after r104560.

Modified: trunk/Source/_javascript_Core/runtime/GetterSetter.h (104835 => 104836)


--- trunk/Source/_javascript_Core/runtime/GetterSetter.h	2012-01-12 18:20:05 UTC (rev 104835)
+++ trunk/Source/_javascript_Core/runtime/GetterSetter.h	2012-01-12 18:38:24 UTC (rev 104836)
@@ -56,9 +56,9 @@
         static void visitChildren(JSCell*, SlotVisitor&);
 
         JSObject* getter() const { return m_getter.get(); }
-        void setGetter(JSGlobalData& globalData, JSObject* getter) { m_getter.set(globalData, this, getter); }
+        void setGetter(JSGlobalData& globalData, JSObject* getter) { m_getter.setMayBeNull(globalData, this, getter); }
         JSObject* setter() const { return m_setter.get(); }
-        void setSetter(JSGlobalData& globalData, JSObject* setter) { m_setter.set(globalData, this, setter); }
+        void setSetter(JSGlobalData& globalData, JSObject* setter) { m_setter.setMayBeNull(globalData, this, setter); }
         static Structure* createStructure(JSGlobalData& globalData, JSGlobalObject* globalObject, JSValue prototype)
         {
             return Structure::create(globalData, globalObject, prototype, TypeInfo(GetterSetterType, OverridesVisitChildren), &s_info);

Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (104835 => 104836)


--- trunk/Source/_javascript_Core/runtime/JSArray.cpp	2012-01-12 18:20:05 UTC (rev 104835)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp	2012-01-12 18:38:24 UTC (rev 104836)
@@ -354,19 +354,15 @@
 
     if (descriptor.isAccessorDescriptor()) {
         JSObject* getter = 0;
-        if (descriptor.getter() && descriptor.getter().isObject())
-            getter = asObject(descriptor.getter());
-        if (!getter && oldDescriptor.isAccessorDescriptor()) {
-            if (oldDescriptor.getter() && oldDescriptor.getter().isObject())
-                getter = asObject(oldDescriptor.getter());
-        }
+        if (descriptor.getterPresent())
+            getter = descriptor.getterObject();
+        else if (oldDescriptor.isAccessorDescriptor())
+            getter = oldDescriptor.getterObject();
         JSObject* setter = 0;
-        if (descriptor.setter() && descriptor.setter().isObject())
-            setter = asObject(descriptor.setter());
-        if (!setter && oldDescriptor.isAccessorDescriptor()) {
-            if (oldDescriptor.setter() && oldDescriptor.setter().isObject())
-                setter = asObject(oldDescriptor.setter());
-        }
+        if (descriptor.setterPresent())
+            setter = descriptor.setterObject();
+        else if (oldDescriptor.isAccessorDescriptor())
+            setter = oldDescriptor.setterObject();
 
         GetterSetter* accessor = GetterSetter::create(exec);
         if (getter)
@@ -491,6 +487,7 @@
             }
             // 10.b. else, the [[Configurable]] field of current is true, so any change is acceptable.
         } else {
+            ASSERT(current.isAccessorDescriptor() && current.getterPresent() && current.setterPresent());
             // 11. Else, IsAccessorDescriptor(current) and IsAccessorDescriptor(Desc) are both true so, if the [[Configurable]] field of current is false, then
             if (!current.configurable()) {
                 // 11.i. Reject, if the [[Set]] field of Desc is present and SameValue(Desc.[[Set]], current.[[Set]]) is false.

Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (104835 => 104836)


--- trunk/Source/_javascript_Core/runtime/JSObject.cpp	2012-01-12 18:20:05 UTC (rev 104835)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp	2012-01-12 18:38:24 UTC (rev 104836)
@@ -691,13 +691,13 @@
     if (descriptor.isGenericDescriptor() || descriptor.isDataDescriptor()) {
         if (descriptor.isGenericDescriptor() && oldDescriptor.isAccessorDescriptor()) {
             GetterSetter* accessor = GetterSetter::create(exec);
-            if (oldDescriptor.getter()) {
+            if (oldDescriptor.getterPresent()) {
                 attributes |= Accessor;
-                accessor->setGetter(exec->globalData(), asObject(oldDescriptor.getter()));
+                accessor->setGetter(exec->globalData(), oldDescriptor.getterObject());
             }
-            if (oldDescriptor.setter()) {
+            if (oldDescriptor.setterPresent()) {
                 attributes |= Accessor;
-                accessor->setSetter(exec->globalData(), asObject(oldDescriptor.setter()));
+                accessor->setSetter(exec->globalData(), oldDescriptor.setterObject());
             }
             target->methodTable()->putWithAttributes(target, exec, propertyName, accessor, attributes);
             return true;
@@ -711,12 +711,12 @@
         return true;
     }
     attributes &= ~ReadOnly;
-    if (descriptor.getter() && descriptor.getter().isObject())
-        target->methodTable()->defineGetter(target, exec, propertyName, asObject(descriptor.getter()), attributes);
+    if (descriptor.getterPresent())
+        target->methodTable()->defineGetter(target, exec, propertyName, descriptor.getterObject(), attributes);
     if (exec->hadException())
         return false;
-    if (descriptor.setter() && descriptor.setter().isObject())
-        target->methodTable()->defineSetter(target, exec, propertyName, asObject(descriptor.setter()), attributes);
+    if (descriptor.setterPresent())
+        target->methodTable()->defineSetter(target, exec, propertyName, descriptor.setterObject(), attributes);
     return !exec->hadException();
 }
 
@@ -823,15 +823,15 @@
         return false;
     GetterSetter* getterSetter = asGetterSetter(accessor);
     if (current.attributesEqual(descriptor)) {
-        if (descriptor.setter())
-            getterSetter->setSetter(exec->globalData(), asObject(descriptor.setter()));
-        if (descriptor.getter())
-            getterSetter->setGetter(exec->globalData(), asObject(descriptor.getter()));
+        if (descriptor.setterPresent())
+            getterSetter->setSetter(exec->globalData(), descriptor.setterObject());
+        if (descriptor.getterPresent())
+            getterSetter->setGetter(exec->globalData(), descriptor.getterObject());
         return true;
     }
     object->methodTable()->deleteProperty(object, exec, propertyName);
     unsigned attrs = current.attributesWithOverride(descriptor);
-    if (descriptor.setter() || descriptor.getter())
+    if (descriptor.setterPresent() || descriptor.getterPresent())
         attrs |= Accessor;
     object->putDirect(exec->globalData(), propertyName, getterSetter, attrs);
     return true;

Modified: trunk/Source/_javascript_Core/runtime/ObjectConstructor.cpp (104835 => 104836)


--- trunk/Source/_javascript_Core/runtime/ObjectConstructor.cpp	2012-01-12 18:20:05 UTC (rev 104835)
+++ trunk/Source/_javascript_Core/runtime/ObjectConstructor.cpp	2012-01-12 18:38:24 UTC (rev 104836)
@@ -164,8 +164,10 @@
         description->putDirect(exec->globalData(), exec->propertyNames().value, descriptor.value() ? descriptor.value() : jsUndefined(), 0);
         description->putDirect(exec->globalData(), exec->propertyNames().writable, jsBoolean(descriptor.writable()), 0);
     } else {
-        description->putDirect(exec->globalData(), exec->propertyNames().get, descriptor.getter() ? descriptor.getter() : jsUndefined(), 0);
-        description->putDirect(exec->globalData(), exec->propertyNames().set, descriptor.setter() ? descriptor.setter() : jsUndefined(), 0);
+        ASSERT(descriptor.getter());
+        ASSERT(descriptor.setter());
+        description->putDirect(exec->globalData(), exec->propertyNames().get, descriptor.getter(), 0);
+        description->putDirect(exec->globalData(), exec->propertyNames().set, descriptor.setter(), 0);
     }
     
     description->putDirect(exec->globalData(), exec->propertyNames().enumerable, jsBoolean(descriptor.enumerable()), 0);
@@ -251,8 +253,7 @@
                 throwError(exec, createTypeError(exec, "Getter must be a function."));
                 return false;
             }
-        } else
-            get = JSValue();
+        }
         desc.setGetter(get);
     }
 
@@ -267,9 +268,7 @@
                 throwError(exec, createTypeError(exec, "Setter must be a function."));
                 return false;
             }
-        } else
-            set = JSValue();
-
+        }
         desc.setSetter(set);
     }
 

Modified: trunk/Source/_javascript_Core/runtime/PropertyDescriptor.cpp (104835 => 104836)


--- trunk/Source/_javascript_Core/runtime/PropertyDescriptor.cpp	2012-01-12 18:20:05 UTC (rev 104835)
+++ trunk/Source/_javascript_Core/runtime/PropertyDescriptor.cpp	2012-01-12 18:38:24 UTC (rev 104836)
@@ -84,6 +84,18 @@
     return m_setter;
 }
 
+JSObject* PropertyDescriptor::getterObject() const
+{
+    ASSERT(isAccessorDescriptor() && getterPresent());
+    return m_getter.isObject() ? asObject(m_getter) : 0;
+}
+
+JSObject* PropertyDescriptor::setterObject() const
+{
+    ASSERT(isAccessorDescriptor() && setterPresent());
+    return m_setter.isObject() ? asObject(m_setter) : 0;
+}
+
 void PropertyDescriptor::setDescriptor(JSValue value, unsigned attributes)
 {
     ASSERT(value);
@@ -91,14 +103,12 @@
 
     m_attributes = attributes;
     if (value.isGetterSetter()) {
+        m_attributes &= ~ReadOnly; // FIXME: we should be able to ASSERT this!
+
         GetterSetter* accessor = asGetterSetter(value);
-
-        m_getter = accessor->getter();
-        m_setter = accessor->setter();
-        ASSERT(m_getter || m_setter);
-
+        m_getter = accessor->getter() ? accessor->getter() : jsUndefined();
+        m_setter = accessor->setter() ? accessor->setter() : jsUndefined();
         m_seenAttributes = EnumerablePresent | ConfigurablePresent;
-        m_attributes &= ~ReadOnly;
     } else {
         m_value = value;
         m_seenAttributes = EnumerablePresent | ConfigurablePresent | WritablePresent;
@@ -108,11 +118,11 @@
 void PropertyDescriptor::setAccessorDescriptor(GetterSetter* accessor, unsigned attributes)
 {
     ASSERT(attributes & Accessor);
-    ASSERT(accessor->getter() || accessor->setter());
+    attributes &= ~ReadOnly; // FIXME: we should be able to ASSERT this!
+
     m_attributes = attributes;
-    m_getter = accessor->getter();
-    m_setter = accessor->setter();
-    m_attributes &= ~ReadOnly;
+    m_getter = accessor->getter() ? accessor->getter() : jsUndefined();
+    m_setter = accessor->setter() ? accessor->setter() : jsUndefined();
     m_seenAttributes = EnumerablePresent | ConfigurablePresent;
 }
 
@@ -212,6 +222,7 @@
 
 unsigned PropertyDescriptor::attributesOverridingCurrent(const PropertyDescriptor& current) const
 {
+    unsigned currentAttributes = current.m_attributes;
     unsigned overrideMask = 0;
     if (writablePresent())
         overrideMask |= ReadOnly;
@@ -221,7 +232,7 @@
         overrideMask |= DontDelete;
     if (isAccessorDescriptor())
         overrideMask |= Accessor;
-    return (m_attributes & overrideMask) | (current.m_attributes & ~overrideMask);
+    return (m_attributes & overrideMask) | (currentAttributes & ~overrideMask);
 }
 
 }

Modified: trunk/Source/_javascript_Core/runtime/PropertyDescriptor.h (104835 => 104836)


--- trunk/Source/_javascript_Core/runtime/PropertyDescriptor.h	2012-01-12 18:20:05 UTC (rev 104835)
+++ trunk/Source/_javascript_Core/runtime/PropertyDescriptor.h	2012-01-12 18:38:24 UTC (rev 104836)
@@ -51,6 +51,8 @@
         JSValue value() const { return m_value; }
         JSValue getter() const;
         JSValue setter() const;
+        JSObject* getterObject() const;
+        JSObject* setterObject() const;
         void setUndefined();
         void setDescriptor(JSValue value, unsigned attributes);
         void setAccessorDescriptor(GetterSetter* accessor, unsigned attributes);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to