Bug 88340 – [WK2] Add implementation for dispatchIntent in WebFrameLoaderClient

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

The dispatchIntent() method in WebFrameLoaderClient has currently no implementation.
We need to implement it in order to bring support for Web Intents to WebKit2.

[Chris Dumez]

Created attachment 145826 [details]
Patch

[Chris Dumez]

Created attachment 145831 [details]
Patch

[Chris Dumez]

Created attachment 145832 [details]
Patch

Fix small indentation problem.

[Build Bot]

Comment on attachment 145832 [details]
Patch

Attachment 145832 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12899594

[Chris Dumez]

Created attachment 145839 [details]
Patch

Attempt to make ews-mac happy.

[WebKit Review Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Greg Billock]

Comment on attachment 145839 [details]
Patch

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

> Source/WebKit2/UIProcess/WebIntentData.h:48
> +    String title() const { return m_store.action; }

I'd prefer keeping these as action/type. Type specifiers can include string literals as well as mime type specifiers, and 'title' is what we'll probably call the name of registered intent handlers.

[Chris Dumez]

Comment on attachment 145839 [details]
Patch

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

>> Source/WebKit2/UIProcess/WebIntentData.h:48
>> +    String title() const { return m_store.action; }
> 
> I'd prefer keeping these as action/type. Type specifiers can include string literals as well as mime type specifiers, and 'title' is what we'll probably call the name of registered intent handlers.

My bad. This method is supposed to be action() not title(). This is a copy/paste error on my part.
Note that I had to rename the type() method to mimeType() because there is already a type() method inherited from APIObject. I know that according to the spec, this does not have to be a MIME type. Any other suggestion for the method name then?

[Chris Dumez]

Created attachment 145944 [details]
Patch

- Rename title() method to action(): Was a bad copy/paste.
- Rename mimeType() method to payloadType(): Since type() is already taken, I think it is better but feel free to suggest something better.
- Add payload data, extras hashmap and suggestion URLs to IntentData (only MessagePorts are missing now but I'd like to do that in a separate patch)

[Anders Carlsson]

Comment on attachment 145944 [details]
Patch

Why did you add the bundle API? We shouldn't add bundle API unless there is a clear need for it. Also, you need to bump the WKPageLoaderClient version.

[Chris Dumez]

Created attachment 146213 [details]
Patch

Take Andersca's feedback into consideration, thanks.

[Chris Dumez]

Created attachment 147273 [details]
Patch

Rebase on master and take into consideration feedback from Kenneth at Bug 88399.

[Early Warning System Bot]

Comment on attachment 147273 [details]
Patch

Attachment 147273 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12939980

[Chris Dumez]

Created attachment 147281 [details]
Patch

Should fix the qt-wk2 build.

[WebKit Review Bot]

Comment on attachment 147281 [details]
Patch

Clearing flags on attachment: 147281

Committed r120209: <http://trac.webkit.org/changeset/120209>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by 89007

[Anders Carlsson]

This broke the Mac WK2 build - IntentData.h and .cpp were not added to the Xcode project and those includes were not surrounded by the correct #ifdefs.

[Chris Dumez]

Created attachment 147349 [details]
Patch

Protect the header includes using #ifdefs, hopefully fixing the Mac build.

[Chris Dumez]

Could someone please cq+ ?

[WebKit Review Bot]

Comment on attachment 147349 [details]
Patch

Rejecting attachment 147349 [details] from commit-queue.

New failing tests:
animations/first-letter-play-state.html
Full output: http://queues.webkit.org/results/12942974

[WebKit Review Bot]

Created attachment 147403 [details]
Archive of layout-test-results from ec2-cq-03

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Chris Dumez]

Comment on attachment 147349 [details]
Patch

The chromium test failure is unrelated. Setting cq? Flag again. Could someone please cq+?

[WebKit Review Bot]

Comment on attachment 147349 [details]
Patch

Clearing flags on attachment: 147349

Committed r120278: <http://trac.webkit.org/changeset/120278>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.