[webkit-reviews] review granted: [Bug 216428] Send TestRendered event after running a test but before dumping : [Attachment 408584] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Darin Adler <darin at apple.com>'s request
for review:
Bug 216428: Send TestRendered event after running a test but before dumping
https://bugs.webkit.org/show_bug.cgi?id=216428

Attachment 408584: Patch

https://bugs.webkit.org/attachment.cgi?id=408584&action=review




--- Comment #4 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 408584
  --> https://bugs.webkit.org/attachment.cgi?id=408584
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=408584&action=review

> Tools/TestRunnerShared/Bindings/JSBasics.h:38
> +JSValueRef JSValueMakeBooleanOrNull(JSContextRef, Optional<bool>);
> +Optional<bool> JSValueToNullableBoolean(JSContextRef, JSValueRef);
> +
> +JSValueRef JSValueMakeStringOrNull(JSContextRef, JSStringRef);

Since you are just moving this, we shouldn't change this for this change, but I
don't think we should mimic the JavaScriptCore c-API naming style here. Should
instead be something like:

JSValueMakeBooleanOrNull(JSContextRef, Optional<bool>) ->
makeValue(JSContextRef, Optional<bool>)
JSValueToNullableBoolean(JSContextRef, JSValueRef) ->
toOptionalBool(JSContextRef, JSValueRef)
JSValueMakeStringOrNull(JSContextRef, JSStringRef) ->
makeValueAllowingNull(JSContextRef, JSStringRef)