From 43c48fb9f34da682482635dae6ebf1b3dcdae560 Mon Sep 17 00:00:00 2001 From: "rsimha@chromium.org" Date: Mon, 2 May 2011 19:47:43 +0000 Subject: Refactor sync passphrase setup flow and fix passphrase tests The passphrase sync integration tests are in a state of disrepair due to several recent changes to the sync implementation. In particular, passphrase sync uses two similar methods called OnPassphraseRequired and OnPassphraseFailed to differentiate between cases where a passphrase is required for a first attempt at decryption and cases where decryption with a cached passphrase failed and the user needs to be prompted. This patch refactors the "for_decryption" boolean flag into an enum called PassphraseRequiredReason, thereby eliminating the need for OnPassphraseFailed. It also fixes the tests and updates the test framework to account for scenarios in which a client is waiting for a passphrase that has been set by one of its partners, and enables it to exit early when forward progress is impossible without a call to SetPassphrase. BUG=78840, 80180, 81018 TEST=sync_integration_tests Review URL: http://codereview.chromium.org/6902101 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@83764 0039d316-1c4b-4281-b951-d872f2087c98 --- .../browser/sync/profile_sync_service_harness.cc | 66 +++++++++++++++------- 1 file changed, 46 insertions(+), 20 deletions(-) (limited to '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 f8ab166..60529f2 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; if (result == false) { - std::string pss_status = GetServiceStatus(); - VLOG(0) << pss_status; + std::string status = GetServiceStatus(); + LOG(ERROR) << "Client " << id_ << ": SetupSync failed. Syncer status:\n" + << status; + } else { + VLOG(1) << "Client " << id_ << ": SetupSync successful."; } return result; } @@ -183,6 +184,12 @@ bool ProfileSyncServiceHarness::SetupSync( return false; } + if (wait_state_ == SET_PASSPHRASE_FAILED) { + // 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,37 @@ bool ProfileSyncServiceHarness::RunStateChangeMachine() { if (IsSynced()) { // The first sync cycle is now complete. We can start running tests. SignalStateCompleteWithNextState(FULLY_SYNCED); + break; + } + if (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(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()->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(SET_PASSPHRASE_FAILED); + 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: { @@ -248,7 +271,7 @@ bool ProfileSyncServiceHarness::RunStateChangeMachine() { case WAITING_FOR_PASSPHRASE_ACCEPTED: { LogClientInfo("WAITING_FOR_PASSPHRASE_ACCEPTED"); if (service()->ShouldPushChanges() && - !service()->observed_passphrase_required()) { + !service()->ObservedPassphraseRequired()) { // The passphrase has been accepted, and sync has been restarted. SignalStateCompleteWithNextState(FULLY_SYNCED); } @@ -279,6 +302,12 @@ bool ProfileSyncServiceHarness::RunStateChangeMachine() { } break; } + case SET_PASSPHRASE_FAILED: { + // A passphrase is required for decryption. There is nothing the sync + // client can do until SetPassphrase() is called. + LogClientInfo("SET_PASSPHRASE_FAILED"); + break; + } case FULLY_SYNCED: { // The client is online and fully synced. There is nothing to do. LogClientInfo("FULLY_SYNCED"); @@ -308,10 +337,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()->ObservedPassphraseRequired()) { // Passphrase is already accepted; don't wait. return true; } @@ -620,8 +647,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) { @@ -634,7 +660,7 @@ void ProfileSyncServiceHarness::LogClientInfo(const std::string& message) { << ", has_unsynced_items: " << service()->HasUnsyncedItems() << ", observed_passphrase_required: " - << service()->observed_passphrase_required() + << service()->ObservedPassphraseRequired() << ", notifications_enabled: " << GetStatus().notifications_enabled << ", service_is_pushing_changes: " << ServiceIsPushingChanges() -- cgit v1.1