Bug 90066 - [EFL][WK2] Add API to inspect a Web Intent service
Summary: [EFL][WK2] Add API to inspect a Web Intent service
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: 89276
Blocks: 90064
  Show dependency treegraph
 
Reported: 2012-06-27 04:57 PDT by Chris Dumez
Modified: 2012-07-02 23:27 PDT (History)
5 users (show)

See Also:


Attachments
Patch (19.38 KB, patch)
2012-06-29 00:54 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (19.49 KB, patch)
2012-07-02 04:17 PDT, Chris Dumez
kenneth: review+
Details | Formatted Diff | Diff
Patch for landing (19.53 KB, patch)
2012-07-02 22:34 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.