[Sync] Refactor GetEncryptedTypes usage.

sync/internal_api/sync_encryption_handler_impl.cc

 // Copyright (c) 2012 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 "sync/internal_api/sync_encryption_handler_impl.h"
 
 #include <queue>
 #include <string>
 
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/tracked_objects.h"
 #include "base/metrics/histogram.h"
 #include "sync/internal_api/public/read_node.h"
 #include "sync/internal_api/public/read_transaction.h"
-#include "sync/internal_api/public/user_share.h"
 #include "sync/internal_api/public/util/experiments.h"
 #include "sync/internal_api/public/write_node.h"
 #include "sync/internal_api/public/write_transaction.h"
 #include "sync/protocol/encryption.pb.h"
 #include "sync/protocol/nigori_specifics.pb.h"
+#include "sync/protocol/sync.pb.h"
 #include "sync/syncable/base_transaction.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/entry.h"
 #include "sync/syncable/nigori_util.h"
 #include "sync/util/cryptographer.h"
 
 namespace syncer {
 
 namespace {
 // The maximum number of times we will automatically overwrite the nigori node
 // because the encryption keys don't match (per chrome instantiation).
 // We protect ourselves against nigori rollbacks, but it's possible two
 // different clients might have contrasting view of what the nigori node state
 // should be, in which case they might ping pong (see crbug.com/119207).
 static const int kNigoriOverwriteLimit = 10;
 }
 
+template <typename T>
+const T& TransactionalHolder<T>::Get(
+    syncable::BaseTransaction* const trans) const {
+  DCHECK_EQ(user_share_->directory.get(), trans->directory());
+  return *obj_;
+}
+
+template <typename T>
+T* TransactionalHolder<T>::GetMutable(
+    syncable::BaseTransaction* const trans) {
+  DCHECK_EQ(user_share_->directory.get(), trans->directory());
+  return obj_;
+}
+
 SyncEncryptionHandlerImpl::SyncEncryptionHandlerImpl(
     UserShare* user_share,
-    Cryptographer* cryptographer)
+    Encryptor* encryptor)
     : weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       user_share_(user_share),
-      cryptographer_(cryptographer),
-      encrypted_types_(SensitiveTypes()),
+      cryptographer_unsafe_(encryptor),
+      encrypted_types_unsafe_(SensitiveTypes()),
+      cryptographer_holder_(user_share_, &cryptographer_unsafe_),
+      encrypted_types_holder_(user_share_, &encrypted_types_unsafe_),
       encrypt_everything_(false),
       explicit_passphrase_(false),
       nigori_overwrite_count_(0) {
 }
 
 SyncEncryptionHandlerImpl::~SyncEncryptionHandlerImpl() {}
 
 void SyncEncryptionHandlerImpl::AddObserver(Observer* observer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!observers_.HasObserver(observer));
   observers_.AddObserver(observer);
 }
 
 void SyncEncryptionHandlerImpl::RemoveObserver(Observer* observer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(observers_.HasObserver(observer));
   observers_.RemoveObserver(observer);
 }
 
 void SyncEncryptionHandlerImpl::Init() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   WriteTransaction trans(FROM_HERE, user_share_);
   WriteNode node(&trans);
-  Cryptographer* cryptographer = trans.GetCryptographer();
-  cryptographer_ = cryptographer;
 
   if (node.InitByTagLookup(kNigoriTag) != BaseNode::INIT_OK)
     return;
   if (!ApplyNigoriUpdateImpl(node.GetNigoriSpecifics(),
                              trans.GetWrappedTrans())) {
     WriteEncryptionStateToNigori(&trans);
   }
 
-  FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
-                    OnCryptographerStateChanged(cryptographer));
+  // Always trigger an encrypted types and cryptographer state change event at
+  // init time so observers get the initial values.
+  FOR_EACH_OBSERVER(
+      Observer, observers_,
+      OnEncryptedTypesChanged(
+          encrypted_types_holder_.Get(trans.GetWrappedTrans()),
+          encrypt_everything_));
+  FOR_EACH_OBSERVER(
+      SyncEncryptionHandler::Observer,
+      observers_,
+      OnCryptographerStateChanged(
+          cryptographer_holder_.GetMutable(trans.GetWrappedTrans())));
 
   // If the cryptographer is not ready (either it has pending keys or we
   // failed to initialize it), we don't want to try and re-encrypt the data.
   // If we had encrypted types, the DataTypeManager will block, preventing
   // sync from happening until the the passphrase is provided.
