From 88dee5e7f00ebf94465d3bf4399ef50484e9d8a2 Mon Sep 17 00:00:00 2001 From: "rsimha@chromium.org" Date: Tue, 3 May 2011 18:23:53 +0000 Subject: Add method IsPassphraseRequiredForDecryption to ProfileSyncService ProfileSyncService needs to know in multiple places whether a passphrase is required for decryption. Right now, the logic involves checking passphrase_required_reason_ against a couple of states. This patch refactors this logic into the methods: IsPassphraseRequired IsPassphraseRequiredForDecryption BUG=81313, 81341, 81018 TEST=sync_integration_tests Review URL: http://codereview.chromium.org/6910012 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@83923 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/sync/profile_sync_service.cc | 29 ++++++++-------------- chrome/browser/sync/profile_sync_service.h | 8 +++++- .../browser/sync/profile_sync_service_harness.cc | 13 ++++++---- chrome/browser/sync/sync_ui_util.cc | 11 +++----- 4 files changed, 29 insertions(+), 32 deletions(-) (limited to 'chrome/browser/sync') diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index db2fd10..a8606cb 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -616,16 +616,12 @@ void ProfileSyncService::OnPassphraseRequired( // We will skip the passphrase prompt and suppress the warning if the // passphrase is needed for decryption but the user is not syncing an // encrypted data type on this machine. Otherwise we prompt. - if (!IsEncryptedDatatypeEnabled() && - (reason == sync_api::REASON_DECRYPTION || - reason == sync_api::REASON_SET_PASSPHRASE_FAILED)) { + if (!IsEncryptedDatatypeEnabled() && IsPassphraseRequiredForDecryption()) { OnPassphraseAccepted(); return; } - if (WizardIsVisible() && - (reason == sync_api::REASON_DECRYPTION || - reason == sync_api::REASON_SET_PASSPHRASE_FAILED)) { + if (WizardIsVisible() && IsPassphraseRequiredForDecryption()) { wizard_.Step(SyncSetupWizard::ENTER_PASSPHRASE); } @@ -689,7 +685,7 @@ void ProfileSyncService::ShowLoginDialog(gfx::NativeWindow parent_window) { } void ProfileSyncService::ShowErrorUI(gfx::NativeWindow parent_window) { - if (ObservedPassphraseRequired()) { + if (IsPassphraseRequired()) { if (IsUsingSecondaryPassphrase()) PromptForExistingPassphrase(parent_window); else @@ -945,7 +941,7 @@ void ProfileSyncService::GetRegisteredDataTypes( bool ProfileSyncService::IsUsingSecondaryPassphrase() const { return backend_.get() && (backend_->IsUsingExplicitPassphrase() || (tried_implicit_gaia_remove_when_bug_62103_fixed_ && - ObservedPassphraseRequired())); + IsPassphraseRequired())); } bool ProfileSyncService::IsCryptographerReady( @@ -986,8 +982,7 @@ void ProfileSyncService::ConfigureDataTypeManager() { encrypted_types_.clear(); if (types.count(syncable::PASSWORDS) > 0) encrypted_types_.insert(syncable::PASSWORDS); - if (passphrase_required_reason_ == sync_api::REASON_DECRYPTION || - passphrase_required_reason_ == sync_api::REASON_SET_PASSPHRASE_FAILED) { + if (IsPassphraseRequiredForDecryption()) { if (IsEncryptedDatatypeEnabled()) { // We need a passphrase still. Prompt the user for a passphrase, and // DataTypeManager::Configure() will get called once the passphrase is @@ -1105,7 +1100,7 @@ void ProfileSyncService::DeactivateDataType( void ProfileSyncService::SetPassphrase(const std::string& passphrase, bool is_explicit, bool is_creation) { - if (ShouldPushChanges() || ObservedPassphraseRequired()) { + if (ShouldPushChanges() || IsPassphraseRequired()) { backend_->SetPassphrase(passphrase, is_explicit); } else { cached_passphrase_.value = passphrase; @@ -1160,11 +1155,9 @@ void ProfileSyncService::Observe(NotificationType type, } // We should never get in a state where we have no encrypted datatypes - // enabled, and yet we still think we require a passphrase. - DCHECK(passphrase_required_reason_ == - sync_api::REASON_PASSPHRASE_NOT_REQUIRED || - passphrase_required_reason_ == sync_api::REASON_ENCRYPTION || - IsEncryptedDatatypeEnabled()); + // enabled, and yet we still think we require a passphrase for decryption. + DCHECK(!(IsPassphraseRequiredForDecryption() && + !IsEncryptedDatatypeEnabled())); // TODO(sync): Less wizard, more toast. wizard_.Step(SyncSetupWizard::DONE); @@ -1210,9 +1203,7 @@ void ProfileSyncService::Observe(NotificationType type, // If this signin was to initiate a passphrase migration (on the // first computer, thus not for decryption), continue the migration. if (passphrase_migration_in_progress_ && - passphrase_required_reason_ != sync_api::REASON_DECRYPTION && - passphrase_required_reason_ != - sync_api::REASON_SET_PASSPHRASE_FAILED) { + !IsPassphraseRequiredForDecryption()) { wizard_.Step(SyncSetupWizard::PASSPHRASE_MIGRATION); passphrase_migration_in_progress_ = false; } diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h index a0e1cb8..e343b12 100644 --- a/chrome/browser/sync/profile_sync_service.h +++ b/chrome/browser/sync/profile_sync_service.h @@ -283,11 +283,17 @@ class ProfileSyncService : public browser_sync::SyncFrontend, } // Returns true if OnPassphraseRequired has been called for any reason. - bool ObservedPassphraseRequired() const { + bool IsPassphraseRequired() const { return passphrase_required_reason_ != sync_api::REASON_PASSPHRASE_NOT_REQUIRED; } + // Returns true if OnPassphraseRequired has been called for decryption. + bool IsPassphraseRequiredForDecryption() const { + return (passphrase_required_reason_ == sync_api::REASON_DECRYPTION || + passphrase_required_reason_ == sync_api::REASON_SET_PASSPHRASE_FAILED); + } + sync_api::PassphraseRequiredReason passphrase_required_reason() const { return passphrase_required_reason_; } diff --git a/chrome/browser/sync/profile_sync_service_harness.cc b/chrome/browser/sync/profile_sync_service_harness.cc index 60529f2..1988768 100644 --- a/chrome/browser/sync/profile_sync_service_harness.cc +++ b/chrome/browser/sync/profile_sync_service_harness.cc @@ -271,7 +271,7 @@ bool ProfileSyncServiceHarness::RunStateChangeMachine() { case WAITING_FOR_PASSPHRASE_ACCEPTED: { LogClientInfo("WAITING_FOR_PASSPHRASE_ACCEPTED"); if (service()->ShouldPushChanges() && - !service()->ObservedPassphraseRequired()) { + !service()->IsPassphraseRequired()) { // The passphrase has been accepted, and sync has been restarted. SignalStateCompleteWithNextState(FULLY_SYNCED); } @@ -338,7 +338,7 @@ bool ProfileSyncServiceHarness::AwaitPassphraseAccepted() { } if (service()->ShouldPushChanges() && - !service()->ObservedPassphraseRequired()) { + !service()->IsPassphraseRequired()) { // Passphrase is already accepted; don't wait. return true; } @@ -524,7 +524,9 @@ bool ProfileSyncServiceHarness::IsSynced() { !service()->HasUnsyncedItems() && !snap->has_more_to_sync && snap->unsynced_count == 0 && - !service()->HasPendingBackendMigration()); + !service()->HasPendingBackendMigration() && + service()->passphrase_required_reason() != + sync_api::REASON_SET_PASSPHRASE_FAILED); } bool ProfileSyncServiceHarness::MatchesOtherClient( @@ -659,8 +661,9 @@ void ProfileSyncServiceHarness::LogClientInfo(const std::string& message) { << ", num_conflicting_updates: " << snap->num_conflicting_updates << ", has_unsynced_items: " << service()->HasUnsyncedItems() - << ", observed_passphrase_required: " - << service()->ObservedPassphraseRequired() + << ", passphrase_required_reason: " + << sync_api::PassphraseRequiredReasonToString( + service()->passphrase_required_reason()) << ", notifications_enabled: " << GetStatus().notifications_enabled << ", service_is_pushing_changes: " << ServiceIsPushingChanges() diff --git a/chrome/browser/sync/sync_ui_util.cc b/chrome/browser/sync/sync_ui_util.cc index e894e1b7..989a996 100644 --- a/chrome/browser/sync/sync_ui_util.cc +++ b/chrome/browser/sync/sync_ui_util.cc @@ -103,7 +103,7 @@ MessageType GetStatusInfo(ProfileSyncService* service, // Either show auth error information with a link to re-login, auth in prog, // or note that everything is OK with the last synced time. - if (status.authenticated && !service->ObservedPassphraseRequired()) { + if (status.authenticated && !service->IsPassphraseRequired()) { // Everything is peachy. if (status_label) { status_label->assign(GetSyncedStateStatusLabel(service)); @@ -115,11 +115,8 @@ MessageType GetStatusInfo(ProfileSyncService* service, l10n_util::GetStringUTF16(IDS_SYNC_AUTHENTICATING_LABEL)); } result_type = PRE_SYNCED; - } else if (service->ObservedPassphraseRequired()) { - if (service->passphrase_required_reason() == - sync_api::REASON_DECRYPTION || - service->passphrase_required_reason() == - sync_api::REASON_SET_PASSPHRASE_FAILED) { + } else if (service->IsPassphraseRequired()) { + if (service->IsPassphraseRequiredForDecryption()) { // NOT first machine. // Show a link ("needs attention"), but still indicate the // current synced status. Return SYNC_PROMO so that @@ -190,7 +187,7 @@ MessageType GetStatusInfoForNewTabPage(ProfileSyncService* service, DCHECK(link_label); if (service->HasSyncSetupCompleted() && - service->ObservedPassphraseRequired()) { + service->IsPassphraseRequired()) { if (service->passphrase_required_reason() == sync_api::REASON_ENCRYPTION) { // First machine migrating to passwords. Show as a promotion. if (status_label && link_label) { -- cgit v1.1