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.
Created attachment 144734 [details] Patch
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
Created attachment 144828 [details] Patch Fix typo reported by Greg, thanks.
This looks good to me.
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.
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."
(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));
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!
Comment on attachment 144828 [details] Patch Clearing flags on attachment: 144828 Committed r118962: <http://trac.webkit.org/changeset/118962>
All reviewed patches have been landed. Closing bug.