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

Issue 892829 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

Prevent misleading 'new user' notifications

Project Member Reported by jordynass@chromium.org, Oct 5

Issue description

There are a few issues with the 'new user' notification logic that will cause it to be on screen when it shouldn't be. I'm not 100% confident in this, but my understanding of the treatment of notifications that come in during OOBE is that appear in the background as soon as they are triggered so the user can't see them until OOBE is finished, but for purposes of handling them in our logic, they're already on screen. If I'm off in that understanding, the solutions below are probably going to require revision.

ISSUES

(1) User completes setup flow on Chromebook A, logs into Chromebook B for the first time, and then forgets their phone on Chromebook B. See  crbug.com/881142#c11  for more detail.

(2) If the user completes setup in OOBE, the notification will be visible when they finish. Note: at the moment this would most likely be replaced by the 'Chromebook added' (which is also undesirable behavior) but once  crbug.com/884373  is complete, the 'new user' notification will be there instead.

(3) If the notification comes up on Chromebook A and the user sets a host on Chromebook B, while waiting for verification the 'new user' notification will still be visible on A.

(4) If the user skips setup in OOBE, the 'new user' notification will be visible when they finish, (which is spammy since they just refused). Note: this is listed separately from (2) because its solution is different.

-----

SOLUTIONS

(1) Never show the 'new user' notification on Chromebook B if the user has seen any of the MultiDevice notifications on Chromebook B before because, if they're in this position, they should have seen the 'Chromebook added' notification when they first logged onto Chromebook B.

(2) & (3) Whenever there is a host status change, if the new host status is not 'potential hosts exist' (i.e. there are eligible hosts but none have been set), we remove the 'new user' notification if it's present.

(4) When the user presses the skip button in OOBE, we remove the 'new user' notification if present.
 
Status: Assigned (was: Available)
 Issue 881142  has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/971180fc6c175baa4d6b40fda809fe3a81191111

commit 971180fc6c175baa4d6b40fda809fe3a81191111
Author: Jordy Greenblatt <jordynass@chromium.org>
Date: Tue Oct 09 19:04:55 2018

[CrOS MultiDevice] Make SetupCompletionRecorder record its own events

In order to ensure that the 'new host' and 'Chromebook added'
notifications to get new users to set up multidevice features and to
inform them that their current device is connected respectively are
providing correct and useful information, we need to record when the
user completes setup and when they receive an update that their host
phone as been verified. The former behavior was implemented in an
earlier CL. This CL implements the latter and observes
HostStatusProvider to keep track of both timestamps.

-----

MANUAL TEST

Completion and verification of pref recording in chronos log output:

localhost ~ # cat /home/chronos/user/log/chrome | grep -e SetupFlowCompletionRecorderImpl
[13763:13763:1008/180345.702849:INFO:setup_flow_completion_recorder_impl.cc(105)] SetupFlowCompletionRecorderImpl::OnHostStatusChange(): Setup flow successfully completed. Recording timestamp 1539047025702.
[13763:13763:1008/180346.310752:INFO:setup_flow_completion_recorder_impl.cc(115)] SetupFlowCompletionRecorderImpl::OnHostStatusChange(): New host verified. Recording timestamp 1539047026310.



Bug:  892829 ,  884373 
Change-Id: I81faae473d5c784f54cb122df37d68856fb6a27f
Reviewed-on: https://chromium-review.googlesource.com/c/1270004
Commit-Queue: Jordy Greenblatt <jordynass@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598022}
[modify] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/BUILD.gn
[modify] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc
[modify] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.h
[modify] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl_unittest.cc
[add] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/fake_host_device_timestamp_manager.cc
[add] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/fake_host_device_timestamp_manager.h
[delete] https://crrev.com/b75ef9be7f158f67a17b52b2d79f6a7877ede0a1/chromeos/services/multidevice_setup/fake_setup_flow_completion_recorder.cc
[delete] https://crrev.com/b75ef9be7f158f67a17b52b2d79f6a7877ede0a1/chromeos/services/multidevice_setup/fake_setup_flow_completion_recorder.h
[add] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/host_device_timestamp_manager.h
[add] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/host_device_timestamp_manager_impl.cc
[add] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/host_device_timestamp_manager_impl.h
[add] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/host_device_timestamp_manager_impl_unittest.cc
[modify] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/multidevice_setup_impl.cc
[modify] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/multidevice_setup_impl.h
[modify] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/multidevice_setup_impl_unittest.cc
[modify] https://crrev.com/971180fc6c175baa4d6b40fda809fe3a81191111/chromeos/services/multidevice_setup/multidevice_setup_service.cc
[delete] https://crrev.com/b75ef9be7f158f67a17b52b2d79f6a7877ede0a1/chromeos/services/multidevice_setup/setup_flow_completion_recorder.h
[delete] https://crrev.com/b75ef9be7f158f67a17b52b2d79f6a7877ede0a1/chromeos/services/multidevice_setup/setup_flow_completion_recorder_impl.cc
[delete] https://crrev.com/b75ef9be7f158f67a17b52b2d79f6a7877ede0a1/chromeos/services/multidevice_setup/setup_flow_completion_recorder_impl.h
[delete] https://crrev.com/b75ef9be7f158f67a17b52b2d79f6a7877ede0a1/chromeos/services/multidevice_setup/setup_flow_completion_recorder_impl_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b61a6f8015146b4c844320d333261d26b7313fd

