[Android] Battery Status API: some tweaks to the BatteryStatusManager.java.

[Android] Battery Status API: some tweaks to the BatteryStatusManager.java.

Make sure battery status works with proper default values
on Android. Add an exception for battery present check for
Galaxy Nexus because it wrongly reports that the battery
is not present (see http://crbug.com/384348).

BUG=122593, 384348, 360068
TEST=http://jsbin.com/battery-status-diagnostics (manual)

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278829

[timvolodine, 2014-06-19 18:19:54]

The CQ bit was checked by timvolodine@chromium.org

[commit-bot: I haz the power, 2014-06-19 18:22:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/timvolodine@chromium.org/330333003/20001

[Peter Beverloo, 2014-06-19 18:25:11]

The CQ bit was unchecked by peter@chromium.org

[Peter Beverloo, 2014-06-19 18:26:00]

On 2014/06/19 18:25:11, Peter Beverloo wrote:
> The CQ bit was unchecked by mailto:peter@chromium.org

Tim, please get a review from either Michael or Mounir. TBRing code changes is
not good practice, and we sit right next to you..

[timvolodine, 2014-06-19 18:29:55]

Mounir, Michael, please review this is a small patch and would like to get it in
before the cut tomorrow.

thanks,
Tim

[mlamouri (slow - plz ping), 2014-06-19 18:54:07]

Thanks for this patch!

LGTM with the comments below applied.

https://codereview.chromium.org/330333003/diff/20001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/BatteryStatusManager.java
(right):

https://codereview.chromium.org/330333003/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/BatteryStatusManager.java:93:
|| batteryPresentByDefault();
What about this:

boolean present = ignoreBatteryPresentState() ||
intent.getBooleanExtra(BatteryManager.EXTRA_PRESENT, false);

https://codereview.chromium.org/330333003/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/BatteryStatusManager.java:120:
protected boolean batteryPresentByDefault() {
Could you add a small comment explaining at a high level what this is doing? The
comment inside the definition is fine, it explains the problem for the specific
device. Also, I would name this function something more clear. Maybe
ignoreBatteryPresentState() ?

What about:
/**
 * Returns whether the BatteryStatusManager should ignore the battery present
state.
 * It might be required for some devices that have a broken EXTRA_PRESENT.
 */
protected boolean ignoreBatteryPresentState() {

[timvolodine, 2014-06-20 12:32:43]

thanks for the quick review Mounir!

https://codereview.chromium.org/330333003/diff/20001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/BatteryStatusManager.java
(right):

https://codereview.chromium.org/330333003/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/BatteryStatusManager.java:93:
|| batteryPresentByDefault();
On 2014/06/19 18:54:07, Mounir Lamouri wrote:
> What about this:
> 
> boolean present = ignoreBatteryPresentState() ||
> intent.getBooleanExtra(BatteryManager.EXTRA_PRESENT, false);

yes that's slightly different semantics so I've made it:
ignoreBatteryPresentState() ? true :
intent.getBooleanExtra(BatteryManager.EXTRA_PRESENT, false);
Done.

https://codereview.chromium.org/330333003/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/BatteryStatusManager.java:120:
protected boolean batteryPresentByDefault() {
On 2014/06/19 18:54:07, Mounir Lamouri wrote:
> Could you add a small comment explaining at a high level what this is doing?
The
> comment inside the definition is fine, it explains the problem for the
specific
> device. Also, I would name this function something more clear. Maybe
> ignoreBatteryPresentState() ?
> 
> What about:
> /**
>  * Returns whether the BatteryStatusManager should ignore the battery present
> state.
>  * It might be required for some devices that have a broken EXTRA_PRESENT.
>  */
> protected boolean ignoreBatteryPresentState() {

Done.

[timvolodine, 2014-06-20 13:20:46]

The CQ bit was checked by timvolodine@chromium.org

[commit-bot: I haz the power, 2014-06-20 13:23:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/timvolodine@chromium.org/330333003/40001

[commit-bot: I haz the power, 2014-06-20 17:02:50]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-06-20 21:40:14]

Change committed as 278829