Summary: | [WK2] Add implementation for dispatchIntent in WebFrameLoaderClient | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||||||||||||
Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||
Severity: | Normal | CC: | abarth, andersca, beidson, cgarcia, cmarcelo, gbillock, gustavo, kenneth, menard, mrobinson, rakuco, ryuan.choi, sam, webkit.review.bot, zoltan | ||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||
Bug Depends on: | 89007 | ||||||||||||||||||||||||
Bug Blocks: | 88303 | ||||||||||||||||||||||||
Attachments: |
|
Description
Chris Dumez
2012-06-05 09:44:46 PDT
Created attachment 145826 [details]
Patch
Created attachment 145831 [details]
Patch
Created attachment 145832 [details]
Patch
Fix small indentation problem.
Comment on attachment 145832 [details] Patch Attachment 145832 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12899594 Created attachment 145839 [details]
Patch
Attempt to make ews-mac happy.
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 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 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? 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 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.
Created attachment 146213 [details]
Patch
Take Andersca's feedback into consideration, thanks.
Created attachment 147273 [details] Patch Rebase on master and take into consideration feedback from Kenneth at Bug 88399. Comment on attachment 147273 [details] Patch Attachment 147273 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12939980 Created attachment 147281 [details]
Patch
Should fix the qt-wk2 build.
Comment on attachment 147281 [details] Patch Clearing flags on attachment: 147281 Committed r120209: <http://trac.webkit.org/changeset/120209> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by 89007 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. Created attachment 147349 [details]
Patch
Protect the header includes using #ifdefs, hopefully fixing the Mac build.
Could someone please cq+ ? 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 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 on attachment 147349 [details]
Patch
The chromium test failure is unrelated. Setting cq? Flag again. Could someone please cq+?
Comment on attachment 147349 [details] Patch Clearing flags on attachment: 147349 Committed r120278: <http://trac.webkit.org/changeset/120278> All reviewed patches have been landed. Closing bug. |