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

Sudarsana Nagineni (babu)
Reported 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.
Attachments
Patch (9.28 KB, patch)
2013-01-24 08:29 PST, Sudarsana Nagineni (babu) 		no flags
DetailsFormatted DiffDiff
patch (9.44 KB, patch)
2013-01-30 06:56 PST, Sudarsana Nagineni (babu) 		no flags
DetailsFormatted DiffDiff
Patch (9.60 KB, patch)
2013-02-11 09:59 PST, Sudarsana Nagineni (babu) 		no flags
DetailsFormatted DiffDiff
Patch (9.58 KB, patch)
2013-02-12 08:48 PST, Sudarsana Nagineni (babu) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Sudarsana Nagineni (babu)
Comment 1 2013-01-24 08:29:19 PST
Created attachment 184507 [details] Patch
Mikhail Pozdnyakov
Comment 2 2013-01-24 08:31:53 PST
Comment on attachment 184507 [details] Patch LGTM
Sudarsana Nagineni (babu)
Comment 3 2013-01-30 06:56:59 PST
Created attachment 185496 [details] patch Rebased.
Chris Dumez
Comment 4 2013-01-30 07:11:16 PST
Comment on attachment 185496 [details] patch LGTM.
Gyuyoung Kim
Comment 5 2013-01-30 18:35:23 PST
Comment on attachment 185496 [details] patch Looks fine.
Sudarsana Nagineni (babu)
Comment 6 2013-02-11 09:59:42 PST
Created attachment 187604 [details] Patch Rebased on master.
Kenneth Rohde Christiansen
Comment 7 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?
Chris Dumez
Comment 8 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()) ?
Sudarsana Nagineni (babu)
Comment 9 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.
Sudarsana Nagineni (babu)
Comment 10 2013-02-12 08:48:14 PST
Created attachment 187877 [details] Patch Take comments from Kenneth and Chris into consideration.
Kenneth Rohde Christiansen
Comment 11 2013-02-15 08:19:07 PST
Comment on attachment 187877 [details] Patch LGTM
WebKit Review Bot
Comment 12 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>
WebKit Review Bot
Comment 13 2013-02-15 09:11:18 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.