Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2899803003:
Do not destroy the life cycle when the user is invalid. (Closed)
|
Created:
3 years, 7 months ago by sacomoto Modified:
3 years, 7 months ago Reviewers:
Tim Song CC:
chromium-reviews, jlklein+watch-tether_chromium.org, tengs+watch-tether_chromium.org, hansberry+watch-tether_chromium.org, jhawkins+watch-tether_chromium.org, lesliewatkins+watch-tether_chromium.org, khorimoto+watch-tether_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionDo not destroy the life cycle when the user is invalid.
BUG=725142
Review-Url: https://codereview.chromium.org/2899803003
Cr-Commit-Position: refs/heads/master@{#474064}
Committed: https://chromium.googlesource.com/chromium/src/+/9265ff00a94993e1db869cb7e540117102bc85d5
Patch Set 1 #Patch Set 2 : fixing tests #Messages
Total messages: 19 (13 generated)
Description was changed from ========== Do not destroy the life cycle when the user is invalid. BUG=725142 ========== to ========== Do not destroy the life cycle when the user is invalid. BUG=725142 ==========
sacomoto@chromium.org changed reviewers: + tengs@chromium.org
PTAL.
lgtm
The CQ bit was checked by sacomoto@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by sacomoto@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I changed where the invalid user check is done. Please take another look. I believe this is the right fix, but I don't I think have the full picture of how the ProximityAuthSystem interact with the ScreenLockBridge. For instance, the ProximityAuthSystem::OnScreenDidLock() triggered here: https://cs.chromium.org/chromium/src/components/proximity_auth/screenlock_bri... will always have the empty account as the ScreenlockBridge::Get()->focused_account_id().
lgtm++ It seems the ScreenlockBridge behaviour is entirely dependent on the login UI code in ChromeOS. This will affect what users are focused at what time. Given that ChromeOS doesn't have this deep level of integration testing, we should probably be as defensive as possible. I had to fix a similar issue earlier where the same user is being refocused multiple times.
The CQ bit was checked by sacomoto@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495574621566710,
"parent_rev": "3808a61e679c88a041a6bca1abb392a452d151ef", "commit_rev":
"9265ff00a94993e1db869cb7e540117102bc85d5"}
Message was sent while issue was closed.
Description was changed from ========== Do not destroy the life cycle when the user is invalid. BUG=725142 ========== to ========== Do not destroy the life cycle when the user is invalid. BUG=725142 Review-Url: https://codereview.chromium.org/2899803003 Cr-Commit-Position: refs/heads/master@{#474064} Committed: https://chromium.googlesource.com/chromium/src/+/9265ff00a94993e1db869cb7e540... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/9265ff00a94993e1db869cb7e540... |