-  if (cryptographer->is_ready())
+  if (cryptographer_holder_.Get(trans.GetWrappedTrans()).is_ready())
     ReEncryptEverything(&trans);
 }
 
-// Note: this is called from within a syncable transaction, so we need to post
-// tasks if we want to do any work that creates a new sync_api transaction.
-void SyncEncryptionHandlerImpl::ApplyNigoriUpdate(
-    const sync_pb::NigoriSpecifics& nigori,
-    syncable::BaseTransaction* const trans) {
-  DCHECK(trans);
-  if (!ApplyNigoriUpdateImpl(nigori, trans)) {
-    MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&SyncEncryptionHandlerImpl::RewriteNigori,
-                   weak_ptr_factory_.GetWeakPtr()));
-  }
-
-  FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
-                    OnCryptographerStateChanged(cryptographer_));
-}
-
-// Note: this is always called via the Cryptographer interface right now,
-// so a transaction is already held. Once we remove that interface, we'll
-// need to enforce holding a transaction when calling this method.
-ModelTypeSet SyncEncryptionHandlerImpl::GetEncryptedTypes() const {
-  return encrypted_types_;
-}
-
 void SyncEncryptionHandlerImpl::SetEncryptionPassphrase(
     const std::string& passphrase,
     bool is_explicit) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // We do not accept empty passphrases.
   if (passphrase.empty()) {
     NOTREACHED() << "Cannot encrypt with an empty passphrase.";
     return;
   }
 
   // All accesses to the cryptographer are protected by a transaction.
   WriteTransaction trans(FROM_HERE, user_share_);
-  Cryptographer* cryptographer = trans.GetCryptographer();
   KeyParams key_params = {"localhost", "dummy", passphrase};
   WriteNode node(&trans);
   if (node.InitByTagLookup(kNigoriTag) != BaseNode::INIT_OK) {
     NOTREACHED();
     return;
   }
 
   bool nigori_has_explicit_passphrase =
       node.GetNigoriSpecifics().using_explicit_passphrase();
   std::string bootstrap_token;
   sync_pb::EncryptedData pending_keys;
+  Cryptographer* cryptographer =
+      cryptographer_holder_.GetMutable(trans.GetWrappedTrans());
   if (cryptographer->has_pending_keys())
     pending_keys = cryptographer->GetPendingKeys();
   bool success = false;
 
-
   // There are six cases to handle here:
   // 1. The user has no pending keys and is setting their current GAIA password
   //    as the encryption passphrase. This happens either during first time sync
   //    with a clean profile, or after re-authenticating on a profile that was
   //    already signed in with the cryptographer ready.
   // 2. The user has no pending keys, and is overwriting an (already provided)
   //    implicit passphrase with an explicit (custom) passphrase.
   // 3. The user has pending keys for an explicit passphrase that is somehow set
   //    to their current GAIA passphrase.
   // 4. The user has pending keys encrypted with their current GAIA passphrase
@@ skipping 68 matching lines @@
   DVLOG_IF(1, success)
       << "Successfully set encryption passphrase; updating nigori and "
          "reencrypting.";
 
   FinishSetPassphrase(
       success, bootstrap_token, is_explicit, &trans, &node);
 }
 
 void SyncEncryptionHandlerImpl::SetDecryptionPassphrase(
     const std::string& passphrase) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // We do not accept empty passphrases.
   if (passphrase.empty()) {
     NOTREACHED() << "Cannot decrypt with an empty passphrase.";
     return;
   }
 
   // All accesses to the cryptographer are protected by a transaction.
   WriteTransaction trans(FROM_HERE, user_share_);
-  Cryptographer* cryptographer = trans.GetCryptographer();
   KeyParams key_params = {"localhost", "dummy", passphrase};
   WriteNode node(&trans);
   if (node.InitByTagLookup(kNigoriTag) != BaseNode::INIT_OK) {
     NOTREACHED();
     return;
   }
 
