Diff
Modified: branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackFunction.cpp (111256 => 111257)
--- branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackFunction.cpp 2012-03-19 22:03:19 UTC (rev 111256)
+++ branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackFunction.cpp 2012-03-19 22:03:25 UTC (rev 111257)
@@ -30,7 +30,6 @@
#include "APICast.h"
#include "CodeBlock.h"
#include "ExceptionHelpers.h"
-#include "JSCallbackObject.h"
#include "JSFunction.h"
#include "FunctionPrototype.h"
#include <runtime/JSGlobalObject.h>
@@ -75,6 +74,10 @@
if (exception)
throwError(exec, toJS(exec, exception));
+ // result must be a valid JSValue.
+ if (!result)
+ return throwVMTypeError(exec);
+
return JSValue::encode(toJS(exec, result));
}
@@ -84,35 +87,4 @@
return CallTypeHost;
}
-JSValueRef JSCallbackFunction::toStringCallback(JSContextRef ctx, JSObjectRef, JSObjectRef thisObject, size_t, const JSValueRef[], JSValueRef* exception)
-{
- JSObject* object = toJS(thisObject);
- if (object->inherits(&JSCallbackObject<JSNonFinalObject>::s_info)) {
- for (JSClassRef jsClass = jsCast<JSCallbackObject<JSNonFinalObject>*>(object)->classRef(); jsClass; jsClass = jsClass->parentClass)
- if (jsClass->convertToType)
- return jsClass->convertToType(ctx, thisObject, kJSTypeString, exception);
- } else if (object->inherits(&JSCallbackObject<JSGlobalObject>::s_info)) {
- for (JSClassRef jsClass = jsCast<JSCallbackObject<JSGlobalObject>*>(object)->classRef(); jsClass; jsClass = jsClass->parentClass)
- if (jsClass->convertToType)
- return jsClass->convertToType(ctx, thisObject, kJSTypeString, exception);
- }
- return 0;
-}
-
-JSValueRef JSCallbackFunction::valueOfCallback(JSContextRef ctx, JSObjectRef, JSObjectRef thisObject, size_t, const JSValueRef[], JSValueRef* exception)
-{
- JSObject* object = toJS(thisObject);
- if (object->inherits(&JSCallbackObject<JSNonFinalObject>::s_info)) {
- for (JSClassRef jsClass = jsCast<JSCallbackObject<JSNonFinalObject>*>(object)->classRef(); jsClass; jsClass = jsClass->parentClass)
- if (jsClass->convertToType)
- return jsClass->convertToType(ctx, thisObject, kJSTypeNumber, exception);
- } else if (object->inherits(&JSCallbackObject<JSGlobalObject>::s_info)) {
- for (JSClassRef jsClass = jsCast<JSCallbackObject<JSGlobalObject>*>(object)->classRef(); jsClass; jsClass = jsClass->parentClass)
- if (jsClass->convertToType)
- return jsClass->convertToType(ctx, thisObject, kJSTypeNumber, exception);
- }
- return 0;
-}
-
-
} // namespace JSC
Modified: branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackFunction.h (111256 => 111257)
--- branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackFunction.h 2012-03-19 22:03:19 UTC (rev 111256)
+++ branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackFunction.h 2012-03-19 22:03:25 UTC (rev 111257)
@@ -55,9 +55,6 @@
return Structure::create(globalData, globalObject, proto, TypeInfo(ObjectType, StructureFlags), &s_info);
}
- static JSValueRef toStringCallback(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception);
- static JSValueRef valueOfCallback(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception);
-
private:
static CallType getCallData(JSCell*, CallData&);
Modified: branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackObject.h (111256 => 111257)
--- branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackObject.h 2012-03-19 22:03:19 UTC (rev 111256)
+++ branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackObject.h 2012-03-19 22:03:25 UTC (rev 111257)
@@ -177,6 +177,8 @@
private:
static UString className(const JSObject*);
+ static JSValue defaultValue(const JSObject*, ExecState*, PreferredPrimitiveType);
+
static bool getOwnPropertySlot(JSCell*, ExecState*, const Identifier&, PropertySlot&);
static bool getOwnPropertyDescriptor(JSObject*, ExecState*, const Identifier&, PropertyDescriptor&);
Modified: branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackObjectFunctions.h (111256 => 111257)
--- branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackObjectFunctions.h 2012-03-19 22:03:19 UTC (rev 111256)
+++ branches/safari-534.55-branch/Source/_javascript_Core/API/JSCallbackObjectFunctions.h 2012-03-19 22:03:25 UTC (rev 111257)
@@ -184,6 +184,30 @@
}
template <class Parent>
+JSValue JSCallbackObject<Parent>::defaultValue(const JSObject* object, ExecState* exec, PreferredPrimitiveType hint)
+{
+ const JSCallbackObject* thisObject = jsCast<const JSCallbackObject*>(object);
+ JSContextRef ctx = toRef(exec);
+ JSObjectRef thisRef = toRef(thisObject);
+ ::JSType jsHint = hint == PreferString ? kJSTypeString : kJSTypeNumber;
+
+ for (JSClassRef jsClass = thisObject->classRef(); jsClass; jsClass = jsClass->parentClass) {
+ if (JSObjectConvertToTypeCallback convertToType = jsClass->convertToType) {
+ JSValueRef exception = 0;
+ JSValueRef result = convertToType(ctx, thisRef, jsHint, &exception);
+ if (exception) {
+ throwError(exec, toJS(exec, exception));
+ return jsUndefined();
+ }
+ if (result)
+ return toJS(exec, result);
+ }
+ }
+
+ return Parent::defaultValue(object, exec, hint);
+}
+
+template <class Parent>
bool JSCallbackObject<Parent>::getOwnPropertyDescriptor(JSObject* object, ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor)
{
JSCallbackObject* thisObject = jsCast<JSCallbackObject*>(object);
Modified: branches/safari-534.55-branch/Source/_javascript_Core/API/JSClassRef.cpp (111256 => 111257)
--- branches/safari-534.55-branch/Source/_javascript_Core/API/JSClassRef.cpp 2012-03-19 22:03:19 UTC (rev 111256)
+++ branches/safari-534.55-branch/Source/_javascript_Core/API/JSClassRef.cpp 2012-03-19 22:03:25 UTC (rev 111257)
@@ -214,17 +214,6 @@
* DerivedClass | DerivedClassPrototype
*/
- if (convertToType) {
- if (!prototypeClass)
- prototypeClass = OpaqueJSClass::create(&kJSClassDefinitionEmpty).leakRef();
- if (!prototypeClass->m_staticFunctions)
- prototypeClass->m_staticFunctions = adoptPtr(new OpaqueJSClassStaticFunctionsTable);
- const Identifier& toString = exec->propertyNames().toString;
- const Identifier& valueOf = exec->propertyNames().valueOf;
- prototypeClass->m_staticFunctions->add(StringImpl::create(toString.characters(), toString.length()), adoptPtr(new StaticFunctionEntry(&JSCallbackFunction::toStringCallback, 0)));
- prototypeClass->m_staticFunctions->add(StringImpl::create(valueOf.characters(), valueOf.length()), adoptPtr(new StaticFunctionEntry(&JSCallbackFunction::valueOfCallback, 0)));
- }
-
if (!prototypeClass)
return 0;
Modified: branches/safari-534.55-branch/Source/_javascript_Core/API/tests/testapi.c (111256 => 111257)
--- branches/safari-534.55-branch/Source/_javascript_Core/API/tests/testapi.c 2012-03-19 22:03:19 UTC (rev 111256)
+++ branches/safari-534.55-branch/Source/_javascript_Core/API/tests/testapi.c 2012-03-19 22:03:25 UTC (rev 111257)
@@ -311,6 +311,16 @@
return JSValueMakeNull(context);
}
+static JSValueRef MyObject_convertToTypeWrapper(JSContextRef context, JSObjectRef object, JSType type, JSValueRef* exception)
+{
+ UNUSED_PARAM(context);
+ UNUSED_PARAM(object);
+ UNUSED_PARAM(type);
+ UNUSED_PARAM(exception);
+ // Forward to default object class
+ return 0;
+}
+
static bool MyObject_set_nullGetForwardSet(JSContextRef ctx, JSObjectRef object, JSStringRef propertyName, JSValueRef value, JSValueRef* exception)
{
UNUSED_PARAM(ctx);
@@ -355,14 +365,65 @@
MyObject_convertToType,
};
+JSClassDefinition MyObject_convertToTypeWrapperDefinition = {
+ 0,
+ kJSClassAttributeNone,
+
+ "MyObject",
+ NULL,
+
+ NULL,
+ NULL,
+
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ MyObject_convertToTypeWrapper,
+};
+
+JSClassDefinition MyObject_nullWrapperDefinition = {
+ 0,
+ kJSClassAttributeNone,
+
+ "MyObject",
+ NULL,
+
+ NULL,
+ NULL,
+
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+};
+
static JSClassRef MyObject_class(JSContextRef context)
{
UNUSED_PARAM(context);
static JSClassRef jsClass;
- if (!jsClass)
- jsClass = JSClassCreate(&MyObject_definition);
-
+ if (!jsClass) {
+ JSClassRef baseClass = JSClassCreate(&MyObject_definition);
+ MyObject_convertToTypeWrapperDefinition.parentClass = baseClass;
+ JSClassRef wrapperClass = JSClassCreate(&MyObject_convertToTypeWrapperDefinition);
+ MyObject_nullWrapperDefinition.parentClass = wrapperClass;
+ jsClass = JSClassCreate(&MyObject_nullWrapperDefinition);
+ }
+
return jsClass;
}
Modified: branches/safari-534.55-branch/Source/_javascript_Core/API/tests/testapi.js (111256 => 111257)
--- branches/safari-534.55-branch/Source/_javascript_Core/API/tests/testapi.js 2012-03-19 22:03:19 UTC (rev 111256)
+++ branches/safari-534.55-branch/Source/_javascript_Core/API/tests/testapi.js 2012-03-19 22:03:25 UTC (rev 111257)
@@ -155,7 +155,7 @@
shouldBe("MyObject ? 1 : 0", true); // toBoolean
shouldBe("+MyObject", 1); // toNumber
shouldBe("(Object.prototype.toString.call(MyObject))", "[object MyObject]"); // Object.prototype.toString
-shouldBe("(MyObject.toString())", "MyObjectAsString"); // toString
+shouldBe("(MyObject.toString())", "[object MyObject]"); // toString
shouldBe("String(MyObject)", "MyObjectAsString"); // toString
shouldBe("MyObject - 0", 1); // toNumber
shouldBe("MyObject.valueOf()", 1); // valueOf
Modified: branches/safari-534.55-branch/Source/_javascript_Core/ChangeLog (111256 => 111257)
--- branches/safari-534.55-branch/Source/_javascript_Core/ChangeLog 2012-03-19 22:03:19 UTC (rev 111256)
+++ branches/safari-534.55-branch/Source/_javascript_Core/ChangeLog 2012-03-19 22:03:25 UTC (rev 111257)
@@ -1,5 +1,58 @@
2012-03-19 Lucas Forschler <[email protected]>
+ Merge 111162
+
+ 2012-03-19 Gavin Barraclough <[email protected]>
+
+ JSCallbackFunction::toStringCallback/valueOfCallback do not handle 0 return value from convertToType
+ https://bugs.webkit.org/show_bug.cgi?id=81468 <rdar://problem/11034745>
+
+ Reviewed by Oliver Hunt.
+
+ The API specifies that convertToType may opt not to handle a conversion:
+ "@result The objects's converted value, or NULL if the object was not converted."
+ In which case, it would propagate first up the JSClass hierarchy, calling its superclass's
+ conversion functions, and failing that call the JSObject::defaultValue function.
+
+ Unfortunately this behaviour was removed in bug#69677/bug#69858, and instead we now rely on
+ the toStringCallback/valueOfCallback function introduced in bug#69156. Even after a fix in
+ bug#73368, these will return the result from the first convertToType they find, regardless
+ of whether this result is null, and if no convertToType method is found in the api class
+ hierarchy (possible if toStringCallback/valueOfCallback was accessed off the prototype
+ chain), they will also return a null pointer. This is unsafe.
+
+ It would be easy to make the approach based around toStringCallback/valueOfCallback continue
+ to walk the api class hierarchy, but making the fallback to defaultValue would be problematic
+ (since defaultValue calls toStringCallback/valueOfCallback, this would infinitely recurse).
+ Making the fallback work with toString/valueOf methods attached to api objects is probably
+ not the right thing to do – instead, we should just implement the defaultValue trap for api
+ objects.
+
+ In addition, this bug highlights that fact that JSCallbackFunction::call will allow a hard
+ null to be returned from C to _javascript_ - this is not okay. Handle with an exception.
+
+ * API/JSCallbackFunction.cpp:
+ (JSC::JSCallbackFunction::call):
+ - Should be null checking the return value.
+ (JSC):
+ - Remove toStringCallback/valueOfCallback.
+ * API/JSCallbackFunction.h:
+ (JSCallbackFunction):
+ - Remove toStringCallback/valueOfCallback.
+ * API/JSCallbackObject.h:
+ (JSCallbackObject):
+ - Add defaultValue mthods to JSCallbackObject.
+ * API/JSCallbackObjectFunctions.h:
+ (JSC::::defaultValue):
+ - Add defaultValue mthods to JSCallbackObject.
+ * API/JSClassRef.cpp:
+ (OpaqueJSClass::prototype):
+ - Remove toStringCallback/valueOfCallback.
+ * API/tests/testapi.js:
+ - Revert this test, now we no longer artificially introduce a toString method onto the api object.
+
+2012-03-19 Lucas Forschler <[email protected]>
+
Merge 110818
from http://svn.webkit.org/repository/webkit/branches/safari-534.54-branch
