[webkit-reviews] review denied: [Bug 90538] [chromium] Re-enable Battery Status API : [Attachment 150757] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied
sadrul at chromium.org's request for review:
Bug 90538: [chromium] Re-enable Battery Status API
https://bugs.webkit.org/show_bug.cgi?id=90538

Attachment 150757: Patch
https://bugs.webkit.org/attachment.cgi?id=150757&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=150757&action=review


If you start by thinking of the battery status as a global property of the
computer, then it should be possible to deduce a simpler interface...

> Source/WebKit/chromium/public/WebBatteryMonitor.h:33
> +class WebBatteryMonitorClient {

please use a separate file for each top-level type

> Source/WebKit/chromium/public/WebBatteryMonitor.h:44
> +    virtual void start(WebBatteryMonitorClient*) = 0;

why does the client interface need to be passed to both start and stop methods?


why do we need the client interface at all?  couldn't the webkit API just
export a static function to receive updates to the battery status?  perhaps
we don't even need WebBatteryMonitor?  or is it important to have some way
to limit the frequency of updates?  maybe you only want to push updates to
renderers that have an interest in receiving them?