"ASSERTION: Battery API: When charging and level at 1.0, remaining time should be 0. Please fix your backend!"

RESOLVED FIXED in mozilla11

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: m_kato)

Tracking

({assertion})

Trunk
mozilla11
assertion
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
+++ This bug was initially created as a clone of Bug #707085 +++

Calling navigator.mozBattery triggers:

###!!! ASSERTION: Battery API: When charging and level at 1.0, remaining time should be 0. Please fix your backend!: 'Error', file dom/battery/BatteryManager.cpp, line 200

My Windows XP desktop has no battery.
Makoto, it seems that your code doesn't take into account situations were there is no battery. Can you check if there actually is a battery with the Windows API? In that case, you should file the structure with the default values like you do when there is no status.

I should have catch that in the review of the initial patch, I'm sorry :(

If you don't have time to work on that soon let me know, I will try to have a look.
maybe, some environment always returns unknown (-1) for remain time.  We should check whether level is 1.0.
Assignee: nobody → m_kato
(In reply to Makoto Kato from comment #2)
> maybe, some environment always returns unknown (-1) for remain time.  We
> should check whether level is 1.0.

We could do that but I think the best solution would be to check if there is a battery in the device. On Linux, that information is easily available. Can't we get it on Windows?
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #3)
> (In reply to Makoto Kato from comment #2)
> > maybe, some environment always returns unknown (-1) for remain time.  We
> > should check whether level is 1.0.
> 
> We could do that but I think the best solution would be to check if there is
> a battery in the device. On Linux, that information is easily available.
> Can't we get it on Windows?

If no battery, we can check other member of SYSTEM_POWER_STATUS.  But it can return unknown, so we should check whether level is 1.0.   If API cannot return unknown, I think it is better.
Created attachment 582233 [details] [diff] [review]
fix
(Assignee)

Updated

5 years ago
Attachment #582233 - Flags: review?(mounir)
Comment on attachment 582233 [details] [diff] [review]
fix

Review of attachment 582233 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the change.

Thank you for the quick fix :)

::: hal/windows/WindowsBattery.cpp
@@ +224,5 @@
>    if (status.ACLineStatus != 0) {
> +    if (aBattryInfo->level() == 1.0) {
> +      // GetSystemPowerStatus API may returns -1 for BatteryFullLifeTime.
> +      // So, if battery is 100%, set 0 at force.
> +      aBatteryInfo->remainingTime() = 0;

Please use kDefaultRemainingTime instead of 0.
Attachment #582233 - Flags: review?(mounir) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/5efce975a9e7
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/5efce975a9e7
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.