Bug 87829 – [EFL] Ewk_Intent_Request's postResult/postFailure should take serialized script values in argument

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Chris Dumez]

The ewk_intent_request_result_post() and ewk_intent_request_failure_post() methods currently take "const char*" results in argument, which are then converted internally into SerializedScriptValues.
As a consequence, only string type results can be passed in argument, which is not according to the spec. The client should be able to post results of any type, not just string.
We should therefore use WebCore::SerializedScriptValue as argument type and make those methods private.

[Chris Dumez]

Created attachment 144734 [details]
Patch

[Greg Billock]

Comment on attachment 144734 [details]
Patch

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

> Source/WebKit/efl/ewk/ewk_intent_request.cpp:107
> + * The serialied payload data will be passed to the error callback registered by the

serialized

[Chris Dumez]

Created attachment 144828 [details]
Patch

Fix typo reported by Greg, thanks.

[Greg Billock]

This looks good to me.

[Adam Barth]

Comment on attachment 144828 [details]
Patch

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

> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:676
> +        ewk_intent_request_failure_post(request, WebCore::SerializedScriptValue::create(String::fromUTF8("ERROR")));

Should we send a serialization of null instead?  It seems strange to have this hard-coded error string.

[Greg Billock]

Comment on attachment 144828 [details]
Patch

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

>> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:676
>> +        ewk_intent_request_failure_post(request, WebCore::SerializedScriptValue::create(String::fromUTF8("ERROR")));
> 
> Should we send a serialization of null instead?  It seems strange to have this hard-coded error string.

Could do. It's basically just "some string that we can check for to make sure the postFailure pathway got called."

[Chris Dumez]

(In reply to comment #5)
> (From update of attachment 144828 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=144828&action=review
> 
> > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:676
> > +        ewk_intent_request_failure_post(request, WebCore::SerializedScriptValue::create(String::fromUTF8("ERROR")));
> 
> Should we send a serialization of null instead?  It seems strange to have this hard-coded error string.

This is matching the chromium port implementation:
v8::Handle<v8::Value> v8value = v8::String::New("ERROR");
request->postFailure(WebKit::WebSerializedScriptValue::serialize(v8value));

[Adam Barth]

Comment on attachment 144828 [details]
Patch

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

>>>> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:676
>>>> +        ewk_intent_request_failure_post(request, WebCore::SerializedScriptValue::create(String::fromUTF8("ERROR")));
>>> 
>>> Should we send a serialization of null instead?  It seems strange to have this hard-coded error string.
>> 
>> Could do. It's basically just "some string that we can check for to make sure the postFailure pathway got called."
> 
> This is matching the chromium port implementation:
> v8::Handle<v8::Value> v8value = v8::String::New("ERROR");
> request->postFailure(WebKit::WebSerializedScriptValue::serialize(v8value));

Ah, ok.  Makes sense.  Thanks!

[WebKit Review Bot]

Comment on attachment 144828 [details]
Patch

Clearing flags on attachment: 144828

Committed r118962: <http://trac.webkit.org/changeset/118962>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.