Last Comment Bug 700262 - Add remainingTime support to upower backend
: Add remainingTime support to upower backend
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Mounir Lamouri (:mounir)
:
Mentors:
Depends on: 700261
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-07 04:11 PST by Mounir Lamouri (:mounir)
Modified: 2011-11-15 14:47 PST (History)
4 users (show)
mounir: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (5.00 KB, patch)
2011-11-07 04:11 PST, Mounir Lamouri (:mounir)
no flags Details | Diff | Review
Patch v1.1 (5.00 KB, patch)
2011-11-08 00:57 PST, Mounir Lamouri (:mounir)
no flags Details | Diff | Review
Patch v1.1 with a comment (6.02 KB, patch)
2011-11-08 12:50 PST, Mounir Lamouri (:mounir)
cjones.bugs: review+
asa: approval‑mozilla‑aurora-
Details | Diff | Review

Description Mounir Lamouri (:mounir) 2011-11-07 04:11:52 PST
Created attachment 572429 [details] [diff] [review]
Patch v1
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-11-07 20:29:26 PST
Comment on attachment 572429 [details] [diff] [review]
Patch v1

>+  aBatteryInfo->remainingTime() = upowerClient->GetRemainingTime();;

Empty statement here (double semicolon).  Some compilers are picky
about that.

>+  if (isFull) {
>+    mRemainingTime = 0;

This seems strange to me.  Shouldn't we use TimeToEmpty in this case?
Does the spec say something about it?
Comment 2 Mounir Lamouri (:mounir) 2011-11-08 00:57:04 PST
(In reply to Chris Jones [:cjones] [:warhammer] from comment #1)
> >+  if (isFull) {
> >+    mRemainingTime = 0;
> 
> This seems strange to me.  Shouldn't we use TimeToEmpty in this case?
> Does the spec say something about it?

Basically, when .charging is true, we have to use .chargingTime and .charging is false, we use .dischargingTime so if we are full (and charging), the chargingTime should be 0. Which is the case here. We could be using TimeToFull here but I'm not sure what the value would be given that 0 is for unknown values is upower...
Comment 3 Mounir Lamouri (:mounir) 2011-11-08 00:57:39 PST
Created attachment 572753 [details] [diff] [review]
Patch v1.1
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-11-08 12:31:16 PST
Comment on attachment 572753 [details] [diff] [review]
Patch v1.1

Waiting to hear back if FullyCharged => Charging.
Comment 5 Mounir Lamouri (:mounir) 2011-11-08 12:50:09 PST
Created attachment 572965 [details] [diff] [review]
Patch v1.1 with a comment
Comment 6 Mounir Lamouri (:mounir) 2011-11-09 06:23:10 PST
Comment on attachment 572965 [details] [diff] [review]
Patch v1.1 with a comment

This has just landed in mozilla-inbound. Requesting aurora approval for API completeness in Firefox 10. It would be better to ship Firefox 10 with the entire API if we say it is present. The risk is very low given that there is no use of the API for the moment.
Comment 7 Marco Bonardo [::mak] 2011-11-10 03:10:51 PST
https://hg.mozilla.org/mozilla-central/rev/09eb49521f13
Comment 8 Asa Dotzler [:asa] 2011-11-15 14:47:38 PST
Comment on attachment 572965 [details] [diff] [review]
Patch v1.1 with a comment

It looks like this feature wasn't ready for 10. Please re-nominate if you have a good justification for trying to get this into 10 rather than just waiting for 11.

Note You need to log in before you can comment on or make changes to this bug.