[chromium] Re-enable Battery Status API

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Sadrul Habib Chowdhury]

[chromium] Re-enable Battery Status API

[Sadrul Habib Chowdhury]

Created attachment 150757 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Sadrul Habib Chowdhury]

This patch addresses https://bugs.webkit.org/show_bug.cgi?id=83284#c50. I have also updated the corresponding chromium CL: http://codereview.chromium.org/10024013/.

[Darin Fisher (:fishd, Google)]

Comment on attachment 150757 [details]
Patch

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?

[Sadrul Habib Chowdhury]

Comment on attachment 150757 [details]
Patch

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

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

Yes. The purpose here is to send the battery-status updates only when a client explicitly requests for them. This is why we send the client as a parameter to start and stop, so that the monitor can keep track of the clients correctly, and also notify the browser-process when to start/stop sending the updates (and what renderers to send the updates to).

[Darin Fisher (:fishd, Google)]

Since battery status is a global property of the device, I think the Platform interface should only support a single client.  Multi-plexing multiple interested consumers should be the job of WebCore.  The platform layer is just an OS abstraction.  Therefore, a Platform API like this makes more sense to me:

  class WebBatteryStatus {
  public:
      // member variables and constructor
  };

  class WebBatteryMonitorClient {
  public:
      virtual void didChangeBatteryStatus(const WebBatteryStatus&) = 0;
  };

  class WebBatteryMonitor {
  public:
      virtual void start(WebBatteryMonitorClient*) { }
      virtual void stop() { }
  };

This means that WebCore will need to maintain a list of folks who are ultimately interested in battery status notifications.  WebBatteryStatus also needs to be moved to the Platform API.

[Sadrul Habib Chowdhury]

(In reply to comment #6)
> Since battery status is a global property of the device, I think the Platform interface should only support a single client.  Multi-plexing multiple interested consumers should be the job of WebCore.  The platform layer is just an OS abstraction.  Therefore, a Platform API like this makes more sense to me:
> 
>   class WebBatteryStatus {
>   public:
>       // member variables and constructor
>   };
> 
>   class WebBatteryMonitorClient {
>   public:
>       virtual void didChangeBatteryStatus(const WebBatteryStatus&) = 0;
>   };
> 
>   class WebBatteryMonitor {
>   public:
>       virtual void start(WebBatteryMonitorClient*) { }
>       virtual void stop() { }
>   };
> 
> This means that WebCore will need to maintain a list of folks who are ultimately interested in battery status notifications.  WebBatteryStatus also needs to be moved to the Platform API.

Right now, a WebCore::BatteryController is created for each page (WebViewImpl), in WebCore::BatteryController::provideBatteryTo. If I understand correctly, you are suggesting that instead of this, there should be a single BatteryController in a renderer-process?

[Stephen Chenney]

Marked LayoutTest bugs, bugs with Chromium IDs, and some others as WontFix. Test failure bugs still are trackable via TestExpectations or disabled unit tests.

[Takahiro Ichihashi]

What is the status of battery API for chromium/webkit? 

I saw below issue as "Resolved", but now we don't see any relevant methods/properties in Chrome/Chromium.
https://bugs.webkit.org/show_bug.cgi?id=62698

Monitoring battery status might become important for web application, especially for those generate high CPU loads (WebRTC for example). Probably developers would become want to change behavior of the application depending on the battery status - disable peer to peer connections and switch to server-to-client model with low resolution for laptops not charging battery, for example.