Settings People Revamp: Fix some small bugs in the handler

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 144 matching lines @@
 // static
 const char PeopleHandler::kSpinnerPageStatus[] = "spinner";
 const char PeopleHandler::kConfigurePageStatus[] = "configure";
 const char PeopleHandler::kTimeoutPageStatus[] = "timeout";
 const char PeopleHandler::kDonePageStatus[] = "done";
 const char PeopleHandler::kPassphraseFailedPageStatus[] = "passphraseFailed";
 
 PeopleHandler::PeopleHandler(Profile* profile)
     : profile_(profile),
       configuring_sync_(false),
+      signin_observer_(this),
       sync_service_observer_(this) {}
 
 PeopleHandler::~PeopleHandler() {
   // Early exit if running unit tests (no actual WebUI is attached).
   if (!web_ui())
     return;
 
   // This case is hit when the user performs a back navigation.
   CloseSyncSetup();
 }
@@ skipping 43 matching lines @@
 #endif
 }
 
 void PeopleHandler::OnJavascriptAllowed() {
   PrefService* prefs = profile_->GetPrefs();
   profile_pref_registrar_.Init(prefs);
   profile_pref_registrar_.Add(
       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)
     sync_service_observer_.Add(sync_service);
 }
 
 void PeopleHandler::OnJavascriptDisallowed() {
   profile_pref_registrar_.RemoveAll();
+  signin_observer_.RemoveAll();
   sync_service_observer_.RemoveAll();
 }
 
 #if !defined(OS_CHROMEOS)
 void PeopleHandler::DisplayGaiaLogin(signin_metrics::AccessPoint access_point) {
   DCHECK(!sync_startup_tracker_);
   // Advanced options are no longer being configured if the login screen is
   // visible. If the user exits the signin wizard after this without
   // configuring sync, CloseSyncSetup() will ensure they are logged out.
   configuring_sync_ = false;
@@ skipping 324 matching lines @@
     webui::DeleteProfileAtPath(profile_->GetPath(), web_ui());
   }
 }
 #endif
 
 void PeopleHandler::HandleCloseTimeout(const base::ListValue* args) {
   CloseSyncSetup();
 }
 
 void PeopleHandler::HandleGetSyncStatus(const base::ListValue* args) {
+  AllowJavascript();
+
   CHECK_EQ(1U, args->GetSize());
   const base::Value* callback_id;
   CHECK(args->Get(0, &callback_id));
 
   ResolveJavascriptCallback(*callback_id, *GetSyncStatusDictionary());
 }
 
 void PeopleHandler::HandleManageOtherPeople(const base::ListValue* /* args */) {
   UserManager::Show(base::FilePath(), profiles::USER_MANAGER_NO_TUTORIAL,
                     profiles::USER_MANAGER_SELECT_PROFILE_NO_ACTION);
@@ skipping 62 matching lines @@
   // 3) Previously working credentials have expired.
   // 4) User is signed in, but has stopped sync via the google dashboard, and
   //    signout is prohibited by policy so we need to force a re-auth.
   // 5) User clicks [Advanced Settings] button on options page while already
   //    logged in.
   // 6) One-click signin (credentials are already available, so should display
   //    sync configure UI, not login UI).
   // 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
     // because previously working credentials have expired (case 3). Close sync
     // setup including any visible overlays, and display the gaia auth page.
     // Control will be returned to the sync settings page once auth is complete.
     CloseUI();
     DisplayGaiaLogin(
         creating_supervised_user ?
             signin_metrics::AccessPoint::ACCESS_POINT_SUPERVISED_USER :
             signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS);
     return;
   }
 #endif
   if (!GetSyncService()) {
     // This can happen if the user directly navigates to /settings/syncSetup.
     DLOG(WARNING) << "Cannot display sync UI when sync is disabled";
     CloseUI();
     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();
   FocusUI();
 }
 
-void PeopleHandler::OpenConfigureSync() {
-  if (!PrepareSyncSetup())
-    return;
-
-  PushSyncPrefs();
-  FocusUI();
-}
-
 void PeopleHandler::FocusUI() {
   DCHECK(IsActiveLogin());
   WebContents* web_contents = web_ui()->GetWebContents();
   web_contents->GetDelegate()->ActivateContents(web_contents);
 }
 
 void PeopleHandler::CloseUI() {
   CloseSyncSetup();
   CallJavascriptFunction("cr.webUIListenerCallback",
                          base::StringValue("page-status-changed"),
@@ skipping 82 matching lines @@
 bool PeopleHandler::FocusExistingWizardIfPresent() {
   if (!IsExistingWizardPresent())
     return false;
 
   LoginUIService* service = GetLoginUIService();
   DCHECK(service);
   service->current_login_ui()->FocusUI();
   return true;
 }
 
 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()) {
     service->RequestStart();
 
     // See if it's even possible to bring up the sync backend - 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_) !=
@@ skipping 124 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.
   service->SetSetupInProgress(false);
   service->SetFirstSetupComplete();
 }
 
 }  // namespace settings