Enhanced Sync Confirmation modal

chrome/browser/ui/sync/one_click_signin_sync_starter.cc

 // Copyright (c) 2012 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/ui/sync/one_click_signin_sync_starter.h"
 
 #include <stddef.h>
 
 #include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 380 matching lines @@
 }
 
 void OneClickSigninSyncStarter::OnSyncConfirmationUIClosed(
     LoginUIService::SyncConfirmationUIClosedResults results) {
   switch (results) {
     case LoginUIService::CONFIGURE_SYNC_FIRST:
       content::RecordAction(
           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;
     }
     case LoginUIService::ABORT_SIGNIN:
       FinishProfileSyncServiceSetup();
       break;
   }
 
   delete this;
 }
@@ skipping 36 matching lines @@
     const GoogleServiceAuthError& error) {
   // Regardless of whether the account was successfully added or not,
   // continue with sync starting.
 
   if (!sync_setup_completed_callback_.is_null())
     sync_setup_completed_callback_.Run(SYNC_SETUP_SUCCESS);
 
   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);
         }
         DisplayFinalConfirmationBubble(message);
       }
       break;
     }
     case CONFIRM_SYNC_SETTINGS_FIRST:
       if (switches::UsePasswordSeparatedSigninFlow()) {
@@ skipping 112 matching lines @@
   }
 }
 
 ProfileSyncService* OneClickSigninSyncStarter::GetProfileSyncService() {
   ProfileSyncService* service = NULL;
   if (profile_->IsSyncAllowed())
     service = ProfileSyncServiceFactory::GetForProfile(profile_);
   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_);
   if (service)
     service->SetSetupInProgress(false);
 }
 
 void OneClickSigninSyncStarter::ShowSettingsPageInWebContents(
     content::WebContents* contents,
     const std::string& sub_page) {
@@ skipping 19 matching lines @@
                                             false /* user_gesture */);
 }
 
 void OneClickSigninSyncStarter::LoadContinueUrl() {
   web_contents()->GetController().LoadURL(
       continue_url_,
       content::Referrer(),
       ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
       std::string());
 }