+  Cryptographer* cryptographer =
+      cryptographer_holder_.GetMutable(trans.GetWrappedTrans());
   if (!cryptographer->has_pending_keys()) {
     // Note that this *can* happen in a rare situation where data is
     // re-encrypted on another client while a SetDecryptionPassphrase() call is
     // in-flight on this client. It is rare enough that we choose to do nothing.
     NOTREACHED() << "Attempt to set decryption passphrase failed because there "
                  << "were no pending keys.";
     return;
   }
 
   bool nigori_has_explicit_passphrase =
@@ skipping 104 matching lines @@
          "reencrypting.";
 
   FinishSetPassphrase(success,
                       bootstrap_token,
                       nigori_has_explicit_passphrase,
                       &trans,
                       &node);
 }
 
 void SyncEncryptionHandlerImpl::EnableEncryptEverything() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  WriteTransaction trans(FROM_HERE, user_share_);
+  ModelTypeSet* encrypted_types =
+      encrypted_types_holder_.GetMutable(trans.GetWrappedTrans());
   if (encrypt_everything_) {
-    DCHECK(encrypted_types_.Equals(ModelTypeSet::All()));
+    DCHECK(encrypted_types->Equals(ModelTypeSet::All()));
     return;
   }
-  WriteTransaction trans(FROM_HERE, user_share_);
+  DVLOG(1) << "Enabling encrypt everything.";
   encrypt_everything_ = true;
   // Change |encrypted_types_| directly to avoid sending more than one
   // notification.
-  encrypted_types_ = ModelTypeSet::All();
+  *encrypted_types = ModelTypeSet::All();
   FOR_EACH_OBSERVER(
       Observer, observers_,
-      OnEncryptedTypesChanged(encrypted_types_, encrypt_everything_));
+      OnEncryptedTypesChanged(*encrypted_types, encrypt_everything_));
   WriteEncryptionStateToNigori(&trans);
-  ReEncryptEverything(&trans);
+  if (cryptographer_holder_.Get(trans.GetWrappedTrans()).is_ready())
+    ReEncryptEverything(&trans);
 }
 
 bool SyncEncryptionHandlerImpl::EncryptEverythingEnabled() const {
-  ReadTransaction trans(FROM_HERE, user_share_);
+  DCHECK(thread_checker_.CalledOnValidThread());
   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_;
 }
 
