New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 10 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
link

Issue 136885: Battery level indicator wrong for a while

Reported by kuscher@chromium.org, Jul 11 2012 Project Member

Issue description

Google Chrome	22.0.1201.0 (Official Build 145644) canary
Platform	2575.0.0 (Official Build) canary-channel

Report:
When I start my lumpy the battery indicator in the status area shows the battery as full for a second or two before showing the real level.  Today I booted the device to check the battery level, so I was staring right at it and went through the cycle of it's full (hooray!) no, it's empty (argh!).

1) @Simon, Can we speed up the readings from the battery?
2) @Sebastien, if (1) is not feasible should we think about a pulsing battery like we do for WiFi or a ? or no battery? What would be the best UX here.
 

Comment 1 by sgabr...@chromium.org, Jul 11 2012

The best Ux would be the 1st solution. In case it's not possible, yes we could do a pulsing one or a inactive one.
We could also let it appear once the reading is done (the way the mute icon appears, pushing other icons).

An inactive/searching state for a Wifi icon feels normal because we're used to it. In my opinion, it not the same for a battery level icon. That's why I would go for the "display only when the reading is done" solution.

Comment 2 by kuscher@chromium.org, Jul 12 2012

Cc: sadrul@chromium.org
That sounds reasonable to me. 

1) @Simon, Can we speed up the readings from the battery?
2) If (1) is unfeasible we should make the battery icon only appear when we know the battery level

Comment 3 by arscott@chromium.org, Jul 12 2012

Cc: snanda@chromium.org
I don't think we can speed them up, but Sameer can give a better answer than I can.

Comment 4 by snanda@chromium.org, Jul 12 2012

Labels: Area-Power
Owner: rharrison@chromium.org
Ryan -- since you've been looking into power manager's battery code recently can you investigate this?

We should be able get the remaining charge level pretty quickly.  The charging/discharging rate takes some time to stabilize after power state transitions so there can be some delay in reporting an accurate reading for the remaining (dis)charging time.

Comment 5 by rharrison@chromium.org, Jul 12 2012

I think given that powerd is in a separate process and it takes time to do the IPC between the process and gather the information about the state of the battery, there will always be a window of time where there isn't a value for the battery state in the browser. I expected the UX to display the "Calculating..." type dialogue, that we use other places, before it gets its first value from powerd instead of the behaviour we have here. I think irregardless of any performance improvements in the information flow we should have handling for this case in the display correctly.

The fact that this is occurring for a second or more at the start of the session is a bit odd to me. I don't think that the sequence of messages to get that information should take that long, but I will admit I have not profiled it. To me this suggests that the first request for information from the display layer is arriving relatively late in the start up process. To mitigate this I could have the handler for when powerd sees a session start message send up the power status to chrome. One problem with this is, is that chrome is not guaranteed to be in a state to receive the message until it sends the request. There is also the possibility that the start session message and data request are arriving close together, ie. later.

Comment 6 by kuscher@chromium.org, Jul 12 2012

How complex is this. This is a minor issue so waiting 2-3 seconds before showing the battery on the login screen will not be noticed by 99% of people and we can do that too.

Comment 7 by rharrison@chromium.org, Jul 12 2012

I am not sure for having it display "calculating" since I haven't worked on that code directly. jennyz or sadrul I think would be better suited to comment on that.

For sending up the message when we get the session started message that should be relatively easy to do, I can write up a CL to do it this afternoon, but it might not be that useful given the timing of messages.

Comment 8 by rharrison@chromium.org, Jul 12 2012

I stand corrected on my statement on how easy it would be to add in the message being sent up at session start. I had assumed that the messages were being handled as signals, thus asynchronous messages. Unfortunately the message that sends up the data is explicitly a reply to a method call from chrome. Thus I cannot just send the message up.

I can though send a signal that causes Chrome to request an update of the power state, so I can at least initiate the sequence as early as possible, though there will be more potential delays going this route.

Comment 9 by rharrison@chromium.org, Jul 12 2012

