RESOLVED FIXED 130495
Refine BatteryStatus module
https://bugs.webkit.org/show_bug.cgi?id=130495
Summary Refine BatteryStatus module
Jinwoo Song
Reported 2014-03-19 20:27:44 PDT
Small refinements: - Return PassRef instead of PassRefPtr in create method - Switched to nullptr instead of 0 where appropriate. - Removed unused function and header file. - Removed unnecessary empty lines.
Attachments
Patch (7.80 KB, patch)
2014-03-19 20:31 PDT, Jinwoo Song
no flags
Patch (7.83 KB, patch)
2014-03-19 23:33 PDT, Jinwoo Song
no flags
Jinwoo Song
Comment 1 2014-03-19 20:31:05 PDT
Andreas Kling
Comment 2 2014-03-19 23:07:53 PDT
Comment on attachment 227250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227250&action=review > Source/WebCore/Modules/battery/BatteryManager.cpp:39 > - RefPtr<BatteryManager> batteryManager(adoptRef(new BatteryManager(navigator))); > + Ref<BatteryManager> batteryManager(adoptRef(*new BatteryManager(navigator))); > batteryManager->suspendIfNeeded(); > - return batteryManager.release(); > + return batteryManager.get(); This will ref() and deref() the object twice, it would be more efficient to write it like this: auto batteryManager = adoptRef(*new BatteryManager(navigator)); batteryManager.get().suspendIfNeeded(); return std::move(batteryManager);
Jinwoo Song
Comment 3 2014-03-19 23:30:56 PDT
(In reply to comment #2) > (From update of attachment 227250 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=227250&action=review > > > Source/WebCore/Modules/battery/BatteryManager.cpp:39 > > - RefPtr<BatteryManager> batteryManager(adoptRef(new BatteryManager(navigator))); > > + Ref<BatteryManager> batteryManager(adoptRef(*new BatteryManager(navigator))); > > batteryManager->suspendIfNeeded(); > > - return batteryManager.release(); > > + return batteryManager.get(); > > This will ref() and deref() the object twice, it would be more efficient to write it like this: > > auto batteryManager = adoptRef(*new BatteryManager(navigator)); > batteryManager.get().suspendIfNeeded(); > return std::move(batteryManager); Thanks for advice! I'll modify it.
Jinwoo Song
Comment 4 2014-03-19 23:33:53 PDT
Created attachment 227260 [details] Patch Applied Kling's comment.
WebKit Commit Bot
Comment 5 2014-03-20 00:12:00 PDT
Comment on attachment 227260 [details] Patch Clearing flags on attachment: 227260 Committed r165950: <http://trac.webkit.org/changeset/165950>
WebKit Commit Bot
Comment 6 2014-03-20 00:12:10 PDT
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.