Bug 107821 – [EFL][WK2] Use C API inside BatteryProvider and NetworkInfoProvider

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Sudarsana Nagineni (babu)]

As per Bug 107657, we should start using the C API instead of accessing internal C++ classes directly.

[Sudarsana Nagineni (babu)]

Created attachment 184507 [details]
Patch

[Mikhail Pozdnyakov]

Comment on attachment 184507 [details]
Patch

LGTM

[Sudarsana Nagineni (babu)]

Created attachment 185496 [details]
patch

Rebased.

[Chris Dumez]

Comment on attachment 185496 [details]
patch

LGTM.

[Gyuyoung Kim]

Comment on attachment 185496 [details]
patch

Looks fine.

[Sudarsana Nagineni (babu)]

Created attachment 187604 [details]
Patch

Rebased on master.

[Kenneth Rohde Christiansen]

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?

[Chris Dumez]

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()) ?

[Sudarsana Nagineni (babu)]

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.

[Sudarsana Nagineni (babu)]

Created attachment 187877 [details]
Patch

Take comments from Kenneth and Chris into consideration.

[Kenneth Rohde Christiansen]

Comment on attachment 187877 [details]
Patch

LGTM

[WebKit Review Bot]

Comment on attachment 187877 [details]
Patch

Clearing flags on attachment: 187877

Committed r143008: <http://trac.webkit.org/changeset/143008>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.