[Sync] Refactor GetEncryptedTypes usage.

The cryptographer is now owned by the SyncEncryptionHandlerImpl, and
no longer needs any connection to the nigori handler or encrypted types.
Those are accessed via the directory, which now has a GetNigoriHandler
method.

Because of this, we can now clean up the thread safety guarantees in the
sync encryption handler impl, enforcing that everything except
GetEncryptedTypes (and IsUsingExplicitPassphrase, which will be fixed in a
followup patch) are invoked on the syncer thread. This lets us not have to
open unnecessary transactions.

At the chrome layer, encrypted types are accessed via BaseTransaction::
GetEncryptedTypes(). At the syncer layer, they're accessed via directory::
GetNigoriHandler()->GetEncryptedTypes(BaseTransaction* trans const).

BUG=142476,139848


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

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

4 files changed, 31 insertions, 28 deletions

diff --git a/sync/test/engine/test_directory_setter_upper.cc b/sync/test/engine/test_directory_setter_upper.cc
index 1a68389..318900d 100644
--- a/sync/test/engine/test_directory_setter_upper.cc
+++ b/sync/test/engine/test_directory_setter_upper.cc
@@ -23,8 +23,12 @@ TestDirectorySetterUpper::TestDirectorySetterUpper() : name_("Test") {}
 TestDirectorySetterUpper::~TestDirectorySetterUpper() {}
 
 void TestDirectorySetterUpper::SetUp() {
-  directory_.reset(new syncable::Directory(&encryptor_, &handler_, NULL,
-      new syncable::InMemoryDirectoryBackingStore(name_)));
+  directory_.reset(new syncable::Directory(
+      new syncable::InMemoryDirectoryBackingStore(name_),
+      &handler_,
+      NULL,
+      &encryption_handler_,
+      encryption_handler_.cryptographer()));
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
   ASSERT_EQ(syncable::OPENED, directory_->Open(
           name_, &delegate_, NullTransactionObserver()));
diff --git a/sync/test/engine/test_directory_setter_upper.h b/sync/test/engine/test_directory_setter_upper.h
index b492064..5dbce2d 100644
--- a/sync/test/engine/test_directory_setter_upper.h
+++ b/sync/test/engine/test_directory_setter_upper.h
@@ -35,7 +35,7 @@
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/scoped_temp_dir.h"
-#include "sync/test/fake_encryptor.h"
+#include "sync/test/fake_sync_encryption_handler.h"
 #include "sync/test/null_directory_change_delegate.h"
 #include "sync/util/test_unrecoverable_error_handler.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -61,6 +61,8 @@ class TestDirectorySetterUpper {
 
   syncable::Directory* directory() { return directory_.get(); }
 
+  SyncEncryptionHandler* encryption_handler() { return &encryption_handler_; }
+
  private:
   syncable::NullDirectoryChangeDelegate delegate_;
   TestUnrecoverableErrorHandler handler_;
@@ -68,7 +70,7 @@ class TestDirectorySetterUpper {
   void RunInvariantCheck();
 
   ScopedTempDir temp_dir_;
-  FakeEncryptor encryptor_;
+  FakeSyncEncryptionHandler encryption_handler_;
   scoped_ptr<syncable::Directory> directory_;
   std::string name_;
 
diff --git a/sync/test/fake_sync_encryption_handler.cc b/sync/test/fake_sync_encryption_handler.cc
index b490862..c589558 100644
--- a/sync/test/fake_sync_encryption_handler.cc
+++ b/sync/test/fake_sync_encryption_handler.cc
@@ -6,7 +6,6 @@
 
 #include "sync/protocol/nigori_specifics.pb.h"
 #include "sync/syncable/nigori_util.h"
-#include "sync/util/cryptographer.h"
 
 namespace syncer {
 
@@ -14,7 +13,7 @@ FakeSyncEncryptionHandler::FakeSyncEncryptionHandler()
     : encrypted_types_(SensitiveTypes()),
       encrypt_everything_(false),
       explicit_passphrase_(false),
-      cryptographer_(NULL) {
+      cryptographer_(&encryptor_) {
 }
 FakeSyncEncryptionHandler::~FakeSyncEncryptionHandler() {}
 
@@ -30,21 +29,18 @@ void FakeSyncEncryptionHandler::ApplyNigoriUpdate(
   if (nigori.using_explicit_passphrase())
     explicit_passphrase_ = true;
 
-  if (!cryptographer_)
-    return;
-
-  if (cryptographer_->CanDecrypt(nigori.encrypted()))
-    cryptographer_->InstallKeys(nigori.encrypted());
-  else
-    cryptographer_->SetPendingKeys(nigori.encrypted());
+  if (cryptographer_.CanDecrypt(nigori.encrypted()))
+    cryptographer_.InstallKeys(nigori.encrypted());
+  else if (nigori.has_encrypted())
+    cryptographer_.SetPendingKeys(nigori.encrypted());
 
-  if (cryptographer_->has_pending_keys()) {
+  if (cryptographer_.has_pending_keys()) {
     DVLOG(1) << "OnPassPhraseRequired Sent";
-    sync_pb::EncryptedData pending_keys = cryptographer_->GetPendingKeys();
+    sync_pb::EncryptedData pending_keys = cryptographer_.GetPendingKeys();
     FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
                       OnPassphraseRequired(REASON_DECRYPTION,
                                            pending_keys));
-  } else if (!cryptographer_->is_ready()) {
+  } else if (!cryptographer_.is_ready()) {
     DVLOG(1) << "OnPassphraseRequired sent because cryptographer is not "
              << "ready";
     FOR_EACH_OBSERVER(SyncEncryptionHandler::Observer, observers_,
@@ -61,6 +57,11 @@ void FakeSyncEncryptionHandler::UpdateNigoriFromEncryptedTypes(
                                            nigori);
 }
 
+ModelTypeSet FakeSyncEncryptionHandler::GetEncryptedTypes(
+    syncable::BaseTransaction* const trans) const {
+  return encrypted_types_;
+}
+
 void FakeSyncEncryptionHandler::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);
 }
@@ -95,10 +96,6 @@ bool FakeSyncEncryptionHandler::EncryptEverythingEnabled() const {
   return encrypt_everything_;
 }
 
-ModelTypeSet FakeSyncEncryptionHandler::GetEncryptedTypes() const {
-  return encrypted_types_;
-}
-
 bool FakeSyncEncryptionHandler::IsUsingExplicitPassphrase() const {
   return explicit_passphrase_;
 }
diff --git a/sync/test/fake_sync_encryption_handler.h b/sync/test/fake_sync_encryption_handler.h
index 83a2e63..7456a08 100644
--- a/sync/test/fake_sync_encryption_handler.h
+++ b/sync/test/fake_sync_encryption_handler.h
@@ -11,11 +11,11 @@
 #include "base/observer_list.h"
 #include "sync/internal_api/public/sync_encryption_handler.h"
 #include "sync/syncable/nigori_handler.h"
+#include "sync/test/fake_encryptor.h"
+#include "sync/util/cryptographer.h"
 
 namespace syncer {
 
-class Cryptographer;
-
 // A fake sync encryption handler capable of keeping track of the encryption
 // state without opening any transactions or interacting with the nigori node.
 // Note that this only performs basic interactions with the cryptographer
@@ -28,10 +28,6 @@ class FakeSyncEncryptionHandler : public SyncEncryptionHandler,
   FakeSyncEncryptionHandler();
   virtual ~FakeSyncEncryptionHandler();
 
-  void set_cryptographer(Cryptographer* cryptographer) {
-    cryptographer_ = cryptographer;
-  }
-
   // SyncEncryptionHandler implementation.
   virtual void AddObserver(Observer* observer) OVERRIDE;
   virtual void RemoveObserver(Observer* observer) OVERRIDE;
@@ -47,10 +43,13 @@ class FakeSyncEncryptionHandler : public SyncEncryptionHandler,
   virtual void ApplyNigoriUpdate(
       const sync_pb::NigoriSpecifics& nigori,
       syncable::BaseTransaction* const trans) OVERRIDE;
-  virtual ModelTypeSet GetEncryptedTypes() const OVERRIDE;
   virtual void UpdateNigoriFromEncryptedTypes(
       sync_pb::NigoriSpecifics* nigori,
       syncable::BaseTransaction* const trans) const OVERRIDE;
+  virtual ModelTypeSet GetEncryptedTypes(
+      syncable::BaseTransaction* const trans) const OVERRIDE;
+
+  Cryptographer* cryptographer() { return &cryptographer_; }
 
  private:
   ObserverList<SyncEncryptionHandler::Observer> observers_;
@@ -58,7 +57,8 @@ class FakeSyncEncryptionHandler : public SyncEncryptionHandler,
   bool encrypt_everything_;
   bool explicit_passphrase_;
 
-  Cryptographer* cryptographer_;
+  FakeEncryptor encryptor_;
+  Cryptographer cryptographer_;
 };
 
 }  // namespace syncer