[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}

chrome/browser/chromeos/BUILD.gn

@@ -100,6 +100,7 @@ source_set("chromeos") {
     "//chromeos/services/multidevice_setup/public/cpp:android_sms_app_helper_delegate",
     "//chromeos/services/multidevice_setup/public/cpp:android_sms_pairing_state_tracker",
     "//chromeos/services/multidevice_setup/public/cpp:auth_token_validator",
+    "//chromeos/services/multidevice_setup/public/cpp:oobe_completion_tracker",
     "//chromeos/services/multidevice_setup/public/cpp:prefs",
     "//chromeos/services/secure_channel/public/cpp/client",
     "//components/arc",
@@ -1361,6 +1362,8 @@ source_set("chromeos") {
     "multidevice_setup/auth_token_validator_impl.h",
     "multidevice_setup/multidevice_setup_client_factory.cc",
     "multidevice_setup/multidevice_setup_client_factory.h",
+    "multidevice_setup/oobe_completion_tracker_factory.cc",
+    "multidevice_setup/oobe_completion_tracker_factory.h",
     "net/cert_verify_proc_chromeos.cc",
     "net/cert_verify_proc_chromeos.h",
     "net/client_cert_filter_chromeos.cc",

chrome/browser/chromeos/login/screens/multidevice_setup_screen.cc

@@ -8,9 +8,11 @@
 #include "base/logging.h"
 #include "chrome/browser/chromeos/login/screens/multidevice_setup_screen_view.h"
 #include "chrome/browser/chromeos/multidevice_setup/multidevice_setup_client_factory.h"
+#include "chrome/browser/chromeos/multidevice_setup/oobe_completion_tracker_factory.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chromeos/chromeos_features.h"
 #include "chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
+#include "chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h"
 
 namespace chromeos {
 
@@ -67,6 +69,14 @@ void MultiDeviceSetupScreen::Show() {
   }
 
   view_->Show();
+
+  // Record that user was presented with setup flow to prevent spam
+  // notifications from suggesting setup in the future.
+  multidevice_setup::OobeCompletionTracker* oobe_completion_tracker =
+      multidevice_setup::OobeCompletionTrackerFactory::GetForProfile(
+          ProfileManager::GetActiveUserProfile());
+  DCHECK(oobe_completion_tracker);
+  oobe_completion_tracker->MarkOobeShown();
 }
 
 void MultiDeviceSetupScreen::Hide() {

chrome/browser/chromeos/multidevice_setup/oobe_completion_tracker_factory.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/multidevice_setup/oobe_completion_tracker_factory.h"
+
+#include "base/macros.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/keyed_service/core/keyed_service.h"
+#include "content/public/browser/browser_context.h"
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+// static
+OobeCompletionTracker* OobeCompletionTrackerFactory::GetForProfile(
+    Profile* profile) {
+  return static_cast<OobeCompletionTracker*>(
+      GetInstance()->GetServiceForBrowserContext(profile, true));
+}
+
+// static
+OobeCompletionTrackerFactory* OobeCompletionTrackerFactory::GetInstance() {
+  return base::Singleton<OobeCompletionTrackerFactory>::get();
+}
+
+OobeCompletionTrackerFactory::OobeCompletionTrackerFactory()
+    : BrowserContextKeyedServiceFactory(
+          "OobeCompletionTrackerFactory",
+          BrowserContextDependencyManager::GetInstance()) {}
+
+OobeCompletionTrackerFactory::~OobeCompletionTrackerFactory() = default;
+
+KeyedService* OobeCompletionTrackerFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  return new OobeCompletionTracker();
+}
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos

chrome/browser/chromeos/multidevice_setup/oobe_completion_tracker_factory.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_CHROMEOS_MULTIDEVICE_SETUP_OOBE_COMPLETION_TRACKER_FACTORY_H_
+#define CHROME_BROWSER_CHROMEOS_MULTIDEVICE_SETUP_OOBE_COMPLETION_TRACKER_FACTORY_H_
+
+#include "base/memory/singleton.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+class KeyedService;
+class Profile;
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+class OobeCompletionTracker;
+
+// Owns OobeCompletionTracker instances and associates them with Profiles.
+class OobeCompletionTrackerFactory : public BrowserContextKeyedServiceFactory {
+ public:
+  static OobeCompletionTracker* GetForProfile(Profile* profile);
+
+  static OobeCompletionTrackerFactory* GetInstance();
+
+ private:
+  friend struct base::DefaultSingletonTraits<OobeCompletionTrackerFactory>;
+
+  OobeCompletionTrackerFactory();
+  ~OobeCompletionTrackerFactory() override;
+
+  // BrowserContextKeyedServiceFactory:
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override;
+
+  DISALLOW_COPY_AND_ASSIGN(OobeCompletionTrackerFactory);
+};
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_MULTIDEVICE_SETUP_OOBE_COMPLETION_TRACKER_FACTORY_H_

chrome/browser/profiles/profile_impl.cc

@@ -152,6 +152,7 @@
 #include "chrome/browser/chromeos/multidevice_setup/android_sms_pairing_state_tracker_impl.h"
 #include "chrome/browser/chromeos/multidevice_setup/auth_token_validator_factory.h"
 #include "chrome/browser/chromeos/multidevice_setup/auth_token_validator_impl.h"
+#include "chrome/browser/chromeos/multidevice_setup/oobe_completion_tracker_factory.h"
 #include "chrome/browser/chromeos/net/delay_network_call.h"
 #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h"
 #include "chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.h"
@@ -1497,6 +1498,8 @@ ProfileImpl::CreateMultiDeviceSetupService() {
       chromeos::device_sync::DeviceSyncClientFactory::GetForProfile(this),
       chromeos::multidevice_setup::AuthTokenValidatorFactory::GetForProfile(
           this),
+      chromeos::multidevice_setup::OobeCompletionTrackerFactory::GetForProfile(
+          this),
       std::make_unique<
           chromeos::multidevice_setup::AndroidSmsAppHelperDelegateImpl>(this),
       std::make_unique<

chromeos/services/multidevice_setup/BUILD.gn

@@ -65,6 +65,7 @@ static_library("multidevice_setup") {
     "//chromeos/services/multidevice_setup/public/cpp:android_sms_app_helper_delegate",
     "//chromeos/services/multidevice_setup/public/cpp:android_sms_pairing_state_tracker",
     "//chromeos/services/multidevice_setup/public/cpp:auth_token_validator",
+    "//chromeos/services/multidevice_setup/public/cpp:oobe_completion_tracker",
     "//chromeos/services/multidevice_setup/public/cpp:prefs",
     "//chromeos/services/multidevice_setup/public/mojom",
     "//chromeos/services/secure_channel/public/cpp/client",
@@ -142,6 +143,7 @@ source_set("unit_tests") {
     "//base",
     "//base/test:test_support",
     "//chromeos/services/device_sync/public/cpp:test_support",
+    "//chromeos/services/multidevice_setup/public/cpp:oobe_completion_tracker",
     "//chromeos/services/multidevice_setup/public/cpp:prefs",
     "//chromeos/services/multidevice_setup/public/cpp:test_support",
     "//chromeos/services/multidevice_setup/public/cpp:unit_tests",

chromeos/services/multidevice_setup/DEPS

@@ -2,6 +2,7 @@ include_rules = [
   "+chrome/common/url_constants.h",
   "+components/proximity_auth/logging/logging.h",
   "+components/cryptauth",
+  '+components/keyed_service/core',
   "+components/sync_preferences/testing_pref_service_syncable.h",
   "+components/variations",
   "+mojo/public/cpp/bindings",

chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc

@@ -54,10 +54,11 @@ AccountStatusChangeDelegateNotifierImpl::Factory::BuildInstance(
     HostStatusProvider* host_status_provider,
     PrefService* pref_service,
     HostDeviceTimestampManager* host_device_timestamp_manager,
+    OobeCompletionTracker* oobe_completion_tracker,
     base::Clock* clock) {
   return base::WrapUnique(new AccountStatusChangeDelegateNotifierImpl(
       host_status_provider, pref_service, host_device_timestamp_manager,
-      clock));
+      oobe_completion_tracker, clock));
 }
 
 // static
@@ -72,6 +73,9 @@ void AccountStatusChangeDelegateNotifierImpl::RegisterPrefs(
                               kTimestampNotSet);
   registry->RegisterInt64Pref(kExistingUserChromebookAddedPrefName,
                               kTimestampNotSet);
+
+  registry->RegisterInt64Pref(kOobeSetupFlowTimestampPrefName,
+                              kTimestampNotSet);
   registry->RegisterStringPref(
       kVerifiedHostDeviceIdFromMostRecentHostStatusUpdatePrefName, kNoHost);
 }
@@ -79,6 +83,7 @@ void AccountStatusChangeDelegateNotifierImpl::RegisterPrefs(
 AccountStatusChangeDelegateNotifierImpl::
     ~AccountStatusChangeDelegateNotifierImpl() {
   host_status_provider_->RemoveObserver(this);
+  oobe_completion_tracker_->RemoveObserver(this);
 }
 
 void AccountStatusChangeDelegateNotifierImpl::OnDelegateSet() {
@@ -108,19 +113,30 @@ const char AccountStatusChangeDelegateNotifierImpl::
     kVerifiedHostDeviceIdFromMostRecentHostStatusUpdatePrefName[] =
         "multidevice_setup.host_device_id_from_most_recent_sync";
 
+// The timestamps (in milliseconds since UNIX Epoch, aka JavaTime) of the user
+// seeing setup flow in OOBE. If it is 0, the user did not see the setup flow in
+// OOBE.
+// static
+const char
+    AccountStatusChangeDelegateNotifierImpl::kOobeSetupFlowTimestampPrefName[] =
+        "multidevice_setup.oobe_setup_flow_timestamp ";
+
 AccountStatusChangeDelegateNotifierImpl::
     AccountStatusChangeDelegateNotifierImpl(
         HostStatusProvider* host_status_provider,
         PrefService* pref_service,
         HostDeviceTimestampManager* host_device_timestamp_manager,
+        OobeCompletionTracker* oobe_completion_tracker,
         base::Clock* clock)
     : host_status_provider_(host_status_provider),
       pref_service_(pref_service),
       host_device_timestamp_manager_(host_device_timestamp_manager),
+      oobe_completion_tracker_(oobe_completion_tracker),
       clock_(clock) {
   verified_host_device_id_from_most_recent_update_ =
       LoadHostDeviceIdFromEndOfPreviousSession();
   host_status_provider_->AddObserver(this);
+  oobe_completion_tracker_->AddObserver(this);
 }
 
 void AccountStatusChangeDelegateNotifierImpl::OnHostStatusChange(
@@ -128,6 +144,13 @@ void AccountStatusChangeDelegateNotifierImpl::OnHostStatusChange(
   CheckForMultiDeviceEvents(host_status_with_device);
 }
 
+void AccountStatusChangeDelegateNotifierImpl::OnOobeCompleted() {
+  pref_service_->SetInt64(kOobeSetupFlowTimestampPrefName,
+                          clock_->Now().ToJavaTime());
+  if (delegate())
+    delegate()->OnNoLongerNewUser();
+}
+
 void AccountStatusChangeDelegateNotifierImpl::CheckForMultiDeviceEvents(
     const HostStatusProvider::HostStatusWithDevice& host_status_with_device) {
   if (!delegate()) {
@@ -174,6 +197,13 @@ void AccountStatusChangeDelegateNotifierImpl::
     CheckForNewUserPotentialHostExistsEvent(
         const HostStatusProvider::HostStatusWithDevice&
             host_status_with_device) {
+  // We do not notify the user if they already had a chance to go through setup
+  // flow in OOBE.
+  if (pref_service_->GetInt64(kOobeSetupFlowTimestampPrefName) !=
+      kTimestampNotSet) {
+    return;
+  }
+
   // We only check for new user events if there is no enabled host.
   if (verified_host_device_id_from_most_recent_update_)
     return;

chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.h

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "chromeos/services/multidevice_setup/account_status_change_delegate_notifier.h"
 #include "chromeos/services/multidevice_setup/host_status_provider.h"
+#include "chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h"
 #include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
 
 class PrefRegistrySimple;
@@ -31,7 +32,8 @@ class HostDeviceTimestampManager;
 // previous notifications.
 class AccountStatusChangeDelegateNotifierImpl
     : public AccountStatusChangeDelegateNotifier,
-      public HostStatusProvider::Observer {
+      public HostStatusProvider::Observer,
+      public OobeCompletionTracker::Observer {
  public:
   class Factory {
    public:
@@ -42,6 +44,7 @@ class AccountStatusChangeDelegateNotifierImpl
         HostStatusProvider* host_status_provider,
         PrefService* pref_service,
         HostDeviceTimestampManager* host_device_timestamp_manager,
+        OobeCompletionTracker* oobe_completion_tracker,
         base::Clock* clock);
 
    private:
@@ -62,6 +65,7 @@ class AccountStatusChangeDelegateNotifierImpl
   static const char kExistingUserHostSwitchedPrefName[];
   static const char kExistingUserChromebookAddedPrefName[];
 
+  static const char kOobeSetupFlowTimestampPrefName[];
   static const char
       kVerifiedHostDeviceIdFromMostRecentHostStatusUpdatePrefName[];
 
@@ -69,6 +73,7 @@ class AccountStatusChangeDelegateNotifierImpl
       HostStatusProvider* host_status_provider,
       PrefService* pref_service,
       HostDeviceTimestampManager* host_device_timestamp_manager,
+      OobeCompletionTracker* oobe_completion_tracker,
       base::Clock* clock);
 
   // AccountStatusChangeDelegateNotifier:
@@ -78,6 +83,9 @@ class AccountStatusChangeDelegateNotifierImpl
   void OnHostStatusChange(const HostStatusProvider::HostStatusWithDevice&
                               host_status_with_device) override;
 
+  // OobeCompletionTracker::Observer:
+  void OnOobeCompleted() override;
+
   void CheckForMultiDeviceEvents(
       const HostStatusProvider::HostStatusWithDevice& host_status_with_device);
 
@@ -107,6 +115,7 @@ class AccountStatusChangeDelegateNotifierImpl
   HostStatusProvider* host_status_provider_;
   PrefService* pref_service_;
   HostDeviceTimestampManager* host_device_timestamp_manager_;
+  OobeCompletionTracker* oobe_completion_tracker_;
   base::Clock* clock_;
 
   DISALLOW_COPY_AND_ASSIGN(AccountStatusChangeDelegateNotifierImpl);

chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl_unittest.cc

@@ -14,6 +14,7 @@
 #include "chromeos/services/multidevice_setup/fake_account_status_change_delegate.h"
 #include "chromeos/services/multidevice_setup/fake_host_device_timestamp_manager.h"
 #include "chromeos/services/multidevice_setup/fake_host_status_provider.h"
+#include "chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h"
 #include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
 #include "components/cryptauth/remote_device_ref.h"
 #include "components/cryptauth/remote_device_test_util.h"
@@ -27,14 +28,31 @@ namespace multidevice_setup {
 namespace {
 
 const int64_t kTestTimeMillis = 1500000000000;
-const std::string kFakePhoneKey = "fake-phone-key";
-const std::string kFakePhoneName = "Phony Phone";
+const char kFakePhoneKey[] = "fake-phone-key";
+const char kFakePhoneName[] = "Phony Phone";
+const char kFakePhoneKeyA[] = "fake-phone-key-A";
+const char kFakePhoneNameA[] = "Phony Phone A";
+const char kFakePhoneKeyB[] = "fake-phone-key-B";
+const char kFakePhoneNameB[] = "Phony Phone B";
+
 const cryptauth::RemoteDeviceRef kFakePhone =
     cryptauth::RemoteDeviceRefBuilder()
         .SetPublicKey(kFakePhoneKey)
         .SetName(kFakePhoneName)
         .Build();
 
+// Alternate hosts for multi host tests
+const cryptauth::RemoteDeviceRef kFakePhoneA =
+    cryptauth::RemoteDeviceRefBuilder()
+        .SetPublicKey("fake-phone-key-A")
+        .SetName("Phony Phone A")
+        .Build();
+const cryptauth::RemoteDeviceRef kFakePhoneB =
+    cryptauth::RemoteDeviceRefBuilder()
+        .SetPublicKey("fake-phone-key-B")
+        .SetName("Phony Phone B")
+        .Build();
+
 }  // namespace
 
 class MultiDeviceSetupAccountStatusChangeDelegateNotifierTest
@@ -55,6 +73,7 @@ class MultiDeviceSetupAccountStatusChangeDelegateNotifierTest
     test_clock_ = std::make_unique<base::SimpleTestClock>();
     fake_host_device_timestamp_manager_ =
         std::make_unique<FakeHostDeviceTimestampManager>();
+    fake_oobe_completion_tracker_ = std::make_unique<OobeCompletionTracker>();
     test_clock_->SetNow(base::Time::FromJavaTime(kTestTimeMillis));
   }
 
@@ -62,11 +81,18 @@ class MultiDeviceSetupAccountStatusChangeDelegateNotifierTest
     delegate_notifier_ =
         AccountStatusChangeDelegateNotifierImpl::Factory::Get()->BuildInstance(
             fake_host_status_provider_.get(), test_pref_service_.get(),
-            fake_host_device_timestamp_manager_.get(), test_clock_.get());
+            fake_host_device_timestamp_manager_.get(),
+            fake_oobe_completion_tracker_.get(), test_clock_.get());
+  }
+
+  cryptauth::RemoteDeviceRef BuildFakePhone(std::string public_key,
+                                            std::string name) {
+    return cryptauth::RemoteDeviceRefBuilder()
+        .SetPublicKey(public_key)
+        .SetName(name)
+        .Build();
   }
 
-  // Following HostStatusWithDevice contract, this sets the device to null when
-  // there is no set host (or the default kFakePhone otherwise).
   void SetHostWithStatus(
       mojom::HostStatus host_status,
       const base::Optional<cryptauth::RemoteDeviceRef>& host_device) {
@@ -122,6 +148,16 @@ class MultiDeviceSetupAccountStatusChangeDelegateNotifierTest
     delegate_notifier_->FlushForTesting();
   }
 
+  // Simulates completing setup flow in OOBE.
+  void CompleteOobeSetupFlow() {
+    fake_host_status_provider_->SetHostWithStatus(
+        mojom::HostStatus::kEligibleHostExistsButNoHostSet,
+        base::nullopt /* host_device */);
+    fake_oobe_completion_tracker_->MarkOobeShown();
+    if (fake_delegate_)
+      delegate_notifier_->FlushForTesting();
+  }
+
   int64_t GetNewUserPotentialHostExistsTimestamp() {
     return test_pref_service_->GetInt64(
         AccountStatusChangeDelegateNotifierImpl::
@@ -134,6 +170,12 @@ class MultiDeviceSetupAccountStatusChangeDelegateNotifierTest
             kExistingUserChromebookAddedPrefName);
   }
 
+  int64_t GetOobeSetupFlowTimestamp() {
+    return test_pref_service_->GetInt64(
+        AccountStatusChangeDelegateNotifierImpl::
+            kOobeSetupFlowTimestampPrefName);
+  }
+
   std::string GetMostRecentVerifiedHostDeviceIdPref() {
     return test_pref_service_->GetString(
         AccountStatusChangeDelegateNotifierImpl::
@@ -153,6 +195,7 @@ class MultiDeviceSetupAccountStatusChangeDelegateNotifierTest
       test_pref_service_;
   std::unique_ptr<FakeHostDeviceTimestampManager>
       fake_host_device_timestamp_manager_;
+  std::unique_ptr<OobeCompletionTracker> fake_oobe_completion_tracker_;
   std::unique_ptr<base::SimpleTestClock> test_clock_;
 
   std::unique_ptr<AccountStatusChangeDelegateNotifier> delegate_notifier_;
@@ -202,15 +245,17 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
 
   SetHostWithStatus(
       mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation,
-      kFakePhone);
+      BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   EXPECT_EQ(0u, fake_delegate()->num_new_user_potential_host_events_handled());
   EXPECT_EQ(0u, GetNewUserPotentialHostExistsTimestamp());
 
-  SetHostWithStatus(mojom::HostStatus::kHostSetButNotYetVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostSetButNotYetVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   EXPECT_EQ(0u, fake_delegate()->num_new_user_potential_host_events_handled());
   EXPECT_EQ(0u, GetNewUserPotentialHostExistsTimestamp());
 
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   EXPECT_EQ(0u, fake_delegate()->num_new_user_potential_host_events_handled());
   EXPECT_EQ(0u, GetNewUserPotentialHostExistsTimestamp());
 }
@@ -282,7 +327,39 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
   // A potential host was set.
   SetHostWithStatus(
       mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation,
-      kFakePhone);
+      BuildFakePhone(kFakePhoneKey, kFakePhoneName));
+  EXPECT_EQ(1u, fake_delegate()->num_new_user_potential_host_events_handled());
+  EXPECT_EQ(1u, fake_delegate()->num_no_longer_new_user_events_handled());
+}
+
+TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
+       CompletingOobeSetupFlowBlocksNewUserEventIfNoDelegateIsSet) {
+  BuildAccountStatusChangeDelegateNotifier();
+
+  // Complete OOBE MultiDevice setup flow before delegate is set.
+  EXPECT_EQ(0u, GetOobeSetupFlowTimestamp());
+  CompleteOobeSetupFlow();
+  EXPECT_EQ(kTestTimeMillis, GetOobeSetupFlowTimestamp());
+
+  // Set delegate, which triggers event check.
+  SetAccountStatusChangeDelegatePtr();
+
+  EXPECT_EQ(0u, fake_delegate()->num_new_user_potential_host_events_handled());
+  EXPECT_EQ(0u, GetNewUserPotentialHostExistsTimestamp());
+}
+
+TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
+       CompletingOobeSetupFlowWithDelegateSetTriggersNoLongerNewUserEvent) {
+  BuildAccountStatusChangeDelegateNotifier();
+  SetAccountStatusChangeDelegatePtr();
+  SetHostWithStatus(mojom::HostStatus::kEligibleHostExistsButNoHostSet,
+                    base::nullopt /* host_device */);
+
+  // Complete OOBE MultiDevice setup flow before delegate is set.
+  EXPECT_EQ(0u, GetOobeSetupFlowTimestamp());
+  CompleteOobeSetupFlow();
+  EXPECT_EQ(kTestTimeMillis, GetOobeSetupFlowTimestamp());
+
   EXPECT_EQ(1u, fake_delegate()->num_new_user_potential_host_events_handled());
   EXPECT_EQ(1u, fake_delegate()->num_no_longer_new_user_events_handled());
 }
@@ -304,12 +381,15 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
   // event in the absence of the Chromebook added timestamp.
   SetHostWithStatus(
       mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation,
-      kFakePhone);
+      BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   EXPECT_EQ(0u, fake_delegate()->num_no_longer_new_user_events_handled());
 }
 
 TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
        NotifiesObserverForHostSwitchEvents) {
+  const cryptauth::RemoteDeviceRef fakePhone =
+      BuildFakePhone(kFakePhoneKey, kFakePhoneName);
+
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
   // Check the delegate initializes to 0.
@@ -317,31 +397,25 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 
   // Set initially verified host.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified, fakePhone);
   // Host was set but has never been switched.
   EXPECT_EQ(0u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 
   // Switch to new verified host.
   SetHostWithStatus(mojom::HostStatus::kHostVerified,
-                    cryptauth::RemoteDeviceRefBuilder()
-                        .SetPublicKey(kFakePhoneKey + "A")
-                        .SetName(kFakePhoneName + "A")
-                        .Build());
+                    BuildFakePhone(kFakePhoneKeyA, kFakePhoneNameA));
   EXPECT_EQ(1u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 
   // Switch to a different new verified host.
   SetHostWithStatus(mojom::HostStatus::kHostVerified,
-                    cryptauth::RemoteDeviceRefBuilder()
-                        .SetPublicKey(kFakePhoneKey + "B")
-                        .SetName(kFakePhoneName + "B")
-                        .Build());
+                    BuildFakePhone(kFakePhoneKeyB, kFakePhoneNameB));
   EXPECT_EQ(2u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 
   // Switch back to initial host (verified).
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified, fakePhone);
   EXPECT_EQ(3u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 }
@@ -351,13 +425,11 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
   // Set initially verified host.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   // Set to host with identical information.
   SetHostWithStatus(mojom::HostStatus::kHostVerified,
-                    cryptauth::RemoteDeviceRefBuilder()
-                        .SetPublicKey(kFakePhoneKey)
-                        .SetName(kFakePhoneName)
-                        .Build());
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   EXPECT_EQ(0u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 }
@@ -367,25 +439,26 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
   // Set initially verified host.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   // Set to verified host with same name but different key.
   SetHostWithStatus(mojom::HostStatus::kHostVerified,
-                    cryptauth::RemoteDeviceRefBuilder()
-                        .SetPublicKey(kFakePhoneKey + "alternate")
-                        .SetName(kFakePhoneName)
-                        .Build());
+                    BuildFakePhone(kFakePhoneKeyA, kFakePhoneName));
   EXPECT_EQ(1u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 }
 
 TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
        VerifyingSetHostTriggersNoHostSwtichEvent) {
+  const cryptauth::RemoteDeviceRef fakePhone =
+      BuildFakePhone(kFakePhoneKey, kFakePhoneName);
+
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
   // Set initial host but do not verify.
-  SetHostWithStatus(mojom::HostStatus::kHostSetButNotYetVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostSetButNotYetVerified, fakePhone);
   // Verify host.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified, fakePhone);
   EXPECT_EQ(0u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 }
@@ -395,17 +468,15 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
   // Set initially verified host.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
 
   EXPECT_EQ(0u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 
   // Set a new host without verifying.
   SetHostWithStatus(mojom::HostStatus::kHostSetButNotYetVerified,
-                    cryptauth::RemoteDeviceRefBuilder()
-                        .SetPublicKey(kFakePhoneKey + "A")
-                        .SetName(kFakePhoneName + "A")
-                        .Build());
+                    BuildFakePhone(kFakePhoneKeyA, kFakePhoneNameA));
   EXPECT_EQ(0u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 
@@ -413,10 +484,7 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
   // unverified.
   SetHostWithStatus(
       mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation,
-      cryptauth::RemoteDeviceRefBuilder()
-          .SetPublicKey(kFakePhoneKey + "B")
-          .SetName(kFakePhoneName + "B")
-          .Build());
+      BuildFakePhone(kFakePhoneKeyB, kFakePhoneNameB));
   EXPECT_EQ(0u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 }
@@ -426,7 +494,8 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
   // Set initially verified host.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
 
   EXPECT_EQ(0u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
@@ -437,20 +506,20 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
 
   // Set a new verified host.
   SetHostWithStatus(mojom::HostStatus::kHostVerified,
-                    cryptauth::RemoteDeviceRefBuilder()
-                        .SetPublicKey(kFakePhoneKey + "alternate")
-                        .SetName(kFakePhoneName + "alternate")
-                        .Build());
+                    BuildFakePhone(kFakePhoneKeyA, kFakePhoneNameA));
   EXPECT_EQ(0u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 }
 
 TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
        HostSwitchedBetweenSessions) {
-  SetHostFromPreviousSession(kFakePhoneKey + "-old");
+  // Host set in some previous session.
+  SetHostFromPreviousSession(
+      BuildFakePhone(kFakePhoneKeyA, kFakePhoneNameA).GetDeviceId());
   BuildAccountStatusChangeDelegateNotifier();
   // Host switched and verified between sessions.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   SetAccountStatusChangeDelegatePtr();
   EXPECT_EQ(1u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
@@ -460,7 +529,7 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
        NoHostSwitchedEventWithoutExistingHost) {
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
-  SetUpHost(kFakePhone);
+  SetUpHost(BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   EXPECT_EQ(0u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 }
@@ -469,14 +538,12 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
        NoHostSwitchedEventWithoutObserverSet) {
   BuildAccountStatusChangeDelegateNotifier();
   // Set initially verified host.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   // All conditions for host switched event are satisfied except for setting a
   // delegate.
   SetHostWithStatus(mojom::HostStatus::kHostVerified,
-                    cryptauth::RemoteDeviceRefBuilder()
-                        .SetPublicKey(kFakePhoneKey + "alternate")
-                        .SetName(kFakePhoneName + "alternate")
-                        .Build());
+                    BuildFakePhone(kFakePhoneKeyA, kFakePhoneNameA));
   EXPECT_EQ(0u,
             fake_delegate()->num_existing_user_host_switched_events_handled());
 }
@@ -491,13 +558,17 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
 
   // Host is set and verified from another Chromebook while this one is logged
   // in.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   EXPECT_EQ(
       1u, fake_delegate()->num_existing_user_chromebook_added_events_handled());
 }
 
 TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
        OnlyVerifiedHostCausesChromebookAddedEvent) {
+  const cryptauth::RemoteDeviceRef fakePhone =
+      BuildFakePhone(kFakePhoneKey, kFakePhoneName);
+
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
   // Start with potential hosts but none set.
@@ -505,12 +576,12 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
                     base::nullopt /* host_device */);
 
   // Set a host without verifying.
-  SetHostWithStatus(mojom::HostStatus::kHostSetButNotYetVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostSetButNotYetVerified, fakePhone);
   EXPECT_EQ(
       0u, fake_delegate()->num_existing_user_chromebook_added_events_handled());
 
   // Verify the new host.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified, fakePhone);
   EXPECT_EQ(
       1u, fake_delegate()->num_existing_user_chromebook_added_events_handled());
 }