I have created a CL (https://gerrit.chromium.org/gerrit/27295), which adds starting the information upload when powerd sees a start of session. I am going to test this and see if it has much effect.

Comment 10 by rharrison@chromium.org, Jul 12 2012

Status: Started
I have tested the above CL and from my perspective the display for power appears faster on the login screen and has the correct values. I haven't done extensive testing, but it appears to resolve some of this issue.

Comment 11 by bugdroid1@chromium.org, Jul 12 2012

Project Member
Commit: c5e8ee5042d4fa4156fefa8fb1df85740fb1458b
 Email: rharrison@chromium.org

Send up power information at start of session

We know that at the start of session that the UX layer does not have information
about the state of the battery and power system. Thus when we get a session
started message immediately send this information.

BUG= chromium:136885 
TEST=Built package and ran unit tests.
     Built image and checked for regressions.
     Observed state of power status in tray for improper state on session
     start. <TBD>

Change-Id: Icf4b47dbd11159419187c1c45ea4e66e9be6cd03
Reviewed-on: https://gerrit.chromium.org/gerrit/27295
Reviewed-by: Sameer Nanda <snanda@chromium.org>
Tested-by: Ryan Harrison <rharrison@chromium.org>

M	powerd.cc

Comment 12 by rharrison@chromium.org, Jul 16 2012

Kuscher, when you get the opportunity could you take a look at a canary image that has my patch for power_manager in it and see if the improvement is sufficient? If not we will have to work on the code path in the browser.

Comment 13 by kuscher@chromium.org, Jul 16 2012

Owner: kuscher@chromium.org
Assigning to myself. Will update the bug when I looked at it.

Comment 14 by kuscher@chromium.org, Aug 2 2012

Labels: -Pri-2 -Mstone-23 Pri-1 Mstone-22
Owner: rharrison@chromium.org
Ryan/Jenny, on the current canary with your changes in I don't see a battery icon appear at all until I click on the status bar. Sounds like there is a bug somewhere?

Comment 15 by rharrison@chromium.org, Aug 2 2012

I have been able to reproduce this on lumpy. If one waits 30 seconds it will appear also. If you click immediately it appears, which I am not sure if it causes a query in the UI or is it just refreshes the icon using the previously received information. There should be 3 messages at the start of a sessions; 0s, 5s, and 30s. So for some reason 0 and 5 are not causing causing an update.

There are two possibilities here: a) powerd is not sending 0 and 5, which I am investigating or b) the UI is either not receiving the messages or not taking action on them.

Comment 16 by jen...@chromium.org, Aug 2 2012

I will repo sync to get the latest build and take a look at the UI code too.

Comment 17 by kuscher@chromium.org, Aug 2 2012

Thanks to both of you for looking into this.

Comment 18 by rharrison@chromium.org, Aug 2 2012

Cc: derat@chromium.org sque@chromium.org
I am seeing at least two bugs here that could be causing this. a) We do not appear to be getting a message about the fact the login session has started in powerd, so the messages @ 0s and 5s are never going to be sent. This I think is a regression
derat, can you comment on that?

Beyond that, it appears that when a poll event occurs the poll signal is getting eaten, so the browser never gets it and requests an update. When you click on the status are it pulls the current data without the poll from what I can see. Specifically I am seeing in the log about every 30s right after the messages related to the poll:
[0802/123922:ERROR:powerd.cc(725)] Could not find handler for org.chromium.PowerManager:PowerSupplyPoll in method handler table.

This signal is sent from powerd to inform chrome there is data to be pulled. I am not sure why powerd is trying to handle it as a method since it is a signal :-/

Comment 19 by derat@chromium.org, Aug 2 2012

#18: This bug is about the battery level not showing up on the login screen before a user has logged in, right?  I don't believe that the session manager sends a SessionStateChanged D-Bus signal until the user has logged in.

In powerd, Daemon::Init() calls RetrieveSessionState(), which should call OnSessionStateChange().  https://gerrit.chromium.org/gerrit/#/c/27295/1/powerd.cc looks like it only fetches the power status for state "starting".

Comment 20 by rharrison@chromium.org, Aug 2 2012

Owner: sque@chromium.org
Status: Assigned
Thanks derat, I hadn't noticed that it was only for starting. That needs to be changed, most likely have Init() call the ResumePolling method, which should initiate an immediate update.

That solves one part of the issue, but I think there still is a bug wrt to the polling signal.

Since I am going away on vacation I am going to pass off this bug to sque@. I will continue to look for when this issue was introduced until I leave.

Comment 21 by sque@chromium.org, Aug 2 2012

Here's my powerd.LATEST.
powerd.LATEST
20.8 KB View Download

Comment 22 by saintlou@chromium.org, Aug 6 2012

Labels: Iteration-62
All Pri-1 for M22 -> Iteration-62

Comment 23 by saintlou@chromium.org, Aug 9 2012

