Refactor sync passphrase setup flow and fix passphrase tests

chrome/browser/sync/profile_sync_service_harness.cc

Index: chrome/browser/sync/profile_sync_service_harness.cc
diff --git a/chrome/browser/sync/profile_sync_service_harness.cc b/chrome/browser/sync/profile_sync_service_harness.cc
index f8ab1669d8897d5788ffcc5887c88fcce18fe884..ca8b212ceab9d72ec5c3d54eaf7b844cb4822c41 100644
--- a/chrome/browser/sync/profile_sync_service_harness.cc
+++ b/chrome/browser/sync/profile_sync_service_harness.cc
@@ -134,11 +134,12 @@ bool ProfileSyncServiceHarness::SetupSync() {
     synced_datatypes.insert(syncable::ModelTypeFromInt(i));
   }
   bool result = SetupSync(synced_datatypes);
-  VLOG(0) << "Client " << id_ << ": Set up sync completed with result "
-          << result;
+  // TODO(lipalani): Change VLOG(0) to VLOG(1) -- See http://crbug.com/80706.
+  VLOG(0) << "Client " << id_ << ": SetupSync completed with result "
+          << result << ".";
   if (result == false) {
     std::string pss_status = GetServiceStatus();
-    VLOG(0) << pss_status;
+    LOG(ERROR) << "SetupSync() failed. service_ details:\n" << pss_status;
   }
   return result;
 }
@@ -183,6 +184,12 @@ bool ProfileSyncServiceHarness::SetupSync(
     return false;
   }
 
+  if (wait_state_ == PASSPHRASE_REQUIRED_FOR_DECRYPTION) {
+    // A passphrase is required for decryption. Sync cannot proceed until
+    // SetPassphrase is called.
+    return false;
+  }
+
   // Indicate to the browser that sync setup is complete.
   service()->SetSyncSetupCompleted();
 
@@ -215,21 +222,39 @@ bool ProfileSyncServiceHarness::RunStateChangeMachine() {
       if (IsSynced()) {
         // The first sync cycle is now complete. We can start running tests.
         SignalStateCompleteWithNextState(FULLY_SYNCED);
+        break;
+      }
+      if (service()->observed_passphrase_required() &&
+          service()->passphrase_required_reason() ==
+              sync_api::REASON_SET_PASSPHRASE_FAILED) {
+        // A passphrase is required for decryption and we don't have it. Do not
+        // wait any more.
+        SignalStateCompleteWithNextState(PASSPHRASE_REQUIRED_FOR_DECRYPTION);

[tim (not reviewing), 2011/04/29 23:02:13]

It's a bit unintuitive now that we say if REASON_SET_PASSPHRASE_REQUIRED (and
_not_ REASON_DECRYPTON) then PASSPHRASE_REQUIRED_FOR_DECRYPTION...

[Raghu Simha, 2011/04/30 00:43:06]

Changed to SET_PASSPHRASE_FAILED.

+        break;
       }
       break;
     }
     case WAITING_FOR_SYNC_TO_FINISH: {
       LogClientInfo("WAITING_FOR_SYNC_TO_FINISH");
-      if (!IsSynced()) {
-        // The client is not yet fully synced. Continue waiting.
-        if (!GetStatus().server_reachable) {
-          // The client cannot reach the sync server because the network is
-          // disabled. There is no need to wait anymore.
-          SignalStateCompleteWithNextState(SERVER_UNREACHABLE);
-        }
+      if (IsSynced()) {
+        // The sync cycle we were waiting for is complete.
+        SignalStateCompleteWithNextState(FULLY_SYNCED);
+        break;
+      }
+      if (service()->observed_passphrase_required() &&
+          service()->passphrase_required_reason() ==
+              sync_api::REASON_SET_PASSPHRASE_FAILED) {
+        // A passphrase is required for decryption and we don't have it. Do not
+        // wait any more.
+        SignalStateCompleteWithNextState(PASSPHRASE_REQUIRED_FOR_DECRYPTION);
+        break;
+      }
+      if (!GetStatus().server_reachable) {
+        // The client cannot reach the sync server because the network is
+        // disabled. There is no need to wait anymore.
+        SignalStateCompleteWithNextState(SERVER_UNREACHABLE);
         break;
       }
-      SignalStateCompleteWithNextState(FULLY_SYNCED);
       break;
     }
     case WAITING_FOR_UPDATES: {
@@ -279,6 +304,12 @@ bool ProfileSyncServiceHarness::RunStateChangeMachine() {
       }
       break;
     }
+    case PASSPHRASE_REQUIRED_FOR_DECRYPTION: {
+      // A passphrase is required for decryption. There is nothing the sync
+      // client can do until SetPassphrase() is called.
+      LogClientInfo("PASSPHRASE_REQUIRED_FOR_DECRYPTION");
+      break;
+    }
     case FULLY_SYNCED: {
       // The client is online and fully synced. There is nothing to do.
       LogClientInfo("FULLY_SYNCED");
@@ -308,10 +339,8 @@ bool ProfileSyncServiceHarness::AwaitPassphraseAccepted() {
     return false;
   }
 
-  // TODO(atwilson): After ProfileSyncService::OnPassphraseAccepted() is
-  // fixed, add an extra check to make sure that the value of
-  // service()->observed_passphrase_required() is false.
-  if (service()->ShouldPushChanges()) {
+  if (service()->ShouldPushChanges() &&
+      !service()->observed_passphrase_required()) {
     // Passphrase is already accepted; don't wait.
     return true;
   }
@@ -620,8 +649,7 @@ std::string ProfileSyncServiceHarness::GetUpdatedTimestamp(
 }
 
 void ProfileSyncServiceHarness::LogClientInfo(const std::string& message) {
-  // TODO(lipalani): Change VLOG(0) to VLOG(1)
-  // http://crbug.com/80706
+  // TODO(lipalani): Change VLOG(0) to VLOG(1) -- See http://crbug.com/80706.
   if (service()) {
     const SyncSessionSnapshot* snap = GetLastSessionSnapshot();
     if (snap) {