[webkit-reviews] review denied: [Bug 122531] ObjCCallbackFunctionImpl shouldn't store a JSContext : [Attachment 214218] Patch

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has denied Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 122531: ObjCCallbackFunctionImpl shouldn't store a JSContext
https://bugs.webkit.org/show_bug.cgi?id=122531

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

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=214218&action=review


> Source/JavaScriptCore/API/JSContextRef.h:136
> +#if defined(__MAC_OS_X_VERSION_MIN_REQUIRED) &&
__MAC_OS_X_VERSION_MIN_REQUIRED >= 1070
> +/*!
> + at function
> + at abstract Gets the global context of a JavaScript execution context.
> + at param ctx The JSContext whose global context you want to get.
> + at result ctx's global context.
> +*/
> +JS_EXPORT JSGlobalContextRef JSContextGetGlobalContext(JSContextRef ctx);
> +#endif

This isn't how availability macros are used. You use something like
NS_AVAILABLE_MAC(10_7) at the end of the method declaration:

 JS_EXPORT JSGlobalContextRef JSContextGetGlobalContext(JSContextRef ctx)
NS_AVAILABLE_MAC(10_7);

There's a slightly different macro to use if this should be exposed on iOS as
well.

If this header file is used by non-Mac platforms you may need to add some
creative #ifing so that other platforms don't choke on the availability macros.