Summary: | Better JSContext API for named evaluations (other than //# sourceURL) | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ggaren, joepeck, mhahnenberg, timothy | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Joseph Pecoraro
2014-03-07 13:49:05 PST
Created attachment 226162 [details]
[PATCH] Proposed Fix
Comment on attachment 226162 [details] [PATCH] Proposed Fix 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 > +@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. |