diff options
| author | rsimha@chromium.org <rsimha@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-05 09:23:27 +0000 |
|---|---|---|
| committer | rsimha@chromium.org <rsimha@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-05 09:23:27 +0000 |
| commit | 439dce0e361ea60656fa91eeb84d50b19a405027 (patch) | |
| tree | b1ae0bfe3ed3b609495cad030e3b720ea4adb4ef /chrome/browser/ui/webui/sync_setup_handler.cc | |
| parent | 5cb80b934436dfb0bf9b810a7a9f45c9c9155f75 (diff) | |
| download | chromium_src-439dce0e361ea60656fa91eeb84d50b19a405027.zip chromium_src-439dce0e361ea60656fa91eeb84d50b19a405027.tar.gz chromium_src-439dce0e361ea60656fa91eeb84d50b19a405027.tar.bz2 | |
[sync] Eliminate reload loop in sync settings page
During initial sync setup, if the user refreshes the settings page when
the advanced settings page is visible, and then hits the back button, we
enter a page reload loop.
A couple of salient features of the loop are:
1) When the back button is pressed, SyncSetupHandler::CloseSyncSetup()
is called, which causes ProfileSyncService::NotifyObservers() to
again indirectly call SyncSetupHandler::CloseSyncSetup(), due to
SigninTracker::OnStateChanged() incorrectly detecting a signin
failure.
2) When the sync setup page is loaded, SyncSetupHandler::OpenSyncSetup()
calls OptionsSyncSetupHandler::ShowSetupUI(), resulting in an
unnecessary navigation to chrome://settings/syncSetup, given that we
are already at the sync setup page.
This patch does a few things to eliminate reload loops:
1) Removes SyncSetupHandler::ShowSetupUI(), and eliminates the derived
class OptionsSyncSetupHandler. With web based sign in, there is no
longer the need for SyncSetupHandler to re-navigate to
chrome://settings/syncSetup.
2) In SyncSetupHandler::CloseSyncSetup(), we clear SigninTracker
before potentially aborting sync setup. This eliminates the case
where SigninTracker detects an abandoned sync setup as a signin
failure.
3) In ProfileSyncService::SetSetupInProgress(), we only call
NotifyObservers() if the value of |setup_in_progress_| changes.
This eliminates additional recursive calls to
SyncSetupHandler::CloseSyncSetup() in cases where it is called in
response to ProfileSyncService::NotifyObservers() originating from
ProfileSyncService::SetSetupInProgress().
BUG=245219
TEST=Repro steps in the above bug shouldn't result in a reload loop
Review URL: https://chromiumcodereview.appspot.com/16085012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204217 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/ui/webui/sync_setup_handler.cc')
| -rw-r--r-- | chrome/browser/ui/webui/sync_setup_handler.cc | 19 |
1 files changed, 10 insertions, 9 deletions
diff --git a/chrome/browser/ui/webui/sync_setup_handler.cc b/chrome/browser/ui/webui/sync_setup_handler.cc index 6c67046..228cf8d 100644 --- a/chrome/browser/ui/webui/sync_setup_handler.cc +++ b/chrome/browser/ui/webui/sync_setup_handler.cc @@ -1150,6 +1150,14 @@ void SyncSetupHandler::HandleCloseTimeout(const ListValue* args) { } void SyncSetupHandler::CloseSyncSetup() { + // Stop a timer to handle timeout in waiting for checking network connection. + backend_start_timer_.reset(); + + // Clear the signin tracker before canceling sync setup, as it may incorrectly + // flag a signin failure. + bool was_signing_in = (signin_tracker_.get() != NULL); + signin_tracker_.reset(); + // TODO(atwilson): Move UMA tracking of signin events out of sync module. ProfileSyncService* sync_service = GetSyncService(); if (IsActiveLogin()) { @@ -1157,7 +1165,8 @@ void SyncSetupHandler::CloseSyncSetup() { // automatically closed due to an auth error. if (!sync_service || (!sync_service->HasSyncSetupCompleted() && sync_service->GetAuthError().state() == GoogleServiceAuthError::NONE)) { - if (signin_tracker_.get()) { + if (was_signing_in) { + // TODO(rsimha): Remove this. Sync should not be logging sign in events. ProfileSyncService::SyncEvent( ProfileSyncService::CANCEL_DURING_SIGNON); } else if (configuring_sync_) { @@ -1203,10 +1212,6 @@ void SyncSetupHandler::CloseSyncSetup() { #endif configuring_sync_ = false; - signin_tracker_.reset(); - - // Stop a timer to handle timeout in waiting for checking network connection. - backend_start_timer_.reset(); } void SyncSetupHandler::OpenSyncSetup() { @@ -1233,8 +1238,6 @@ void SyncSetupHandler::OpenSyncSetup() { // User is not logged in, or login has been specially requested - need to // display login UI (cases 1-3). DisplayGaiaLogin(false); - if (!SyncPromoUI::UseWebBasedSigninFlow()) - ShowSetupUI(); return; } #endif @@ -1249,7 +1252,6 @@ void SyncSetupHandler::OpenSyncSetup() { // via the "Advanced..." button or through One-Click signin (cases 4-6), or // they are re-enabling sync after having disabled it (case 7). DisplayConfigureSync(true, false); - ShowSetupUI(); } void SyncSetupHandler::OpenConfigureSync() { @@ -1257,7 +1259,6 @@ void SyncSetupHandler::OpenConfigureSync() { return; DisplayConfigureSync(true, false); - ShowSetupUI(); } void SyncSetupHandler::FocusUI() { |