[Sync] Refactor passphrase state handling

sync/internal_api/sync_encryption_handler_impl.cc

Index: sync/internal_api/sync_encryption_handler_impl.cc
diff --git a/sync/internal_api/sync_encryption_handler_impl.cc b/sync/internal_api/sync_encryption_handler_impl.cc
index 2873bba03143b19cdb5cc77cd7a0cb200986dfaa..9798d9e441d62200519a6777081c83074884125c 100644
--- a/sync/internal_api/sync_encryption_handler_impl.cc
+++ b/sync/internal_api/sync_encryption_handler_impl.cc
@@ -60,7 +60,7 @@ SyncEncryptionHandlerImpl::SyncEncryptionHandlerImpl(
       cryptographer_holder_(user_share_, &cryptographer_unsafe_),
       encrypted_types_holder_(user_share_, &encrypted_types_unsafe_),
       encrypt_everything_(false),
-      explicit_passphrase_(false),
+      passphrase_state_(IMPLICIT_PASSPHRASE),
       nigori_overwrite_count_(0) {
 }
 
@@ -389,12 +389,9 @@ bool SyncEncryptionHandlerImpl::EncryptEverythingEnabled() const {
   return encrypt_everything_;
 }
 
-bool SyncEncryptionHandlerImpl::IsUsingExplicitPassphrase() const {
-  // TODO(zea): this is called from the UI thread, so we have to have a
-  // transaction while accessing it. Add an OnPassphraseTypeChanged observer
-  // and have the SBH cache the value on the UI thread.
-  ReadTransaction trans(FROM_HERE, user_share_);
-  return explicit_passphrase_;
+PassphraseState SyncEncryptionHandlerImpl::GetPassphraseState() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return passphrase_state_;
 }
 
 // Note: this is called from within a syncable transaction, so we need to post
@@ -506,10 +503,15 @@ bool SyncEncryptionHandlerImpl::ApplyNigoriUpdateImpl(
     const sync_pb::NigoriSpecifics& nigori,
     syncable::BaseTransaction* const trans) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(1) << "Applying nigori node update.";
   bool nigori_types_need_update = !UpdateEncryptedTypesFromNigori(nigori,
                                                                   trans);
-  if (nigori.using_explicit_passphrase())
-    explicit_passphrase_ = true;
+  if (nigori.using_explicit_passphrase() &&
+      passphrase_state_ != CUSTOM_PASSPHRASE) {
+    passphrase_state_ = CUSTOM_PASSPHRASE;
+    FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
+                      OnPassphraseStateChanged(passphrase_state_));
+  }
 
   Cryptographer* cryptographer = cryptographer_holder_.GetMutable(trans);
   bool nigori_needs_new_keys = false;
@@ -556,7 +558,8 @@ bool SyncEncryptionHandlerImpl::ApplyNigoriUpdateImpl(
   // Check if the current local encryption state is stricter/newer than the
   // nigori state. If so, we need to overwrite the nigori node with the local
   // state.
-  if (nigori.using_explicit_passphrase() != explicit_passphrase_ ||
+  if (nigori.using_explicit_passphrase() !=
+          (passphrase_state_ == CUSTOM_PASSPHRASE) ||

[akalin, 2012/08/22 20:10:39]

might be clearer to have an intermediate variable 'explicit_passphrase =
passphrase_state_ == CUSTOM_PASSPHRASE'

[Nicolas Zea, 2012/08/22 20:18:33]

Done.

       nigori.encrypt_everything() != encrypt_everything_ ||
       nigori_types_need_update ||
       nigori_needs_new_keys) {
@@ -695,8 +698,6 @@ void SyncEncryptionHandlerImpl::FinishSetPassphrase(
     return;
   }
 
-  FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
-                    OnPassphraseAccepted());
   DCHECK(cryptographer.is_ready());
 
   sync_pb::NigoriSpecifics specifics(nigori_node->GetNigoriSpecifics());
@@ -704,10 +705,19 @@ void SyncEncryptionHandlerImpl::FinishSetPassphrase(
   // the same.
   if (!cryptographer.GetKeys(specifics.mutable_encrypted()))
     NOTREACHED();
-  explicit_passphrase_ = is_explicit;
+  if (is_explicit && passphrase_state_ != CUSTOM_PASSPHRASE) {
+    passphrase_state_ = CUSTOM_PASSPHRASE;
+    FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
+                      OnPassphraseStateChanged(passphrase_state_));
+  }
   specifics.set_using_explicit_passphrase(is_explicit);
   nigori_node->SetNigoriSpecifics(specifics);
 
+  // Must do this after OnPassphraseStateChanged, in order to ensure the PSS
+  // checks the passphrase state after it has been set.
+  FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
+                    OnPassphraseAccepted());
+
   // Does nothing if everything is already encrypted.
   ReEncryptEverything(trans);
 }