Writable attribute not set correctly when redefining an accessor to a data descriptor
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Feb 2012 19:59:43 +0000 (19:59 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Feb 2012 19:59:43 +0000 (19:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=79931

Reviewed by Oliver Hunt.

Source/JavaScriptCore:

* runtime/JSObject.cpp:
(JSC::JSObject::defineOwnProperty):
    - use attributesOverridingCurrent instead of attributesWithOverride.
* runtime/PropertyDescriptor.cpp:
* runtime/PropertyDescriptor.h:
    - remove attributesWithOverride - attributesOverridingCurrent does the same thing.

LayoutTests:

* fast/js/Object-defineProperty-expected.txt:
* fast/js/script-tests/Object-defineProperty.js:
    - Added tests.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@109240 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/js/Object-defineProperty-expected.txt
LayoutTests/fast/js/script-tests/Object-defineProperty.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/PropertyDescriptor.cpp
Source/JavaScriptCore/runtime/PropertyDescriptor.h

index 295ffa6..1dc1735 100644 (file)
@@ -1,3 +1,14 @@
+2012-02-29  Gavin Barraclough  <barraclough@apple.com>
+
+        Writable attribute not set correctly when redefining an accessor to a data descriptor
+        https://bugs.webkit.org/show_bug.cgi?id=79931
+
+        Reviewed by Oliver Hunt.
+
+        * fast/js/Object-defineProperty-expected.txt:
+        * fast/js/script-tests/Object-defineProperty.js:
+            - Added tests.
+
 2012-02-29  Max Feil  <mfeil@rim.com>
 
         [BlackBerry] Add support for FLAC audio and OGG/Vorbis audio
index 5d4a7b7..d6b94d1 100644 (file)
@@ -116,6 +116,9 @@ PASS 0 in Object.prototype is true
 PASS '0' in Object.prototype is true
 PASS var o = {}; o.readOnly = false; o.readOnly is true
 PASS 'use strict'; var o = {}; o.readOnly = false; o.readOnly threw exception TypeError: Attempted to assign to readonly property..
+PASS Object.getOwnPropertyDescriptor(Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return false; }, configurable: true}), 'foo', {value:false}), 'foo').writable is false
+PASS Object.getOwnPropertyDescriptor(Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return false; }, configurable: true}), 'foo', {value:false, writable: false}), 'foo').writable is false
+PASS Object.getOwnPropertyDescriptor(Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return false; }, configurable: true}), 'foo', {value:false, writable: true}), 'foo').writable is true
 PASS successfullyParsed is true
 
 TEST COMPLETE
index a35f6a8..2015a4d 100644 (file)
@@ -167,4 +167,7 @@ shouldBeTrue("var o = {}; o.readOnly = false; o.readOnly");
 shouldThrow("'use strict'; var o = {}; o.readOnly = false; o.readOnly");
 delete Object.prototype.readOnly;
 
-
+// Check the writable attribute is set correctly when redefining an accessor as a data descriptor.
+shouldBeFalse("Object.getOwnPropertyDescriptor(Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return false; }, configurable: true}), 'foo', {value:false}), 'foo').writable");
+shouldBeFalse("Object.getOwnPropertyDescriptor(Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return false; }, configurable: true}), 'foo', {value:false, writable: false}), 'foo').writable");
+shouldBeTrue("Object.getOwnPropertyDescriptor(Object.defineProperty(Object.defineProperty({}, 'foo', {get: function() { return false; }, configurable: true}), 'foo', {value:false, writable: true}), 'foo').writable");
index 908508b..1d1f71b 100644 (file)
@@ -1,3 +1,17 @@
+2012-02-29  Gavin Barraclough  <barraclough@apple.com>
+
+        Writable attribute not set correctly when redefining an accessor to a data descriptor
+        https://bugs.webkit.org/show_bug.cgi?id=79931
+
+        Reviewed by Oliver Hunt.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::defineOwnProperty):
+            - use attributesOverridingCurrent instead of attributesWithOverride.
+        * runtime/PropertyDescriptor.cpp:
+        * runtime/PropertyDescriptor.h:
+            - remove attributesWithOverride - attributesOverridingCurrent does the same thing.
+
 2012-02-29  Kevin Ollivier  <kevino@theolliviers.com>
 
         Add JSCore symbol exports needed by wx port
index a1703a7..edc214b 100644 (file)
@@ -702,7 +702,7 @@ bool JSObject::defineOwnProperty(JSObject* object, ExecState* exec, const Identi
     if (descriptor.isGenericDescriptor()) {
         if (!current.attributesEqual(descriptor)) {
             object->methodTable()->deleteProperty(object, exec, propertyName);
-            return putDescriptor(exec, object, propertyName, descriptor, current.attributesWithOverride(descriptor), current);
+            return putDescriptor(exec, object, propertyName, descriptor, descriptor.attributesOverridingCurrent(current), current);
         }
         return true;
     }
@@ -715,7 +715,7 @@ bool JSObject::defineOwnProperty(JSObject* object, ExecState* exec, const Identi
             return false;
         }
         object->methodTable()->deleteProperty(object, exec, propertyName);
-        return putDescriptor(exec, object, propertyName, descriptor, current.attributesWithOverride(descriptor), current);
+        return putDescriptor(exec, object, propertyName, descriptor, descriptor.attributesOverridingCurrent(current), current);
     }
 
     // Changing the value and attributes of an existing property
@@ -737,7 +737,7 @@ bool JSObject::defineOwnProperty(JSObject* object, ExecState* exec, const Identi
         if (current.attributesEqual(descriptor) && !descriptor.value())
             return true;
         object->methodTable()->deleteProperty(object, exec, propertyName);
-        return putDescriptor(exec, object, propertyName, descriptor, current.attributesWithOverride(descriptor), current);
+        return putDescriptor(exec, object, propertyName, descriptor, descriptor.attributesOverridingCurrent(current), current);
     }
 
     // Changing the accessor functions of an existing accessor property
index 0cb6295..236a8e5 100644 (file)
@@ -206,22 +206,6 @@ bool PropertyDescriptor::attributesEqual(const PropertyDescriptor& other) const
     return true;
 }
 
-unsigned PropertyDescriptor::attributesWithOverride(const PropertyDescriptor& other) const
-{
-    unsigned mismatch = other.m_attributes ^ m_attributes;
-    unsigned sharedSeen = other.m_seenAttributes & m_seenAttributes;
-    unsigned newAttributes = m_attributes & defaultAttributes;
-    if (sharedSeen & WritablePresent && mismatch & ReadOnly)
-        newAttributes ^= ReadOnly;
-    if (sharedSeen & ConfigurablePresent && mismatch & DontDelete)
-        newAttributes ^= DontDelete;
-    if (sharedSeen & EnumerablePresent && mismatch & DontEnum)
-        newAttributes ^= DontEnum;
-    if (isAccessorDescriptor() && other.isDataDescriptor())
-        newAttributes |= ReadOnly;
-    return newAttributes;
-}
-
 unsigned PropertyDescriptor::attributesOverridingCurrent(const PropertyDescriptor& current) const
 {
     unsigned currentAttributes = current.m_attributes;
index 98af02e..2c3878f 100644 (file)
@@ -70,7 +70,6 @@ namespace JSC {
         bool getterPresent() const { return m_getter; }
         bool equalTo(ExecState* exec, const PropertyDescriptor& other) const;
         bool attributesEqual(const PropertyDescriptor& other) const;
-        unsigned attributesWithOverride(const PropertyDescriptor& other) const;
         unsigned attributesOverridingCurrent(const PropertyDescriptor& current) const;
 
     private: