[webkit-reviews] review granted: [Bug 129911] Better JSContext API for named evaluations (other than //# sourceURL) : [Attachment 226162] [PATCH] Proposed Fix
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Mar 10 16:18:44 PDT 2014
Geoffrey Garen <ggaren at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 129911: Better JSContext API for named evaluations (other than //#
sourceURL)
https://bugs.webkit.org/show_bug.cgi?id=129911
Attachment 226162: [PATCH] Proposed Fix
https://bugs.webkit.org/attachment.cgi?id=226162&action=review
------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=226162&action=review
r=me, but I think you can do a bit better here.
> Source/JavaScriptCore/API/JSContext.h:87
> + at param sourceURL A URL for the script's source path and name. Used by
debuggers and when reporting exceptions.
I think I would just say "source file" instead of "source path and name". I
didn't immediately understand what you meant by "path and name".
I would also say "This parameter is informative only: it does not change the
behavior of the script."
> Source/JavaScriptCore/API/JSContext.mm:113
> - (JSValue *)evaluateScript:(NSString *)script
> {
> - JSValueRef exceptionValue = 0;
> + JSValueRef exceptionValue = nullptr;
> JSStringRef scriptJS = JSStringCreateWithCFString((CFStringRef)script);
> - JSValueRef result = JSEvaluateScript(m_context, scriptJS, 0, 0, 0,
&exceptionValue);
> + JSValueRef result = JSEvaluateScript(m_context, scriptJS, nullptr,
nullptr, 0, &exceptionValue);
> + JSStringRelease(scriptJS);
> +
> + if (exceptionValue)
> + return [self valueFromNotifyException:exceptionValue];
> +
> + return [JSValue valueWithJSValueRef:result inContext:self];
> +}
> +
> +- (JSValue *)evaluateScript:(NSString *)script withSourceURL:(NSURL
*)sourceURL
> +{
> + if (!sourceURL)
> + return [self evaluateScript:script];
> +
> + JSValueRef exceptionValue = nullptr;
> + JSStringRef scriptJS = JSStringCreateWithCFString((CFStringRef)script);
> + JSStringRef sourceURLJS =
JSStringCreateWithCFString((CFStringRef)[sourceURL absoluteString]);
> + JSValueRef result = JSEvaluateScript(m_context, scriptJS, nullptr,
sourceURLJS, 0, &exceptionValue);
> + JSStringRelease(sourceURLJS);
> JSStringRelease(scriptJS);
I would have done this the other way around, and had -evaluateScript: call
-evaluateScript:withURL: passing a null or empty URL. That way, there's only
one copy of the string creation, script evaluation, and exception handling
code.
More information about the webkit-reviews
mailing list