Settings People Revamp: Fix some small bugs in the handler

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

Index: chrome/browser/ui/webui/settings/people_handler.cc
diff --git a/chrome/browser/ui/webui/settings/people_handler.cc b/chrome/browser/ui/webui/settings/people_handler.cc
index e24047bdbd55122077eb9a5f4f46666cb1c45e44..fb617f53090d4c141281eaa4659cfed9fdc912b8 100644
--- a/chrome/browser/ui/webui/settings/people_handler.cc
+++ b/chrome/browser/ui/webui/settings/people_handler.cc
@@ -162,6 +162,7 @@ const char PeopleHandler::kPassphraseFailedPageStatus[] = "passphraseFailed";
 PeopleHandler::PeopleHandler(Profile* profile)
     : profile_(profile),
       configuring_sync_(false),
+      signin_observer_(this),
       sync_service_observer_(this) {}
 
 PeopleHandler::~PeopleHandler() {
@@ -225,6 +226,11 @@ void PeopleHandler::OnJavascriptAllowed() {
       prefs::kSigninAllowed,
       base::Bind(&PeopleHandler::UpdateSyncStatus, base::Unretained(this)));
 
+  SigninManagerBase* signin_manager(
+      SigninManagerFactory::GetInstance()->GetForProfile(profile_));
+  if (signin_manager)
+    signin_observer_.Add(signin_manager);
+
   ProfileSyncService* sync_service(
       ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile_));
   if (sync_service)
@@ -233,6 +239,7 @@ void PeopleHandler::OnJavascriptAllowed() {
 
 void PeopleHandler::OnJavascriptDisallowed() {
   profile_pref_registrar_.RemoveAll();
+  signin_observer_.RemoveAll();
   sync_service_observer_.RemoveAll();
 }
 
@@ -577,6 +584,8 @@ void PeopleHandler::HandleCloseTimeout(const base::ListValue* args) {
 }
 
 void PeopleHandler::HandleGetSyncStatus(const base::ListValue* args) {
+  AllowJavascript();
+
   CHECK_EQ(1U, args->GetSize());
   const base::Value* callback_id;
   CHECK(args->Get(0, &callback_id));
@@ -659,7 +668,6 @@ void PeopleHandler::OpenSyncSetup(bool creating_supervised_user) {
   // 7) User re-enables sync after disabling it via advanced settings.
 #if !defined(OS_CHROMEOS)
   SigninManagerBase* signin = SigninManagerFactory::GetForProfile(profile_);
-
   if (!signin->IsAuthenticated() ||
       SigninErrorControllerFactory::GetForProfile(profile_)->HasError()) {
     // User is not logged in (cases 1-2), or login has been specially requested
@@ -688,14 +696,6 @@ void PeopleHandler::OpenSyncSetup(bool creating_supervised_user) {
   FocusUI();
 }
 
-void PeopleHandler::OpenConfigureSync() {
-  if (!PrepareSyncSetup())
-    return;
-
-  PushSyncPrefs();
-  FocusUI();
-}
-
 void PeopleHandler::FocusUI() {
   DCHECK(IsActiveLogin());
   WebContents* web_contents = web_ui()->GetWebContents();
@@ -799,8 +799,18 @@ bool PeopleHandler::FocusExistingWizardIfPresent() {
 }
 
 void PeopleHandler::PushSyncPrefs() {

[tommycli, 2016/05/11 19:19:08]

This method was previously only called when the dialog was initially opened.

Since now this method is called in response to Sync preferences changes, some of
the DCHECKs turn into early exits.

-  // Should never call this when we are not signed in.
-  DCHECK(SigninManagerFactory::GetForProfile(profile_)->IsAuthenticated());
+#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;
+
   ProfileSyncService* service = GetSyncService();
   DCHECK(service);
   if (!service->IsBackendInitialized()) {