[Sync] Remove SET_PASSPHRASE_FAILED reason.

We only call OnPassphraseRequired if the cryptographer is not ready, and
we set the reason based on whether there are pending keys.

Also make sure SetDecryptionPassphrase properly handles return value.

BUG=118243
TEST=

Review URL: https://chromiumcodereview.appspot.com/9796001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128088 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 36 insertions, 47 deletions

diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index 7320a1c..42301510 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -202,7 +202,8 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
   // otherwise. If new encrypted keys arrive during the asynchronous call,
   // OnPassphraseRequired may be triggered at a later time. It is an error to
   // call this when there are no pending keys.
-  bool SetDecryptionPassphrase(const std::string& passphrase);
+  bool SetDecryptionPassphrase(const std::string& passphrase)
+      WARN_UNUSED_RESULT;
 
   // Called on |frontend_loop_| to kick off shutdown procedure. After this,
   // no further sync activity will occur with the sync server and no further
diff --git a/chrome/browser/sync/internal_api/js_sync_manager_observer_unittest.cc b/chrome/browser/sync/internal_api/js_sync_manager_observer_unittest.cc
index 7637ea7..0bf4d2a 100644
--- a/chrome/browser/sync/internal_api/js_sync_manager_observer_unittest.cc
+++ b/chrome/browser/sync/internal_api/js_sync_manager_observer_unittest.cc
@@ -139,7 +139,6 @@ TEST_F(JsSyncManagerObserverTest, OnPassphraseRequired) {
   DictionaryValue reason_passphrase_not_required_details;
   DictionaryValue reason_encryption_details;
   DictionaryValue reason_decryption_details;
-  DictionaryValue reason_set_passphrase_failed_details;
 
   reason_passphrase_not_required_details.SetString(
       "reason",
@@ -151,10 +150,6 @@ TEST_F(JsSyncManagerObserverTest, OnPassphraseRequired) {
   reason_decryption_details.SetString(
       "reason",
       sync_api::PassphraseRequiredReasonToString(sync_api::REASON_DECRYPTION));
-  reason_set_passphrase_failed_details.SetString(
-      "reason",
-      sync_api::PassphraseRequiredReasonToString(
-          sync_api::REASON_SET_PASSPHRASE_FAILED));
 
   EXPECT_CALL(mock_js_event_handler_,
               HandleJsEvent("onPassphraseRequired",
@@ -166,10 +161,6 @@ TEST_F(JsSyncManagerObserverTest, OnPassphraseRequired) {
   EXPECT_CALL(mock_js_event_handler_,
               HandleJsEvent("onPassphraseRequired",
                            HasDetailsAsDictionary(reason_decryption_details)));
-  EXPECT_CALL(mock_js_event_handler_,
-              HandleJsEvent("onPassphraseRequired",
-                           HasDetailsAsDictionary(
-                               reason_set_passphrase_failed_details)));
 
   js_sync_manager_observer_.OnPassphraseRequired(
       sync_api::REASON_PASSPHRASE_NOT_REQUIRED,
@@ -178,9 +169,6 @@ TEST_F(JsSyncManagerObserverTest, OnPassphraseRequired) {
                                                  sync_pb::EncryptedData());
   js_sync_manager_observer_.OnPassphraseRequired(sync_api::REASON_DECRYPTION,
                                                  sync_pb::EncryptedData());
-  js_sync_manager_observer_.OnPassphraseRequired(
-      sync_api::REASON_SET_PASSPHRASE_FAILED,
-      sync_pb::EncryptedData());
   PumpLoop();
 }
 
diff --git a/chrome/browser/sync/internal_api/sync_manager.cc b/chrome/browser/sync/internal_api/sync_manager.cc
index 14b37fd..481061c 100644
--- a/chrome/browser/sync/internal_api/sync_manager.cc
+++ b/chrome/browser/sync/internal_api/sync_manager.cc
@@ -242,7 +242,7 @@ class SyncManager::SyncInternal
   // a flag in the nigori node specifying whether the current passphrase is
   // explicit (custom passphrase) or non-explicit (GAIA). If the existing
   // encryption passphrase is "explicit", the data cannot be re-encrypted and
-  // OnPassphraseRequired is triggered with REASON_SET_PASSPHRASE_FAILED.
+  // SetEncryptionPassphrase will do nothing.
   // If !is_explicit and there are pending keys, we will attempt to decrypt them
   // using this passphrase. If this fails, we will save this encryption key to
   // be applied later after the pending keys are resolved.
@@ -264,8 +264,6 @@ class SyncManager::SyncInternal
   //            success == false, we send an OnPassphraseRequired notification.
   // |bootstrap_token|: used to inform observers if the cryptographer's
   //                    bootstrap token was updated.
-  // |pending_keys|: used to pass on the cryptographer's pending keys to the UI
-  //                 thread so they can be cached. Ignored if |success| == true.
   // |is_explicit|: used to differentiate between a custom passphrase (true) and
   //                a GAIA passphrase that is implicitly used for encryption
   //                (false).
@@ -273,7 +271,6 @@ class SyncManager::SyncInternal
   void FinishSetPassphrase(
       bool success,
       const std::string& bootstrap_token,
-      const sync_pb::EncryptedData& pending_keys,
       bool is_explicit,
       WriteTransaction* trans,
       WriteNode* nigori_node);
@@ -1307,14 +1304,14 @@ void SyncManager::SyncInternal::SetEncryptionPassphrase(
     success = false;
   }
 
-  DVLOG_IF(1, success)
-      << "Failure in SetEncryptionPassphrase; notifying and returning.";
   DVLOG_IF(1, !success)
+      << "Failure in SetEncryptionPassphrase; notifying and returning.";
+  DVLOG_IF(1, success)
       << "Successfully set encryption passphrase; updating nigori and "
          "reencrypting.";
 
   FinishSetPassphrase(
-      success, bootstrap_token, pending_keys, is_explicit, &trans, &node);
+      success, bootstrap_token, is_explicit, &trans, &node);
 }
 
 void SyncManager::SyncInternal::SetDecryptionPassphrase(
@@ -1446,15 +1443,14 @@ void SyncManager::SyncInternal::SetDecryptionPassphrase(
     }
   }  // nigori_has_explicit_passphrase
 
-  DVLOG_IF(1, success)
-      << "Failure in SetDecryptionPassphrase; notifying and returning.";
   DVLOG_IF(1, !success)
+      << "Failure in SetDecryptionPassphrase; notifying and returning.";
+  DVLOG_IF(1, success)
       << "Successfully set decryption passphrase; updating nigori and "
          "reencrypting.";
 
   FinishSetPassphrase(success,
                       bootstrap_token,
-                      pending_keys,
                       nigori_has_explicit_passphrase,
                       &trans,
                       &node);
@@ -1463,7 +1459,6 @@ void SyncManager::SyncInternal::SetDecryptionPassphrase(
 void SyncManager::SyncInternal::FinishSetPassphrase(
     bool success,
     const std::string& bootstrap_token,
-    const sync_pb::EncryptedData& pending_keys,
     bool is_explicit,
     WriteTransaction* trans,
     WriteNode* nigori_node) {
@@ -1477,10 +1472,19 @@ void SyncManager::SyncInternal::FinishSetPassphrase(
   }
 
   if (!success) {
-    FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
-                      OnPassphraseRequired(
-                          sync_api::REASON_SET_PASSPHRASE_FAILED,
-                          pending_keys));
+    Cryptographer* cryptographer = trans->GetCryptographer();
+    if (cryptographer->is_ready()) {
+      LOG(ERROR) << "Attempt to change passphrase failed while cryptographer "
+                 << "was ready.";
+    } else if (cryptographer->has_pending_keys()) {
+      FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
+                        OnPassphraseRequired(sync_api::REASON_DECRYPTION,
+                                             cryptographer->GetPendingKeys()));
+    } else {
+      FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
+                        OnPassphraseRequired(sync_api::REASON_ENCRYPTION,
+                                             sync_pb::EncryptedData()));
+    }
     return;
   }
 
@@ -2465,8 +2469,6 @@ const char* PassphraseRequiredReasonToString(
       return "REASON_ENCRYPTION";
     case REASON_DECRYPTION:
       return "REASON_DECRYPTION";
-    case REASON_SET_PASSPHRASE_FAILED:
-      return "REASON_SET_PASSPHRASE_FAILED";
     default:
       NOTREACHED();
       return "INVALID_REASON";
diff --git a/chrome/browser/sync/internal_api/sync_manager.h b/chrome/browser/sync/internal_api/sync_manager.h
index c9d45b8..3ebaa91 100644
--- a/chrome/browser/sync/internal_api/sync_manager.h
+++ b/chrome/browser/sync/internal_api/sync_manager.h
@@ -66,10 +66,6 @@ enum PassphraseRequiredReason {
   REASON_DECRYPTION = 2,               // The cryptographer requires a
                                        // passphrase for its first attempt at
                                        // decryption.
-  REASON_SET_PASSPHRASE_FAILED = 3,    // The cryptographer requires a new
-                                       // passphrase because its attempt at
-                                       // decryption with the cached passphrase
-                                       // was unsuccessful.
 };
 
 
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 0444247..2b584ec 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -873,8 +873,8 @@ void ProfileSyncService::OnPassphraseRequired(
     if (backend_->IsUsingExplicitPassphrase()) {
       DVLOG(1) << "Attempting explicit passphrase.";
       // The passphrase will be re-cached if the syncer isn't ready.
-      SetDecryptionPassphrase(explicit_passphrase);
-      return;
+      if (SetDecryptionPassphrase(explicit_passphrase))
+        return;
     }
   }
 
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index b4b0a51..7e26cdb 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -463,7 +463,8 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // Asynchronously decrypts pending keys using |passphrase|. Returns false
   // immediately if the passphrase could not be used to decrypt a locally cached
   // copy of encrypted keys; returns true otherwise.
-  virtual bool SetDecryptionPassphrase(const std::string& passphrase);
+  virtual bool SetDecryptionPassphrase(const std::string& passphrase)
+      WARN_UNUSED_RESULT;
 
   // Turns on encryption for all data. Callers must call OnUserChoseDatatypes()
   // after calling this to force the encryption to occur.
diff --git a/chrome/browser/sync/profile_sync_service_harness.cc b/chrome/browser/sync/profile_sync_service_harness.cc
index aff06e0..f2694ac 100644
--- a/chrome/browser/sync/profile_sync_service_harness.cc
+++ b/chrome/browser/sync/profile_sync_service_harness.cc
@@ -270,7 +270,7 @@ bool ProfileSyncServiceHarness::RunStateChangeMachine() {
         break;
       }
       if (service()->passphrase_required_reason() ==
-              sync_api::REASON_SET_PASSPHRASE_FAILED) {
+              sync_api::REASON_DECRYPTION) {
         // A passphrase is required for decryption and we don't have it. Do not
         // wait any more.
         SignalStateCompleteWithNextState(SET_PASSPHRASE_FAILED);
diff --git a/chrome/browser/sync/sync_setup_flow.cc b/chrome/browser/sync/sync_setup_flow.cc
index afc842a..deebc99 100644
--- a/chrome/browser/sync/sync_setup_flow.cc
+++ b/chrome/browser/sync/sync_setup_flow.cc
@@ -354,11 +354,13 @@ void SyncSetupFlow::OnUserConfigured(const SyncConfiguration& configuration) {
     // Caller passed a gaia passphrase. This is illegal if we are currently
     // using a secondary passphrase.
     DCHECK(!service_->IsUsingSecondaryPassphrase());
-    service_->SetDecryptionPassphrase(configuration.gaia_passphrase);
+
     // Since the user entered the passphrase manually, set this flag so we can
     // report an error if the passphrase setting failed.
     user_tried_setting_passphrase_ = true;
-    set_new_decryption_passphrase = true;
+    if (service_->SetDecryptionPassphrase(configuration.gaia_passphrase)) {
+      set_new_decryption_passphrase = true;
+    }
   }
 
   // Set the secondary passphrase, either as a decryption passphrase, or
@@ -372,11 +374,9 @@ void SyncSetupFlow::OnUserConfigured(const SyncConfiguration& configuration) {
     // "enter passphrase" dialog without sending the passphrase to the syncer
     // thread.
     if (service_->IsPassphraseRequiredForDecryption()) {
-      if (!service_->SetDecryptionPassphrase(
-          configuration.secondary_passphrase)) {
-        user_tried_setting_passphrase_ = true;
-        Advance(SyncSetupWizard::ENTER_PASSPHRASE);
-        return;
+      if (service_->SetDecryptionPassphrase(
+              configuration.secondary_passphrase)) {
+        set_new_decryption_passphrase = true;
       }
     } else {
       service_->SetEncryptionPassphrase(configuration.secondary_passphrase,
@@ -384,7 +384,6 @@ void SyncSetupFlow::OnUserConfigured(const SyncConfiguration& configuration) {
     }
     if (service_->IsUsingSecondaryPassphrase()) {
       user_tried_setting_passphrase_ = true;
-      set_new_decryption_passphrase = true;
     } else {
       user_tried_creating_explicit_passphrase_ = true;
     }
@@ -412,8 +411,10 @@ void SyncSetupFlow::OnUserConfigured(const SyncConfiguration& configuration) {
 
 void SyncSetupFlow::OnPassphraseEntry(const std::string& passphrase) {
   Advance(SyncSetupWizard::SETTING_UP);
-  service_->SetDecryptionPassphrase(passphrase);
   user_tried_setting_passphrase_ = true;
+  if (!service_->SetDecryptionPassphrase(passphrase)) {
+    Advance(SyncSetupWizard::ENTER_PASSPHRASE);
+  }
 }
 
 void SyncSetupFlow::OnPassphraseCancel() {