Implement [[DefineOwnProperty]] for the arguments object

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

Reviewed by Sam Weinig.

Source/JavaScriptCore:

* runtime/Arguments.cpp:
(JSC::Arguments::deletePropertyByIndex):
(JSC::Arguments::deleteProperty):
    - Deleting an argument should also delete the copy on the object, if any.
(JSC::Arguments::defineOwnProperty):
    - Defining a property may override the live mapping.
* runtime/Arguments.h:
(Arguments):

LayoutTests:

* fast/js/arguments-expected.txt:
* fast/js/script-tests/arguments.js:
    - Added test cases

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index f13181e..e88fdf1 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2012-02-22  Gavin Barraclough  <barraclough@apple.com>
+
+        Implement [[DefineOwnProperty]] for the arguments object
+        https://bugs.webkit.org/show_bug.cgi?id=79309
+
+        Reviewed by Sam Weinig.
+
+        * fast/js/arguments-expected.txt:
+        * fast/js/script-tests/arguments.js:
+            - Added test cases
+
 2012-02-22  Adrienne Walker  <enne@google.com>
 
         [chromium] Unreviewed gardening. Update expectations after skia roll.

diff --git a/LayoutTests/fast/js/arguments-expected.txt b/LayoutTests/fast/js/arguments-expected.txt
index 8f95b06..09b39f5 100644 (file)
--- a/LayoutTests/fast/js/arguments-expected.txt
+++ b/LayoutTests/fast/js/arguments-expected.txt
@@ -145,6 +145,30 @@ PASS descriptor.value is "one"
 PASS descriptor.writable is true
 PASS descriptor.enumerable is true
 PASS descriptor.configurable is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
+PASS true is true
 PASS successfullyParsed is true
 
 TEST COMPLETE

diff --git a/LayoutTests/fast/js/script-tests/arguments.js b/LayoutTests/fast/js/script-tests/arguments.js
index 790e148..ca83842 100644 (file)
--- a/LayoutTests/fast/js/script-tests/arguments.js
+++ b/LayoutTests/fast/js/script-tests/arguments.js
@@ -594,3 +594,53 @@ shouldBe("descriptor.value", '"one"');
 shouldBe("descriptor.writable", 'true');
 shouldBe("descriptor.enumerable", 'true');
 shouldBe("descriptor.configurable", 'true');
+
+// Test cases for [[DefineOwnProperty]] applied to the arguments object.
+(function(a0,a1,a2,a3){
+    Object.defineProperties(arguments, {
+        1: { get: function(){ return 201; } },
+        2: { value: 202, writable: false },
+        3: { writable: false },
+    });
+
+    // Test a0 is a live mapped argument.
+    shouldBeTrue(String( a0 === 100 ));
+    shouldBeTrue(String( arguments[0] === 100 ));
+    a0 = 300;
+    shouldBeTrue(String( a0 === 300 ));
+    shouldBeTrue(String( arguments[0] === 300 ));
+    arguments[0] = 400;
+    shouldBeTrue(String( a0 === 400 ));
+    shouldBeTrue(String( arguments[0] === 400 ));
+
+    // When a1 is redefined as an accessor, it is no longer live.
+    shouldBeTrue(String( a1 === 101 ));
+    shouldBeTrue(String( arguments[1] === 201 ));
+    a1 = 301;
+    shouldBeTrue(String( a1 === 301 ));
+    shouldBeTrue(String( arguments[1] === 201 ));
+    arguments[1] = 401;
+    shouldBeTrue(String( a1 === 301 ));
+    shouldBeTrue(String( arguments[1] === 201 ));
+
+    // When a2 is make read-only the value is set, but it is no longer live.
+    shouldBeTrue(String( a2 === 202 ));
+    shouldBeTrue(String( arguments[2] === 202 ));
+    a2 = 302;
+    shouldBeTrue(String( a2 === 302 ));
+    shouldBeTrue(String( arguments[2] === 202 ));
+    arguments[2] = 402;
+    shouldBeTrue(String( a2 === 302 ));
+    shouldBeTrue(String( arguments[2] === 202 ));
+
+    // When a3 is make read-only it remains live.
+    shouldBeTrue(String( a3 === 103 ));
+    shouldBeTrue(String( arguments[3] === 103 ));
+    a3 = 303;
+    shouldBeTrue(String( a3 === 303 ));
+    shouldBeTrue(String( arguments[3] === 303 ));
+    arguments[3] = 403;
+    shouldBeTrue(String( a3 === 403 ));
+    shouldBeTrue(String( arguments[3] === 403 ));
+
+})(100,101,102,103);

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index cfb82d9..cf188a4 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,5 +1,21 @@
 2012-02-22  Gavin Barraclough  <barraclough@apple.com>
 
+        Implement [[DefineOwnProperty]] for the arguments object
+        https://bugs.webkit.org/show_bug.cgi?id=79309
+
+        Reviewed by Sam Weinig.
+
+        * runtime/Arguments.cpp:
+        (JSC::Arguments::deletePropertyByIndex):
+        (JSC::Arguments::deleteProperty):
+            - Deleting an argument should also delete the copy on the object, if any.
+        (JSC::Arguments::defineOwnProperty):
+            - Defining a property may override the live mapping.
+        * runtime/Arguments.h:
+        (Arguments):
+
+2012-02-22  Gavin Barraclough  <barraclough@apple.com>
+
         Fix Object.freeze for non-final objects.
         https://bugs.webkit.org/show_bug.cgi?id=79286
 

diff --git a/Source/JavaScriptCore/runtime/Arguments.cpp b/Source/JavaScriptCore/runtime/Arguments.cpp
index 6a675ab..2bbad02 100644 (file)
--- a/Source/JavaScriptCore/runtime/Arguments.cpp
+++ b/Source/JavaScriptCore/runtime/Arguments.cpp
@@ -243,6 +243,9 @@ bool Arguments::deletePropertyByIndex(JSCell* cell, ExecState* exec, unsigned i)
 {
     Arguments* thisObject = jsCast<Arguments*>(cell);
     if (i < thisObject->d->numArguments) {
+        if (!Base::deletePropertyByIndex(cell, exec, i))
+            return false;
+
         if (!thisObject->d->deletedArguments) {
             thisObject->d->deletedArguments = adoptArrayPtr(new bool[thisObject->d->numArguments]);
             memset(thisObject->d->deletedArguments.get(), 0, sizeof(bool) * thisObject->d->numArguments);
@@ -258,10 +261,16 @@ bool Arguments::deletePropertyByIndex(JSCell* cell, ExecState* exec, unsigned i)
 
 bool Arguments::deleteProperty(JSCell* cell, ExecState* exec, const Identifier& propertyName) 
 {
+    if (exec->globalData().isInDefineOwnProperty())
+        return Base::deleteProperty(cell, exec, propertyName);
+
     Arguments* thisObject = jsCast<Arguments*>(cell);
     bool isArrayIndex;
     unsigned i = propertyName.toArrayIndex(isArrayIndex);
     if (isArrayIndex && i < thisObject->d->numArguments) {
+        if (!Base::deleteProperty(cell, exec, propertyName))
+            return false;
+
         if (!thisObject->d->deletedArguments) {
             thisObject->d->deletedArguments = adoptArrayPtr(new bool[thisObject->d->numArguments]);
             memset(thisObject->d->deletedArguments.get(), 0, sizeof(bool) * thisObject->d->numArguments);
@@ -285,12 +294,44 @@ bool Arguments::deleteProperty(JSCell* cell, ExecState* exec, const Identifier&
         thisObject->createStrictModeCalleeIfNecessary(exec);
     }
     
-    if (propertyName == exec->propertyNames().caller && !thisObject->d->isStrictMode)
+    if (propertyName == exec->propertyNames().caller && thisObject->d->isStrictMode)
         thisObject->createStrictModeCallerIfNecessary(exec);
 
     return JSObject::deleteProperty(thisObject, exec, propertyName);
 }
 
+bool Arguments::defineOwnProperty(JSObject* object, ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor, bool shouldThrow)
+{
+    if (!Base::defineOwnProperty(object, exec, propertyName, descriptor, shouldThrow))
+        return false;
+
+    Arguments* thisObject = jsCast<Arguments*>(object);
+    bool isArrayIndex;
+    unsigned i = propertyName.toArrayIndex(isArrayIndex);
+    if (isArrayIndex && i < thisObject->d->numArguments) {
+        if (!thisObject->d->deletedArguments) {
+            thisObject->d->deletedArguments = adoptArrayPtr(new bool[thisObject->d->numArguments]);
+            memset(thisObject->d->deletedArguments.get(), 0, sizeof(bool) * thisObject->d->numArguments);
+        }
+        // From ES 5.1, 10.6 Arguments Object
+        // 5. If the value of isMapped is not undefined, then
+        if (!thisObject->d->deletedArguments[i]) {
+            // a. If IsAccessorDescriptor(Desc) is true, then
+            if (descriptor.isAccessorDescriptor()) {
+                // i. Call the [[Delete]] internal method of map passing P, and false as the arguments.
+                thisObject->d->deletedArguments[i] = true;
+            } else if (descriptor.value()) { // b. Else i. If Desc.[[Value]] is present, then
+                // 1. Call the [[Put]] internal method of map passing P, Desc.[[Value]], and Throw as the arguments.
+                // ii. If Desc.[[Writable]] is present and its value is false, then
+                thisObject->argument(i).set(exec->globalData(), thisObject, descriptor.value());
+                if (descriptor.writablePresent() && !descriptor.writable())
+                    thisObject->d->deletedArguments[i] = true; // 1. Call the [[Delete]] internal method of map passing P and false as arguments.
+            }
+        }
+    }
+    return true;
+}
+
 void Arguments::tearOff(CallFrame* callFrame)
 {
     if (isTornOff())

diff --git a/Source/JavaScriptCore/runtime/Arguments.h b/Source/JavaScriptCore/runtime/Arguments.h
index 3564fe4..ee54a49 100644 (file)
--- a/Source/JavaScriptCore/runtime/Arguments.h
+++ b/Source/JavaScriptCore/runtime/Arguments.h
@@ -117,6 +117,7 @@ namespace JSC {
         static void putByIndex(JSCell*, ExecState*, unsigned propertyName, JSValue);
         static bool deleteProperty(JSCell*, ExecState*, const Identifier& propertyName);
         static bool deletePropertyByIndex(JSCell*, ExecState*, unsigned propertyName);
+        static bool defineOwnProperty(JSObject*, ExecState*, const Identifier& propertyName, PropertyDescriptor&, bool shouldThrow);
         void createStrictModeCallerIfNecessary(ExecState*);
         void createStrictModeCalleeIfNecessary(ExecState*);