commit 5b61a6f8015146b4c844320d333261d26b7313fd
Author: Jordy Greenblatt <jordynass@chromium.org>
Date: Tue Oct 09 19:10:44 2018

[CrOS MultiDevice] Block new user notification after 'Chromebook added'

If the Chromebook has ever received a 'Chromebook added' notification,
it means the user already took care of MultiDevice setup on a different
device and should not receive the 'new user' notification. At present,
this issue shows up when a user with a verified host logs into a new
Chromebook B and forgets the host on Chromebook B, causing the 'new
host' notification to show up. I manually verified this is no longer
the case (on a powerwashed Chromebook).

Bug:  892829 
Change-Id: Ib81abed11cf1a937b7f8ad23fe5e5e33aa935f23
Reviewed-on: https://chromium-review.googlesource.com/c/1271555
Commit-Queue: Jordy Greenblatt <jordynass@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598026}
[modify] https://crrev.com/5b61a6f8015146b4c844320d333261d26b7313fd/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc
[modify] https://crrev.com/5b61a6f8015146b4c844320d333261d26b7313fd/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl_unittest.cc

Update: https://chromium-review.googlesource.com/c/1271555 fixed ISSUE (1) above
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7337169affd052c8e7605422dff25c325ed4656

commit b7337169affd052c8e7605422dff25c325ed4656
Author: Jordy Greenblatt <jordynass@chromium.org>
Date: Thu Oct 11 00:08:35 2018

[CrOS MultiDevice] Remove new user notification when it is inaccurate

When someone who has not yet gone through the MultiDevice setup flow is
no longer in a state where they should be prompted to start setup flow,
this CL automatically removes the notification. This involves adding a
callback to the account status change delegate to check if the
notification is no longer accurate whenever the state of the phones on
the GAIA account changes and having the callback remove the
notification.

-----

TESTING

To manually test forgot my host, powerwashed, received the 'new user'
notification, and ran through setup (via settings so I didn't click the
notification). As expected, the notification disappeared on its own.

Bug:  892829 
Change-Id: Ic6d6dbfe16f2bdb40416e35b0efc06d6cdc0d8e9
Reviewed-on: https://chromium-review.googlesource.com/c/1272803
Commit-Queue: Jordy Greenblatt <jordynass@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598579}
[modify] https://crrev.com/b7337169affd052c8e7605422dff25c325ed4656/ash/multi_device_setup/multi_device_notification_presenter.cc
[modify] https://crrev.com/b7337169affd052c8e7605422dff25c325ed4656/ash/multi_device_setup/multi_device_notification_presenter.h
[modify] https://crrev.com/b7337169affd052c8e7605422dff25c325ed4656/ash/multi_device_setup/multi_device_notification_presenter_unittest.cc
[modify] https://crrev.com/b7337169affd052c8e7605422dff25c325ed4656/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc
[modify] https://crrev.com/b7337169affd052c8e7605422dff25c325ed4656/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.h
[modify] https://crrev.com/b7337169affd052c8e7605422dff25c325ed4656/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl_unittest.cc
[modify] https://crrev.com/b7337169affd052c8e7605422dff25c325ed4656/chromeos/services/multidevice_setup/fake_account_status_change_delegate.cc
[modify] https://crrev.com/b7337169affd052c8e7605422dff25c325ed4656/chromeos/services/multidevice_setup/fake_account_status_change_delegate.h
[modify] https://crrev.com/b7337169affd052c8e7605422dff25c325ed4656/chromeos/services/multidevice_setup/multidevice_setup_impl_unittest.cc
[modify] https://crrev.com/b7337169affd052c8e7605422dff25c325ed4656/chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom

Update: https://chromium-review.googlesource.com/c/1272803 fixed ISSUE (3) (verified) and ISSUE (2) (not yet verified).
Mergedinto: 893354
Status: Duplicate (was: Assigned)
Marking dupblicate because all the remiaining work is contained in 893354.
*duplicate
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9bc3529fa3e30165627cdb56d378c207dd118c8c