+// Note: this is called from within a syncable transaction, so we need to post
+// tasks if we want to do any work that creates a new sync_api transaction.
+void SyncEncryptionHandlerImpl::ApplyNigoriUpdate(
+    const sync_pb::NigoriSpecifics& nigori,
+    syncable::BaseTransaction* const trans) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(trans);
+  if (!ApplyNigoriUpdateImpl(nigori, trans)) {
+    MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&SyncEncryptionHandlerImpl::RewriteNigori,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  FOR_EACH_OBSERVER(
+      SyncEncryptionHandler::Observer,
+      observers_,
+      OnCryptographerStateChanged(

[tim (not reviewing), 2012/08/23 18:38:10]

Should we bubble the trans up through this observer? To make it obvious, and to
avoid observers shooting themselves in the foot (by doing something like we
cleverly avoid above by posting a task)?

[Nicolas Zea, 2012/08/23 22:49:32]

We'd have to do that with most other observer methods too. I think it's just
safer to state up front that all observer methods are assumed to be called while
a transaction is open. I don't feel too strongly either way though.

+          cryptographer_holder_.GetMutable(trans)));
+}
+
+void SyncEncryptionHandlerImpl::UpdateNigoriFromEncryptedTypes(
+    sync_pb::NigoriSpecifics* nigori,
+    syncable::BaseTransaction* const trans) const {
+  syncable::UpdateNigoriFromEncryptedTypes(encrypted_types_holder_.Get(trans),
+                                           encrypt_everything_,
+                                           nigori);
+}
+
+ModelTypeSet SyncEncryptionHandlerImpl::GetEncryptedTypes(
+    syncable::BaseTransaction* const trans) const {
+  return encrypted_types_holder_.Get(trans);
+}
+
 // This function iterates over all encrypted types.  There are many scenarios in
 // which data for some or all types is not currently available.  In that case,
 // the lookup of the root node will fail and we will skip encryption for that
 // type.
 void SyncEncryptionHandlerImpl::ReEncryptEverything(
     WriteTransaction* trans) {
-  Cryptographer* cryptographer = trans->GetCryptographer();
-  if (!cryptographer->is_ready())
-    return;
-  ModelTypeSet encrypted_types = GetEncryptedTypes();
-  for (ModelTypeSet::Iterator iter = encrypted_types.First();
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(cryptographer_holder_.Get(trans->GetWrappedTrans()).is_ready());
+  for (ModelTypeSet::Iterator iter = encrypted_types_holder_.Get(
+           trans->GetWrappedTrans()).First();
        iter.Good(); iter.Inc()) {
     if (iter.Get() == PASSWORDS || iter.Get() == NIGORI)
       continue; // These types handle encryption differently.
 
     ReadNode type_root(trans);
     std::string tag = ModelTypeToRootTag(iter.Get());
     if (type_root.InitByTagLookup(tag) != BaseNode::INIT_OK)
       continue; // Don't try to reencrypt if the type's data is unavailable.
 
     // Iterate through all children of this datatype.
@@ skipping 34 matching lines @@
       WriteNode child(trans);
       if (child.InitByIdLookup(child_id) != BaseNode::INIT_OK) {
         NOTREACHED();
         return;
       }
       child.SetPasswordSpecifics(child.GetPasswordSpecifics());
       child_id = child.GetSuccessorId();
     }
   }
 
+  DVLOG(1) << "Re-encrypt everything complete.";
+
   // NOTE: We notify from within a transaction.
   FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
                     OnEncryptionComplete());
 }
 
 bool SyncEncryptionHandlerImpl::ApplyNigoriUpdateImpl(
     const sync_pb::NigoriSpecifics& nigori,
     syncable::BaseTransaction* const trans) {
-  Cryptographer* cryptographer = trans->directory()->GetCryptographer(trans);
-  bool nigori_types_need_update = !UpdateEncryptedTypesFromNigori(nigori);
+  DCHECK(thread_checker_.CalledOnValidThread());
+  bool nigori_types_need_update = !UpdateEncryptedTypesFromNigori(nigori,
+                                                                  trans);
   if (nigori.using_explicit_passphrase())
     explicit_passphrase_ = true;
 
+  Cryptographer* cryptographer = cryptographer_holder_.GetMutable(trans);
   bool nigori_needs_new_keys = false;
   if (!nigori.encrypted().blob().empty()) {
     if (cryptographer->CanDecrypt(nigori.encrypted())) {
       cryptographer->InstallKeys(nigori.encrypted());
       // We only update the default passphrase if this was a new explicit
       // passphrase. Else, since it was decryptable, it must not have been a new
       // key.
       if (nigori.using_explicit_passphrase())
         cryptographer->SetDefaultKey(nigori.encrypted().key_name());
 
@@ skipping 34 matching lines @@
   if (nigori.using_explicit_passphrase() != explicit_passphrase_ ||
       nigori.encrypt_everything() != encrypt_everything_ ||
       nigori_types_need_update ||
       nigori_needs_new_keys) {
     return false;
   }
   return true;
 }
 
 void SyncEncryptionHandlerImpl::RewriteNigori() {
+  DVLOG(1) << "Overwriting stale nigori node.";
+  DCHECK(thread_checker_.CalledOnValidThread());
   WriteTransaction trans(FROM_HERE, user_share_);
   WriteEncryptionStateToNigori(&trans);
 }
 
 void SyncEncryptionHandlerImpl::WriteEncryptionStateToNigori(
     WriteTransaction* trans) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   WriteNode nigori_node(trans);
   // This can happen in tests that don't have nigori nodes.
-  if (!nigori_node.InitByTagLookup(kNigoriTag) == BaseNode::INIT_OK)
+  if (nigori_node.InitByTagLookup(kNigoriTag) != BaseNode::INIT_OK)
     return;
   sync_pb::NigoriSpecifics nigori = nigori_node.GetNigoriSpecifics();
-  Cryptographer* cryptographer = trans->GetCryptographer();
-  if (cryptographer->is_ready() &&
+  const Cryptographer& cryptographer = cryptographer_holder_.Get(
+      trans->GetWrappedTrans());
+  if (cryptographer.is_ready() &&
       nigori_overwrite_count_ < kNigoriOverwriteLimit) {
     // Does not modify the encrypted blob if the unencrypted data already
     // matches what is about to be written.
     sync_pb::EncryptedData original_keys = nigori.encrypted();
-    if (!cryptographer->GetKeys(nigori.mutable_encrypted()))
+    if (!cryptographer.GetKeys(nigori.mutable_encrypted()))
       NOTREACHED();
 
     if (nigori.encrypted().SerializeAsString() !=
         original_keys.SerializeAsString()) {
       // We've updated the nigori node's encryption keys. In order to prevent
       // a possible looping of two clients constantly overwriting each other,
       // we limit the absolute number of overwrites per client instantiation.
       nigori_overwrite_count_++;
       UMA_HISTOGRAM_COUNTS("Sync.AutoNigoriOverwrites",
                            nigori_overwrite_count_);
     }
 
     // Note: we don't try to set using_explicit_passphrase here since if that
     // is lost the user can always set it again. The main point is to preserve
     // the encryption keys so all data remains decryptable.
   }
-  syncable::UpdateNigoriFromEncryptedTypes(encrypted_types_,
-                                           encrypt_everything_,
-                                           &nigori);
+  syncable::UpdateNigoriFromEncryptedTypes(
+      encrypted_types_holder_.Get(trans->GetWrappedTrans()),
+      encrypt_everything_,
+      &nigori);
 
   // If nothing has changed, this is a no-op.
   nigori_node.SetNigoriSpecifics(nigori);
 }
 
 bool SyncEncryptionHandlerImpl::UpdateEncryptedTypesFromNigori(
-    const sync_pb::NigoriSpecifics& nigori) {
+    const sync_pb::NigoriSpecifics& nigori,
+    syncable::BaseTransaction* const trans) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  ModelTypeSet* encrypted_types = encrypted_types_holder_.GetMutable(trans);
   if (nigori.encrypt_everything()) {
     if (!encrypt_everything_) {
       encrypt_everything_ = true;
-      encrypted_types_ = ModelTypeSet::All();
+      *encrypted_types = ModelTypeSet::All();
+      DVLOG(1) << "Enabling encrypt everything via nigori node update";
       FOR_EACH_OBSERVER(
           Observer, observers_,
-          OnEncryptedTypesChanged(encrypted_types_, encrypt_everything_));
+          OnEncryptedTypesChanged(*encrypted_types, encrypt_everything_));
     }
-    DCHECK(encrypted_types_.Equals(ModelTypeSet::All()));
+    DCHECK(encrypted_types->Equals(ModelTypeSet::All()));
     return true;
   }
 
-  ModelTypeSet encrypted_types;
-  encrypted_types = syncable::GetEncryptedTypesFromNigori(nigori);
-  encrypted_types.PutAll(SensitiveTypes());
+  ModelTypeSet nigori_encrypted_types;
+  nigori_encrypted_types = syncable::GetEncryptedTypesFromNigori(nigori);
+  nigori_encrypted_types.PutAll(SensitiveTypes());
 
   // If anything more than the sensitive types were encrypted, and
   // encrypt_everything is not explicitly set to false, we assume it means
   // a client intended to enable encrypt everything.
   if (!nigori.has_encrypt_everything() &&
-      !Difference(encrypted_types, SensitiveTypes()).Empty()) {
+      !Difference(nigori_encrypted_types, SensitiveTypes()).Empty()) {
     if (!encrypt_everything_) {
       encrypt_everything_ = true;
-      encrypted_types_ = ModelTypeSet::All();
+      *encrypted_types = ModelTypeSet::All();
       FOR_EACH_OBSERVER(
           Observer, observers_,
-          OnEncryptedTypesChanged(encrypted_types_, encrypt_everything_));
+          OnEncryptedTypesChanged(*encrypted_types, encrypt_everything_));
     }
-    DCHECK(encrypted_types_.Equals(ModelTypeSet::All()));
+    DCHECK(encrypted_types->Equals(ModelTypeSet::All()));
     return false;
   }
 
-  MergeEncryptedTypes(encrypted_types);
-  return encrypted_types_.Equals(encrypted_types);
-}
-
-void SyncEncryptionHandlerImpl::UpdateNigoriFromEncryptedTypes(
-    sync_pb::NigoriSpecifics* nigori,
-    syncable::BaseTransaction* const trans) const {
-  syncable::UpdateNigoriFromEncryptedTypes(encrypted_types_,
-                                           encrypt_everything_,
-                                           nigori);
+  MergeEncryptedTypes(nigori_encrypted_types, trans);
+  return encrypted_types->Equals(nigori_encrypted_types);
 }
 
 void SyncEncryptionHandlerImpl::FinishSetPassphrase(
     bool success,
     const std::string& bootstrap_token,
     bool is_explicit,
     WriteTransaction* trans,
     WriteNode* nigori_node) {
-  Cryptographer* cryptographer = trans->GetCryptographer();
-  FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
-                    OnCryptographerStateChanged(cryptographer));
+  DCHECK(thread_checker_.CalledOnValidThread());
+  FOR_EACH_OBSERVER(
+      SyncEncryptionHandler::Observer,
+      observers_,
+      OnCryptographerStateChanged(
+          cryptographer_holder_.GetMutable(trans->GetWrappedTrans())));
 
   // It's possible we need to change the bootstrap token even if we failed to
   // set the passphrase (for example if we need to preserve the new GAIA
   // passphrase).
   if (!bootstrap_token.empty()) {
     DVLOG(1) << "Bootstrap token updated.";
     FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
                       OnBootstrapTokenUpdated(bootstrap_token));
   }
 
