EFL's Ewk_Intent does not expose WebCore::Intent::messagePorts(). As a consequence, the following test case cannot be unskipped: webintents/web-intents-invoke-port.html
Created attachment 144717 [details] Patch
Comment on attachment 144717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144717&action=review > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:673 > + const WebCore::MessagePortChannelArray* messagePorts = ewk_intent_message_ports_get(intent); Would it make sense to inline this in DumpRenderTreeChrome.cpp?
(In reply to comment #2) > (From update of attachment 144717 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144717&action=review > > > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:673 > > + const WebCore::MessagePortChannelArray* messagePorts = ewk_intent_message_ports_get(intent); > > Would it make sense to inline this in DumpRenderTreeChrome.cpp? I was trying to avoid exposing a WebCore type (MessagePortChannelArray) to DumpRenderTree but you're right: It seems a few methods in DumpRenderTreeSupportEfl expose WebCore types and it is more extensible this way. I'll fix the patch, thanks.
Created attachment 144837 [details] Patch
Looks good to me
Comment on attachment 144837 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144837&action=review > Source/WebKit/efl/ewk/ewk_intent_private.h:39 > +WebCore::MessagePortChannelArray* ewk_intent_message_ports_get(const Ewk_Intent* intent); Is it ok to return WebCore types in this API? I guess we're doing that a bunch in this header already.
(In reply to comment #6) > (From update of attachment 144837 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144837&action=review > > > Source/WebKit/efl/ewk/ewk_intent_private.h:39 > > +WebCore::MessagePortChannelArray* ewk_intent_message_ports_get(const Ewk_Intent* intent); > > Is it ok to return WebCore types in this API? I guess we're doing that a bunch in this header already. It is not uncommon in private Ewk headers.
As we were discussing on IRC, these days we tend to add a function to the EWKPrivate namespace to extract a WebCore type wrapped inside an ewk type (we have EWKPrivate::corePage() and EWKPrivate::corePageClient(), for example). While Chris' approach isn't wrong, since we started writing our DumpRenderTree implementation different reviewers have suggested that cleaner approach I mentioned above, so we're slowly moving away from these small extractor functions to leaving it up to the caller code to do whatever is needed with a core type.
Comment on attachment 144837 [details] Patch Clearing cq flag until I update the patch based on rakuco's input.
Created attachment 144919 [details] Patch for landing Take Rakuco's feedback into consideration and rebase on master so that the patch applies.
Comment on attachment 144919 [details] Patch for landing Looks great, thanks.
Committed r118977: <http://trac.webkit.org/changeset/118977>