Refactor sync passphrase setup flow and fix passphrase tests

chrome/browser/sync/profile_sync_service_harness.cc

 // Copyright (c) 2011 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/sync/profile_sync_service_harness.h"
 
 #include <stddef.h>
 #include <algorithm>
 #include <iterator>
 #include <ostream>
@@ skipping 116 matching lines @@
   return profile_->HasProfileSyncService();
 }
 
 bool ProfileSyncServiceHarness::SetupSync() {
   syncable::ModelTypeSet synced_datatypes;
   for (int i = syncable::FIRST_REAL_MODEL_TYPE;
       i < syncable::MODEL_TYPE_COUNT; ++i) {
     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;
 }
 
 bool ProfileSyncServiceHarness::SetupSync(
     const syncable::ModelTypeSet& synced_datatypes) {
   // Initialize the sync client's profile sync service object.
   service_ = profile_->GetProfileSyncService("");
   if (service_ == NULL) {
     LOG(ERROR) << "SetupSync(): service_ is null.";
@@ skipping 24 matching lines @@
   // Wait for initial sync cycle to be completed.
   DCHECK_EQ(wait_state_, WAITING_FOR_INITIAL_SYNC);
   if (!AwaitStatusChangeWithTimeout(kLiveSyncOperationTimeoutMs,
       "Waiting for initial sync cycle to complete.")) {
     LOG(ERROR) << "Initial sync cycle did not complete after "
                << kLiveSyncOperationTimeoutMs / 1000
                << " seconds.";
     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();
 
   return true;
 }
 
 void ProfileSyncServiceHarness::SignalStateCompleteWithNextState(
     WaitState next_state) {
   wait_state_ = next_state;
   SignalStateComplete();
@@ skipping 12 matching lines @@
         // The sync backend is initialized.
         SignalStateCompleteWithNextState(WAITING_FOR_INITIAL_SYNC);
       }
       break;
     }
     case WAITING_FOR_INITIAL_SYNC: {
       LogClientInfo("WAITING_FOR_INITIAL_SYNC");
       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;
       }
-      SignalStateCompleteWithNextState(FULLY_SYNCED);
+      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;
+      }
       break;
     }
     case WAITING_FOR_UPDATES: {
       LogClientInfo("WAITING_FOR_UPDATES");
       DCHECK(timestamp_match_partner_);
       if (!MatchesOtherClient(timestamp_match_partner_)) {
         // The client is not yet fully synced; keep waiting until we converge.
         break;
       }
       timestamp_match_partner_->service()->RemoveObserver(this);
@@ skipping 29 matching lines @@
     }
     case SERVER_UNREACHABLE: {
       LogClientInfo("SERVER_UNREACHABLE");
       if (GetStatus().server_reachable) {
         // The client was offline due to the network being disabled, but is now
         // back online. Wait for the pending sync cycle to complete.
         SignalStateCompleteWithNextState(WAITING_FOR_SYNC_TO_FINISH);
       }
       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");
       break;
     }
     case SYNC_DISABLED: {
       // Syncing is disabled for the client. There is nothing to do.
       LogClientInfo("SYNC_DISABLED");
       break;
     }
     default:
       // Invalid state during observer callback which may be triggered by other
       // classes using the the UI message loop.  Defer to their handling.
       break;
   }
   return original_wait_state != wait_state_;
 }
 
 void ProfileSyncServiceHarness::OnStateChanged() {
   RunStateChangeMachine();
 }
 
 bool ProfileSyncServiceHarness::AwaitPassphraseAccepted() {
   LogClientInfo("AwaitPassphraseAccepted");
   if (wait_state_ == SYNC_DISABLED) {
     LOG(ERROR) << "Sync disabled for Client " << id_ << ".";
     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;
   }
 
   wait_state_ = WAITING_FOR_PASSPHRASE_ACCEPTED;
   return AwaitStatusChangeWithTimeout(kLiveSyncOperationTimeoutMs,
                                       "Waiting for passphrase accepted.");
 }
 
 bool ProfileSyncServiceHarness::AwaitBackendInitialized() {
@@ skipping 288 matching lines @@
 }
 
 std::string ProfileSyncServiceHarness::GetUpdatedTimestamp(
     syncable::ModelType model_type) {
   const SyncSessionSnapshot* snap = GetLastSessionSnapshot();
   DCHECK(snap != NULL) << "GetUpdatedTimestamp(): Sync snapshot is NULL.";
   return snap->download_progress_markers[model_type];
 }
 
 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) {
       VLOG(0) << "Client " << id_ << ": " << message
               << ": num_updates_downloaded : "
               << snap->syncer_status.num_updates_downloaded_total
               << ", has_more_to_sync: " << snap->has_more_to_sync
               << ", unsynced_count: " << snap->unsynced_count
               << ", num_conflicting_updates: " << snap->num_conflicting_updates
               << ", has_unsynced_items: "
@@ skipping 48 matching lines @@
   return true;
 }
 
 std::string ProfileSyncServiceHarness::GetServiceStatus() {
   DictionaryValue value;
   sync_ui_util::ConstructAboutInformation(service_, &value);
   std::string service_status;
   base::JSONWriter::Write(&value, true, &service_status);
   return service_status;
 }