2011-05-27 Geoffrey Garen <ggaren@apple.com>

        Reviewed by Oliver Hunt.

        JS API is too aggressive about throwing exceptions for NULL get or set operations
        https://bugs.webkit.org/show_bug.cgi?id=61678

        * API/JSCallbackObject.h: Changed our staticValueGetter to a regular
        function that returns a JSValue, so it can fail and still forward to
        normal property lookup.

        * API/JSCallbackObjectFunctions.h:
        (JSC::::getOwnPropertySlot): Don't throw an exception when failing to
        access a static property -- just forward the access. This allows objects
        to observe get/set operations but still let the JS object manage lifetime.

        (JSC::::put): Ditto.

        (JSC::::getStaticValue): Same as JSCallbackObject.h.

        * API/tests/testapi.c:
        (MyObject_set_nullGetForwardSet):
        * API/tests/testapi.js: Updated tests to reflect slightly less strict
        behavior, which matches headerdoc claims.

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

diff --git a/Source/JavaScriptCore/API/JSCallbackObject.h b/Source/JavaScriptCore/API/JSCallbackObject.h
index 468589a..397ccda 100644 (file)
--- a/Source/JavaScriptCore/API/JSCallbackObject.h
+++ b/Source/JavaScriptCore/API/JSCallbackObject.h
@@ -188,7 +188,7 @@ private:
     static EncodedJSValue JSC_HOST_CALL call(ExecState*);
     static EncodedJSValue JSC_HOST_CALL construct(ExecState*);
    
-    static JSValue staticValueGetter(ExecState*, JSValue, const Identifier&);
+    JSValue getStaticValue(ExecState*, const Identifier&);
     static JSValue staticFunctionGetter(ExecState*, JSValue, const Identifier&);
     static JSValue callbackGetter(ExecState*, JSValue, const Identifier&);
 

diff --git a/Source/JavaScriptCore/API/JSCallbackObjectFunctions.h b/Source/JavaScriptCore/API/JSCallbackObjectFunctions.h
index 8639e1a..cc8f3a8 100644 (file)
--- a/Source/JavaScriptCore/API/JSCallbackObjectFunctions.h
+++ b/Source/JavaScriptCore/API/JSCallbackObjectFunctions.h
@@ -149,8 +149,11 @@ bool JSCallbackObject<Base>::getOwnPropertySlot(ExecState* exec, const Identifie
         
         if (OpaqueJSClassStaticValuesTable* staticValues = jsClass->staticValues(exec)) {
             if (staticValues->contains(propertyName.impl())) {
-                slot.setCustom(this, staticValueGetter);
-                return true;
+                JSValue value = getStaticValue(exec, propertyName);
+                if (value) {
+                    slot.setValue(value);
+                    return true;
+                }
             }
         }
         
@@ -225,8 +228,7 @@ void JSCallbackObject<Base>::put(ExecState* exec, const Identifier& propertyName
                         throwError(exec, toJS(exec, exception));
                     if (result || exception)
                         return;
-                } else
-                    throwError(exec, createReferenceError(exec, "Attempt to set a property that is not settable."));
+                }
             }
         }
         
@@ -515,14 +517,12 @@ bool JSCallbackObject<Base>::inherits(JSClassRef c) const
 }
 
 template <class Base>
