Diff
Modified: trunk/Source/_javascript_Core/API/JSCallbackFunction.cpp (111161 => 111162)
--- trunk/Source/_javascript_Core/API/JSCallbackFunction.cpp 2012-03-19 08:01:24 UTC (rev 111161)
+++ trunk/Source/_javascript_Core/API/JSCallbackFunction.cpp 2012-03-19 08:12:05 UTC (rev 111162)
@@ -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>
@@ -76,6 +75,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));
}
@@ -85,35 +88,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: trunk/Source/_javascript_Core/API/JSCallbackFunction.h (111161 => 111162)
--- trunk/Source/_javascript_Core/API/JSCallbackFunction.h 2012-03-19 08:01:24 UTC (rev 111161)
+++ trunk/Source/_javascript_Core/API/JSCallbackFunction.h 2012-03-19 08:12:05 UTC (rev 111162)
@@ -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: trunk/Source/_javascript_Core/API/JSCallbackObject.h (111161 => 111162)
--- trunk/Source/_javascript_Core/API/JSCallbackObject.h 2012-03-19 08:01:24 UTC (rev 111161)
+++ trunk/Source/_javascript_Core/API/JSCallbackObject.h 2012-03-19 08:12:05 UTC (rev 111162)
@@ -175,6 +175,8 @@
static void destroy(JSCell*);
+ static JSValue defaultValue(const JSObject*, ExecState*, PreferredPrimitiveType);
+
static bool getOwnPropertySlot(JSCell*, ExecState*, const Identifier&, PropertySlot&);
static bool getOwnPropertyDescriptor(JSObject*, ExecState*, const Identifier&, PropertyDescriptor&);
Modified: trunk/Source/_javascript_Core/API/JSCallbackObjectFunctions.h (111161 => 111162)
--- trunk/Source/_javascript_Core/API/JSCallbackObjectFunctions.h 2012-03-19 08:01:24 UTC (rev 111161)
+++ trunk/Source/_javascript_Core/API/JSCallbackObjectFunctions.h 2012-03-19 08:12:05 UTC (rev 111162)
@@ -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: trunk/Source/_javascript_Core/API/JSClassRef.cpp (111161 => 111162)
--- trunk/Source/_javascript_Core/API/JSClassRef.cpp 2012-03-19 08:01:24 UTC (rev 111161)
+++ trunk/Source/_javascript_Core/API/JSClassRef.cpp 2012-03-19 08:12:05 UTC (rev 111162)
@@ -204,17 +204,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: trunk/Source/_javascript_Core/API/tests/testapi.c (111161 => 111162)
--- trunk/Source/_javascript_Core/API/tests/testapi.c 2012-03-19 08:01:24 UTC (rev 111161)
+++ trunk/Source/_javascript_Core/API/tests/testapi.c 2012-03-19 08:12:05 UTC (rev 111162)
@@ -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: trunk/Source/_javascript_Core/API/tests/testapi.js (111161 => 111162)
--- trunk/Source/_javascript_Core/API/tests/testapi.js 2012-03-19 08:01:24 UTC (rev 111161)
+++ trunk/Source/_javascript_Core/API/tests/testapi.js 2012-03-19 08:12:05 UTC (rev 111162)
@@ -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: trunk/Source/_javascript_Core/ChangeLog (111161 => 111162)
--- trunk/Source/_javascript_Core/ChangeLog 2012-03-19 08:01:24 UTC (rev 111161)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-03-19 08:12:05 UTC (rev 111162)
@@ -1,3 +1,52 @@
+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-18 Raphael Kubo da Costa <[email protected]>
[EFL] Include ICU_INCLUDE_DIRS when building.
Modified: trunk/Source/_javascript_Core/runtime/Arguments.cpp (111161 => 111162)
--- trunk/Source/_javascript_Core/runtime/Arguments.cpp 2012-03-19 08:01:24 UTC (rev 111161)
+++ trunk/Source/_javascript_Core/runtime/Arguments.cpp 2012-03-19 08:12:05 UTC (rev 111162)
@@ -306,6 +306,7 @@
bool isArrayIndex;
unsigned i = propertyName.toArrayIndex(isArrayIndex);
if (isArrayIndex && i < thisObject->d->numArguments) {
+ object->putDirect(exec->globalData(), propertyName, thisObject->argument(i).get(), 0);
if (!Base::defineOwnProperty(object, exec, propertyName, descriptor, shouldThrow))
return false;
@@ -331,35 +332,16 @@
thisObject->d->deletedArguments[i] = true;
}
}
-
return true;
}
if (propertyName == exec->propertyNames().length && !thisObject->d->overrodeLength) {
+ thisObject->putDirect(exec->globalData(), propertyName, jsNumber(thisObject->d->numArguments), DontEnum);
thisObject->d->overrodeLength = true;
- if (!descriptor.isAccessorDescriptor()) {
- if (!descriptor.value())
- descriptor.setValue(jsNumber(thisObject->d->numArguments));
- if (!descriptor.configurablePresent())
- descriptor.setConfigurable(true);
- }
- if (!descriptor.configurablePresent())
- descriptor.setConfigurable(true);
- }
-
- if (propertyName == exec->propertyNames().callee && !thisObject->d->overrodeCallee) {
+ } else if (propertyName == exec->propertyNames().callee && !thisObject->d->overrodeCallee) {
+ thisObject->putDirect(exec->globalData(), propertyName, thisObject->d->callee.get(), DontEnum);
thisObject->d->overrodeCallee = true;
- if (!descriptor.isAccessorDescriptor()) {
- if (!descriptor.value())
- descriptor.setValue(thisObject->d->callee.get());
- if (!descriptor.configurablePresent())
- descriptor.setConfigurable(true);
- }
- if (!descriptor.configurablePresent())
- descriptor.setConfigurable(true);
- }
-
- if (propertyName == exec->propertyNames().caller && thisObject->d->isStrictMode)
+ } else if (propertyName == exec->propertyNames().caller && thisObject->d->isStrictMode)
thisObject->createStrictModeCallerIfNecessary(exec);
return Base::defineOwnProperty(object, exec, propertyName, descriptor, shouldThrow);
Modified: trunk/Source/_javascript_Core/runtime/JSFunction.cpp (111161 => 111162)
--- trunk/Source/_javascript_Core/runtime/JSFunction.cpp 2012-03-19 08:01:24 UTC (rev 111161)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.cpp 2012-03-19 08:12:05 UTC (rev 111162)
@@ -371,11 +371,6 @@
PropertySlot slot;
thisObject->methodTable()->getOwnPropertySlot(thisObject, exec, propertyName, slot);
} else if (propertyName == exec->propertyNames().arguments || propertyName == exec->propertyNames().length || propertyName == exec->propertyNames().caller) {
- if (!object->isExtensible()) {
- if (throwException)
- throwError(exec, createTypeError(exec, "Attempting to define property on object that is not extensible."));
- return false;
- }
if (descriptor.configurablePresent() && descriptor.configurable()) {
if (throwException)
throwError(exec, createTypeError(exec, "Attempting to configurable attribute of unconfigurable property."));
