Bug 88340 - [WK2] Add implementation for dispatchIntent in WebFrameLoaderClient
Summary: [WK2] Add implementation for dispatchIntent in WebFrameLoaderClient
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on: 89007
Blocks: 88303
  Show dependency treegraph
 
Reported: 2012-06-05 09:44 PDT by Chris Dumez
Modified: 2012-06-13 21:37 PDT (History)
15 users (show)

See Also:


Attachments
Patch (29.31 KB, patch)
2012-06-05 10:08 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (29.29 KB, patch)
2012-06-05 10:26 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (29.27 KB, patch)
2012-06-05 10:30 PDT, Chris Dumez
buildbot: commit-queue-
Details | Formatted Diff | Diff
Patch (31.18 KB, patch)
2012-06-05 10:51 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (32.25 KB, patch)
2012-06-05 23:35 PDT, Chris Dumez
andersca: review-
Details | Formatted Diff | Diff
Patch (29.04 KB, patch)
2012-06-07 00:05 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (28.40 KB, patch)
2012-06-13 03:19 PDT, Chris Dumez
kenneth: review+
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Patch (29.06 KB, patch)
2012-06-13 03:44 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (28.68 KB, patch)
2012-06-13 09:59 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cq-03 (656.07 KB, application/zip)
2012-06-13 13:45 PDT, WebKit Review Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2012-06-05 09:44:46 PDT
The dispatchIntent() method in WebFrameLoaderClient has currently no implementation.
We need to implement it in order to bring support for Web Intents to WebKit2.
Comment 1 Chris Dumez 2012-06-05 10:08:04 PDT
Created attachment 145826 [details]
Patch
Comment 2 Chris Dumez 2012-06-05 10:26:58 PDT
Created attachment 145831 [details]
Patch
Comment 3 Chris Dumez 2012-06-05 10:30:11 PDT
Created attachment 145832 [details]
Patch

Fix small indentation problem.
Comment 4 Build Bot 2012-06-05 10:38:27 PDT
Comment on attachment 145832 [details]
Patch

Attachment 145832 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12899594
Comment 5 Chris Dumez 2012-06-05 10:51:33 PDT
Created attachment 145839 [details]
Patch

Attempt to make ews-mac happy.
Comment 6 WebKit Review Bot 2012-06-05 10:53:41 PDT
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
Comment 7 Greg Billock 2012-06-05 15:49:28 PDT
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.
Comment 8 Chris Dumez 2012-06-05 21:33:39 PDT
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?
Comment 9 Chris Dumez 2012-06-05 23:35:24 PDT
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)
Comment 10 Anders Carlsson 2012-06-06 13:23:07 PDT
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.
Comment 11 Chris Dumez 2012-06-07 00:05:19 PDT
Created attachment 146213 [details]
Patch

Take Andersca's feedback into consideration, thanks.
Comment 12 Chris Dumez 2012-06-13 03:19:28 PDT
Created attachment 147273 [details]
Patch

Rebase on master and take into consideration feedback from Kenneth at Bug 88399.
Comment 13 Early Warning System Bot 2012-06-13 03:33:51 PDT
Comment on attachment 147273 [details]
Patch

Attachment 147273 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12939980
Comment 14 Chris Dumez 2012-06-13 03:44:03 PDT
Created attachment 147281 [details]
Patch

Should fix the qt-wk2 build.
Comment 15 WebKit Review Bot 2012-06-13 08:06:03 PDT
Comment on attachment 147281 [details]
Patch

Clearing flags on attachment: 147281

Committed r120209: <http://trac.webkit.org/changeset/120209>
Comment 16 WebKit Review Bot 2012-06-13 08:06:15 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 WebKit Review Bot 2012-06-13 09:05:20 PDT
Re-opened since this is blocked by 89007
Comment 18 Anders Carlsson 2012-06-13 09:07:31 PDT
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.
Comment 19 Chris Dumez 2012-06-13 09:59:10 PDT
Created attachment 147349 [details]
Patch

Protect the header includes using #ifdefs, hopefully fixing the Mac build.
Comment 20 Chris Dumez 2012-06-13 11:44:56 PDT
Could someone please cq+ ?
Comment 21 WebKit Review Bot 2012-06-13 13:45:26 PDT
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
Comment 22 WebKit Review Bot 2012-06-13 13:45:32 PDT
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
Comment 23 Chris Dumez 2012-06-13 20:38:02 PDT
Comment on attachment 147349 [details]
Patch

The chromium test failure is unrelated. Setting cq? Flag again. Could someone please cq+?
Comment 24 WebKit Review Bot 2012-06-13 21:37:38 PDT
Comment on attachment 147349 [details]
Patch

Clearing flags on attachment: 147349

Committed r120278: <http://trac.webkit.org/changeset/120278>
Comment 25 WebKit Review Bot 2012-06-13 21:37:47 PDT
All reviewed patches have been landed.  Closing bug.