Bug 107821

Summary: [EFL][WK2] Use C API inside BatteryProvider and NetworkInfoProvider
Product: WebKit Reporter: Sudarsana Nagineni (babu) <naginenis>
Component: WebKit EFLAssignee: Sudarsana Nagineni (babu) <naginenis>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, benjamin, cdumez, gyuyoung.kim, kenneth, kling, lucas.de.marchi, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 107657    
Attachments:
Description Flags
Patch
none
patch
none
Patch
none
Patch none

Description Sudarsana Nagineni (babu) 2013-01-24 06:19:25 PST 
As per Bug 107657, we should start using the C API instead of accessing internal C++ classes directly.
Comment 1 Sudarsana Nagineni (babu) 2013-01-24 08:29:19 PST 
Created attachment 184507 [details]
Patch
Comment 2 Mikhail Pozdnyakov 2013-01-24 08:31:53 PST 
Comment on attachment 184507 [details]
Patch

LGTM
Comment 3 Sudarsana Nagineni (babu) 2013-01-30 06:56:59 PST 
Created attachment 185496 [details]
patch

Rebased.
Comment 4 Chris Dumez 2013-01-30 07:11:16 PST 
Comment on attachment 185496 [details]
patch

LGTM.
Comment 5 Gyuyoung Kim 2013-01-30 18:35:23 PST 
Comment on attachment 185496 [details]
patch

Looks fine.
Comment 6 Sudarsana Nagineni (babu) 2013-02-11 09:59:42 PST 
Created attachment 187604 [details]
Patch

Rebased on master.
Comment 7 Kenneth Rohde Christiansen 2013-02-12 05:33:46 PST 
Comment on attachment 187604 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=187604&action=review

> Source/WebKit2/UIProcess/efl/BatteryProvider.cpp:94
> +    WKBatteryStatusRef wkBatteryStatus = WKBatteryStatusCreate(status->charging(), status->chargingTime(), status->dischargingTime(), status->level());

So why not WKRetainPtr? Are you releasing it manually later?
Comment 8 Chris Dumez 2013-02-12 05:40:29 PST 
Comment on attachment 187604 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=187604&action=review

>> Source/WebKit2/UIProcess/efl/BatteryProvider.cpp:94
>> +    WKBatteryStatusRef wkBatteryStatus = WKBatteryStatusCreate(status->charging(), status->chargingTime(), status->dischargingTime(), status->level());
> 
> So why not WKRetainPtr? Are you releasing it manually later?

Right. Good catch Kenneth. I also believe you need to adopt the returned value here.

> Source/WebKit2/UIProcess/efl/BatteryProvider.cpp:97
> +    WKBatteryManagerProviderDidChangeBatteryStatus(m_batteryManager.get(), wkEventType.get(), wkBatteryStatus);

Have you tried toAPI(eventType.impl()) ?
Comment 9 Sudarsana Nagineni (babu) 2013-02-12 07:35:18 PST 
Comment on attachment 187604 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=187604&action=review

>>> Source/WebKit2/UIProcess/efl/BatteryProvider.cpp:94
>>> +    WKBatteryStatusRef wkBatteryStatus = WKBatteryStatusCreate(status->charging(), status->chargingTime(), status->dischargingTime(), status->level());
>> 
>> So why not WKRetainPtr? Are you releasing it manually later?
> 
> Right. Good catch Kenneth. I also believe you need to adopt the returned value here.

Okay.

>> Source/WebKit2/UIProcess/efl/BatteryProvider.cpp:97
>> +    WKBatteryManagerProviderDidChangeBatteryStatus(m_batteryManager.get(), wkEventType.get(), wkBatteryStatus);
> 
> Have you tried toAPI(eventType.impl()) ?

I have not, but I will check it.
Comment 10 Sudarsana Nagineni (babu) 2013-02-12 08:48:14 PST 
Created attachment 187877 [details]
Patch

Take comments from Kenneth and Chris into consideration.
Comment 11 Kenneth Rohde Christiansen 2013-02-15 08:19:07 PST 
Comment on attachment 187877 [details]
Patch

LGTM
Comment 12 WebKit Review Bot 2013-02-15 09:11:13 PST 
Comment on attachment 187877 [details]
Patch

Clearing flags on attachment: 187877

Committed r143008: <http://trac.webkit.org/changeset/143008>
Comment 13 WebKit Review Bot 2013-02-15 09:11:18 PST 
All reviewed patches have been landed.  Closing bug.