Issue metadata
Sign in to add a comment
|
Bluetooth sometimes won't turn off at the signin screen |
||||||||||||||||||||||||
Issue description
Pixel 1 ("link") with ToT chrome r499741 and OS 9914
I'm seeing an issue on device when I boot to the login screen, open the system tray menu, go to the Bluetooth section, and click the toggle to turn off bluetooth. Sometimes after I click it it turns off for a fraction of a second, then turns back on.
Sonny found it's caused by my CL https://chromium-review.googlesource.com/634329 which adds support for signin screen prefs in ash. That CL made OnActiveUserPrefServiceChanged() get triggered for the signin screen profile prefs, which is consistent with how chrome interprets ProfileManager::GetActiveUserProfile(). However, I suspect BluetoothPowerController thinks the first call to OnActiveUserPrefServiceChanged() indicates a user login (and I think I told Sonny it was OK to assume that - sorry).
If the problem is caused by my CL then this should not affect M61, since my CL is only in M62. However, it will probably need backport to M62 since it's a user-visible breakage.
,
Sep 7 2017
Fixed by this CL (which has the wrong bug number, whoops): The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/48e7ecb90f096621fd69889f813ba999f881cdcf commit 48e7ecb90f096621fd69889f813ba999f881cdcf Author: James Cook <jamescook@chromium.org> Date: Wed Sep 06 23:53:22 2017 cros: Fix Bluetooth toggle at login screen crrev.com/499078 added the ability for ash to access signin screen prefs to support accessibility features. That CL tied the prefs to the existing OnActiveUserPrefServiceChanged() notification. This broke the Bluetooth toggle at the login screen because the bluetooth power code makes the reasonable assumption that the notification only happens for real users. * Add a separate OnSigninScreenPrefServiceInitialized() notification * Make accessibility code explicitly handle signin screen prefs * Bluetooth code is unchanged, but I added a regression test Bug: 762310 Test: ash_unittests, manually toggle bluetooth at login screen on device Change-Id: I5eb8491cd107a28a3c12482d923ee5614143b91f Reviewed-on: https://chromium-review.googlesource.com/654000 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: James Cook <jamescook@chromium.org> Cr-Commit-Position: refs/heads/master@{#500140} [modify] https://crrev.com/48e7ecb90f096621fd69889f813ba999f881cdcf/ash/accessibility/accessibility_controller.cc [modify] https://crrev.com/48e7ecb90f096621fd69889f813ba999f881cdcf/ash/accessibility/accessibility_controller.h [modify] https://crrev.com/48e7ecb90f096621fd69889f813ba999f881cdcf/ash/accessibility/accessibility_controller_unittest.cc [modify] https://crrev.com/48e7ecb90f096621fd69889f813ba999f881cdcf/ash/session/session_controller.cc [modify] https://crrev.com/48e7ecb90f096621fd69889f813ba999f881cdcf/ash/session/session_controller.h [modify] https://crrev.com/48e7ecb90f096621fd69889f813ba999f881cdcf/ash/session/session_observer.h [modify] https://crrev.com/48e7ecb90f096621fd69889f813ba999f881cdcf/ash/session/test_session_controller_client.cc [modify] https://crrev.com/48e7ecb90f096621fd69889f813ba999f881cdcf/ash/system/bluetooth/bluetooth_power_controller.h [modify] https://crrev.com/48e7ecb90f096621fd69889f813ba999f881cdcf/ash/system/bluetooth/bluetooth_power_controller_unittest.cc
,
Sep 8 2017
This is a regression I caused shortly before the M62 branch point. It's a user-visible breakage. I've verified the fix on device.
,
Sep 9 2017
Your change meets the bar and is auto-approved for M62. Please go ahead and merge the CL to branch 3202 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 11 2017
Merged to branch as: cros: Fix Bluetooth toggle at login screen crrev.com/499078 added the ability for ash to access signin screen prefs to support accessibility features. That CL tied the prefs to the existing OnActiveUserPrefServiceChanged() notification. This broke the Bluetooth toggle at the login screen because the bluetooth power code makes the reasonable assumption that the notification only happens for real users. * Add a separate OnSigninScreenPrefServiceInitialized() notification * Make accessibility code explicitly handle signin screen prefs * Bluetooth code is unchanged, but I added a regression test TBR=jamescook@chromium.org (cherry picked from commit 48e7ecb90f096621fd69889f813ba999f881cdcf) Bug: 762310 Test: ash_unittests, manually toggle bluetooth at login screen on device Change-Id: I5eb8491cd107a28a3c12482d923ee5614143b91f Reviewed-on: https://chromium-review.googlesource.com/654000 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: James Cook <jamescook@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#500140} Reviewed-on: https://chromium-review.googlesource.com/660777 Reviewed-by: James Cook <jamescook@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#120} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
,
Sep 13 2017
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
,
Sep 13 2017
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by sonnysasaka@chromium.org
, Sep 6 2017