"Smart Lock is turned on" notification shows when Smart Lock is prohibited |
||||
Issue descriptionOriginally discovered in crbug.com/902260 : the user enabled Better Together via MultiDevice Setup, which enabled their phone as an EASY_UNLOCK_HOST. However, they are a Googler and not in the g/easyunlock-dogfooder group, which means Smart Lock is prohibited for them on the client. The logic which displays the "Smart Lock is turned on" notification only checks that the user has an EASY_UNLOCK_HOST==kEnabled, and doesn't verify that local SmartLock FeatureState==kEnabledByUser [1]. The correct way to resolve this issue is to simply listen on the OnFeatureStatesChanged callback, and show the notification when FeatureState==kEnabledByUser. A pref should be created that makes sure the notification is only shown once, and it should be reset when HostStatus=kNoVerifiedHost. This solution will allow us to remove messy and confusing logic about "remote_device_unlock_keys_before_sync_". The only issue with this clean refactor is that the "pairing changed" notification will still need this messy logic [2]. In order to accommodate the "pairing changed" notification in this refactor, we need to add OnConnectedHostSwitchedForExistingUser() [3] logic to MultiDeviceSetupClient -- this is a fair bit of work. I've opened up crbug.com/905385 (and marked it as blocking) in order to discuss if this notification is still necessary. We will probably be getting a lot of very similar reports to crbug.com/902260 from Googlers who are confused by this strange behavior. Given that, I felt it appropriate to mark this P1. We don't have the time before M72 to resolve this open question around the "pairing changed" notification, so for now, we will follow a "hacky" solution within the existing logic that checks FeatureState. At that point we can downgrade this bug. 1) https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/easy_unlock/easy_unlock_service_regular.cc?sq=package:chromium&g=0&l=704 2) https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/easy_unlock/easy_unlock_service_regular.cc?sq=package:chromium&g=0&l=707 3) https://cs.chromium.org/chromium/src/chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom?sq=package:chromium&dr=CSs&g=0&l=112
,
Nov 14
,
Nov 14
,
Nov 14
Adding this thread for additional context [1]. TLDR; corp users are also receiving a "Smart Lock is turned on" email. This is email issue is low-priority and high-effort and therefore will not be changed. This notification issue is still valuable to fix. As I mentioned, I only set this to a high priority because I was concerned about a large volume of reports from Googlers. However, even if we were to fix this issue ASAP, the ship has already sailed; this fix can't land in M71 now, so Googlers will run into this issue regardless of anything we can do today. By the time a fix lands in M72-stable, most Googlers will have gone through setup, so this fix will have less impact. Downgrading priority. 1) https://groups.google.com/a/google.com/forum/#!topic/better-together-dev/Ya4xsBsBQpA/discussion
,
Nov 16
|
||||
►
Sign in to add a comment |
||||
Comment 1 by hansberry@chromium.org
, Nov 14Labels: -Pri-2 M-72 OS-Chrome Pri-1