Should be able to reconfigure a non-configurable property as read-only

https://bugs.webkit.org/show_bug.cgi?id=79170

Reviewed by Sam Weinig.

See ES5.1 8.12.9 10.a.i - the spec prohibits making a read-only property writable,
but does not inhibit making a writable property read-only.

Source/JavaScriptCore:

* runtime/JSGlobalData.cpp:
(JSC::JSGlobalData::JSGlobalData):
* runtime/JSGlobalData.h:
(JSC::JSGlobalData::setInDefineOwnProperty):
(JSGlobalData):
(JSC::JSGlobalData::isInDefineOwnProperty):
    - Added flag, tracking whether we are in JSObject::defineOwnProperty.
* runtime/JSObject.cpp:
(JSC::JSObject::deleteProperty):
(DefineOwnPropertyScope):
    - Always allow properties to be deleted by DefineOwnProperty - assume it knows what it is doing!
(JSC::DefineOwnPropertyScope::DefineOwnPropertyScope):
(JSC::DefineOwnPropertyScope::~DefineOwnPropertyScope):
    - Added RAII helper.
(JSC::JSObject::defineOwnProperty):
    - Track on the globalData when we are in this method.

LayoutTests:

* fast/js/Object-defineProperty-expected.txt:
* fast/js/script-tests/Object-defineProperty.js:
    - Update test result (this was enforcing incorrect behaviour).

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 1abd227..f651b53 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2012-02-21  Gavin Barraclough  <barraclough@apple.com>
+
+        Should be able to reconfigure a non-configurable property as read-only
+        https://bugs.webkit.org/show_bug.cgi?id=79170
+
+        Reviewed by Sam Weinig.
+
+        See ES5.1 8.12.9 10.a.i - the spec prohibits making a read-only property writable,
+        but does not inhibit making a writable property read-only.
+
+        * fast/js/Object-defineProperty-expected.txt:
+        * fast/js/script-tests/Object-defineProperty.js:
+            - Update test result (this was enforcing incorrect behaviour).
+
 2012-02-21  Adam Klein  <adamk@chromium.org>
 
         Setting innerText causes DOMSubtreeModified to be dispatched too early

diff --git a/LayoutTests/fast/js/Object-defineProperty-expected.txt b/LayoutTests/fast/js/Object-defineProperty-expected.txt
index 5932471..5d4a7b7 100644 (file)
--- a/LayoutTests/fast/js/Object-defineProperty-expected.txt
+++ b/LayoutTests/fast/js/Object-defineProperty-expected.txt
@@ -28,7 +28,7 @@ PASS Object.defineProperty(createUnconfigurableProperty({}, 'foo'), 'foo', {conf
 PASS Object.defineProperty(createUnconfigurableProperty({}, 'foo'), 'foo', {writable: true}) threw exception TypeError: Attempting to change writable attribute of unconfigurable property..
 PASS Object.defineProperty(createUnconfigurableProperty({}, 'foo'), 'foo', {enumerable: true}) threw exception TypeError: Attempting to change enumerable attribute of unconfigurable property..
 PASS Object.defineProperty(createUnconfigurableProperty({}, 'foo', false, true), 'foo', {enumerable: false}), 'foo' threw exception TypeError: Attempting to change enumerable attribute of unconfigurable property..
-PASS JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty(createUnconfigurableProperty({}, 'foo', true), 'foo', {writable: false}), 'foo')) is JSON.stringify({value: 1, writable: true, enumerable: false, configurable: false})
+PASS JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty(createUnconfigurableProperty({}, 'foo', true), 'foo', {writable: false}), 'foo')) is JSON.stringify({value: 1, writable: false, enumerable: false, configurable: false})
 PASS Object.defineProperty({}, 'foo', {value:1, get: function(){}}) threw exception TypeError: Invalid property.  'value' present on property with getter or setter..
 PASS Object.defineProperty({}, 'foo', {value:1, set: function(){}}) threw exception TypeError: Invalid property.  'value' present on property with getter or setter..
 PASS Object.defineProperty({}, 'foo', {writable:true, get: function(){}}) threw exception TypeError: Invalid property.  'writable' present on property with getter or setter..

diff --git a/LayoutTests/fast/js/script-tests/Object-defineProperty.js b/LayoutTests/fast/js/script-tests/Object-defineProperty.js
index 34e4a32..a35f6a8 100644 (file)
--- a/LayoutTests/fast/js/script-tests/Object-defineProperty.js
+++ b/LayoutTests/fast/js/script-tests/Object-defineProperty.js
@@ -44,7 +44,7 @@ shouldThrow("Object.defineProperty(createUnconfigurableProperty({}, 'foo'), 'foo
 shouldThrow("Object.defineProperty(createUnconfigurableProperty({}, 'foo', false, true), 'foo', {enumerable: false}), 'foo'");
 
 shouldBe("JSON.stringify(Object.getOwnPropertyDescriptor(Object.defineProperty(createUnconfigurableProperty({}, 'foo', true), 'foo', {writable: false}), 'foo'))",
-         "JSON.stringify({value: 1, writable: true, enumerable: false, configurable: false})");
+         "JSON.stringify({value: 1, writable: false, enumerable: false, configurable: false})");
 
 shouldThrow("Object.defineProperty({}, 'foo', {value:1, get: function(){}})");
 shouldThrow("Object.defineProperty({}, 'foo', {value:1, set: function(){}})");

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 2c2d697..9766fb6 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,30 @@
+2012-02-21  Gavin Barraclough  <barraclough@apple.com>
+
+        Should be able to reconfigure a non-configurable property as read-only
+        https://bugs.webkit.org/show_bug.cgi?id=79170
+
+        Reviewed by Sam Weinig.
+
+        See ES5.1 8.12.9 10.a.i - the spec prohibits making a read-only property writable,
+        but does not inhibit making a writable property read-only.
+
+        * runtime/JSGlobalData.cpp:
+        (JSC::JSGlobalData::JSGlobalData):
+        * runtime/JSGlobalData.h:
+        (JSC::JSGlobalData::setInDefineOwnProperty):
+        (JSGlobalData):
+        (JSC::JSGlobalData::isInDefineOwnProperty):
+            - Added flag, tracking whether we are in JSObject::defineOwnProperty.
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::deleteProperty):
+        (DefineOwnPropertyScope):
+            - Always allow properties to be deleted by DefineOwnProperty - assume it knows what it is doing!
+        (JSC::DefineOwnPropertyScope::DefineOwnPropertyScope):
+        (JSC::DefineOwnPropertyScope::~DefineOwnPropertyScope):
+            - Added RAII helper.
+        (JSC::JSObject::defineOwnProperty):
+            - Track on the globalData when we are in this method.
+
 2012-02-21  Oliver Hunt  <oliver@apple.com>
 
         Make TypedArrays be available in commandline jsc

