New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 881142 link

Starred by 3 users

Issue metadata

Status: Duplicate
Merged: issue 892829
Owner:
Last visit > 30 days ago
Closed: Oct 8
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocked on:
issue 856848


Participants' hotlists:
Better-Together-Launch-Blockers


Sign in to add a comment

Prevent MultiDevice "new user" event from firing immediately after forgetting a host on new Chromebook

Project Member Reported by jordynass@chromium.org, Sep 6

Issue description

This is expected behavior given the implementation of https://chromium-review.googlesource.com/c/chromium/src/+/1205495 but it should be prevented.
 
Blockedon: 856848
Owner: jordynass@chromium.org
Status: Assigned (was: Untriaged)
Cc: jlklein@chromium.org lesliewatkins@chromium.org jordynass@chromium.org khorimoto@chromium.org jhawkins@chromium.org hansberry@chromium.org
Labels: M-71
Jordy, please provide more context on this issue.  Specifically, what is the actual user experience and what should it be instead?  What changes are necessary in the codebase to affect this change?
TLDR:

We should prevent the 'new user' event which triggers a notification prompting the user to go through Unified Setup if the user has ever seen the 'Chromebook added' notification before on that machine. It should only involve making the method AccountStatusChangeDelegateNotifierImpl::CheckForNewUserPotentialHostExistsEvent() (https://cs.chromium.org/chromium/src/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc?q=%22CheckForNewUserPotentialHostExistsEvent(%22+f:chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc&sq=package:chromium&g=0&l=163) return early if there is a non-zero timestamp for that event.


Full detail/context:

In the Settings UI subpage, a click on the 'Forget device' line will prevent the Chromebook from connecting to the host until it is set up again. Under the hood, this changes the account's host status (i.e. chromeos::multidevice_setup::mojom::HostStatus) from 
one of the 'host set' statuses (i.e. waiting for verification, waiting for server, or verified) to the 'eligible hosts exist' mode.

One result of https://chromium-review.googlesource.com/c/chromium/src/+/1205495 is that any change in the account's host status triggers a check for MultiDevice notifications. At this stage, the notification check sees that the host status is 'eligible hosts exist' so the user is a candidate for the Setup Flow so it can trigger the 'new user' notification.

If the user has has ever received the 'new user' on the Chromebook they're currently using (call it Chromebook A), the currently logic prevents. However, if setup took place on a different Chromebook (call it Chromebook B) so they were never shown the 'new user' notification on Chromebook A, it will come up (on Chromebook A) when the host is forgotten.

So basically the problem case is:
1) User goes through Unified Setup on Chromebook B
2) User logs onto Chromebook A
3) User clicks 'forget device on Chromebook A'

The simplest solution is just to have the check prevent the 'new user' notification from showing if the 'Chromebook added' notification has ever appeared on Chromebook A because, if setup took place on Chromebook B, the 'Chromebook added' notification should have appeared on Chromebook A the next time it was logged in.
Is it not simpler to have a 'host set' status of 'forgotten' or similar?
Issue 884088 has been merged into this issue.
Cc: nohle@chromium.org
 Issue 884373  has been merged into this issue.
 Issue 887597  has been merged into this issue.
Components: -UI>ProximityAuth UI>Multidevice
Cc: -lesliewatkins@chromium.org
Labels: -Pri-2 Pri-3
Owner: ----
Status: Available (was: Assigned)
Jordy's suggestion of "making the method AccountStatusChangeDelegateNotifierImpl::CheckForNewUserPotentialHostExistsEvent()... return early if there is a non-zero timestamp for that event" has actually already been implemented, see [1]. Has anyone repro'd this issue recently? I personally cannot (on HEAD), and from my reading of the code [1], I can't see how it can happen. 

However, I'm sure the issue he described of multiple client devices still occurs, that is:  
1) User goes through Unified Setup on Chromebook B
2) User logs onto Chromebook A
3) User clicks 'forget device' on Chromebook A

The best way to solve this particular issue is probably to follow James's suggestion in #5.

AFAICT, this bug can only cause issues for users with multiple Chromebooks, or who remove their account and sign back in. That's common for us, but definitely an edge case for most users -- I think it's best if we lower the priority of this for now. If I'm wrong about not being able to repro this in the single-client case, please correct me and fix the priority.

1) https://cs.chromium.org/chromium/src/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc?q=%22CheckForNewUserPotentialHostExistsEvent(%22+f:chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc&sq=package:chromium&g=0&l=175
Currently, our logic is to show the notification the first time that the Chromebook notices that the user has an eligible phone but no active host phone. In this case, we go through setup flow on Chromebook A, then log into Chromebook B and forget the device. Since it is the first time that Chromebook B has seen an eligible device without any host set, it shows the notification.

We should change the logic so that we simply never show the promo notification after the user has set a host, regardless of what Chromebook the user set the host on. The easiest way to do this would be to modify SetupFlowCompletionRecorder to observer HostStatusProvider; if at any time HostStatusProvider has a status of kHostVerified, SetupFlowCompletionRecorder should record this event. Then, when AccountStatusChangeDelegateNotifier checks to see if it should show the "new user" notification, it should ask SetupFlowCompletionRecorder if the host has ever been verified. If it has, it should not show the notification.

The two changes involved are:
(1) Modify SetupFlowCompletionRecorderImpl so that it observes HostStatusProvider, and remove SetupFlowCompletionRecorder's RecordCompletion() function which is currently unused.
(2) Modify AccountStatusChangeDelegateNotifierImpl to check for whether the host has ever been verified within CheckForNewUserPotentialHostExistsEvent().
Yeah, I'm not seeing the original bug either. I'm not sure off the top of my head why it stopped, but I'm about to dive into that code for  crbug.com/884373  so I'll double check there if it makes sense.

Kyle, could you put the issue in comment 11 in a separate bug?
Summary: Prevent MultiDevice "new user" event from firing immediately after forgetting a host on new Chromebook (was: Prevent MultiDevice "new user" event from firing immediately after forgetting a host)
The bug still is that you get the "new user" notification after forgetting a host, so I don't think it warrants a separate bug. I adjusted the naming of this bug to be more specific, though :)
Owner: jordynass@chromium.org
Status: Started (was: Available)
Mergedinto: 892829
Status: Duplicate (was: Started)

Sign in to add a comment