[sync] Display an error when sync settings aren't confirmed

chrome/browser/sync/sync_ui_util_unittest.cc

Index: chrome/browser/sync/sync_ui_util_unittest.cc
diff --git a/chrome/browser/sync/sync_ui_util_unittest.cc b/chrome/browser/sync/sync_ui_util_unittest.cc
index 0077699140d70340b29dc2caa1b867d26ec62314..8753f2a9378f4d0d75271f751ca5a3e9359a4ee7 100644
--- a/chrome/browser/sync/sync_ui_util_unittest.cc
+++ b/chrome/browser/sync/sync_ui_util_unittest.cc
@@ -48,6 +48,7 @@ enum DistinctState {
   STATUS_CASE_AUTH_ERROR,
   STATUS_CASE_PROTOCOL_ERROR,
   STATUS_CASE_PASSPHRASE_ERROR,
+  STATUS_CASE_CONFIRM_SYNC_SETTINGS,
   STATUS_CASE_SYNCED,
   STATUS_CASE_SYNC_DISABLED_BY_POLICY,
   NUMBER_OF_STATUS_CASES
@@ -173,6 +174,17 @@ void GetDistinctCase(ProfileSyncServiceMock* service,
           .WillRepeatedly(Return(false));
       return;
     }
+    case STATUS_CASE_CONFIRM_SYNC_SETTINGS: {
+      EXPECT_CALL(*service, IsFirstSetupComplete())
+          .WillRepeatedly(Return(false));
+      EXPECT_CALL(*service, IsSyncRequested()).WillRepeatedly(Return(true));
+      EXPECT_CALL(*service, IsPassphraseRequired())
+          .WillRepeatedly(Return(false));
+      syncer::SyncEngine::Status status;
+      EXPECT_CALL(*service, QueryDetailedSyncStatus(_))
+          .WillRepeatedly(DoAll(SetArgPointee<0>(status), Return(false)));
+      return;
+    }
     case STATUS_CASE_PASSPHRASE_ERROR: {
       EXPECT_CALL(*service, IsFirstSetupComplete())
           .WillRepeatedly(Return(true));
@@ -238,6 +250,8 @@ sync_ui_util::ActionType GetActionTypeforDistinctCase(int case_number) {
       return sync_ui_util::UPGRADE_CLIENT;
     case STATUS_CASE_PASSPHRASE_ERROR:
       return sync_ui_util::ENTER_PASSPHRASE;
+    case STATUS_CASE_CONFIRM_SYNC_SETTINGS:
+      return sync_ui_util::CONFIRM_SYNC_SETTINGS;
     case STATUS_CASE_SYNCED:
       return sync_ui_util::NO_ACTION;
     case STATUS_CASE_SYNC_DISABLED_BY_POLICY:
@@ -413,3 +427,24 @@ TEST_F(SyncUIUtilTest, ActionableErrorWithPassiveMessage) {
   EXPECT_NE(first_actionable_error_status_label,
             second_actionable_error_status_label);
 }
+
+TEST_F(SyncUIUtilTest, SyncSettingsConfirmationNeededTest) {

[skym, 2017/06/01 19:48:44]

Can you make a unit test that has nullptr for the labels too?

[Patrick Noland, 2017/06/07 19:07:23]

Alas, GetStatusLabels DCHECKS that link_label and status_label are non-null,
even though GetStatusInfo doesn't require that. GetStatusInfo is internal to
sync_ui_utils so there's not a good way to pass nullptr without doing a lot of
refactoring. And if I was refactoring this, I would change it so much that this
change wouldn't apply any longer. 

+  std::unique_ptr<Profile> profile(MakeSignedInTestingProfile());
+  SigninManagerBase* signin =
+      SigninManagerFactory::GetForProfile(profile.get());
+
+  ProfileSyncServiceMock service(
+      CreateProfileSyncServiceParamsForTest(profile.get()));
+  EXPECT_CALL(service, IsFirstSetupComplete()).WillRepeatedly(Return(false));
+  EXPECT_CALL(service, IsSyncRequested()).WillRepeatedly(Return(true));
+
+  base::string16 actionable_error_status_label;
+  base::string16 link_label;
+  sync_ui_util::ActionType action_type = sync_ui_util::NO_ACTION;
+
+  sync_ui_util::GetStatusLabels(
+      profile.get(), &service, *signin, sync_ui_util::PLAIN_TEXT,
+      &actionable_error_status_label, &link_label, &action_type);
+
+  EXPECT_EQ(action_type, sync_ui_util::CONFIRM_SYNC_SETTINGS);
+}