diff --git a/Source/JavaScriptCore/runtime/JSGlobalData.cpp b/Source/JavaScriptCore/runtime/JSGlobalData.cpp
index 35ecdcf..e3593ff 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSGlobalData.cpp
+++ b/Source/JavaScriptCore/runtime/JSGlobalData.cpp
@@ -160,6 +160,7 @@ JSGlobalData::JSGlobalData(GlobalDataType globalDataType, ThreadStackType thread
 #if ENABLE(GC_VALIDATION)
     , m_isInitializingObject(false)
 #endif
+    , m_inDefineOwnProperty(false)
 {
     interpreter = new Interpreter;
 

diff --git a/Source/JavaScriptCore/runtime/JSGlobalData.h b/Source/JavaScriptCore/runtime/JSGlobalData.h
index b30d1bf..96a69de 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSGlobalData.h
+++ b/Source/JavaScriptCore/runtime/JSGlobalData.h
@@ -211,6 +211,16 @@ namespace JSC {
             codeBlocksBeingCompiled.removeLast();
         }
 
+        void setInDefineOwnProperty(bool inDefineOwnProperty)
+        {
+            m_inDefineOwnProperty = inDefineOwnProperty;
+        }
+
+        bool isInDefineOwnProperty()
+        {
+            return m_inDefineOwnProperty;
+        }
+
 #if ENABLE(ASSEMBLER)
         ExecutableAllocator executableAllocator;
 #endif
@@ -355,6 +365,8 @@ namespace JSC {
 #if ENABLE(GC_VALIDATION)
         bool m_isInitializingObject;
 #endif
+        bool m_inDefineOwnProperty;
+
         TypedArrayDescriptor m_int8ArrayDescriptor;
         TypedArrayDescriptor m_int16ArrayDescriptor;
         TypedArrayDescriptor m_int32ArrayDescriptor;

diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
index 5875a5c..acc4a18 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSObject.cpp
@@ -268,7 +268,7 @@ bool JSObject::deleteProperty(JSCell* cell, ExecState* exec, const Identifier& p
     unsigned attributes;
     JSCell* specificValue;
     if (thisObject->structure()->get(exec->globalData(), propertyName, attributes, specificValue) != WTF::notFound) {
-        if ((attributes & DontDelete))
+        if (attributes & DontDelete && !exec->globalData().isInDefineOwnProperty())
             return false;
         thisObject->removeDirect(exec->globalData(), propertyName);
         return true;
@@ -276,7 +276,7 @@ bool JSObject::deleteProperty(JSCell* cell, ExecState* exec, const Identifier& p
 
     // Look in the static hashtable of properties
     const HashEntry* entry = thisObject->findPropertyHashEntry(exec, propertyName);
-    if (entry && entry->attributes() & DontDelete)
+    if (entry && entry->attributes() & DontDelete && !exec->globalData().isInDefineOwnProperty())
         return false; // this builtin property can't be deleted
 
     // FIXME: Should the code here actually do some deletion?
@@ -644,8 +644,31 @@ static bool putDescriptor(ExecState* exec, JSObject* target, const Identifier& p
     return true;
 }
 
+class DefineOwnPropertyScope {
+public:
+    DefineOwnPropertyScope(ExecState* exec)
+        : m_globalData(exec->globalData())
+    {
+        m_globalData.setInDefineOwnProperty(true);
+    }
+
+    ~DefineOwnPropertyScope()
+    {
+        m_globalData.setInDefineOwnProperty(false);
+    }
+
+private:
+    JSGlobalData& m_globalData;
+};
+
 bool JSObject::defineOwnProperty(JSObject* object, ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor, bool throwException)
 {
+    // Track on the globaldata that we're in define property.
+    // Currently DefineOwnProperty uses delete to remove properties when they are being replaced
+    // (particularly when changing attributes), however delete won't allow non-configurable (i.e.
+    // DontDelete) properties to be deleted. For now, we can use this flag to make this work.
+    DefineOwnPropertyScope scope(exec);
+
     // If we have a new property we can just put it on normally
     PropertyDescriptor current;
     if (!object->methodTable()->getOwnPropertyDescriptor(object, exec, propertyName, current)) {