Summary: | [EFL][WK2] Add API to inspect a Web Intent service | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | gyuyoung.kim, kenneth, rakuco, tonikitoo, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 89276 | ||||||||||
Bug Blocks: | 90064 | ||||||||||
Attachments: |
|
Description
Chris Dumez
2012-06-27 04:57:27 PDT
Created attachment 150109 [details]
Patch
Created attachment 150396 [details]
Patch
Rebase on master.
Comment on attachment 150396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150396&action=review LGTM > Source/WebKit2/UIProcess/API/efl/ewk_intent_service.cpp:48 > + unsigned int __ref; /**< the reference count of the object */ > +#if ENABLE(WEB_INTENTS_TAG) > + WKRetainPtr<WKIntentServiceInfoRef> wkService; > +#endif > + const char* action; Should this whole thing exist if that is not enabled? I mean the struct wont have the same size anyway > Source/WebKit2/UIProcess/API/efl/ewk_intent_service.cpp:87 > + delete service; are you creating this with 'new'? > Source/WebKit2/UIProcess/API/efl/ewk_intent_service.cpp:169 > + ewkIntentService->wkService = wkService; > + return ewkIntentService; I prefer a newline before return to make it easier to spot Comment on attachment 150396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150396&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_intent_service.cpp:48 >> + const char* action; > > Should this whole thing exist if that is not enabled? I mean the struct wont have the same size anyway I believe the struct is needed by the typedef in the header. I followed the style used in the WK1 ewk classes as well. >> Source/WebKit2/UIProcess/API/efl/ewk_intent_service.cpp:87 >> + delete service; > > are you creating this with 'new'? Good catch, I'm using calloc(). I'll use free() instead. >> Source/WebKit2/UIProcess/API/efl/ewk_intent_service.cpp:169 >> + return ewkIntentService; > > I prefer a newline before return to make it easier to spot Ok. Created attachment 150534 [details]
Patch for landing
- Use free() instead of delete for calloc'd memory
- Add blank lines before return statements
Comment on attachment 150534 [details]
Patch for landing
you forgot to fill in the reviewer, so i am r+ing
Comment on attachment 150534 [details] Patch for landing Clearing flags on attachment: 150534 Committed r121732: <http://trac.webkit.org/changeset/121732> All reviewed patches have been landed. Closing bug. |