Bug 90066

Summary: [EFL][WK2] Add API to inspect a Web Intent service
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: 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 Flags
Patch
none
Patch
kenneth: review+
Patch for landing none

Description Chris Dumez 2012-06-27 04:57:27 PDT
We need to add a Ewk class to wrap a Web Intent service, so that the client can inspect it.
Comment 1 Chris Dumez 2012-06-29 00:54:53 PDT
Created attachment 150109 [details]
Patch
Comment 2 Chris Dumez 2012-07-02 04:17:53 PDT
Created attachment 150396 [details]
Patch

Rebase on master.
Comment 3 Kenneth Rohde Christiansen 2012-07-02 18:42:58 PDT
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 4 Chris Dumez 2012-07-02 22:31:25 PDT
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.
Comment 5 Chris Dumez 2012-07-02 22:34:58 PDT
Created attachment 150534 [details]
Patch for landing

- Use free() instead of delete for calloc'd memory
- Add blank lines before return statements
Comment 6 Kenneth Rohde Christiansen 2012-07-02 22:35:44 PDT
Comment on attachment 150534 [details]
Patch for landing

you forgot to fill in the reviewer, so i am r+ing
Comment 7 WebKit Review Bot 2012-07-02 23:27:27 PDT
Comment on attachment 150534 [details]
Patch for landing

Clearing flags on attachment: 150534

Committed r121732: <http://trac.webkit.org/changeset/121732>
Comment 8 WebKit Review Bot 2012-07-02 23:27:33 PDT
All reviewed patches have been landed.  Closing bug.