-JSValue JSCallbackObject<Base>::staticValueGetter(ExecState* exec, JSValue slotBase, const Identifier& propertyName)
+JSValue JSCallbackObject<Base>::getStaticValue(ExecState* exec, const Identifier& propertyName)
 {
-    JSCallbackObject* thisObj = asCallbackObject(slotBase);
-    
-    JSObjectRef thisRef = toRef(thisObj);
+    JSObjectRef thisRef = toRef(this);
     RefPtr<OpaqueJSString> propertyNameRef;
     
-    for (JSClassRef jsClass = thisObj->classRef(); jsClass; jsClass = jsClass->parentClass)
+    for (JSClassRef jsClass = classRef(); jsClass; jsClass = jsClass->parentClass)
         if (OpaqueJSClassStaticValuesTable* staticValues = jsClass->staticValues(exec))
             if (StaticValueEntry* entry = staticValues->get(propertyName.impl()))
                 if (JSObjectGetPropertyCallback getProperty = entry->getProperty) {
@@ -542,7 +542,7 @@ JSValue JSCallbackObject<Base>::staticValueGetter(ExecState* exec, JSValue slotB
                         return toJS(exec, value);
                 }
 
-    return throwError(exec, createReferenceError(exec, "Static value property defined with NULL getProperty callback."));
+    return JSValue();
 }
 
 template <class Base>

diff --git a/Source/JavaScriptCore/API/tests/testapi.c b/Source/JavaScriptCore/API/tests/testapi.c
index 3af5427..e82d41d 100644 (file)
--- a/Source/JavaScriptCore/API/tests/testapi.c
+++ b/Source/JavaScriptCore/API/tests/testapi.c
@@ -311,8 +311,19 @@ static JSValueRef MyObject_convertToType(JSContextRef context, JSObjectRef objec
     return JSValueMakeNull(context);
 }
 
+static bool MyObject_set_nullGetForwardSet(JSContextRef ctx, JSObjectRef object, JSStringRef propertyName, JSValueRef value, JSValueRef* exception)
+{
+    UNUSED_PARAM(ctx);
+    UNUSED_PARAM(object);
+    UNUSED_PARAM(propertyName);
+    UNUSED_PARAM(value);
+    UNUSED_PARAM(exception);
+    return false; // Forward to parent class.
+}
+
 static JSStaticValue evilStaticValues[] = {
     { "nullGetSet", 0, 0, kJSPropertyAttributeNone },
+    { "nullGetForwardSet", 0, MyObject_set_nullGetForwardSet, kJSPropertyAttributeNone },
     { 0, 0, 0, 0 }
 };
 

diff --git a/Source/JavaScriptCore/API/tests/testapi.js b/Source/JavaScriptCore/API/tests/testapi.js
index be64ed1..61d4533 100644 (file)
--- a/Source/JavaScriptCore/API/tests/testapi.js
+++ b/Source/JavaScriptCore/API/tests/testapi.js
@@ -94,6 +94,9 @@ shouldBe("MyObject('throwOnCall')", "an exception");
 shouldBe("new MyObject('throwOnConstruct')", "an exception");
 shouldBe("'throwOnHasInstance' instanceof MyObject", "an exception");
 
+MyObject.nullGetForwardSet = 1;
+shouldBe("MyObject.nullGetForwardSet", 1);
+
 var foundMyPropertyName = false;
 var foundRegularType = false;
 for (var p in MyObject) {
@@ -162,8 +165,8 @@ shouldBe("constructedObject.value", 1);
 shouldBe("myObject instanceof MyObject", true);
 shouldBe("(new Object()) instanceof MyObject", false);
 
-shouldThrow("MyObject.nullGetSet = 1");
-shouldThrow("MyObject.nullGetSet");
+MyObject.nullGetSet = 1;
+shouldBe("MyObject.nullGetSet", 1);
 shouldThrow("MyObject.nullCall()");
 shouldThrow("MyObject.hasPropertyLie");
 

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 3a73fe9..d4e1f9d 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -2,6 +2,31 @@
 
         Reviewed by Oliver Hunt.
 
+        JS API is too aggressive about throwing exceptions for NULL get or set operations
+        https://bugs.webkit.org/show_bug.cgi?id=61678
+
+        * API/JSCallbackObject.h: Changed our staticValueGetter to a regular
+        function that returns a JSValue, so it can fail and still forward to
+        normal property lookup.
+
+        * API/JSCallbackObjectFunctions.h:
+        (JSC::::getOwnPropertySlot): Don't throw an exception when failing to
+        access a static property -- just forward the access. This allows objects
+        to observe get/set operations but still let the JS object manage lifetime.
+
+        (JSC::::put): Ditto.
+
+        (JSC::::getStaticValue): Same as JSCallbackObject.h.
+
+        * API/tests/testapi.c:
+        (MyObject_set_nullGetForwardSet):
+        * API/tests/testapi.js: Updated tests to reflect slightly less strict
+        behavior, which matches headerdoc claims.
+
+2011-05-27  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Oliver Hunt.
+
         Property caching is too aggressive for API objects
         https://bugs.webkit.org/show_bug.cgi?id=61677