Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2035753003:
Fixed inconsistency with tray icon and user pod on lock screen. Account now switches when focus cha… (Closed)
|
Created:
4 years, 6 months ago by hariank Modified:
4 years, 6 months ago CC:
chromium-reviews, dzhioev+watch_chromium.org, achuith+watch_chromium.org, oshima+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFixed inconsistency with tray icon and user pod on lock screen.
Account now switches when focus changes, not just updated wallpaper.
BUG=614926
Committed: https://crrev.com/b65794b044d1042c761edb46a329ceae9960e47b
Cr-Commit-Position: refs/heads/master@{#398074}
Patch Set 1 #Patch Set 2 : User pod fix #
Total comments: 4
Patch Set 3 : #Patch Set 4 : #Messages
Total messages: 29 (13 generated)
The CQ bit was checked by hariank@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2035753003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2035753003/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by hariank@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2035753003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2035753003/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by hariank@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2035753003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Fixed inconsistency with tray icon and user pod on lock screen. Account now switches when focus changes, not just updated wallpaper. BUG = 614926 ========== to ========== Fixed inconsistency with tray icon and user pod on lock screen. Account now switches when focus changes, not just updated wallpaper. BUG = 614926 ==========
hariank@google.com changed reviewers: + xiyuan@chromium.org
hariank@google.com changed reviewers: + oshima@chromium.org
Hi, can you review this fix? I am currently working on the unit test which will be in a separate cl.
Update CL description's BUG line and get rid of the spaces before and after "=", i.e. BUG=614926 https://codereview.chromium.org/2035753003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right): https://codereview.chromium.org/2035753003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:1278: ash::Shell::GetInstance()->session_state_delegate()->SwitchActiveUser( nit: No need to switch if current user is already active, i.e. also check !user->is_active()
And the the first line of the CL description shorter, usually it should be less than 72 chars. And an empty line between first line and the body. Recommended format: Short CL description less than 72 chars <empty line> Longer CL description if any. Keep lines wrap at 80 chars. BUG=<bug_number> TEST=<Added test, or manual test steps>
https://codereview.chromium.org/2035753003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right): https://codereview.chromium.org/2035753003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:1276: // user may be null in kiosk test nit: // |user| may be nullptr in kiosk mode or unit tests.
Description was changed from ========== Fixed inconsistency with tray icon and user pod on lock screen. Account now switches when focus changes, not just updated wallpaper. BUG = 614926 ========== to ========== Fixed inconsistency with tray icon and user pod on lock screen. Account now switches when focus changes, not just updated wallpaper. BUG=614926 ==========
lgtm
https://codereview.chromium.org/2035753003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right): https://codereview.chromium.org/2035753003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:1276: // user may be null in kiosk test On 2016/06/03 23:09:40, oshima wrote: > nit: // |user| may be nullptr in kiosk mode or unit tests. Done. https://codereview.chromium.org/2035753003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:1278: ash::Shell::GetInstance()->session_state_delegate()->SwitchActiveUser( On 2016/06/03 23:05:57, xiyuan wrote: > nit: No need to switch if current user is already active, > i.e. also check !user->is_active() > Done.
lgtm
The CQ bit was checked by hariank@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2035753003/60001
Message was sent while issue was closed.
Description was changed from ========== Fixed inconsistency with tray icon and user pod on lock screen. Account now switches when focus changes, not just updated wallpaper. BUG=614926 ========== to ========== Fixed inconsistency with tray icon and user pod on lock screen. Account now switches when focus changes, not just updated wallpaper. BUG=614926 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Fixed inconsistency with tray icon and user pod on lock screen. Account now switches when focus changes, not just updated wallpaper. BUG=614926 ========== to ========== Fixed inconsistency with tray icon and user pod on lock screen. Account now switches when focus changes, not just updated wallpaper. BUG=614926 Committed: https://crrev.com/b65794b044d1042c761edb46a329ceae9960e47b Cr-Commit-Position: refs/heads/master@{#398074} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/b65794b044d1042c761edb46a329ceae9960e47b Cr-Commit-Position: refs/heads/master@{#398074} |
