Bug 90538 - [chromium] Re-enable Battery Status API
: [chromium] Re-enable Battery Status API
Status: RESOLVED WONTFIX
Product: WebKit
Classification: Unclassified
Component: New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Sadrul Habib Chowdhury
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-04 03:12 PDT by Sadrul Habib Chowdhury
Modified: 2013-05-06 17:35 PDT (History)
10 users (show)

See Also:


Attachments
Patch (17.38 KB, patch)
2012-07-04 03:13 PDT, Sadrul Habib Chowdhury
fishd: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sadrul Habib Chowdhury 2012-07-04 03:12:03 PDT
[chromium] Re-enable Battery Status API
Comment 1 Sadrul Habib Chowdhury 2012-07-04 03:13:31 PDT
Created attachment 150757 [details]
Patch
Comment 2 WebKit Review Bot 2012-07-04 03:15:57 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Sadrul Habib Chowdhury 2012-07-04 03:20:02 PDT
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/.
Comment 4 Darin Fisher (:fishd, Google) 2012-07-11 11:25:37 PDT
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?
Comment 5 Sadrul Habib Chowdhury 2012-11-05 12:49:27 PST
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).
Comment 6 Darin Fisher (:fishd, Google) 2012-11-19 11:25:34 PST
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.
Comment 7 Sadrul Habib Chowdhury 2012-11-20 01:43:37 PST
(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?
Comment 8 Stephen Chenney 2013-04-09 17:06:35 PDT
Marked LayoutTest bugs, bugs with Chromium IDs, and some others as WontFix. Test failure bugs still are trackable via TestExpectations or disabled unit tests.
Comment 9 Takahiro Ichihashi 2013-05-06 17:35:03 PDT
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.