[webkit-reviews] review granted: [Bug 122561] Objective-C API: blocks aren't callable via 'new' : [Attachment 213795] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 122561: Objective-C API: blocks aren't callable via 'new'
https://bugs.webkit.org/show_bug.cgi?id=122561

Attachment 213795: Patch
https://bugs.webkit.org/attachment.cgi?id=213795&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=213795&action=review


r=me

> Source/JavaScriptCore/API/ObjCCallbackFunction.mm:496
> +    JSGlobalContextRef contextRef = [context JSGlobalContextRef];
> +    if (*exception) {
> +	   ::JSValue *bogusObject = [::JSValue
valueWithNewObjectInContext:context];
> +	   return JSValueToObject(contextRef, [bogusObject JSValueRef], NULL);
> +    }
> +
> +    if (!JSValueIsObject(contextRef, result)) {
> +	   *exception = toRef(JSC::createTypeError(toJS(contextRef),
"Objective-C blocks called as constructors must return an object."));
> +	   ::JSValue *bogusObject = [::JSValue
valueWithNewObjectInContext:context];
> +	   return JSValueToObject(contextRef, [bogusObject JSValueRef], NULL);
> +    }

Can we get away with just returning NULL here? In general, the C API allows a
client to return NULL when throwing an exception. I think that would be nicer
than making a bogus object.

> Source/JavaScriptCore/API/ObjCCallbackFunction.mm:497
> +    return JSValueToObject(contextRef, result, exception);

This can just be a cast, since you've checked JSValueIsObject.