+  const Cryptographer& cryptographer =
+      cryptographer_holder_.Get(trans->GetWrappedTrans());
   if (!success) {
-    if (cryptographer->is_ready()) {
+    if (cryptographer.is_ready()) {
       LOG(ERROR) << "Attempt to change passphrase failed while cryptographer "
                  << "was ready.";
-    } else if (cryptographer->has_pending_keys()) {
+    } else if (cryptographer.has_pending_keys()) {
       FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
                         OnPassphraseRequired(REASON_DECRYPTION,
-                                             cryptographer->GetPendingKeys()));
+                                             cryptographer.GetPendingKeys()));
     } else {
       FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
                         OnPassphraseRequired(REASON_ENCRYPTION,
                                              sync_pb::EncryptedData()));
     }
     return;
   }
 
   FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
                     OnPassphraseAccepted());
-  DCHECK(cryptographer->is_ready());
+  DCHECK(cryptographer.is_ready());
 
   sync_pb::NigoriSpecifics specifics(nigori_node->GetNigoriSpecifics());
   // Does not modify specifics.encrypted() if the original decrypted data was
   // the same.
-  if (!cryptographer->GetKeys(specifics.mutable_encrypted())) {
+  if (!cryptographer.GetKeys(specifics.mutable_encrypted()))
     NOTREACHED();
-    return;
-  }
   explicit_passphrase_ = is_explicit;
   specifics.set_using_explicit_passphrase(is_explicit);
   nigori_node->SetNigoriSpecifics(specifics);
 
-  // Does nothing if everything is already encrypted or the cryptographer has
-  // pending keys.
+  // Does nothing if everything is already encrypted.
   ReEncryptEverything(trans);
 }
 
 void SyncEncryptionHandlerImpl::MergeEncryptedTypes(
-    ModelTypeSet encrypted_types) {
-  if (!encrypted_types_.HasAll(encrypted_types)) {
-    encrypted_types_ = encrypted_types;
+    ModelTypeSet new_encrypted_types,
+    syncable::BaseTransaction* const trans) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  ModelTypeSet* encrypted_types = encrypted_types_holder_.GetMutable(trans);
+  if (!encrypted_types->HasAll(new_encrypted_types)) {
+    *encrypted_types = new_encrypted_types;
     FOR_EACH_OBSERVER(
         Observer, observers_,
-        OnEncryptedTypesChanged(encrypted_types_, encrypt_everything_));
+        OnEncryptedTypesChanged(*encrypted_types, encrypt_everything_));
   }
 }
 
 }  // namespace browser_sync