Enhanced Sync Confirmation modal

chrome/browser/ui/sync/one_click_signin_sync_starter.cc

Index: chrome/browser/ui/sync/one_click_signin_sync_starter.cc
diff --git a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
index 6e500993f6b60a012e03222c1cc7a2993f201c6c..10876e1d118e723963d2739904630c915c7c9a34 100644
--- a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
+++ b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
@@ -398,12 +398,25 @@ void OneClickSigninSyncStarter::OnSyncConfirmationUIClosed(
           base::UserMetricsAction("Signin_Signin_WithAdvancedSyncSettings"));
       chrome::ShowSettingsSubPage(browser_, chrome::kSyncSetupSubPage);
       break;
+    case LoginUIService::SYNC_WITH_ACTIVITY_CONTROLS: {

[anthonyvd, 2016/03/17 20:23:41]

This code is probably changing a bit in
https://codereview.chromium.org/1806193003/. Since this only happens under the
new tab-modal signin flow, and that flow always shows the confirmation dialog,
I'm not sure you'll need this new enum value.

Also, keep in mind that wanting to see activity controls isn't mutually
exclusive with wanting to be taken to settings to configure sync (see my comment
above in sync_confirmation.js)

[Moe, 2016/03/18 21:53:40]

Acknowledged.

+      content::RecordAction(
+          base::UserMetricsAction("Signin_Signin_WithActivityControls"));
+      StartSync();
+      FinishProfileSyncServiceSetup();
+      // Open the 'Activity controls' section of the privacy settings page.
+      GURL url = GURL(chrome::kGoogleAccountActivityControlsURL);
+      content::OpenURLParams params(url,
+                                    content::Referrer(),
+                                    NEW_FOREGROUND_TAB,
+                                    ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
+                                    false);
+      browser_->OpenURL(params);
+      break;
+    }
     case LoginUIService::SYNC_WITH_DEFAULT_SETTINGS: {
       content::RecordAction(
           base::UserMetricsAction("Signin_Signin_WithDefaultSyncSettings"));
-      ProfileSyncService* profile_sync_service = GetProfileSyncService();
-      if (profile_sync_service)
-        profile_sync_service->SetFirstSetupComplete();
+      StartSync();
       FinishProfileSyncServiceSetup();
       break;
     }
@@ -460,13 +473,11 @@ void OneClickSigninSyncStarter::AccountAddedToCookie(
   switch (start_mode_) {
     case SYNC_WITH_DEFAULT_SETTINGS: {
       // Just kick off the sync machine, no need to configure it first.
-      ProfileSyncService* profile_sync_service = GetProfileSyncService();
-      if (profile_sync_service)
-        profile_sync_service->SetFirstSetupComplete();
+      StartSync();
       FinishProfileSyncServiceSetup();
       if (confirmation_required_ == CONFIRM_AFTER_SIGNIN) {
         base::string16 message;
-        if (!profile_sync_service) {
+        if (!GetProfileSyncService()) {
           // Sync is disabled by policy.
           message = l10n_util::GetStringUTF16(
               IDS_ONE_CLICK_SIGNIN_BUBBLE_SYNC_DISABLED_MESSAGE);
@@ -599,6 +610,12 @@ ProfileSyncService* OneClickSigninSyncStarter::GetProfileSyncService() {
   return service;
 }
 
+void OneClickSigninSyncStarter::StartSync() {

[anthonyvd, 2016/03/17 20:23:41]

I'd rename this to SetFirstSetupComplete or something similar since it's more
precise. At this point sync is already "starting" but it's waiting on some
confirmation.

[Moe, 2016/03/18 21:53:40]

Done.

+  ProfileSyncService* profile_sync_service = GetProfileSyncService();
+  if (profile_sync_service)
+    profile_sync_service->SetFirstSetupComplete();
+}
+
 void OneClickSigninSyncStarter::FinishProfileSyncServiceSetup() {
   ProfileSyncService* service =
       ProfileSyncServiceFactory::GetForProfile(profile_);