commit 9bc3529fa3e30165627cdb56d378c207dd118c8c
Author: Jordy Greenblatt <jordynass@chromium.org>
Date: Thu Oct 18 18:21:23 2018

[CrOS MultiDevice] Remove notifications after OOBE setup flow

At present, if a user turns down MultiDevice setup in OOBE, they will
see a notification prompting them to go through our regular setup flow.
In order to avoid spamming people with unwanted notifications, this CL
keeps track of if/when the user saw setup flow in OOBE and prevents
future setup prompting notifications if they have.

Bug:  892829 ,  884401 
Change-Id: I231a25d7879b37912c8594998578eea9afadbf5e
Reviewed-on: https://chromium-review.googlesource.com/c/1275432
Commit-Queue: Jordy Greenblatt <jordynass@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600833}
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chrome/browser/chromeos/login/screens/multidevice_setup_screen.cc
[add] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chrome/browser/chromeos/multidevice_setup/oobe_completion_tracker_factory.cc
[add] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chrome/browser/chromeos/multidevice_setup/oobe_completion_tracker_factory.h
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/BUILD.gn
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/DEPS
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.h
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl_unittest.cc
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/multidevice_setup_impl.cc
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/multidevice_setup_impl.h
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/multidevice_setup_impl_unittest.cc
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/multidevice_setup_initializer.cc
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/multidevice_setup_initializer.h
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/multidevice_setup_service.cc
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/multidevice_setup_service.h
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/multidevice_setup_service_unittest.cc
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/public/cpp/BUILD.gn
[modify] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client_impl_unittest.cc
[add] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.cc
[add] https://crrev.com/9bc3529fa3e30165627cdb56d378c207dd118c8c/chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 22

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a66f261a11f550e1df334991c668ce5721ad960a

commit a66f261a11f550e1df334991c668ce5721ad960a
Author: Jordy Greenblatt <jordynass@chromium.org>
Date: Mon Oct 22 18:29:52 2018

[CrOS MultiDevice] Remove notifications after OOBE setup flow

At present, if a user turns down MultiDevice setup in OOBE, they will
see a notification prompting them to go through our regular setup flow.
In order to avoid spamming people with unwanted notifications, this CL
keeps track of if/when the user saw setup flow in OOBE and prevents
future setup prompting notifications if they have.

(cherry picked from commit 9bc3529fa3e30165627cdb56d378c207dd118c8c)

Bug:  892829 ,  884401 
Change-Id: I231a25d7879b37912c8594998578eea9afadbf5e
Reviewed-on: https://chromium-review.googlesource.com/c/1275432
Commit-Queue: Jordy Greenblatt <jordynass@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600833}
Reviewed-on: https://chromium-review.googlesource.com/c/1289166
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#232}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chrome/browser/chromeos/login/screens/multidevice_setup_screen.cc
[add] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chrome/browser/chromeos/multidevice_setup/oobe_completion_tracker_factory.cc
[add] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chrome/browser/chromeos/multidevice_setup/oobe_completion_tracker_factory.h
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/BUILD.gn
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/DEPS
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.h
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl_unittest.cc
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/multidevice_setup_impl.cc
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/multidevice_setup_impl.h
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/multidevice_setup_impl_unittest.cc
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/multidevice_setup_initializer.cc
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/multidevice_setup_initializer.h
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/multidevice_setup_service.cc
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/multidevice_setup_service.h
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/multidevice_setup_service_unittest.cc
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/public/cpp/BUILD.gn
[modify] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client_impl_unittest.cc
[add] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.cc
[add] https://crrev.com/a66f261a11f550e1df334991c668ce5721ad960a/chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/a66f261a11f550e1df334991c668ce5721ad960a

Commit: a66f261a11f550e1df334991c668ce5721ad960a
Author: jordynass@chromium.org
Commiter: hansberry@chromium.org
Date: 2018-10-22 18:29:52 +0000 UTC

[CrOS MultiDevice] Remove notifications after OOBE setup flow

At present, if a user turns down MultiDevice setup in OOBE, they will
see a notification prompting them to go through our regular setup flow.
In order to avoid spamming people with unwanted notifications, this CL
keeps track of if/when the user saw setup flow in OOBE and prevents
future setup prompting notifications if they have.

(cherry picked from commit 9bc3529fa3e30165627cdb56d378c207dd118c8c)

Bug:  892829 ,  884401 
Change-Id: I231a25d7879b37912c8594998578eea9afadbf5e
Reviewed-on: https://chromium-review.googlesource.com/c/1275432
Commit-Queue: Jordy Greenblatt <jordynass@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600833}
Reviewed-on: https://chromium-review.googlesource.com/c/1289166
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#232}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment