suppress user/password dialog when re-enabling sync

chrome/browser/signin/signin_tracker.cc

Index: chrome/browser/signin/signin_tracker.cc
diff --git a/chrome/browser/signin/signin_tracker.cc b/chrome/browser/signin/signin_tracker.cc
index ec2e02334dc1a553618565f005b94a77f6d7beaa..7174ecd67e0a06502468b25ed2bcce7f0d729aa4 100644
--- a/chrome/browser/signin/signin_tracker.cc
+++ b/chrome/browser/signin/signin_tracker.cc
@@ -23,15 +23,24 @@ static const int kNumSignedInServices =
 
 // Helper to check if the given token service is relevant for sync.
 SigninTracker::SigninTracker(Profile* profile, Observer* observer)
-    : state_(WAITING_FOR_GAIA_VALIDATION),
+    :
+#if !defined(OS_CHROMEOS)
+      state_(WAITING_FOR_GAIA_VALIDATION),

[Andrew T Wilson (Slow), 2012/04/03 17:45:13]

I think it would be better to have an alternate constructor that lets you set
the initial state. That way, you can run tests for this case on any platform.

[kochi, 2012/04/03 21:55:29]

Done.

+#else
+      state_(SERVICES_INITIALIZING),
+#endif
       profile_(profile),
       observer_(observer),
       credentials_valid_(false) {
   DCHECK(observer_);
   // Register for notifications from the SigninManager.
+  // On ChromeOS, we don't register sign in notification because we don't
+  // use it in SyncSetupHandler.
+#if !defined(OS_CHROMEOS)

[Andrew T Wilson (Slow), 2012/04/03 17:45:13]

I'd say you should probably just register for all these notifications even if
you don't need them on cros. For example, you don't care about the
TOKEN_AVAILABLE/REQUEST_FAILED notifications either.

[kochi, 2012/04/03 21:55:29]

Done.

   registrar_.Add(this,
                  chrome::NOTIFICATION_GOOGLE_SIGNIN_SUCCESSFUL,
                  content::Source<Profile>(profile_));
+#endif
   registrar_.Add(this,
                  chrome::NOTIFICATION_GOOGLE_SIGNIN_FAILED,
                  content::Source<Profile>(profile_));
@@ -48,6 +57,11 @@ SigninTracker::SigninTracker(Profile* profile, Observer* observer)
   ProfileSyncService* service =
       ProfileSyncServiceFactory::GetForProfile(profile_);
   service->AddObserver(this);
+
+#if defined(OS_CHROMEOS)
+  DCHECK_EQ(state_, SERVICES_INITIALIZING);

[Andrew T Wilson (Slow), 2012/04/03 17:45:13]

When you change the constructor to allow setting the intitial state, I'd change
this to "if (state_ == SERVICES_INITIALIZING) HandleServiceStateChange();" - no
need to make it cros only, 

[kochi, 2012/04/03 21:55:29]

Done.

+  HandleServiceStateChange();
+#endif
 }
 
 SigninTracker::~SigninTracker() {
@@ -62,6 +76,7 @@ void SigninTracker::Observe(int type,
   // We should not get more than one of these notifications.
   switch (type) {
     case chrome::NOTIFICATION_GOOGLE_SIGNIN_SUCCESSFUL:
+      LOG(WARNING) << "Got NOTIFICATION_GOOGLE_SIGNIN_SUCCESSFUL";
       DCHECK_EQ(state_, WAITING_FOR_GAIA_VALIDATION);
       state_ = SERVICES_INITIALIZING;
       observer_->GaiaCredentialsValid();
@@ -71,6 +86,7 @@ void SigninTracker::Observe(int type,
         HandleServiceStateChange();
       break;
     case chrome::NOTIFICATION_GOOGLE_SIGNIN_FAILED: {
+      LOG(WARNING) << "Got NOTIFICATION_GOOGLE_SIGNIN_FAILED";
       DCHECK_EQ(state_, WAITING_FOR_GAIA_VALIDATION);
       const GoogleServiceAuthError& error =
           *(content::Details<const GoogleServiceAuthError>(details).ptr());
@@ -78,10 +94,12 @@ void SigninTracker::Observe(int type,
       break;
     }
     case chrome::NOTIFICATION_TOKEN_AVAILABLE:
+      LOG(WARNING) << "Got NOTIFICATION_TOKEN_AVAILABLE";
       // A new token is available - check to see if we're all signed in now.
       HandleServiceStateChange();
       break;
     case chrome::NOTIFICATION_TOKEN_REQUEST_FAILED:
+      LOG(WARNING) << "Got NOTIFICATION_TOKEN_REQUEST_FAILED";
       if (state_ == SERVICES_INITIALIZING) {
         const TokenService::TokenRequestFailedDetails& token_details =
             *(content::Details<const TokenService::TokenRequestFailedDetails>(
@@ -90,6 +108,7 @@ void SigninTracker::Observe(int type,
           if (token_details.service() == kSignedInServices[i]) {
             // We got an error loading one of our tokens, so notify our
             // observer.
+            LOG(WARNING) << "resetting state_ to WAITING_FOR_GAIA_VALIDATION";
             state_ = WAITING_FOR_GAIA_VALIDATION;
             observer_->SigninFailed(token_details.error());
           }
@@ -108,6 +127,7 @@ void SigninTracker::OnStateChanged() {
 
 void SigninTracker::HandleServiceStateChange() {
   if (state_ != SERVICES_INITIALIZING) {
+    LOG(WARNING) << "HandleServiceStateChange: state!=SERVICES_INITIALIZING" << (int)state_;
     // Ignore service updates until after our GAIA credentials are validated.
     return;
   }
@@ -120,6 +140,7 @@ void SigninTracker::HandleServiceStateChange() {
   ProfileSyncService* service =
       ProfileSyncServiceFactory::GetForProfile(profile_);
   if (service->waiting_for_auth()) {
+    LOG(WARNING) << "HandleServiceStateChange: service is waiting for auth";

[Andrew T Wilson (Slow), 2012/04/03 17:45:13]

BTW, it might be useful to keep some of these messages around as DVLOG(), so
feel free to do that if you find them useful (I've had to add similar messages
in the past, so it might be something we want to keep).

[kochi, 2012/04/03 21:55:29]

I'm planning to clean all these LOG(WARNING) messages,
but may keep some useful ones as DVLOG.
I'll revisit this.

     // Still waiting for an auth token to come in so stay in the INITIALIZING
     // state (we do this to avoid triggering an early signin error in the case
     // where there's a previous auth error in the sync service that hasn't
@@ -129,12 +150,16 @@ void SigninTracker::HandleServiceStateChange() {
   // If we haven't loaded all our service tokens yet, just exit (we'll be called
   // again when another token is loaded, or will transition to SigninFailed if
   // the loading fails).
-  if (!AreServiceTokensLoaded(profile_))
+  if (!AreServiceTokensLoaded(profile_)) {
+    LOG(WARNING) << "HandleServiceStateChange: ServiceToken not loaded";
     return;
+  }
   if (!AreServicesSignedIn(profile_)) {
+    LOG(WARNING) << "HandleServiceStateChange: not signed in to services";
     state_ = WAITING_FOR_GAIA_VALIDATION;
     observer_->SigninFailed(service->GetAuthError());
   } else if (service->sync_initialized()) {
+    LOG(WARNING) << "HandleServiceStateChange: sign in complete!";
     state_ = SIGNIN_COMPLETE;
     observer_->SigninSuccess();
   }
@@ -146,6 +171,7 @@ bool SigninTracker::AreServiceTokensLoaded(Profile* profile) {
   TokenService* token_service = TokenServiceFactory::GetForProfile(profile);
   for (int i = 0; i < kNumSignedInServices; ++i) {
     if (!token_service->HasTokenForService(kSignedInServices[i])) {
+      LOG(WARNING) << "Token not available for service " << kSignedInServices[i];
       // Don't have a token for one of our signed-in services.
       return false;
     }
@@ -159,8 +185,10 @@ bool SigninTracker::AreServicesSignedIn(Profile* profile) {
     return false;
   ProfileSyncService* service =
       ProfileSyncServiceFactory::GetForProfile(profile);
+  LOG(WARNING) << "AreServicesSignedIn: AreCredentialsAvailable() : " << (service->AreCredentialsAvailable() ? "YES" : "NO");
+  LOG(WARNING) << "AreServicesSignedIn: GetAuthError() : " << (int)service->GetAuthError().state();
+  LOG(WARNING) << "AreServicesSignedIn: unrecoverable error? : " << (service->unrecoverable_error_detected() ? "YES" : "NO");
   return (service->AreCredentialsAvailable() &&
           service->GetAuthError().state() == GoogleServiceAuthError::NONE &&
           !service->unrecoverable_error_detected());
 }
-