Bug 87829

Summary: [EFL] Ewk_Intent_Request's postResult/postFailure should take serialized script values in argument
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit EFLAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, gbillock, gyuyoung.kim, lucas.de.marchi, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 86868    
Attachments:
Description Flags
Patch
none
Patch none

Chris Dumez
Reported 2012-05-30 00:25:22 PDT
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.
Attachments
Patch (9.03 KB, patch)
2012-05-30 00:32 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (9.03 KB, patch)
2012-05-30 08:40 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2012-05-30 00:32:14 PDT
Created attachment 144734 [details] Patch
Greg Billock
Comment 2 2012-05-30 08:13:50 PDT
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
Comment 3 2012-05-30 08:40:20 PDT
Created attachment 144828 [details] Patch Fix typo reported by Greg, thanks.
Greg Billock
Comment 4 2012-05-30 08:46:28 PDT
This looks good to me.
Adam Barth
Comment 5 2012-05-30 11:26:00 PDT
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 6 2012-05-30 11:33:21 PDT
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
Comment 7 2012-05-30 11:40:20 PDT
(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 8 2012-05-30 13:10:58 PDT
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 9 2012-05-30 13:47:21 PDT
Comment on attachment 144828 [details] Patch Clearing flags on attachment: 144828 Committed r118962: <http://trac.webkit.org/changeset/118962>
WebKit Review Bot
Comment 10 2012-05-30 13:47:28 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.