WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
89275
[WK2] Add C API to inspect a Web Intent
https://bugs.webkit.org/show_bug.cgi?id=89275
Summary
[WK2] Add C API to inspect a Web Intent
Chris Dumez
Reported
2012-06-16 00:26:47 PDT
We need a C API for WKIntentData so that it can be queried on client side.
Attachments
Patch
(11.53 KB, patch)
2012-06-16 09:09 PDT
,
Chris Dumez
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Patch
(11.53 KB, patch)
2012-06-16 10:07 PDT
,
Chris Dumez
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Patch
(16.92 KB, patch)
2012-06-16 11:44 PDT
,
Chris Dumez
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Patch
(17.23 KB, patch)
2012-06-16 12:00 PDT
,
Chris Dumez
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Patch
(11.02 KB, patch)
2012-06-16 13:01 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(10.99 KB, patch)
2012-06-16 13:03 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(9.95 KB, patch)
2012-06-17 23:56 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(11.29 KB, patch)
2012-06-17 23:58 PDT
,
Chris Dumez
andersca
: review+
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
Patch for landing
(11.31 KB, patch)
2012-06-21 10:05 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(8)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2012-06-16 09:09:56 PDT
Created
attachment 147971
[details]
Patch
Build Bot
Comment 2
2012-06-16 09:52:03 PDT
Comment on
attachment 147971
[details]
Patch
Attachment 147971
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/12966690
Chris Dumez
Comment 3
2012-06-16 10:07:34 PDT
Created
attachment 147975
[details]
Patch Try to make ews-mac happy.
Build Bot
Comment 4
2012-06-16 10:25:46 PDT
Comment on
attachment 147975
[details]
Patch
Attachment 147975
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/12970254
Build Bot
Comment 5
2012-06-16 10:46:10 PDT
Comment on
attachment 147975
[details]
Patch
Attachment 147975
[details]
did not pass win-ews (win): Output:
http://queues.webkit.org/results/12974016
Chris Dumez
Comment 6
2012-06-16 11:44:11 PDT
Created
attachment 147979
[details]
Patch Add new files to xcode and win project files. This should hopefully make the ews happy.
Build Bot
Comment 7
2012-06-16 11:51:20 PDT
Comment on
attachment 147979
[details]
Patch
Attachment 147979
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/12961897
Chris Dumez
Comment 8
2012-06-16 12:00:04 PDT
Created
attachment 147980
[details]
Patch
Build Bot
Comment 9
2012-06-16 12:37:33 PDT
Comment on
attachment 147980
[details]
Patch
Attachment 147980
[details]
did not pass win-ews (win): Output:
http://queues.webkit.org/results/12973058
Build Bot
Comment 10
2012-06-16 12:40:21 PDT
Comment on
attachment 147980
[details]
Patch
Attachment 147980
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/12973059
Chris Dumez
Comment 11
2012-06-16 13:01:00 PDT
Created
attachment 147986
[details]
Patch
Chris Dumez
Comment 12
2012-06-16 13:03:07 PDT
Created
attachment 147987
[details]
Patch
Gyuyoung Kim
Comment 13
2012-06-17 17:46:31 PDT
Comment on
attachment 147987
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=147987&action=review
> Source/WebKit2/UIProcess/API/C/WKIntentData.h:31 > +#if ENABLE(WEB_INTENTS)
As far as I know, generally, we don't use ENABLE(...) macro in public header file. Because, application need to know ENABLE macro defined in Platform.h. I wonder whether WebKit2 can use ENABLE(...) macro in header file.
Chris Dumez
Comment 14
2012-06-17 21:49:02 PDT
(In reply to
comment #13
)
> (From update of
attachment 147987
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=147987&action=review
> > > Source/WebKit2/UIProcess/API/C/WKIntentData.h:31 > > +#if ENABLE(WEB_INTENTS) > > As far as I know, generally, we don't use ENABLE(...) macro in public header file. Because, application need to know ENABLE macro defined in Platform.h. I wonder whether WebKit2 can use ENABLE(...) macro in header file.
The exact same thing is done in WKInspector.h. Also, I don't believe this is a public header anyway. The public header will be ewk_intent.h, which will use this WKIntentData API internally.
Gyuyoung Kim
Comment 15
2012-06-17 23:22:04 PDT
(In reply to
comment #14
)
> The exact same thing is done in WKInspector.h. Also, I don't believe this is a public header anyway. > The public header will be ewk_intent.h, which will use this WKIntentData API internally.
If WKXXX is not public APIs, I agree to use ENABLE() macro in header file. Of course, I already checked INSPECTOR is used in that files. However, as you can see, MiniBrowser is still using WKXXX functions. So, in my opinion, it is good to not use ENABLE() in WK header files. And also, WK functions can be used by other ports. If other ports don't support port specific APIs, they may want to use WK functions directly.
Chris Dumez
Comment 16
2012-06-17 23:24:33 PDT
(In reply to
comment #15
)
> (In reply to
comment #14
) > > > The exact same thing is done in WKInspector.h. Also, I don't believe this is a public header anyway. > > The public header will be ewk_intent.h, which will use this WKIntentData API internally. > > If WKXXX is not public APIs, I agree to use ENABLE() macro in header file. Of course, I already checked INSPECTOR is used in that files. However, as you can see, MiniBrowser is still using WKXXX functions. So, in my opinion, it is good to not use ENABLE() in WK header files. > > And also, WK functions can be used by other ports. If other ports don't support port specific APIs, they may want to use WK functions directly.
I agree with you. I'll check if I can get rid of it in the header without causing compilation issues on Mac.
Chris Dumez
Comment 17
2012-06-17 23:56:11 PDT
Created
attachment 148061
[details]
Patch Take Gyuyoung's feedback into consideration.
Chris Dumez
Comment 18
2012-06-17 23:58:40 PDT
Created
attachment 148063
[details]
Patch
Gyuyoung Kim
Comment 19
2012-06-18 00:15:03 PDT
Comment on
attachment 148063
[details]
Patch Looks good to me. But, I'm not sure whether WK2 reviewers like this macro process. Anyway, thank you for your update.
WebKit Review Bot
Comment 20
2012-06-21 09:55:45 PDT
Comment on
attachment 148063
[details]
Patch Rejecting
attachment 148063
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: urce/WebKit2/UIProcess/API/C/WKIntentData.cpp patching file Source/WebKit2/UIProcess/API/C/WKIntentData.h patching file Source/WebKit2/UIProcess/WebIntentData.cpp patching file Source/WebKit2/UIProcess/WebIntentData.h Hunk #1 FAILED at 50. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit2/UIProcess/WebIntentData.h.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Anders Car..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output:
http://queues.webkit.org/results/13028175
Chris Dumez
Comment 21
2012-06-21 10:05:31 PDT
Created
attachment 148828
[details]
Patch for landing Could someone please cq+ ?
WebKit Review Bot
Comment 22
2012-06-21 12:08:35 PDT
Comment on
attachment 148828
[details]
Patch for landing
Attachment 148828
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/13026187
Chris Dumez
Comment 23
2012-06-21 12:11:43 PDT
Comment on
attachment 148828
[details]
Patch for landing The chromium problem seems unrelated. Setting cq? again. Could someone please cq+?
WebKit Review Bot
Comment 24
2012-06-21 14:23:45 PDT
Comment on
attachment 148828
[details]
Patch for landing Clearing flags on attachment: 148828 Committed
r120963
: <
http://trac.webkit.org/changeset/120963
>
WebKit Review Bot
Comment 25
2012-06-21 14:23:54 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug