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

chrome/browser/sync/sync_ui_util.cc

Index: chrome/browser/sync/sync_ui_util.cc
diff --git a/chrome/browser/sync/sync_ui_util.cc b/chrome/browser/sync/sync_ui_util.cc
index ce4e1fe76785f402bebb365f9bcf63cf77f1fd55..0a1c17a1db6731d5724d0e4bb13857ebffb129d9 100644
--- a/chrome/browser/sync/sync_ui_util.cc
+++ b/chrome/browser/sync/sync_ui_util.cc
@@ -61,6 +61,12 @@ bool IsChromeDashboardEnabled() {
   return group_name == "Enabled";
 }
 
+bool IsSyncConfirmationNeeded(ProfileSyncService* service) {
+  // The user is signed in and sync is requested, but setup is incomplete.
+  // Sync confirmation needs to be completed.
+  return service->IsSyncRequested() && !service->IsFirstSetupComplete();
+}
+
 // Returns the message that should be displayed when the user is authenticated
 // and can connect to the sync server. If the user hasn't yet authenticated, an
 // empty string is returned.
@@ -323,11 +329,23 @@ MessageType GetStatusInfo(Profile* profile,
                                        link_label, action_type);
       }
     } else if (signin.IsAuthenticated()) {
-      // The user is signed in, but sync has been stopped.
-      result_type = PRE_SYNCED;
-      if (status_label) {
-        status_label->assign(
-            l10n_util::GetStringUTF16(IDS_SIGNED_IN_WITH_SYNC_SUPPRESSED));
+      if (IsSyncConfirmationNeeded(service)) {
+        if (status_label && link_label) {
+          status_label->assign(
+              l10n_util::GetStringUTF16(IDS_SYNC_SETTINGS_NOT_CONFIRMED));
+          link_label->assign(l10n_util::GetStringUTF16(
+              IDS_SYNC_ERROR_USER_MENU_CONFIRM_SYNC_SETTINGS_BUTTON));
+        }
+        *action_type = CONFIRM_SYNC_SETTINGS;
+        if (!status_label->empty())

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

First, what a function. Combines figuring out your state, with setting UI. And
everyone had to deal with status_label and link_label to being null, maybe.

This check of status_label->empty() isn't guarded by if(status_label), right? So
this will deref nullptr, right?

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

Done.

+          return SYNC_ERROR;
+      } else {
+        // The user is signed in, but sync has been stopped.
+        result_type = PRE_SYNCED;
+        if (status_label) {
+          status_label->assign(
+              l10n_util::GetStringUTF16(IDS_SIGNED_IN_WITH_SYNC_SUPPRESSED));
+        }
       }
     }
   }
@@ -401,9 +419,11 @@ MessageType GetStatusLabelsForNewTabPage(Profile* profile,
 }
 
 #if !defined(OS_CHROMEOS)
-AvatarSyncErrorType GetMessagesForAvatarSyncError(Profile* profile,
-                                                  int* content_string_id,
-                                                  int* button_string_id) {
+AvatarSyncErrorType GetMessagesForAvatarSyncError(
+    Profile* profile,
+    const SigninManagerBase& signin,
+    int* content_string_id,
+    int* button_string_id) {
   ProfileSyncService* service =
       ProfileSyncServiceFactory::GetForProfile(profile);
 
@@ -469,6 +489,13 @@ AvatarSyncErrorType GetMessagesForAvatarSyncError(Profile* profile,
       *button_string_id = IDS_SYNC_ERROR_USER_MENU_PASSPHRASE_BUTTON;
       return PASSPHRASE_ERROR;
     }
+
+    // Check for a sync confirmation error.
+    if (signin.IsAuthenticated() && IsSyncConfirmationNeeded(service)) {
+      *content_string_id = IDS_SYNC_SETTINGS_NOT_CONFIRMED;
+      *button_string_id = IDS_SYNC_ERROR_USER_MENU_CONFIRM_SYNC_SETTINGS_BUTTON;
+      return SETTINGS_UNCONFIRMED_ERROR;
+    }
   }
 
   // There is no error.