Labels: ReleaseBlock-Beta

Comment 24 by ddrew@chromium.org, Aug 16 2012

Ping for update?

Comment 25 by saintlou@chromium.org, Aug 20 2012

Labels: Iteration-63
Issues for M22 & post-branch ->  Iteration-63

Comment 26 by ddrew@chromium.org, Aug 20 2012

Labels: -ReleaseBlock-Beta ReleaseBlock-Stable Noteworthy

Comment 27 by rharrison@chromium.org, Aug 22 2012

Owner: rharrison@chromium.org
Taking this back since I am back from vacation.

Comment 28 by rharrison@chromium.org, Aug 22 2012

Status: Started

Comment 29 by jamescook@chromium.org, Aug 22 2012

Cc: -jamescook@chromium.org

Comment 30 by bugdroid1@chromium.org, Aug 23 2012

Project Member
Commit: 1abd1cdc1b2b3a7ea6f1ae0a025c19b75ed9abc1
 Email: rharrison@chromium.org

Change initial start of polling battery poling to a resume.

This causes the daemon to poll the state of battery on start and signal the
browser that there is information about the battery state to pull. This also
schedules the next poll in 5 seconds since the time values from the first poll
might be bogus and need to be ignored. This is the same behaviour that is used
when the device transitions power state.

BUG= chromium:136885 
TEST=Started device and watch the status tray on the login screen. The battery
     icon should appear in about of second, instead of taking upto 30 seconds,
     with the Calculating estimate and then after 5 seconds should transition to
     the correct time estimate.

Change-Id: I948dcfe467e4d7172f8ed57ba8437cf767dd6a52
Reviewed-on: https://gerrit.chromium.org/gerrit/31140
Tested-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Daniel Erat <derat@chromium.org>
Commit-Ready: Ryan Harrison <rharrison@chromium.org>

M	powerd.cc

Comment 31 by rharrison@chromium.org, Aug 23 2012

Labels: Merge-Requested
The second CL has resolved the remaining issues. powerd now polls the battery as part of the initialization sequence and sends up an interim result so that the UX knows everything but the time remaining to full/empty.

Comment 32 by k...@google.com, Aug 23 2012

Labels: -Merge-Requested Merge-Approved

Comment 33 by rharrison@chromium.org, Aug 24 2012

Labels: -Merge-Approved Merge-Pending

Comment 34 by bugdroid1@chromium.org, Aug 24 2012

Project Member
Commit: 1bf371783dd2a5705d4e182ffe17e9680d79b6f8
 Email: rharrison@chromium.org

Change initial start of polling battery poling to a resume.

This causes the daemon to poll the state of battery on start and signal the
browser that there is information about the battery state to pull. This also
schedules the next poll in 5 seconds since the time values from the first poll
might be bogus and need to be ignored. This is the same behaviour that is used
when the device transitions power state.

BUG= chromium:136885 
TEST=Started device and watch the status tray on the login screen. The battery
     icon should appear in about of second, instead of taking upto 30 seconds,
     with the Calculating estimate and then after 5 seconds should transition to
     the correct time estimate.

Change-Id: I8e662b048deacc97059d0d63ccb55e75995118e3
Reviewed-on: https://gerrit.chromium.org/gerrit/31140
Tested-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Daniel Erat <derat@chromium.org>
Commit-Ready: Ryan Harrison <rharrison@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/31348
Reviewed-by: Puneet Kumar <puneetster@chromium.org>

M	powerd.cc

Comment 35 by rharrison@chromium.org, Aug 24 2012

Labels: -Merge-Pending Merge-Merged
Status: Fixed

Comment 36 by bincheng@chromium.org, Aug 27 2012

Status: Verified
ChromeOS 22.0.1229.24    dev
Platform 2723.43.0       dev-channel

Comment 37 by bugdroid1@chromium.org, Oct 13 2012

Project Member
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.

Comment 38 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Labels: -Area-UI -Mstone-22 -Feature-Ash-Tray -Feature-Ash-Startscreen -Area-Power -Noteworthy Cr-OS-Kernel-Power Cr-UI-Shell-LockScreen Cr-UI M-22 Cr-UI-Shell-StatusArea Hotlist-Noteworthy

Comment 39 by laforge@google.com, Mar 12 2013

Labels: -Cr-UI-Shell-LockScreen Cr-UI-Shell-StartScreen

Comment 40 by bugdroid1@chromium.org, Mar 14 2013

Project Member
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Sign in to add a comment