From 439dce0e361ea60656fa91eeb84d50b19a405027 Mon Sep 17 00:00:00 2001
From: "rsimha@chromium.org"
<rsimha@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 5 Jun 2013 09:23:27 +0000
Subject: [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
---
chrome/browser/ui/webui/sync_setup_handler.h | 4 ----
1 file changed, 4 deletions(-)
(limited to 'chrome/browser/ui/webui/sync_setup_handler.h')
diff --git a/chrome/browser/ui/webui/sync_setup_handler.h b/chrome/browser/ui/webui/sync_setup_handler.h
index 80ea45a..cbd3eef 100644
--- a/chrome/browser/ui/webui/sync_setup_handler.h
+++ b/chrome/browser/ui/webui/sync_setup_handler.h
@@ -97,10 +97,6 @@ class SyncSetupHandler : public options::OptionsPageUIHandler,
bool is_configuring_sync() const { return configuring_sync_; }
bool have_signin_tracker() const { return signin_tracker_; }
- // Subclasses must implement this to show the setup UI that's appropriate
- // for the page this is contained in.
- virtual void ShowSetupUI() = 0;
-
// Overridden by subclasses (like SyncPromoHandler) to log stats about the
// user's signin activity.
virtual void RecordSignin();
--
cgit v1.1