Ensure PeopleHandler only calls PSS::RequestStart in response to intention to configure sync

chrome/browser/ui/webui/settings/people_handler.cc

 // Copyright 2015 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/webui/settings/people_handler.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 644 matching lines @@
     return;
   }
 #endif
   if (!service) {
     // This can happen if the user directly navigates to /settings/syncSetup.
     DLOG(WARNING) << "Cannot display sync UI when sync is disabled";
     CloseUI();
     return;
   }
 
+  if (!service->IsEngineInitialized()) {
+    // Requesting the sync service to start may trigger call to PushSyncPrefs.
+    // Setting up the startup tracker beforehand correctly signals the
+    // re-entrant call to early exit.
+    sync_startup_tracker_.reset(new SyncStartupTracker(profile_, this));
+    service->RequestStart();
+
+    // See if it's even possible to bring up the sync engine - if not
+    // (unrecoverable error?), don't bother displaying a spinner that will be
+    // immediately closed because this leads to some ugly infinite UI loop (see
+    // http://crbug.com/244769).
+    if (SyncStartupTracker::GetSyncServiceState(profile_) !=
+        SyncStartupTracker::SYNC_STARTUP_ERROR) {
+      DisplaySpinner();
+    }
+    return;
+  }
+
   // User is already logged in. They must have brought up the config wizard
   // via the "Advanced..." button or through One-Click signin (cases 4-6), or
   // they are re-enabling sync after having disabled it (case 7).
   PushSyncPrefs();
 }
 
 void PeopleHandler::FocusUI() {
   WebContents* web_contents = web_ui()->GetWebContents();
   web_contents->GetDelegate()->ActivateContents(web_contents);
 }
@@ skipping 90 matching lines @@
 #if !defined(OS_CHROMEOS)
   // Early exit if the user has not signed in yet.
   if (!SigninManagerFactory::GetForProfile(profile_)->IsAuthenticated() ||
       SigninErrorControllerFactory::GetForProfile(profile_)->HasError()) {
     return;
   }
 #endif
 
   // Early exit if there is already a preferences push pending sync startup.
   if (sync_startup_tracker_)
     return;

[tommycli, 2017/02/27 21:46:04]

I think you will have to move this early-exit to the above point also.

[pavely, 2017/02/28 01:02:54]

Yes, you are right.
Done.

 
   ProfileSyncService* service = GetSyncService();
   // The sync service may be nullptr if it has been just disabled by policy.
-  if (!service)
-    return;
-
-  if (!service->IsEngineInitialized()) {
-    // Requesting the sync service to start may trigger another reentrant call
-    // to PushSyncPrefs. Setting up the startup tracker beforehand correctly
-    // signals the re-entrant call to early exit.
-    sync_startup_tracker_.reset(new SyncStartupTracker(profile_, this));
-    service->RequestStart();
-
-    // See if it's even possible to bring up the sync engine - if not
-    // (unrecoverable error?), don't bother displaying a spinner that will be
-    // immediately closed because this leads to some ugly infinite UI loop (see
-    // http://crbug.com/244769).
-    if (SyncStartupTracker::GetSyncServiceState(profile_) !=
-        SyncStartupTracker::SYNC_STARTUP_ERROR) {
-      DisplaySpinner();
-    }
+  if (!service || !service->IsEngineInitialized()) {
     return;
   }
 
   configuring_sync_ = true;
   DCHECK(service->IsEngineInitialized())
       << "Cannot configure sync until the sync engine is initialized";
 
   // Setup args for the sync configure screen:
   //   syncAllDataTypes: true if the user wants to sync everything
   //   <data_type>Registered: true if the associated data type is supported
@@ skipping 107 matching lines @@
   base::FilePath profile_file_path = profile_->GetPath();
   ProfileMetrics::LogProfileSyncSignIn(profile_file_path);
 
   // We're done configuring, so notify ProfileSyncService that it is OK to
   // start syncing.
   sync_blocker_.reset();
   service->SetFirstSetupComplete();
 }
 
 }  // namespace settings