Issue metadata
Sign in to add a comment
|
On every reboot seen volume slider on login screen. |
||||||||||||||||||||||||
Issue descriptionChromeOS:8530.0.0/53.0.2784.3 Please specify Cr-* of the system to which this bug/feature applies (add the label below). Steps To Reproduce: (1)Reboot the device or Power On the device (2)Observe the volume slider at login screen Expected Result: Actual Result: Seen volume slider on every reboot of the device. How frequently does this problem reproduce? (Always, sometimes, hard to reproduce?) 100% What is the impact to the user, and is there a workaround? If so, what is it? Please provide any additional information below. Attach a screen shot or log if possible.
,
Jul 1 2016
Possibly caused by the change to how we handle notifications with cras?
,
Jul 1 2016
Likely! Will take a look.
,
Jul 2 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 7 2016
Issue 626226 has been merged into this issue.
,
Jul 7 2016
,
Jul 7 2016
This issue is caused by my change to notify the UI for volume change events based on cras signal OutputNodeVolumeChanged. During the reboot, CrasAudioHandler will intialize audio state, which sets the volume and receives cras signal for volume change signal, therefore, the volume slider bar shows up. I made a cl to add some heuristic for figuring out the volume change signal is in response to the initialization of CrasAudioHandler, if so, do not notify the UI. However, I found out a minor issue caused by cras not sending OutputNodeVolumeChange signal to chrome if chrome sends cras command SetOutputNodeVolume with the same volume cras server has. It cause some minor UI issues as following: 1. When user changes volume, if it reaches the maximum vlue and user press volume up again, volume bar won't pop up. The same happens to minimum volume and volume down. 2. When user logs out a chrome session, CrasAudioHandler will re-initialize its internal state, and sends SetOutputNodeVolume with the same volume value, cras won't respond with OutputNodeVolumeChanged signal. Currently, CrasAudioHandler adds a heuristic to figure out the signal is lost if the first OutputNodeVolumeChanged coming with a different volume value as the volume set during the initialization. If we don't do so, user won't see the volume bar when they press the volume up/down for the first time during a new session. haychao@, will it be possible for cras to ALWAYS send OutputNodeVolumeChanged signal to chrome whenever it receives SetOutputNodeVolume command? cras does so for mute requests.
,
Jul 7 2016
,
Jul 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80330de86e05ab54a5cd17b9d3942d9f65b68983 commit 80330de86e05ab54a5cd17b9d3942d9f65b68983 Author: jennyz <jennyz@chromium.org> Date: Fri Jul 08 18:32:52 2016 Do not notify Volume change event to CrasAudioHandler::Observer when CrasAudioHandler change volume during initialization. BUG= 625222 Review-Url: https://codereview.chromium.org/2127033004 Cr-Commit-Position: refs/heads/master@{#404448} [modify] https://crrev.com/80330de86e05ab54a5cd17b9d3942d9f65b68983/chromeos/audio/cras_audio_handler.cc [modify] https://crrev.com/80330de86e05ab54a5cd17b9d3942d9f65b68983/chromeos/audio/cras_audio_handler.h
,
Jul 8 2016
,
Jul 11 2016
Issue 626832 has been merged into this issue.
,
Jul 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/305b2e610ce5d023aa8cda7adee571c5da8bcc0b commit 305b2e610ce5d023aa8cda7adee571c5da8bcc0b Author: Hsin-Yu Chao <hychao@chromium.org> Date: Sun Jul 10 12:50:15 2016 Revert "CRAS: iodev - Avoid set volume loop by Chrome UI" This reverts commit b9bf4e351d33213f6bd9896b53bb1ff7fde509c0, which was added to avoid volume changed event loop, and the root cause has been fixed in Chrome UI. Because Chrome UI code has been modified to display volume bar base on CRAS volume changed event CRAS reported, we need to revert this commit to fix UI corner case. For example, when system volume is already at 100% max, user press volume up key should make volume bar show up. BUG= chromium:625222 TEST=Adjust volume to 100%, wait for volume bar disappear, press volume up key again to see volume bar show up again. Change-Id: I4960dfe6255a601ef18d02daef25b6ab111f78ea Reviewed-on: https://chromium-review.googlesource.com/359161 Commit-Ready: Hsinyu Chao <hychao@chromium.org> Tested-by: Hsinyu Chao <hychao@chromium.org> Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org> [modify] https://crrev.com/305b2e610ce5d023aa8cda7adee571c5da8bcc0b/cras/src/server/cras_iodev.c
,
Jul 14 2016
Would this be safe to merge to 53? We're seeing reports of the problem there.
,
Jul 15 2016
After hychao's cl in comment #12, it exposes another issue during power on process, I have fixed the issue in the pending cl: https://codereview.chromium.org/2146313005/
,
Jul 15 2016
Regarding #13, we will need to port back both chrome and platform changes to R53.
,
Jul 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e7818501bc53a0f764221189ad2fcf9c7c101fac commit e7818501bc53a0f764221189ad2fcf9c7c101fac Author: jennyz <jennyz@chromium.org> Date: Mon Jul 18 16:51:04 2016 Count the SetOutputNodeVolume requests sent during the audio initialization and track them with the OutputNodeVolumeChanged signals responding from cras, do not pop up the volume bar for such signals. BUG= 625222 Review-Url: https://codereview.chromium.org/2146313005 Cr-Commit-Position: refs/heads/master@{#406014} [modify] https://crrev.com/e7818501bc53a0f764221189ad2fcf9c7c101fac/chromeos/audio/cras_audio_handler.cc [modify] https://crrev.com/e7818501bc53a0f764221189ad2fcf9c7c101fac/chromeos/audio/cras_audio_handler.h
,
Jul 18 2016
,
Aug 10 2016
Issue 634940 has been merged into this issue.
,
Aug 12 2016
Any word on when this release will be pushed? Still happening: Version 53.0.2785.55 beta (64-bit) Platform 8530.49.0 (Official Build) beta-channel samus ARC Version 3127873 Firmware Google_Samus.6300.174.0
,
Aug 12 2016
,
Aug 12 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Aug 12 2016
Seems the CRAS change #12 doesn't need merge to M53 because the CL got reverted wasn't landed there. Jenny: could you help do M53 merge for the CL in #16?
,
Aug 15 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 15 2016
Sorry, I was OOO for vacation last week. Will merge today.
,
Aug 15 2016
Looks like hychao's cl in cras side (comment #12) is not in R53. I will need that change to be merged back to R53 first, then I can merge my chrome side cl. hychao, please merge #12 cl in R53, then assign the issue back to me, so that I can merge chrome cl after.
,
Aug 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/ff8c9b52d413a00041f539b4d7760c61ac0259d2 commit ff8c9b52d413a00041f539b4d7760c61ac0259d2 Author: Hsin-Yu Chao <hychao@chromium.org> Date: Sun Jul 10 12:50:15 2016 Revert "CRAS: iodev - Avoid set volume loop by Chrome UI" This reverts commit b9bf4e351d33213f6bd9896b53bb1ff7fde509c0, which was added to avoid volume changed event loop, and the root cause has been fixed in Chrome UI. Because Chrome UI code has been modified to display volume bar base on CRAS volume changed event CRAS reported, we need to revert this commit to fix UI corner case. For example, when system volume is already at 100% max, user press volume up key should make volume bar show up. BUG= chromium:625222 TEST=Adjust volume to 100%, wait for volume bar disappear, press volume up key again to see volume bar show up again. Change-Id: I4960dfe6255a601ef18d02daef25b6ab111f78ea Reviewed-on: https://chromium-review.googlesource.com/359161 Commit-Ready: Hsinyu Chao <hychao@chromium.org> Tested-by: Hsinyu Chao <hychao@chromium.org> Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org> (cherry picked from commit 305b2e610ce5d023aa8cda7adee571c5da8bcc0b) Reviewed-on: https://chromium-review.googlesource.com/371000 Reviewed-by: Hsinyu Chao <hychao@chromium.org> Commit-Queue: Hsinyu Chao <hychao@chromium.org> [modify] https://crrev.com/ff8c9b52d413a00041f539b4d7760c61ac0259d2/cras/src/server/cras_iodev.c
,
Aug 16 2016
Oops my bad in #comment 22. Done https://chromium-review.googlesource.com/#/c/371000/1 merged to R53.
,
Aug 16 2016
hychao, do you know which build has #26? I will need to try out with my cl merging.
,
Aug 17 2016
Since M53-8530.59.0
,
Aug 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/24448767738300854e3df5ff45e9720f36d0262c commit 24448767738300854e3df5ff45e9720f36d0262c Author: Jenny Zhang <jennyz@chromium.org> Date: Wed Aug 17 17:45:53 2016 Do not notify Volume change event to CrasAudioHandler::Observer when CrasAudioHandler change volume during initialization. BUG= 625222 TBR=hychao@chromium.org Review URL: https://codereview.chromium.org/2256773002 . Review-Url: https://codereview.chromium.org/2127033004 Cr-Original-Commit-Position: refs/heads/master@{#404448} Cr-Commit-Position: refs/branch-heads/2785@{#639} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/24448767738300854e3df5ff45e9720f36d0262c/chromeos/audio/cras_audio_handler.cc [modify] https://crrev.com/24448767738300854e3df5ff45e9720f36d0262c/chromeos/audio/cras_audio_handler.h
,
Aug 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f615009fa3d171ebc5700b8928e349f83f861841 commit f615009fa3d171ebc5700b8928e349f83f861841 Author: Jenny Zhang <jennyz@chromium.org> Date: Wed Aug 17 18:02:54 2016 Count the SetOutputNodeVolume requests sent during the audio initialization and track them with the OutputNodeVolumeChanged signals responding from cras, do not pop up the volume bar for such signals. BUG= 625222 TBR=hychao@chromium.org Review URL: https://codereview.chromium.org/2250963003 . Review-Url: https://codereview.chromium.org/2146313005 Cr-Original-Commit-Position: refs/heads/master@{#406014} Cr-Commit-Position: refs/branch-heads/2785@{#642} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/f615009fa3d171ebc5700b8928e349f83f861841/chromeos/audio/cras_audio_handler.cc [modify] https://crrev.com/f615009fa3d171ebc5700b8928e349f83f861841/chromeos/audio/cras_audio_handler.h
,
Aug 18 2016
The merging is done. But it looks like they are not picked up in the latest R53 build yet.
,
Aug 19 2016
Version 53.0.2785.70 beta (64-bit) Platform 8530.62.0 (Official Build) beta-channel chell Firmware Google_Chell.7820.121.0 Still happening after today's beta update
,
Aug 19 2016
The cls are not in 53.0.2785.70. they are picked up in 53.0.2785.72. Please test 8530.63.0 and beyond.
,
Aug 19 2016
,
Aug 19 2016
Verified on ChromeOS:8530.63.0/chrome:53.0.2785.72
,
Aug 24 2016
HAHAHAHA well good to know it's verified now...any status?
,
Aug 24 2016
It will be fixed for you in the next beta push
,
Aug 25 2016
Issue 640832 has been merged into this issue.
,
Oct 6 2016
I'm seeing this again on samus. Did it regress? 55.0.2878.0 (Official Build) dev (64-bit) 8861.0.0 (Official Build) dev-channel samus
,
Oct 6 2016
(It only happens sometimes, by the way. I saw it on my second-to-last reboot, which was after an update was applied. It did not see it on my most recent reboot.)
,
Oct 6 2016
The workaround we added in chromeos is a heuristic kind of solution, it is not completely bullet-proof. We are discussing to implement a more reliable fix with the help of cras API changes. See detailed discussion in crbug.com/649360.
,
Nov 17 2016
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by abod...@chromium.org
, Jul 1 2016