@@ -522,17 +593,11 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
   // Start with potential hosts but none set.
   // Set initial host but do not verify.
   SetHostWithStatus(mojom::HostStatus::kHostSetButNotYetVerified,
-                    cryptauth::RemoteDeviceRefBuilder()
-                        .SetPublicKey(kFakePhoneKey + "A")
-                        .SetName(kFakePhoneName + "A")
-                        .Build());
+                    BuildFakePhone(kFakePhoneKeyA, kFakePhoneNameA));
 
   // Replace unverified Phone A with verified Phone B.
   SetHostWithStatus(mojom::HostStatus::kHostVerified,
-                    cryptauth::RemoteDeviceRefBuilder()
-                        .SetPublicKey(kFakePhoneKey + "B")
-                        .SetName(kFakePhoneName + "B")
-                        .Build());
+                    BuildFakePhone(kFakePhoneKeyB, kFakePhoneNameB));
   // This causes a 'Chromebook added' event.
   EXPECT_EQ(
       1u, fake_delegate()->num_existing_user_chromebook_added_events_handled());
@@ -545,7 +610,8 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
        ChromebookAddedBetweenSessionsTriggersEvents) {
   BuildAccountStatusChangeDelegateNotifier();
   // Host is set and verified before this Chromebook is logged in.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
 
   SetAccountStatusChangeDelegatePtr();
   EXPECT_EQ(
@@ -554,10 +620,13 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
 
 TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
        ChromebookAddedEventBlockedIfHostWasSetFromThisChromebook) {
+  const cryptauth::RemoteDeviceRef fakePhone =
+      BuildFakePhone(kFakePhoneKey, kFakePhoneName);
+
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
 
-  SetUpHost(kFakePhone);
+  SetUpHost(fakePhone);
   // The host was set on this Chromebook so it should not trigger the Chromebook
   // added event
   EXPECT_EQ(
@@ -565,7 +634,7 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
 
   ForgetHost();
   // Set verified host on another Chromebook.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified, fakePhone);
   EXPECT_EQ(
       1u, fake_delegate()->num_existing_user_chromebook_added_events_handled());
 }
@@ -581,7 +650,8 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
 
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   EXPECT_EQ(
       1u, fake_delegate()->num_existing_user_chromebook_added_events_handled());
   // Timestamp was overwritten by clock.
@@ -593,38 +663,36 @@ TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
   BuildAccountStatusChangeDelegateNotifier();
   // Triggers event check. Note that all conditions for Chromebook added event
   // are satisfied except for setting a delegate.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
+  SetHostWithStatus(mojom::HostStatus::kHostVerified,
+                    BuildFakePhone(kFakePhoneKey, kFakePhoneName));
   EXPECT_EQ(
       0u, fake_delegate()->num_existing_user_chromebook_added_events_handled());
 }
 
 TEST_F(MultiDeviceSetupAccountStatusChangeDelegateNotifierTest,
        VerifiedHostIdStaysUpToDateInPrefs) {
+  const cryptauth::RemoteDeviceRef fakePhone =
+      BuildFakePhone(kFakePhoneKey, kFakePhoneName);
+  const cryptauth::RemoteDeviceRef fakePhoneA =
+      BuildFakePhone(kFakePhoneKeyA, kFakePhoneNameA);
+
   BuildAccountStatusChangeDelegateNotifier();
   SetAccountStatusChangeDelegatePtr();
   // Check the delegate initializes to empty.
   EXPECT_EQ(GetMostRecentVerifiedHostDeviceIdPref(), "");
 
   // Set initially verified host.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhone);
-  EXPECT_EQ(GetMostRecentVerifiedHostDeviceIdPref(), kFakePhone.GetDeviceId());
-
-  const cryptauth::RemoteDeviceRef kFakePhoneAlternate =
-      cryptauth::RemoteDeviceRefBuilder()
-          .SetPublicKey(kFakePhoneKey + "alternate")
-          .SetName(kFakePhoneName + "alternate")
-          .Build();
+  SetHostWithStatus(mojom::HostStatus::kHostVerified, fakePhone);
+  EXPECT_EQ(GetMostRecentVerifiedHostDeviceIdPref(), fakePhone.GetDeviceId());
 
   // Switch to an unverified host.
-  SetHostWithStatus(mojom::HostStatus::kHostSetButNotYetVerified,
-                    kFakePhoneAlternate);
+  SetHostWithStatus(mojom::HostStatus::kHostSetButNotYetVerified, fakePhoneA);
   // The host is set but not verified so the pref should be set to empty.
   EXPECT_EQ(GetMostRecentVerifiedHostDeviceIdPref(), "");
 
   // Verify the new host.
-  SetHostWithStatus(mojom::HostStatus::kHostVerified, kFakePhoneAlternate);
-  EXPECT_EQ(GetMostRecentVerifiedHostDeviceIdPref(),
-            kFakePhoneAlternate.GetDeviceId());
+  SetHostWithStatus(mojom::HostStatus::kHostVerified, fakePhoneA);
+  EXPECT_EQ(GetMostRecentVerifiedHostDeviceIdPref(), fakePhoneA.GetDeviceId());
 
   ForgetHost();
   EXPECT_EQ(GetMostRecentVerifiedHostDeviceIdPref(), "");

chromeos/services/multidevice_setup/multidevice_setup_impl.cc

@@ -21,6 +21,7 @@
 #include "chromeos/services/multidevice_setup/public/cpp/android_sms_app_helper_delegate.h"
 #include "chromeos/services/multidevice_setup/public/cpp/android_sms_pairing_state_tracker.h"
 #include "chromeos/services/multidevice_setup/public/cpp/auth_token_validator.h"
+#include "chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h"
 
 namespace chromeos {
 
@@ -56,6 +57,7 @@ MultiDeviceSetupImpl::Factory::BuildInstance(
     PrefService* pref_service,
     device_sync::DeviceSyncClient* device_sync_client,
     AuthTokenValidator* auth_token_validator,
+    OobeCompletionTracker* oobe_completion_tracker,
     std::unique_ptr<AndroidSmsAppHelperDelegate>
         android_sms_app_helper_delegate,
     std::unique_ptr<AndroidSmsPairingStateTracker>
@@ -63,7 +65,7 @@ MultiDeviceSetupImpl::Factory::BuildInstance(
     const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider) {
   return base::WrapUnique(new MultiDeviceSetupImpl(
       pref_service, device_sync_client, auth_token_validator,
-      std::move(android_sms_app_helper_delegate),
+      oobe_completion_tracker, std::move(android_sms_app_helper_delegate),
       std::move(android_sms_pairing_state_tracker), gcm_device_info_provider));
 }
 
@@ -71,6 +73,7 @@ MultiDeviceSetupImpl::MultiDeviceSetupImpl(
     PrefService* pref_service,
     device_sync::DeviceSyncClient* device_sync_client,
     AuthTokenValidator* auth_token_validator,
+    OobeCompletionTracker* oobe_completion_tracker,
     std::unique_ptr<AndroidSmsAppHelperDelegate>
         android_sms_app_helper_delegate,
     std::unique_ptr<AndroidSmsPairingStateTracker>
@@ -115,6 +118,7 @@ MultiDeviceSetupImpl::MultiDeviceSetupImpl(
               ->BuildInstance(host_status_provider_.get(),
                               pref_service,
                               host_device_timestamp_manager_.get(),
+                              oobe_completion_tracker,
                               base::DefaultClock::GetInstance())),
       device_reenroller_(DeviceReenroller::Factory::Get()->BuildInstance(
           device_sync_client,

chromeos/services/multidevice_setup/multidevice_setup_impl.h

@@ -42,6 +42,7 @@ class HostBackendDelegate;
 class HostDeviceTimestampManager;
 class HostStatusProvider;
 class HostVerifier;
+class OobeCompletionTracker;
 
 // Concrete MultiDeviceSetup implementation.
 class MultiDeviceSetupImpl : public MultiDeviceSetupBase,
@@ -57,6 +58,7 @@ class MultiDeviceSetupImpl : public MultiDeviceSetupBase,
         PrefService* pref_service,
         device_sync::DeviceSyncClient* device_sync_client,
         AuthTokenValidator* auth_token_validator,
+        OobeCompletionTracker* oobe_completion_tracker,
         std::unique_ptr<AndroidSmsAppHelperDelegate>
             android_sms_app_helper_delegate,
         std::unique_ptr<AndroidSmsPairingStateTracker>
@@ -76,6 +78,7 @@ class MultiDeviceSetupImpl : public MultiDeviceSetupBase,
       PrefService* pref_service,
       device_sync::DeviceSyncClient* device_sync_client,
       AuthTokenValidator* auth_token_validator,
+      OobeCompletionTracker* oobe_completion_tracker,
       std::unique_ptr<AndroidSmsAppHelperDelegate>
           android_sms_app_helper_delegate,
       std::unique_ptr<AndroidSmsPairingStateTracker>

chromeos/services/multidevice_setup/multidevice_setup_impl_unittest.cc

@@ -33,6 +33,7 @@
 #include "chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.h"
 #include "chromeos/services/multidevice_setup/public/cpp/fake_android_sms_pairing_state_tracker.h"
 #include "chromeos/services/multidevice_setup/public/cpp/fake_auth_token_validator.h"
+#include "chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h"
 #include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
 #include "components/cryptauth/fake_gcm_device_info_provider.h"
 #include "components/cryptauth/remote_device_test_util.h"
@@ -368,11 +369,13 @@ class FakeAccountStatusChangeDelegateNotifierFactory
       sync_preferences::TestingPrefServiceSyncable*
           expected_testing_pref_service,
       FakeHostDeviceTimestampManagerFactory*
-          fake_host_device_timestamp_manager_factory)
+          fake_host_device_timestamp_manager_factory,
+      OobeCompletionTracker* expected_oobe_completion_tracker)
       : fake_host_status_provider_factory_(fake_host_status_provider_factory),
         expected_testing_pref_service_(expected_testing_pref_service),
         fake_host_device_timestamp_manager_factory_(
-            fake_host_device_timestamp_manager_factory) {}
+            fake_host_device_timestamp_manager_factory),
+        expected_oobe_completion_tracker_(expected_oobe_completion_tracker) {}
 
   ~FakeAccountStatusChangeDelegateNotifierFactory() override = default;
 
@@ -384,6 +387,7 @@ class FakeAccountStatusChangeDelegateNotifierFactory
       HostStatusProvider* host_status_provider,
       PrefService* pref_service,
       HostDeviceTimestampManager* host_device_timestamp_manager,
+      OobeCompletionTracker* oobe_completion_tracker,
       base::Clock* clock) override {
     EXPECT_FALSE(instance_);
     EXPECT_EQ(fake_host_status_provider_factory_->instance(),
@@ -391,6 +395,7 @@ class FakeAccountStatusChangeDelegateNotifierFactory
     EXPECT_EQ(expected_testing_pref_service_, pref_service);
     EXPECT_EQ(fake_host_device_timestamp_manager_factory_->instance(),
               host_device_timestamp_manager);
+    EXPECT_EQ(expected_oobe_completion_tracker_, oobe_completion_tracker);
 
     auto instance = std::make_unique<FakeAccountStatusChangeDelegateNotifier>();
     instance_ = instance.get();
@@ -401,6 +406,7 @@ class FakeAccountStatusChangeDelegateNotifierFactory
   sync_preferences::TestingPrefServiceSyncable* expected_testing_pref_service_;
   FakeHostDeviceTimestampManagerFactory*
       fake_host_device_timestamp_manager_factory_;
+  OobeCompletionTracker* expected_oobe_completion_tracker_;
 
   FakeAccountStatusChangeDelegateNotifier* instance_ = nullptr;
 
@@ -494,6 +500,8 @@ class MultiDeviceSetupImplTest : public testing::Test {
     fake_auth_token_validator_ = std::make_unique<FakeAuthTokenValidator>();
     fake_auth_token_validator_->set_expected_auth_token(kValidAuthToken);
 
+    fake_oobe_completion_tracker_ = std::make_unique<OobeCompletionTracker>();
+
     auto fake_android_sms_app_helper_delegate =
         std::make_unique<FakeAndroidSmsAppHelperDelegate>();
 
@@ -557,7 +565,8 @@ class MultiDeviceSetupImplTest : public testing::Test {
     fake_account_status_change_delegate_notifier_factory_ =
         std::make_unique<FakeAccountStatusChangeDelegateNotifierFactory>(
             fake_host_status_provider_factory_.get(), test_pref_service_.get(),
-            fake_host_device_timestamp_manager_factory_.get());
+            fake_host_device_timestamp_manager_factory_.get(),
+            fake_oobe_completion_tracker_.get());
     AccountStatusChangeDelegateNotifierImpl::Factory::SetFactoryForTesting(
         fake_account_status_change_delegate_notifier_factory_.get());
 
@@ -578,7 +587,7 @@ class MultiDeviceSetupImplTest : public testing::Test {
 
     multidevice_setup_ = MultiDeviceSetupImpl::Factory::Get()->BuildInstance(
         test_pref_service_.get(), fake_device_sync_client_.get(),
-        fake_auth_token_validator_.get(),
+        fake_auth_token_validator_.get(), fake_oobe_completion_tracker_.get(),
         std::move(fake_android_sms_app_helper_delegate),
         std::move(fake_android_sms_pairing_state_tracker),
         fake_gcm_device_info_provider_.get());
@@ -859,6 +868,7 @@ class MultiDeviceSetupImplTest : public testing::Test {
       test_pref_service_;
   std::unique_ptr<device_sync::FakeDeviceSyncClient> fake_device_sync_client_;
   std::unique_ptr<FakeAuthTokenValidator> fake_auth_token_validator_;
+  std::unique_ptr<OobeCompletionTracker> fake_oobe_completion_tracker_;
   std::unique_ptr<cryptauth::FakeGcmDeviceInfoProvider>
       fake_gcm_device_info_provider_;
 

chromeos/services/multidevice_setup/multidevice_setup_initializer.cc

@@ -43,6 +43,7 @@ MultiDeviceSetupInitializer::Factory::BuildInstance(
     PrefService* pref_service,
     device_sync::DeviceSyncClient* device_sync_client,
     AuthTokenValidator* auth_token_validator,
+    OobeCompletionTracker* oobe_completion_tracker,
     std::unique_ptr<AndroidSmsAppHelperDelegate>
         android_sms_app_helper_delegate,
     std::unique_ptr<AndroidSmsPairingStateTracker>
@@ -50,7 +51,7 @@ MultiDeviceSetupInitializer::Factory::BuildInstance(
     const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider) {
   return base::WrapUnique(new MultiDeviceSetupInitializer(
       pref_service, device_sync_client, auth_token_validator,
-      std::move(android_sms_app_helper_delegate),
+      oobe_completion_tracker, std::move(android_sms_app_helper_delegate),
       std::move(android_sms_pairing_state_tracker), gcm_device_info_provider));
 }
 
@@ -73,6 +74,7 @@ MultiDeviceSetupInitializer::MultiDeviceSetupInitializer(
     PrefService* pref_service,
     device_sync::DeviceSyncClient* device_sync_client,
     AuthTokenValidator* auth_token_validator,
+    OobeCompletionTracker* oobe_completion_tracker,
     std::unique_ptr<AndroidSmsAppHelperDelegate>
         android_sms_app_helper_delegate,
     std::unique_ptr<AndroidSmsPairingStateTracker>
@@ -81,6 +83,7 @@ MultiDeviceSetupInitializer::MultiDeviceSetupInitializer(
     : pref_service_(pref_service),
       device_sync_client_(device_sync_client),
       auth_token_validator_(auth_token_validator),
+      oobe_completion_tracker_(oobe_completion_tracker),
       android_sms_app_helper_delegate_(
           std::move(android_sms_app_helper_delegate)),
       android_sms_pairing_state_tracker_(
@@ -269,7 +272,7 @@ void MultiDeviceSetupInitializer::InitializeImplementation() {
 
   multidevice_setup_impl_ = MultiDeviceSetupImpl::Factory::Get()->BuildInstance(
       pref_service_, device_sync_client_, auth_token_validator_,
-      std::move(android_sms_app_helper_delegate_),
+      oobe_completion_tracker_, std::move(android_sms_app_helper_delegate_),
       std::move(android_sms_pairing_state_tracker_), gcm_device_info_provider_);
 
   if (pending_delegate_) {

chromeos/services/multidevice_setup/multidevice_setup_initializer.h

@@ -27,6 +27,7 @@ namespace multidevice_setup {
 class AndroidSmsAppHelperDelegate;
 class AndroidSmsPairingStateTracker;
 class AuthTokenValidator;
+class OobeCompletionTracker;
 
 // Initializes the MultiDeviceSetup service. This class is responsible for
 // waiting for asynchronous initialization steps to complete before creating
@@ -44,6 +45,7 @@ class MultiDeviceSetupInitializer
         PrefService* pref_service,
         device_sync::DeviceSyncClient* device_sync_client,
         AuthTokenValidator* auth_token_validator,
+        OobeCompletionTracker* oobe_completion_tracker,
         std::unique_ptr<AndroidSmsAppHelperDelegate>
             android_sms_app_helper_delegate,
         std::unique_ptr<AndroidSmsPairingStateTracker>
@@ -81,6 +83,7 @@ class MultiDeviceSetupInitializer
       PrefService* pref_service,
       device_sync::DeviceSyncClient* device_sync_client,
       AuthTokenValidator* auth_token_validator,
+      OobeCompletionTracker* oobe_completion_tracker,
       std::unique_ptr<AndroidSmsAppHelperDelegate>
           android_sms_app_helper_delegate,
       std::unique_ptr<AndroidSmsPairingStateTracker>
@@ -123,6 +126,7 @@ class MultiDeviceSetupInitializer
   PrefService* pref_service_;
   device_sync::DeviceSyncClient* device_sync_client_;
   AuthTokenValidator* auth_token_validator_;
+  OobeCompletionTracker* oobe_completion_tracker_;
   std::unique_ptr<AndroidSmsAppHelperDelegate> android_sms_app_helper_delegate_;
   std::unique_ptr<AndroidSmsPairingStateTracker>
       android_sms_pairing_state_tracker_;

chromeos/services/multidevice_setup/multidevice_setup_service.cc

@@ -37,6 +37,7 @@ MultiDeviceSetupService::MultiDeviceSetupService(
     PrefService* pref_service,
     device_sync::DeviceSyncClient* device_sync_client,
     AuthTokenValidator* auth_token_validator,
+    OobeCompletionTracker* oobe_completion_tracker,
     std::unique_ptr<AndroidSmsAppHelperDelegate>
         android_sms_app_helper_delegate,
     std::unique_ptr<AndroidSmsPairingStateTracker>
@@ -47,6 +48,7 @@ MultiDeviceSetupService::MultiDeviceSetupService(
               pref_service,
               device_sync_client,
               auth_token_validator,
+              oobe_completion_tracker,
               std::move(android_sms_app_helper_delegate),
               std::move(android_sms_pairing_state_tracker),
               gcm_device_info_provider)),

chromeos/services/multidevice_setup/multidevice_setup_service.h

@@ -31,6 +31,7 @@ class AndroidSmsPairingStateTracker;
 class AuthTokenValidator;
 class MultiDeviceSetupBase;
 class PrivilegedHostDeviceSetterBase;
+class OobeCompletionTracker;
 
 // Service which provides an implementation for mojom::MultiDeviceSetup. This
 // service creates one implementation and shares it among all connection
@@ -41,6 +42,7 @@ class MultiDeviceSetupService : public service_manager::Service {
       PrefService* pref_service,
       device_sync::DeviceSyncClient* device_sync_client,
       AuthTokenValidator* auth_token_validator,
+      OobeCompletionTracker* oobe_completion_tracker,
       std::unique_ptr<AndroidSmsAppHelperDelegate>
           android_sms_app_helper_delegate,
       std::unique_ptr<AndroidSmsPairingStateTracker>

chromeos/services/multidevice_setup/multidevice_setup_service_unittest.cc

@@ -16,6 +16,7 @@
 #include "chromeos/services/multidevice_setup/public/cpp/fake_android_sms_pairing_state_tracker.h"
 #include "chromeos/services/multidevice_setup/public/cpp/fake_auth_token_validator.h"
 #include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup.h"
+#include "chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h"
 #include "chromeos/services/multidevice_setup/public/mojom/constants.mojom.h"
 #include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
 #include "components/cryptauth/fake_gcm_device_info_provider.h"
@@ -39,6 +40,7 @@ class FakeMultiDeviceSetupFactory : public MultiDeviceSetupImpl::Factory {
           expected_testing_pref_service,
       device_sync::FakeDeviceSyncClient* expected_device_sync_client,
       FakeAuthTokenValidator* expected_auth_token_validator,
+      OobeCompletionTracker* expected_oobe_completion_tracker,
       FakeAndroidSmsAppHelperDelegate* expected_android_sms_app_helper_delegate,
       FakeAndroidSmsPairingStateTracker*
           expected_android_sms_pairing_state_tracker,
@@ -47,6 +49,7 @@ class FakeMultiDeviceSetupFactory : public MultiDeviceSetupImpl::Factory {
       : expected_testing_pref_service_(expected_testing_pref_service),
         expected_device_sync_client_(expected_device_sync_client),
         expected_auth_token_validator_(expected_auth_token_validator),
+        expected_oobe_completion_tracker_(expected_oobe_completion_tracker),
         expected_android_sms_app_helper_delegate_(
             expected_android_sms_app_helper_delegate),
         expected_android_sms_pairing_state_tracker_(
@@ -62,6 +65,7 @@ class FakeMultiDeviceSetupFactory : public MultiDeviceSetupImpl::Factory {
       PrefService* pref_service,
       device_sync::DeviceSyncClient* device_sync_client,
       AuthTokenValidator* auth_token_validator,
+      OobeCompletionTracker* oobe_completion_tracker,
       std::unique_ptr<AndroidSmsAppHelperDelegate>
           android_sms_app_helper_delegate,
       std::unique_ptr<AndroidSmsPairingStateTracker>
@@ -72,6 +76,7 @@ class FakeMultiDeviceSetupFactory : public MultiDeviceSetupImpl::Factory {
     EXPECT_EQ(expected_testing_pref_service_, pref_service);
     EXPECT_EQ(expected_device_sync_client_, device_sync_client);
     EXPECT_EQ(expected_auth_token_validator_, auth_token_validator);
+    EXPECT_EQ(expected_oobe_completion_tracker_, oobe_completion_tracker);
     EXPECT_EQ(expected_android_sms_app_helper_delegate_,
               android_sms_app_helper_delegate.get());
     EXPECT_EQ(expected_android_sms_pairing_state_tracker_,
@@ -86,6 +91,7 @@ class FakeMultiDeviceSetupFactory : public MultiDeviceSetupImpl::Factory {
   sync_preferences::TestingPrefServiceSyncable* expected_testing_pref_service_;
   device_sync::FakeDeviceSyncClient* expected_device_sync_client_;
   FakeAuthTokenValidator* expected_auth_token_validator_;
+  OobeCompletionTracker* expected_oobe_completion_tracker_;
   FakeAndroidSmsAppHelperDelegate* expected_android_sms_app_helper_delegate_;
   FakeAndroidSmsPairingStateTracker*
       expected_android_sms_pairing_state_tracker_;
@@ -113,6 +119,7 @@ class MultiDeviceSetupServiceTest : public testing::Test {
     fake_device_sync_client_ =
         std::make_unique<device_sync::FakeDeviceSyncClient>();
     fake_auth_token_validator_ = std::make_unique<FakeAuthTokenValidator>();
+    fake_oobe_completion_tracker_ = std::make_unique<OobeCompletionTracker>();
     auto fake_android_sms_app_helper_delegate =
         std::make_unique<FakeAndroidSmsAppHelperDelegate>();
     fake_android_sms_app_helper_delegate_ =
@@ -129,6 +136,7 @@ class MultiDeviceSetupServiceTest : public testing::Test {
         std::make_unique<FakeMultiDeviceSetupFactory>(
             test_pref_service_.get(), fake_device_sync_client_.get(),
             fake_auth_token_validator_.get(),
+            fake_oobe_completion_tracker_.get(),
             fake_android_sms_app_helper_delegate_,
             fake_android_sms_pairing_state_tracker_,
             fake_gcm_device_info_provider_.get());
@@ -140,6 +148,7 @@ class MultiDeviceSetupServiceTest : public testing::Test {
             std::make_unique<MultiDeviceSetupService>(
                 test_pref_service_.get(), fake_device_sync_client_.get(),
                 fake_auth_token_validator_.get(),
+                fake_oobe_completion_tracker_.get(),
                 std::move(fake_android_sms_app_helper_delegate),
                 std::move(fake_android_sms_pairing_state_tracker),
                 fake_gcm_device_info_provider_.get()));
@@ -206,6 +215,7 @@ class MultiDeviceSetupServiceTest : public testing::Test {
       test_pref_service_;
   std::unique_ptr<device_sync::FakeDeviceSyncClient> fake_device_sync_client_;
   std::unique_ptr<FakeAuthTokenValidator> fake_auth_token_validator_;
+  std::unique_ptr<OobeCompletionTracker> fake_oobe_completion_tracker_;
   FakeAndroidSmsAppHelperDelegate* fake_android_sms_app_helper_delegate_;
   FakeAndroidSmsPairingStateTracker* fake_android_sms_pairing_state_tracker_;
   std::unique_ptr<cryptauth::FakeGcmDeviceInfoProvider>

chromeos/services/multidevice_setup/public/cpp/BUILD.gn

@@ -78,6 +78,18 @@ source_set("first_run_field_trial") {
   ]
 }
 
+source_set("oobe_completion_tracker") {
+  sources = [
+    "oobe_completion_tracker.cc",
+    "oobe_completion_tracker.h",
+  ]
+
+  deps = [
+    "//base",
+    "//components/keyed_service/core",
+  ]
+}
+
 source_set("url_provider") {
   sources = [
     "url_provider.cc",

chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client_impl_unittest.cc

@@ -46,6 +46,7 @@ class FakeMultiDeviceSetupInitializerFactory
       PrefService* pref_service,
       device_sync::DeviceSyncClient* device_sync_client,
       AuthTokenValidator* auth_token_validator,
+      OobeCompletionTracker* oobe_completion_tracker,
       std::unique_ptr<AndroidSmsAppHelperDelegate>
           android_sms_app_helper_delegate,
       std::unique_ptr<AndroidSmsPairingStateTracker>
@@ -126,6 +127,7 @@ class MultiDeviceSetupClientImplTest : public testing::Test {
     auto multidevice_setup_service = std::make_unique<MultiDeviceSetupService>(
         nullptr /* pref_service */, nullptr /* device_sync_client */,
         nullptr /* auth_token_validator */,
+        nullptr /* oobe_completion_tracker */,
         nullptr /* android_sms_app_helper_delegate */,
         nullptr /* android_sms_pairing_state_tracker */,
         nullptr /* gcm_device_info_provider */);

chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h"
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+OobeCompletionTracker::OobeCompletionTracker() = default;
+
+OobeCompletionTracker::~OobeCompletionTracker() = default;
+
+void OobeCompletionTracker::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void OobeCompletionTracker::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void OobeCompletionTracker::MarkOobeShown() {
+  for (auto& observer : observer_list_)
+    observer.OnOobeCompleted();
+}
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos

chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_SERVICES_MULTIDEVICE_SETUP_PUBLIC_CPP_OOBE_COMPLETION_TRACKER_H_
+#define CHROMEOS_SERVICES_MULTIDEVICE_SETUP_PUBLIC_CPP_OOBE_COMPLETION_TRACKER_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "base/observer_list.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+// Records if/when the user underwent the OOBE MultiDevice setup flow to prevent
+// spamming the user with multiple notifications to set up MultiDevice features.
+class OobeCompletionTracker : public KeyedService {
+ public:
+  class Observer {
+   public:
+    virtual void OnOobeCompleted() = 0;
+
+   protected:
+    virtual ~Observer() = default;
+  };
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  OobeCompletionTracker();
+  ~OobeCompletionTracker() override;
+
+  void MarkOobeShown();
+
+ private:
+  base::ObserverList<Observer>::Unchecked observer_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(OobeCompletionTracker);
+};
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_MULTIDEVICE_SETUP_PUBLIC_CPP_OOBE_COMPLETION_TRACKER_H_