Reviewed by Maciej.

        - Properly document and handle NULL callbacks for static properties. We
        throw an exception in any case other than a ReadOnly property with a NULL
        setProperty callback, because a NULL callback almost certainly indicates
        a programming error. Also throw an exception if hasProperty returns true
        for a property that getProperty can't get.

        - If a static setProperty callback returns 'false', to indicate that the
        property was not set, we no longer forward the set request up the class
        chain, because that's almost certainly not what the programmer expected.

        * API/JSCallbackObject.cpp:
        (KJS::JSCallbackObject::getOwnPropertySlot):
        (KJS::JSCallbackObject::put):
        (KJS::JSCallbackObject::staticValueGetter):
        (KJS::JSCallbackObject::staticFunctionGetter):
        (KJS::JSCallbackObject::callbackGetter):
        * API/JSObjectRef.h:
        * API/minidom.js:
        * API/testapi.c:
        (MyObject_hasProperty):
        * API/testapi.js:

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

diff --git a/JavaScriptCore/API/JSCallbackObject.cpp b/JavaScriptCore/API/JSCallbackObject.cpp
index 3b9e5104a5f333e092c701354e03a81c335b489b..60438d2df33d32569d53f18ff7df463fc7e9bd2b 100644 (file)
--- a/JavaScriptCore/API/JSCallbackObject.cpp
+++ b/JavaScriptCore/API/JSCallbackObject.cpp
@@ -107,11 +107,9 @@ bool JSCallbackObject::getOwnPropertySlot(ExecState* exec, const Identifier& pro
         }
 
         if (__JSClass::StaticValuesTable* staticValues = jsClass->staticValues) {
-            if (StaticValueEntry* entry = staticValues->get(propertyName.ustring().rep())) {
-                if (entry->getProperty) {
-                    slot.setCustom(this, staticValueGetter);
-                    return true;
-                }
+            if (staticValues->contains(propertyName.ustring().rep())) {
+                slot.setCustom(this, staticValueGetter);
+                return true;
             }
         }
         
@@ -148,10 +146,10 @@ void JSCallbackObject::put(ExecState* exec, const Identifier& propertyName, JSVa
             if (StaticValueEntry* entry = staticValues->get(propertyName.ustring().rep())) {
                 if (entry->attributes & kJSPropertyAttributeReadOnly)
                     return;
-                if (JSObjectSetPropertyCallback setProperty = entry->setProperty) {
-                    if (setProperty(context, thisRef, propertyNameRef, valueRef, toRef(exec->exceptionSlot())))
-                        return;
-                }
+                if (JSObjectSetPropertyCallback setProperty = entry->setProperty)
+                    setProperty(context, thisRef, propertyNameRef, valueRef, toRef(exec->exceptionSlot()));
+                else
+                    throwError(exec, ReferenceError, "Writable static value property defined with NULL setProperty callback.");
             }
         }
         
@@ -392,7 +390,7 @@ JSValue* JSCallbackObject::staticValueGetter(ExecState* exec, JSObject*, const I
                     if (JSValueRef value = getProperty(toRef(exec), thisRef, propertyNameRef, toRef(exec->exceptionSlot())))
                         return toJS(value);
 
-    return jsUndefined();
+    return throwError(exec, ReferenceError, "Static value property defined with NULL getProperty callback.");
 }
 
 JSValue* JSCallbackObject::staticFunctionGetter(ExecState* exec, JSObject*, const Identifier& propertyName, const PropertySlot& slot)
@@ -406,14 +404,16 @@ JSValue* JSCallbackObject::staticFunctionGetter(ExecState* exec, JSObject*, cons
     for (JSClassRef jsClass = thisObj->m_class; jsClass; jsClass = jsClass->parentClass) {
         if (__JSClass::StaticFunctionsTable* staticFunctions = jsClass->staticFunctions) {
             if (StaticFunctionEntry* entry = staticFunctions->get(propertyName.ustring().rep())) {
-                JSObject* o = new JSCallbackFunction(exec, entry->callAsFunction, propertyName);
-                thisObj->putDirect(propertyName, o, entry->attributes);
-                return o;
+                if (JSObjectCallAsFunctionCallback callAsFunction = entry->callAsFunction) {
+                    JSObject* o = new JSCallbackFunction(exec, callAsFunction, propertyName);
+                    thisObj->putDirect(propertyName, o, entry->attributes);
+                    return o;
+                }
             }
         }
     }
 
-    return jsUndefined();
+    return throwError(exec, ReferenceError, "Static function property defined with NULL callAsFunction callback.");
 }
 
 JSValue* JSCallbackObject::callbackGetter(ExecState* exec, JSObject*, const Identifier& propertyName, const PropertySlot& slot)
@@ -429,7 +429,7 @@ JSValue* JSCallbackObject::callbackGetter(ExecState* exec, JSObject*, const Iden
             if (JSValueRef value = getProperty(toRef(exec), thisRef, propertyNameRef, toRef(exec->exceptionSlot())))
                 return toJS(value);
 
-    return jsUndefined();
+    return throwError(exec, ReferenceError, "hasProperty callback returned true for a property that doesn't exist.");
 }
 
 } // namespace KJS

diff --git a/JavaScriptCore/API/JSObjectRef.h b/JavaScriptCore/API/JSObjectRef.h
index dafb41ac6764f37a986d3a8d994a3b4d6644c6af..d14fb01096a80ba4dd9c17d892508bb5d6033256 100644 (file)
--- a/JavaScriptCore/API/JSObjectRef.h
+++ b/JavaScriptCore/API/JSObjectRef.h
@@ -262,7 +262,7 @@ typedef JSValueRef
 @abstract This structure describes a statically declared value property.
 @field name A null-terminated UTF8 string containing the property's name.
 @field getProperty A JSObjectGetPropertyCallback to invoke when getting the property's value.
-@field setProperty A JSObjectSetPropertyCallback to invoke when setting the property's value.
+@field setProperty A JSObjectSetPropertyCallback to invoke when setting the property's value. May be NULL if the ReadOnly attribute is set.
 @field attributes A logically ORed set of JSPropertyAttributes to give to the property.
 */
 typedef struct {

diff --git a/JavaScriptCore/API/minidom.js b/JavaScriptCore/API/minidom.js
index 1910a235b91b9a9275dbb959ebafe7e5cba48e5f..4a817a05196f02b70dfb846d163fe29cb846b008 100644 (file)
--- a/JavaScriptCore/API/minidom.js
+++ b/JavaScriptCore/API/minidom.js
@@ -165,6 +165,21 @@ if (typeof window != 'undefined') {
     }
 }
 
+function shouldBe(a, b)
+{
+    var evalA;
+    try {
+        evalA = eval(a);
+    } catch(e) {
+        evalA = e;
+    }
+    
+    if (evalA == b || isNaN(evalA) && typeof evalA == 'number' && isNaN(b) && typeof b == 'number')
+        print("PASS: " + a + " should be " + b + " and is.", "green");
+    else
+        print("__FAIL__: " + a + " should be " + b + " but instead is " + evalA + ".", "red");
+}
+
 function test()
 {
     print("Node is " + Node);
@@ -221,6 +236,10 @@ function test()
     } catch(e) {
         print("caught: " + e);
     }
+    
+    oldNodeType = node.nodeType;
+    node.nodeType = 1;
+    shouldBe("node.nodeType", oldNodeType);
 
     /*
     var element, name, weapon;

diff --git a/JavaScriptCore/API/testapi.c b/JavaScriptCore/API/testapi.c
index 2bf20ffe896bd9af504d337e17ca8e3c3138def9..ae853a9aae976aa9c94654550dfb4c5fe450d0f8 100644 (file)
--- a/JavaScriptCore/API/testapi.c
+++ b/JavaScriptCore/API/testapi.c
@@ -114,7 +114,8 @@ static bool MyObject_hasProperty(JSContextRef context, JSObjectRef object, JSStr
 
     if (JSStringIsEqualToUTF8CString(propertyName, "alwaysOne")
         || JSStringIsEqualToUTF8CString(propertyName, "cantFind")
-        || JSStringIsEqualToUTF8CString(propertyName, "myPropertyName")) {
+        || JSStringIsEqualToUTF8CString(propertyName, "myPropertyName")
+        || JSStringIsEqualToUTF8CString(propertyName, "hasPropertyLie")) {
         return true;
     }
     
@@ -242,14 +243,24 @@ static void MyObject_finalize(JSObjectRef object)
     didFinalize = true;
 }
 
+static JSStaticValue evilStaticValues[] = {
+    { "nullGetSet", 0, 0, kJSPropertyAttributeNone },
+    { 0, 0, 0, 0 }
+};
+
+static JSStaticFunction evilStaticFunctions[] = {
+    { "nullCall", 0, kJSPropertyAttributeNone },
+    { 0, 0, 0 }
+};
+
 JSClassDefinition MyObject_definition = {
     0,
     
     "MyObject",
     NULL,
     
-    NULL,
-    NULL,
+    evilStaticValues,
+    evilStaticFunctions,
     
     MyObject_initialize,
     MyObject_finalize,

diff --git a/JavaScriptCore/API/testapi.js b/JavaScriptCore/API/testapi.js
index 3a4540d0798449ce3a4245fce9507833580d652e..0f8d75341fe51dbd502a97dd8018a84dd7ab1a12 100644 (file)
--- a/JavaScriptCore/API/testapi.js
+++ b/JavaScriptCore/API/testapi.js
@@ -39,6 +39,20 @@ function shouldBe(a, b)
         print("__FAIL__: " + a + " should be " + b + " but instead is " + evalA + ".", "red");
 }
 
+function shouldThrow(a)
+{
+    var result = "__FAIL__: " + a + " did not throw an exception.";
+    
+    var evalA;
+    try {
+        eval(a);
+    } catch(e) {
+        result = "PASS: " + a + " threw: " + e;
+    }
+    
+    print(result);
+}
+
 shouldBe("typeof MyObject", "function"); // our object implements 'call'
 MyObject.cantFind = 1;
 shouldBe("MyObject.cantFind", undefined);
@@ -68,11 +82,13 @@ print(foundRegularType
       ? "PASS: MyObject.regularType was enumerated"
       : "__FAIL__: MyObject.regularType was not enumerated");
 
+myObject = new MyObject();
+
 shouldBe("delete MyObject.regularType", true);
 shouldBe("MyObject.regularType", undefined);
 shouldBe("MyObject(0)", 1);
 shouldBe("MyObject()", undefined);
-shouldBe("typeof new MyObject()", "object");
+shouldBe("typeof myObject", "object");
 shouldBe("MyObject ? 1 : 0", true); // toBoolean
 shouldBe("+MyObject", 1); // toNumber
 shouldBe("(MyObject.toString())", "[object MyObject]"); // toString
@@ -82,5 +98,10 @@ shouldBe("typeof MyConstructor", "object");
 constructedObject = new MyConstructor(1);
 shouldBe("typeof constructedObject", "object");
 shouldBe("constructedObject.value", 1);
-shouldBe("(new MyObject()) instanceof MyObject", true);
+shouldBe("myObject instanceof MyObject", true);
 shouldBe("(new Object()) instanceof MyObject", false);
+
+shouldThrow("MyObject.nullGetSet = 1");
+shouldThrow("MyObject.nullGetSet");
+shouldThrow("MyObject.nullCall()");
+shouldThrow("MyObject.hasPropertyLie");

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 4198af18637a8f46c79d67bf8c265563e4a13e5f..568c762f60b95ecdc0f09d2dfd63a6662101c07e 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,29 @@
+2006-07-16  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Maciej.
+        
+        - Properly document and handle NULL callbacks for static properties. We
+        throw an exception in any case other than a ReadOnly property with a NULL
+        setProperty callback, because a NULL callback almost certainly indicates 
+        a programming error. Also throw an exception if hasProperty returns true
+        for a property that getProperty can't get.
+        
+        - If a static setProperty callback returns 'false', to indicate that the
+        property was not set, we no longer forward the set request up the class
+        chain, because that's almost certainly not what the programmer expected.
+
+        * API/JSCallbackObject.cpp:
+        (KJS::JSCallbackObject::getOwnPropertySlot):
+        (KJS::JSCallbackObject::put):
+        (KJS::JSCallbackObject::staticValueGetter):
+        (KJS::JSCallbackObject::staticFunctionGetter):
+        (KJS::JSCallbackObject::callbackGetter):
+        * API/JSObjectRef.h:
+        * API/minidom.js:
+        * API/testapi.c:
+        (MyObject_hasProperty):
+        * API/testapi.js:
+
 2006-07-16  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Maciej.