Revert of sync: Add non-blocking type encryption support...

Revert of sync: Add non-blocking type encryption support (https://codereview.chromium.org/423193002/)

Reason for revert:
Both LSAN and Valgrind complains about leaks in the tests.

Original issue's description:
> sync: Add non-blocking type encryption support
> 
> Introduces the framework for dealing with sync encryption in
> non-blocking types.  Unlike directory sync types, non-blocking type
> encryption only encrypts data before it is sent to the server.
> Encrypting the data on-disk is a separate problem.
> 
> Adds code to the ModelTypeSyncWorker so it can access the directory's
> cryptographer (through a CryptographerProvider interface) and use it to
> encrypt entities before it sends them to the server.  If the
> cryptographer is unable to encrypt with the desired key, the worker will
> not commit until the cryptographer returns to a good state.
> 
> Adds the concept of a "desired encryption key" to the data type state.
> When the cryptographer key to be used to encrypt a type changes, this
> will be reflected in the data type state.  The ModelTypeSyncProxy is
> responsible for ensuring that all items which have not yet been
> encrypted with this desired key are enqueued for commit.
> 
> Makes the ModelTypeSyncWorker, EntityTracker, and ModelTypeSyncProxy
> collaborate on the management of undecryptable (inapplicable) updates.
> The EntityTracker keeps track of their version numbers and content, and
> prevents the committing of new items to the server until the
> inapplicable update has been dealt with.  The ModelTypeSyncProxy is
> responsible for saving inapplicable updates across restarts.
> 
> This CL alone is not enough to enable encryption support for
> non-blocking types.  It requires additional code to hook up the
> ModelTypeSyncWorkers to receive cryptographer events.  This will be
> added in a future commit.  In the meantime, this CL includes plenty
> of unit tests to verify the functionality that's being added.
> 
> BUG=351005
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287428

TBR=zea@chromium.org,stanisc@chromium.org,rlarocque@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=351005

Review URL: https://codereview.chromium.org/442623002

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

components/sync_driver/non_blocking_data_type_controller_unittest.cc

@@ -85,7 +85,6 @@ class MockSyncContextProxy : public syncer::SyncContextProxy {
   virtual void ConnectTypeToSync(
       syncer::ModelType type,
       const syncer::DataTypeState& data_type_state,
-      const syncer::UpdateResponseDataList& saved_pending_updates,
       const base::WeakPtr<syncer::ModelTypeSyncProxyImpl>& type_proxy)
       OVERRIDE {
     // Normally we'd use MessageLoopProxy::current() as the TaskRunner argument

sync/engine/entity_tracker.cc

@@ -193,14 +193,8 @@ void EntityTracker::ReceiveCommitResponse(const std::string& response_id,
 }
 
 void EntityTracker::ReceiveUpdate(int64 version) {
-  if (version <= highest_gu_response_version_)
-    return;
-
-  highest_gu_response_version_ = version;
-
-  // Got an applicable update newer than any pending updates.  It must be safe
-  // to discard the old pending update, if there was one.
-  ClearPendingUpdate();
+  highest_gu_response_version_ =
+      std::max(highest_gu_response_version_, version);
 
   if (IsInConflict()) {
     // Incoming update clobbers the pending commit on the sync thread.
@@ -209,35 +203,10 @@ void EntityTracker::ReceiveUpdate(int64 version) {
   }
 }
 
-bool EntityTracker::ReceivePendingUpdate(const UpdateResponseData& data) {
-  if (data.response_version < highest_gu_response_version_)
-    return false;
-
-  highest_gu_response_version_ = data.response_version;
-  pending_update_.reset(new UpdateResponseData(data));
-  ClearPendingCommit();
-  return true;
-}
-
-bool EntityTracker::HasPendingUpdate() const {
-  return !!pending_update_;
-}
-
-UpdateResponseData EntityTracker::GetPendingUpdate() const {
-  return *pending_update_;
-}
-
-void EntityTracker::ClearPendingUpdate() {
-  pending_update_.reset();
-}
-
 bool EntityTracker::IsInConflict() const {
   if (!is_commit_pending_)
     return false;
 
-  if (HasPendingUpdate())
-    return true;
-
   if (highest_gu_response_version_ <= highest_commit_response_version_) {
     // The most recent server state was created in a commit made by this
     // client.  We're fully up to date, and therefore not in conflict.

sync/engine/entity_tracker.h

@@ -8,10 +8,8 @@
 #include <string>
 
 #include "base/basictypes.h"
-#include "base/memory/scoped_ptr.h"
 #include "base/time/time.h"
 #include "sync/base/sync_export.h"
-#include "sync/internal_api/public/non_blocking_sync_common.h"
 #include "sync/protocol/sync.pb.h"
 
 namespace syncer {
@@ -83,20 +81,6 @@ class SYNC_EXPORT EntityTracker {
   // Handles receipt of an update from the server.
   void ReceiveUpdate(int64 version);
 
-  // Handles the receipt of an pending update from the server.
-  //
-  // Returns true if the tracker decides this item is worth keeping.  Returns
-  // false if the item is discarded, which could happen if the version number
-  // is out of date.
-  bool ReceivePendingUpdate(const UpdateResponseData& data);
-
-  // Functions to fetch the latest pending update.
-  bool HasPendingUpdate() const;
-  UpdateResponseData GetPendingUpdate() const;
-
-  // Clears the pending update.  Allows us to resume regular commit behavior.
-  void ClearPendingUpdate();
-
  private:
   // Initializes received update state.  Does not initialize state related to
   // pending commits and sets |is_commit_pending_| to false.
@@ -162,11 +146,6 @@ class SYNC_EXPORT EntityTracker {
   bool deleted_;
   sync_pb::EntitySpecifics specifics_;
 
-  // An update for this item which can't be applied right now.  The presence of
-  // an pending update prevents commits.  As of this writing, the only source
-  // of pending updates is updates we can't decrypt right now.
-  scoped_ptr<UpdateResponseData> pending_update_;
-
   DISALLOW_COPY_AND_ASSIGN(EntityTracker);
 };
 

sync/engine/model_type_entity.cc

@@ -24,8 +24,7 @@ scoped_ptr<ModelTypeEntity> ModelTypeEntity::NewLocalItem(
       specifics,
       false,
       now,
-      now,
-      std::string()));
+      now));
 }
 
 scoped_ptr<ModelTypeEntity> ModelTypeEntity::FromServerUpdate(
@@ -36,8 +35,7 @@ scoped_ptr<ModelTypeEntity> ModelTypeEntity::FromServerUpdate(
     const sync_pb::EntitySpecifics& specifics,
     bool deleted,
     base::Time ctime,
-    base::Time mtime,
-    const std::string& encryption_key_name) {
+    base::Time mtime) {
   return scoped_ptr<ModelTypeEntity>(new ModelTypeEntity(0,
                                                          0,
                                                          0,
@@ -49,8 +47,7 @@ scoped_ptr<ModelTypeEntity> ModelTypeEntity::FromServerUpdate(
                                                          specifics,
                                                          deleted,
                                                          ctime,
-                                                         mtime,
-                                                         encryption_key_name));
+                                                         mtime));
 }
 
 ModelTypeEntity::ModelTypeEntity(int64 sequence_number,
@@ -64,8 +61,7 @@ ModelTypeEntity::ModelTypeEntity(int64 sequence_number,
                                  const sync_pb::EntitySpecifics& specifics,
                                  bool deleted,
                                  base::Time ctime,
-                                 base::Time mtime,
-                                 const std::string& encryption_key_name)
+                                 base::Time mtime)
     : sequence_number_(sequence_number),
       commit_requested_sequence_number_(commit_requested_sequence_number),
       acked_sequence_number_(acked_sequence_number),
@@ -77,8 +73,7 @@ ModelTypeEntity::ModelTypeEntity(int64 sequence_number,
       specifics_(specifics),
       deleted_(deleted),
       ctime_(ctime),
-      mtime_(mtime),
-      encryption_key_name_(encryption_key_name) {
+      mtime_(mtime) {
 }
 
 ModelTypeEntity::~ModelTypeEntity() {
@@ -108,8 +103,7 @@ void ModelTypeEntity::ApplyUpdateFromServer(
     int64 update_version,
     bool deleted,
     const sync_pb::EntitySpecifics& specifics,
-    base::Time mtime,
-    const std::string& encryption_key_name) {
+    base::Time mtime) {
   // There was a conflict and the server just won it.
   // This implicitly acks all outstanding commits because a received update
   // will clobber any pending commits on the sync thread.
@@ -127,15 +121,6 @@ void ModelTypeEntity::MakeLocalChange(
   specifics_ = specifics;
 }
 
-void ModelTypeEntity::UpdateDesiredEncryptionKey(const std::string& name) {
-  if (encryption_key_name_ == name)
-    return;
-
-  // Schedule commit with the expectation that the worker will re-encrypt with
-  // the latest encryption key as it does.
-  sequence_number_++;
-}
-
 void ModelTypeEntity::Delete() {
   sequence_number_++;
   specifics_.Clear();
@@ -159,15 +144,12 @@ void ModelTypeEntity::SetCommitRequestInProgress() {
   commit_requested_sequence_number_ = sequence_number_;
 }
 
-void ModelTypeEntity::ReceiveCommitResponse(
-    const std::string& id,
-    int64 sequence_number,
-    int64 response_version,
-    const std::string& encryption_key_name) {
+void ModelTypeEntity::ReceiveCommitResponse(const std::string& id,
+                                            int64 sequence_number,
+                                            int64 response_version) {
   id_ = id;  // The server can assign us a new ID in a commit response.
   acked_sequence_number_ = sequence_number;
   base_version_ = response_version;
-  encryption_key_name_ = encryption_key_name;
 }
 
 void ModelTypeEntity::ClearTransientSyncState() {

sync/engine/model_type_entity.h

@@ -46,8 +46,7 @@ class SYNC_EXPORT_PRIVATE ModelTypeEntity {
       const sync_pb::EntitySpecifics& specifics,
       bool deleted,
       base::Time ctime,
-      base::Time mtime,
-      const std::string& encryption_key_name);
+      base::Time mtime);
 
   // TODO(rlarocque): Implement FromDisk constructor when we implement storage.
 
@@ -80,17 +79,11 @@ class SYNC_EXPORT_PRIVATE ModelTypeEntity {
   void ApplyUpdateFromServer(int64 update_version,
                              bool deleted,
                              const sync_pb::EntitySpecifics& specifics,
-                             base::Time mtime,
-                             const std::string& encryption_key_name);
+                             base::Time mtime);
 
   // Applies a local change to this item.
   void MakeLocalChange(const sync_pb::EntitySpecifics& specifics);
 
-  // Schedule a commit if the |name| does not match this item's last known
-  // encryption key.  The worker that performs the commit is expected to
-  // encrypt the item using the latest available key.
-  void UpdateDesiredEncryptionKey(const std::string& name);
-
   // Applies a local deletion to this item.
   void Delete();
 
@@ -111,8 +104,7 @@ class SYNC_EXPORT_PRIVATE ModelTypeEntity {
   // reached the server.
   void ReceiveCommitResponse(const std::string& id,
                              int64 sequence_number,
-                             int64 response_version,
-                             const std::string& encryption_key_name);
+                             int64 response_version);
 
   // Clears any in-memory sync state associated with outstanding commits.
   void ClearTransientSyncState();
@@ -132,8 +124,7 @@ class SYNC_EXPORT_PRIVATE ModelTypeEntity {
                   const sync_pb::EntitySpecifics& specifics,
                   bool deleted,
                   base::Time ctime,
-                  base::Time mtime,
-                  const std::string& encryption_key_name);
+                  base::Time mtime);
 
   // A sequence number used to track in-progress commits.  Each local change
   // increments this number.
@@ -194,10 +185,6 @@ class SYNC_EXPORT_PRIVATE ModelTypeEntity {
   // doesn't bother to inspect their values.
   base::Time ctime_;
   base::Time mtime_;
-
-  // The name of the encryption key used to encrypt this item on the server.
-  // Empty when no encryption is in use.
-  std::string encryption_key_name_;
 };
 
 }  // namespace syncer

sync/engine/model_type_entity_unittest.cc

@@ -64,8 +64,7 @@ TEST_F(ModelTypeEntityTest, FromServerUpdate) {
                                         specifics,
                                         false,
                                         kCtime,
-                                        kMtime,
-                                        std::string()));
+                                        kMtime));
 
   EXPECT_TRUE(entity->IsWriteRequired());
   EXPECT_FALSE(entity->IsUnsynced());
@@ -88,8 +87,7 @@ TEST_F(ModelTypeEntityTest, TombstoneUpdate) {
                                         sync_pb::EntitySpecifics(),
                                         true,
                                         kCtime,
-                                        kMtime,
-                                        std::string()));
+                                        kMtime));
 
   EXPECT_TRUE(entity->IsWriteRequired());
   EXPECT_FALSE(entity->IsUnsynced());
@@ -109,15 +107,13 @@ TEST_F(ModelTypeEntityTest, ApplyUpdate) {
                                         specifics,
                                         false,
                                         kCtime,
-                                        kMtime,
-                                        std::string()));
+                                        kMtime));
 
   // A deletion update one version later.
   entity->ApplyUpdateFromServer(11,
                                 true,
                                 sync_pb::EntitySpecifics(),
-                                kMtime + base::TimeDelta::FromSeconds(10),
-                                std::string());
+                                kMtime + base::TimeDelta::FromSeconds(10));
 
   EXPECT_TRUE(entity->IsWriteRequired());
   EXPECT_FALSE(entity->IsUnsynced());
@@ -134,8 +130,7 @@ TEST_F(ModelTypeEntityTest, LocalChange) {
                                         specifics,
                                         false,
                                         kCtime,
-                                        kMtime,
-                                        std::string()));
+                                        kMtime));
 
   sync_pb::EntitySpecifics specifics2;
   specifics2.CopyFrom(specifics);
@@ -161,8 +156,7 @@ TEST_F(ModelTypeEntityTest, LocalDeletion) {
                                         specifics,
                                         false,
                                         kCtime,
-                                        kMtime,
-                                        std::string()));
+                                        kMtime));
 
   entity->Delete();
 

sync/engine/model_type_sync_proxy.h

@@ -21,8 +21,7 @@ class SYNC_EXPORT_PRIVATE ModelTypeSyncProxy {
       const CommitResponseDataList& response_list) = 0;
   virtual void OnUpdateReceived(
       const DataTypeState& type_state,
-      const UpdateResponseDataList& response_list,
-      const UpdateResponseDataList& pending_updates) = 0;
+      const UpdateResponseDataList& response_list) = 0;
 };
 
 }  // namespace syncer

sync/engine/model_type_sync_proxy_impl.cc

@@ -18,7 +18,6 @@ ModelTypeSyncProxyImpl::ModelTypeSyncProxyImpl(ModelType type)
       is_preferred_(false),
       is_connected_(false),
       entities_deleter_(&entities_),
-      pending_updates_map_deleter_(&pending_updates_map_),
       weak_ptr_factory_for_ui_(this),
       weak_ptr_factory_for_sync_(this) {
 }
@@ -52,12 +51,10 @@ void ModelTypeSyncProxyImpl::Enable(
   data_type_state_.progress_marker.set_data_type_id(
       GetSpecificsFieldNumberFromModelType(type_));
 
-  UpdateResponseDataList saved_pending_updates = GetPendingUpdates();
   sync_context_proxy_ = sync_context_proxy.Pass();
   sync_context_proxy_->ConnectTypeToSync(
       GetModelType(),
       data_type_state_,
-      saved_pending_updates,
       weak_ptr_factory_for_sync_.GetWeakPtr());
 }
 
@@ -183,18 +180,16 @@ void ModelTypeSyncProxyImpl::OnCommitCompleted(
     } else {
       it->second->ReceiveCommitResponse(response_data.id,
                                         response_data.sequence_number,
-                                        response_data.response_version,
-                                        data_type_state_.encryption_key_name);
+                                        response_data.response_version);
     }
   }
 }
 
 void ModelTypeSyncProxyImpl::OnUpdateReceived(
     const DataTypeState& data_type_state,
-    const UpdateResponseDataList& response_list,
-    const UpdateResponseDataList& pending_updates) {
-  bool got_new_encryption_requirements = data_type_state_.encryption_key_name !=
-                                         data_type_state.encryption_key_name;
+    const UpdateResponseDataList& response_list) {
+  bool initial_sync_just_finished =
+      !data_type_state_.initial_sync_done && data_type_state.initial_sync_done;
 
   data_type_state_ = data_type_state;
 
@@ -204,14 +199,6 @@ void ModelTypeSyncProxyImpl::OnUpdateReceived(
     const UpdateResponseData& response_data = *list_it;
     const std::string& client_tag_hash = response_data.client_tag_hash;
 
-    UpdateMap::iterator old_it = pending_updates_map_.find(client_tag_hash);
-    if (old_it != pending_updates_map_.end()) {
-      // If we're being asked to apply an update to this entity, this overrides
-      // the previous pending updates.
-      delete old_it->second;
-      pending_updates_map_.erase(old_it);
-    }
-
     EntityMap::iterator it = entities_.find(client_tag_hash);
     if (it == entities_.end()) {
       scoped_ptr<ModelTypeEntity> entity =
@@ -222,74 +209,22 @@ void ModelTypeSyncProxyImpl::OnUpdateReceived(
                                             response_data.specifics,
                                             response_data.deleted,
                                             response_data.ctime,
-                                            response_data.mtime,
-                                            response_data.encryption_key_name);
+                                            response_data.mtime);
       entities_.insert(std::make_pair(client_tag_hash, entity.release()));
     } else {
       ModelTypeEntity* entity = it->second;
       entity->ApplyUpdateFromServer(response_data.response_version,
                                     response_data.deleted,
                                     response_data.specifics,
-                                    response_data.mtime,
-                                    response_data.encryption_key_name);
-
+                                    response_data.mtime);
       // TODO: Do something special when conflicts are detected.
     }
-
-    // If the received entity has out of date encryption, we schedule another
-    // commit to fix it.
-    if (data_type_state_.encryption_key_name !=
-        response_data.encryption_key_name) {
-      EntityMap::iterator it2 = entities_.find(client_tag_hash);
-      it2->second->UpdateDesiredEncryptionKey(
-          data_type_state_.encryption_key_name);
-    }
   }
 
-  // Save pending updates in the appropriate data structure.
-  for (UpdateResponseDataList::const_iterator list_it = pending_updates.begin();
-       list_it != pending_updates.end();
-       ++list_it) {
-    const UpdateResponseData& update = *list_it;
-    const std::string& client_tag_hash = update.client_tag_hash;
-
-    UpdateMap::iterator lookup_it = pending_updates_map_.find(client_tag_hash);
-    if (lookup_it == pending_updates_map_.end()) {
-      pending_updates_map_.insert(
-          std::make_pair(client_tag_hash, new UpdateResponseData(update)));
-    } else if (lookup_it->second->response_version <= update.response_version) {
-      delete lookup_it->second;
-      pending_updates_map_.erase(lookup_it);
-      pending_updates_map_.insert(
-          std::make_pair(client_tag_hash, new UpdateResponseData(update)));
-    } else {
-      // Received update is stale, do not overwrite existing.
-    }
-  }
-
-  if (got_new_encryption_requirements) {
-    for (EntityMap::iterator it = entities_.begin(); it != entities_.end();
-         ++it) {
-      it->second->UpdateDesiredEncryptionKey(
-          data_type_state_.encryption_key_name);
-    }
-  }
-
-  // We may have new reasons to commit by the time this function is done.
-  FlushPendingCommitRequests();
+  if (initial_sync_just_finished)
+    FlushPendingCommitRequests();
 
   // TODO: Inform the model of the new or updated data.
-  // TODO: Persist the new data on disk.
-}
-
-UpdateResponseDataList ModelTypeSyncProxyImpl::GetPendingUpdates() {
-  UpdateResponseDataList pending_updates_list;
-  for (UpdateMap::const_iterator it = pending_updates_map_.begin();
-       it != pending_updates_map_.end();
-       ++it) {
-    pending_updates_list.push_back(*it->second);
-  }
-  return pending_updates_list;
 }
 
 void ModelTypeSyncProxyImpl::ClearTransientSyncState() {
@@ -304,7 +239,7 @@ void ModelTypeSyncProxyImpl::ClearSyncState() {
        ++it) {
     it->second->ClearSyncState();
   }
-  STLDeleteValues(&pending_updates_map_);
+
   data_type_state_ = DataTypeState();
 }
 

sync/engine/model_type_sync_proxy_impl.h

@@ -73,16 +73,7 @@ class SYNC_EXPORT_PRIVATE ModelTypeSyncProxyImpl : base::NonThreadSafe {
   // Informs this object that there are some incoming updates is should
   // handle.
   void OnUpdateReceived(const DataTypeState& type_state,
-                        const UpdateResponseDataList& response_list,
-                        const UpdateResponseDataList& pending_updates);
-
-  // Returns the list of pending updates.
-  //
-  // This is used as a helper function, but it's public mainly for testing.
-  // The current test harness setup doesn't allow us to test the data that the
-  // proxy sends to the worker during initialization, so we use this to inspect
-  // its state instead.
-  UpdateResponseDataList GetPendingUpdates();
+                        const UpdateResponseDataList& response_list);
 
   // Returns the long-lived WeakPtr that is intended to be registered with the
   // ProfileSyncService.
@@ -90,7 +81,6 @@ class SYNC_EXPORT_PRIVATE ModelTypeSyncProxyImpl : base::NonThreadSafe {
 
  private:
   typedef std::map<std::string, ModelTypeEntity*> EntityMap;
-  typedef std::map<std::string, UpdateResponseData*> UpdateMap;
 
   // Sends all commit requests that are due to be sent to the sync thread.
   void FlushPendingCommitRequests();
@@ -133,12 +123,6 @@ class SYNC_EXPORT_PRIVATE ModelTypeSyncProxyImpl : base::NonThreadSafe {
   EntityMap entities_;
   STLValueDeleter<EntityMap> entities_deleter_;
 
-  // A set of updates that can not be applied at this time.  These are never
-  // used by the model.  They are kept here only so we can save and restore
-  // them across restarts, and keep them in sync with our progress markers.
-  UpdateMap pending_updates_map_;
-  STLValueDeleter<UpdateMap> pending_updates_map_deleter_;
-
   // We use two different WeakPtrFactories because we want the pointers they
   // issue to have different lifetimes.  When asked to disconnect from the sync
   // thread, we want to make sure that no tasks generated as part of the

sync/engine/model_type_sync_worker_impl.h

@@ -10,13 +10,11 @@
 #include "base/threading/non_thread_safe.h"
 #include "sync/base/sync_export.h"
 #include "sync/engine/commit_contributor.h"
-#include "sync/engine/cryptographer_provider.h"
 #include "sync/engine/model_type_sync_worker.h"
 #include "sync/engine/nudge_handler.h"
 #include "sync/engine/update_handler.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/non_blocking_sync_common.h"
-#include "sync/internal_api/public/sync_encryption_handler.h"
 #include "sync/protocol/sync.pb.h"
 
 namespace base {
@@ -55,19 +53,12 @@ class SYNC_EXPORT ModelTypeSyncWorkerImpl : public UpdateHandler,
  public:
   ModelTypeSyncWorkerImpl(ModelType type,
                           const DataTypeState& initial_state,
-                          const UpdateResponseDataList& saved_pending_updates,
-                          CryptographerProvider* cryptographer_provider,
                           NudgeHandler* nudge_handler,
                           scoped_ptr<ModelTypeSyncProxy> type_sync_proxy);
   virtual ~ModelTypeSyncWorkerImpl();
 
   ModelType GetModelType() const;
 
-  bool IsEncryptionRequired() const;
-  void SetEncryptionKeyName(const std::string& name);
-
-  void OnCryptographerStateChanged();
-
   // UpdateHandler implementation.
   virtual void GetDownloadProgress(
       sync_pb::DataTypeProgressMarker* progress_marker) const OVERRIDE;
@@ -96,45 +87,20 @@ class SYNC_EXPORT ModelTypeSyncWorkerImpl : public UpdateHandler,
 
  private:
   typedef std::map<std::string, EntityTracker*> EntityMap;
-  typedef std::map<std::string, UpdateResponseData*> UpdateMap;
 
   // Stores a single commit request in this object's internal state.
   void StorePendingCommit(const CommitRequestData& request);
 
-  // Returns true if this type has successfully fetched all available updates
-  // from the server at least once.  Our state may or may not be stale, but at
-  // least we know that it was valid at some point in the past.
-  bool IsTypeInitialized() const;
-
-  // Returns true if this type is prepared to commit items.  Currently, this
-  // depends on having downloaded the initial data and having the encryption
-  // settings in a good state.
-  bool CanCommitItems(Cryptographer* cryptographer) const;
+  // Returns true if all data type state required for commits is available.  In
+  // practice, this means that it returns true from the time this object first
+  // receives notice of a successful update fetch from the server.
+  bool CanCommitItems() const;
 
   // Initializes the parts of a commit entity that are the responsibility of
   // this class, and not the EntityTracker.  Some fields, like the
   // client-assigned ID, can only be set by an entity with knowledge of the
   // entire data type's state.
-  void HelpInitializeCommitEntity(Cryptographer* cryptographer,
-                                  sync_pb::SyncEntity* commit_entity);
-
-  // Attempts to decrypt pending updates stored in the EntityMap.  If
-  // successful, will remove the update from the its EntityTracker and forward
-  // it to the proxy thread for application.
-  void TryDecryptPendingUpdates();
-
-  // Attempts to decrypt the given specifics and return them in the |out|
-  // parameter.  Assumes cryptographer->CanDecrypt(specifics) returned true.
-  //
-  // Returns false if the decryption failed.  There are no guarantees about the
-  // contents of |out| when that happens.
-  //
-  // In theory, this should never fail.  Only corrupt or invalid entries could
-  // cause this to fail, and no clients are known to create such entries.  The
-  // failure case is an attempt to be defensive against bad input.
-  static bool DecryptSpecifics(Cryptographer* cryptographer,
-                               const sync_pb::EntitySpecifics& in,
-                               sync_pb::EntitySpecifics* out);
+  void HelpInitializeCommitEntity(sync_pb::SyncEntity* commit_entity);
 
   ModelType type_;
 
@@ -145,10 +111,6 @@ class SYNC_EXPORT ModelTypeSyncWorkerImpl : public UpdateHandler,
   // This is NULL when no proxy is connected..
   scoped_ptr<ModelTypeSyncProxy> type_sync_proxy_;
 
-  // A helper to provide access to the syncable::Directory's cryptographer.
-  // Not owned.
-  CryptographerProvider* cryptographer_provider_;
-
   // Interface used to access and send nudges to the sync scheduler.  Not owned.
   NudgeHandler* nudge_handler_;
 

sync/internal_api/public/non_blocking_sync_common.h

@@ -38,11 +38,6 @@ struct SYNC_EXPORT_PRIVATE DataTypeState {
   // until the first download cycle has completed.
   std::string type_root_id;
 
-  // This value is set if this type's data should be encrypted on the server.
-  // If this key changes, the client will need to re-commit all of its local
-  // data to the server using the new encryption key.
-  std::string encryption_key_name;
-
   // A strictly increasing counter used to generate unique values for the
   // client-assigned IDs.  The incrementing and ID assignment happens on the
   // sync thread, but we store the value here so we can pass it back to the
@@ -56,6 +51,7 @@ struct SYNC_EXPORT_PRIVATE DataTypeState {
   // flag is set.
   bool initial_sync_done;
 };
+
 struct SYNC_EXPORT_PRIVATE CommitRequestData {
   CommitRequestData();
   ~CommitRequestData();
@@ -98,7 +94,6 @@ struct SYNC_EXPORT_PRIVATE UpdateResponseData {
   std::string non_unique_name;
   bool deleted;
   sync_pb::EntitySpecifics specifics;
-  std::string encryption_key_name;
 };
 
 typedef std::vector<CommitRequestData> CommitRequestDataList;

sync/internal_api/public/sync_context.h

@@ -10,11 +10,11 @@
 #include "base/sequenced_task_runner.h"
 #include "sync/base/sync_export.h"
 #include "sync/internal_api/public/base/model_type.h"
-#include "sync/internal_api/public/non_blocking_sync_common.h"
 
 namespace syncer {
 
 class ModelTypeSyncProxyImpl;
+struct DataTypeState;
 
 // An interface of the core parts of sync.
 //
@@ -35,7 +35,6 @@ class SYNC_EXPORT_PRIVATE SyncContext {
   virtual void ConnectSyncTypeToWorker(
       syncer::ModelType type,
       const DataTypeState& data_type_state,
-      const syncer::UpdateResponseDataList& saved_pending_updates,
       const scoped_refptr<base::SequencedTaskRunner>& datatype_task_runner,
       const base::WeakPtr<ModelTypeSyncProxyImpl>& type_sync_proxy) = 0;
 

sync/internal_api/public/sync_context_proxy.h

@@ -7,7 +7,6 @@
 
 #include "base/memory/weak_ptr.h"
 #include "sync/internal_api/public/base/model_type.h"
-#include "sync/internal_api/public/non_blocking_sync_common.h"
 
 namespace syncer {
 
@@ -28,7 +27,6 @@ class SYNC_EXPORT_PRIVATE SyncContextProxy {
   virtual void ConnectTypeToSync(
       syncer::ModelType type,
       const DataTypeState& data_type_state,
-      const UpdateResponseDataList& saved_pending_updates,
       const base::WeakPtr<ModelTypeSyncProxyImpl>& type_sync_proxy) = 0;
 
   // Tells the syncer that we're no longer interested in syncing this type.

sync/internal_api/public/test/null_sync_context_proxy.h

@@ -6,7 +6,6 @@
 #define SYNC_INTERNAL_API_PUBLIC_TEST_NULL_SYNC_CONTEXT_PROXY_H_
 
 #include "base/memory/weak_ptr.h"
-#include "sync/internal_api/public/non_blocking_sync_common.h"
 #include "sync/internal_api/public/sync_context_proxy.h"
 
 namespace syncer {
@@ -24,7 +23,6 @@ class NullSyncContextProxy : public SyncContextProxy {
   virtual void ConnectTypeToSync(
       syncer::ModelType type,
       const DataTypeState& data_type_state,
-      const UpdateResponseDataList& saved_pending_updates,
       const base::WeakPtr<ModelTypeSyncProxyImpl>& type_sync_proxy) OVERRIDE;
   virtual void Disconnect(syncer::ModelType type) OVERRIDE;
   virtual scoped_ptr<SyncContextProxy> Clone() const OVERRIDE;

sync/internal_api/sync_context_proxy_impl.cc

@@ -25,7 +25,6 @@ SyncContextProxyImpl::~SyncContextProxyImpl() {
 void SyncContextProxyImpl::ConnectTypeToSync(
     ModelType type,
     const DataTypeState& data_type_state,
-    const UpdateResponseDataList& saved_pending_updates,
     const base::WeakPtr<ModelTypeSyncProxyImpl>& type_sync_proxy) {
   VLOG(1) << "ConnectTypeToSync: " << ModelTypeToString(type);
   sync_task_runner_->PostTask(FROM_HERE,
@@ -33,7 +32,6 @@ void SyncContextProxyImpl::ConnectTypeToSync(
                                          sync_context_,
                                          type,
                                          data_type_state,
-                                         saved_pending_updates,
                                          base::ThreadTaskRunnerHandle::Get(),
                                          type_sync_proxy));
 }

sync/internal_api/sync_context_proxy_impl.h

@@ -40,7 +40,6 @@ class SYNC_EXPORT_PRIVATE SyncContextProxyImpl : public SyncContextProxy {
   virtual void ConnectTypeToSync(
       syncer::ModelType type,
       const DataTypeState& data_type_state,
-      const UpdateResponseDataList& pending_updates,
       const base::WeakPtr<ModelTypeSyncProxyImpl>& sync_proxy_impl) OVERRIDE;
 
   // Disables syncing for the given type on the sync thread.

sync/internal_api/sync_encryption_handler_impl.cc

@@ -1519,7 +1519,7 @@ bool SyncEncryptionHandlerImpl::GetKeystoreDecryptor(
   DCHECK(!keystore_key.empty());
   DCHECK(cryptographer.is_ready());
   std::string serialized_nigori;
-  serialized_nigori = cryptographer.GetDefaultNigoriKeyData();
+  serialized_nigori = cryptographer.GetDefaultNigoriKey();
   if (serialized_nigori.empty()) {
     LOG(ERROR) << "Failed to get cryptographer bootstrap token.";
     return false;

sync/internal_api/test/null_sync_context_proxy.cc

@@ -15,7 +15,6 @@ NullSyncContextProxy::~NullSyncContextProxy() {
 void NullSyncContextProxy::ConnectTypeToSync(
     syncer::ModelType type,
     const DataTypeState& data_type_state,
-    const UpdateResponseDataList& saved_pending_updates,
     const base::WeakPtr<ModelTypeSyncProxyImpl>& type_sync_proxy) {
   NOTREACHED() << "NullSyncContextProxy is not meant to be used";
 }

sync/sessions/model_type_registry.cc

@@ -15,7 +15,6 @@
 #include "sync/engine/model_type_sync_worker_impl.h"
 #include "sync/internal_api/public/non_blocking_sync_common.h"
 #include "sync/sessions/directory_type_debug_info_emitter.h"
-#include "sync/util/cryptographer.h"
 
 namespace syncer {
 
@@ -33,8 +32,7 @@ class ModelTypeSyncProxyWrapper : public ModelTypeSyncProxy {
       const CommitResponseDataList& response_list) OVERRIDE;
   virtual void OnUpdateReceived(
       const DataTypeState& type_state,
-      const UpdateResponseDataList& response_list,
-      const UpdateResponseDataList& pending_updates) OVERRIDE;
+      const UpdateResponseDataList& response_list) OVERRIDE;
 
  private:
   base::WeakPtr<ModelTypeSyncProxyImpl> processor_;
@@ -63,15 +61,13 @@ void ModelTypeSyncProxyWrapper::OnCommitCompleted(
 
 void ModelTypeSyncProxyWrapper::OnUpdateReceived(
     const DataTypeState& type_state,
-    const UpdateResponseDataList& response_list,
-    const UpdateResponseDataList& pending_updates) {
+    const UpdateResponseDataList& response_list) {
   processor_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&ModelTypeSyncProxyImpl::OnUpdateReceived,
                  processor_,
                  type_state,
-                 response_list,
-                 pending_updates));
+                 response_list));
 }
 
 class ModelTypeSyncWorkerWrapper : public ModelTypeSyncWorker {
@@ -111,7 +107,6 @@ ModelTypeRegistry::ModelTypeRegistry(
     syncable::Directory* directory,
     NudgeHandler* nudge_handler)
     : directory_(directory),
-      cryptographer_provider_(directory_),
       nudge_handler_(nudge_handler),
       weak_ptr_factory_(this) {
   for (size_t i = 0u; i < workers.size(); ++i) {
@@ -190,7 +185,6 @@ void ModelTypeRegistry::SetEnabledDirectoryTypes(
 void ModelTypeRegistry::ConnectSyncTypeToWorker(
     ModelType type,
     const DataTypeState& data_type_state,
-    const UpdateResponseDataList& saved_pending_updates,
     const scoped_refptr<base::SequencedTaskRunner>& type_task_runner,
     const base::WeakPtr<ModelTypeSyncProxyImpl>& proxy_impl) {
   DVLOG(1) << "Enabling an off-thread sync type: " << ModelTypeToString(type);
@@ -198,13 +192,8 @@ void ModelTypeRegistry::ConnectSyncTypeToWorker(
   // Initialize Worker -> Proxy communication channel.
   scoped_ptr<ModelTypeSyncProxy> proxy(
       new ModelTypeSyncProxyWrapper(proxy_impl, type_task_runner));
-  scoped_ptr<ModelTypeSyncWorkerImpl> worker(
-      new ModelTypeSyncWorkerImpl(type,
-                                  data_type_state,
-                                  saved_pending_updates,
-                                  &cryptographer_provider_,
-                                  nudge_handler_,
-                                  proxy.Pass()));
+  scoped_ptr<ModelTypeSyncWorkerImpl> worker(new ModelTypeSyncWorkerImpl(
+      type, data_type_state, nudge_handler_, proxy.Pass()));
 
   // Initialize Proxy -> Worker communication channel.
   scoped_ptr<ModelTypeSyncWorker> wrapped_worker(

sync/sessions/model_type_registry.h

@@ -12,11 +12,9 @@
 #include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "sync/base/sync_export.h"
-#include "sync/engine/directory_cryptographer_provider.h"
 #include "sync/engine/nudge_handler.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/engine/model_safe_worker.h"
-#include "sync/internal_api/public/non_blocking_sync_common.h"
 #include "sync/internal_api/public/sessions/type_debug_info_observer.h"
 #include "sync/internal_api/public/sync_context.h"
 
@@ -33,6 +31,7 @@ class DirectoryTypeDebugInfoEmitter;
 class ModelTypeSyncWorkerImpl;
 class ModelTypeSyncProxyImpl;
 class UpdateHandler;
+struct DataTypeState;
 
 typedef std::map<ModelType, UpdateHandler*> UpdateHandlerMap;
 typedef std::map<ModelType, CommitContributor*> CommitContributorMap;
@@ -58,7 +57,6 @@ class SYNC_EXPORT_PRIVATE ModelTypeRegistry : public SyncContext {
   virtual void ConnectSyncTypeToWorker(
       syncer::ModelType type,
       const DataTypeState& data_type_state,
-      const syncer::UpdateResponseDataList& saved_pending_updates,
       const scoped_refptr<base::SequencedTaskRunner>& type_task_runner,
       const base::WeakPtr<ModelTypeSyncProxyImpl>& proxy) OVERRIDE;
 
@@ -114,9 +112,6 @@ class SYNC_EXPORT_PRIVATE ModelTypeRegistry : public SyncContext {
   // The directory.  Not owned.
   syncable::Directory* directory_;
 
-  // Provides access to the Directory's cryptographer.
-  DirectoryCryptographerProvider cryptographer_provider_;
-
   // The NudgeHandler.  Not owned.
   NudgeHandler* nudge_handler_;
 

sync/sessions/model_type_registry_unittest.cc

@@ -154,7 +154,6 @@ TEST_F(ModelTypeRegistryTest, NonBlockingTypes) {
 
   registry()->ConnectSyncTypeToWorker(syncer::THEMES,
                                       MakeInitialDataTypeState(THEMES),
-                                      UpdateResponseDataList(),
                                       task_runner,
                                       themes_sync_proxy.AsWeakPtrForUI());
   EXPECT_TRUE(registry()->GetEnabledTypes().Equals(
@@ -162,7 +161,6 @@ TEST_F(ModelTypeRegistryTest, NonBlockingTypes) {
 
   registry()->ConnectSyncTypeToWorker(syncer::SESSIONS,
                                       MakeInitialDataTypeState(SESSIONS),
-                                      UpdateResponseDataList(),
                                       task_runner,
                                       sessions_sync_proxy.AsWeakPtrForUI());
   EXPECT_TRUE(registry()->GetEnabledTypes().Equals(
@@ -194,7 +192,6 @@ TEST_F(ModelTypeRegistryTest, NonBlockingTypesWithDirectoryTypes) {
   // Add the themes non-blocking type.
   registry()->ConnectSyncTypeToWorker(syncer::THEMES,
                                       MakeInitialDataTypeState(THEMES),
-                                      UpdateResponseDataList(),
                                       task_runner,
                                       themes_sync_proxy.AsWeakPtrForUI());
   current_types.Put(syncer::THEMES);
@@ -208,7 +205,6 @@ TEST_F(ModelTypeRegistryTest, NonBlockingTypesWithDirectoryTypes) {
   // Add sessions non-blocking type.
   registry()->ConnectSyncTypeToWorker(syncer::SESSIONS,
                                       MakeInitialDataTypeState(SESSIONS),
-                                      UpdateResponseDataList(),
                                       task_runner,
                                       sessions_sync_proxy.AsWeakPtrForUI());
   current_types.Put(syncer::SESSIONS);
@@ -239,12 +235,10 @@ TEST_F(ModelTypeRegistryTest, DeletionOrdering) {
 
   registry()->ConnectSyncTypeToWorker(syncer::THEMES,
                                       MakeInitialDataTypeState(THEMES),
-                                      UpdateResponseDataList(),
                                       task_runner,
                                       themes_sync_proxy->AsWeakPtrForUI());
   registry()->ConnectSyncTypeToWorker(syncer::SESSIONS,
                                       MakeInitialDataTypeState(SESSIONS),
-                                      UpdateResponseDataList(),
                                       task_runner,
                                       sessions_sync_proxy->AsWeakPtrForUI());
   EXPECT_TRUE(registry()->GetEnabledTypes().Equals(

sync/test/engine/injectable_sync_context_proxy.cc

@@ -20,7 +20,6 @@ InjectableSyncContextProxy::~InjectableSyncContextProxy() {
 void InjectableSyncContextProxy::ConnectTypeToSync(
     syncer::ModelType type,
     const DataTypeState& data_type_state,
-    const UpdateResponseDataList& response_list,
     const base::WeakPtr<syncer::ModelTypeSyncProxyImpl>& type_sync_proxy) {
   // This class is allowed to participate in only one connection.
   DCHECK(!is_worker_connected_);

sync/test/engine/injectable_sync_context_proxy.h

@@ -6,7 +6,6 @@
 #define SYNC_TEST_ENGINE_INJECTABLE_SYNC_CONTEXT_PROXY_H_
 
 #include "sync/internal_api/public/base/model_type.h"
-#include "sync/internal_api/public/non_blocking_sync_common.h"
 #include "sync/internal_api/public/sync_context_proxy.h"
 
 namespace syncer {
@@ -25,7 +24,6 @@ class InjectableSyncContextProxy : public syncer::SyncContextProxy {
   virtual void ConnectTypeToSync(
       syncer::ModelType type,
       const DataTypeState& data_type_state,
-      const UpdateResponseDataList& pending_updates,
       const base::WeakPtr<syncer::ModelTypeSyncProxyImpl>& type_sync_proxy)
       OVERRIDE;
   virtual void Disconnect(syncer::ModelType type) OVERRIDE;

sync/test/engine/mock_model_type_sync_proxy.cc

@@ -29,13 +29,11 @@ void MockModelTypeSyncProxy::OnCommitCompleted(
 
 void MockModelTypeSyncProxy::OnUpdateReceived(
     const DataTypeState& type_state,
-    const UpdateResponseDataList& response_list,
-    const UpdateResponseDataList& pending_updates) {
+    const UpdateResponseDataList& response_list) {
   base::Closure task = base::Bind(&MockModelTypeSyncProxy::OnUpdateReceivedImpl,
                                   base::Unretained(this),
                                   type_state,
-                                  response_list,
-                                  pending_updates);
+                                  response_list);
   pending_tasks_.push_back(task);
   if (is_synchronous_)
     RunQueuedTasks();
@@ -113,12 +111,6 @@ UpdateResponseDataList MockModelTypeSyncProxy::GetNthUpdateResponse(
   return received_update_responses_[n];
 }
 
-UpdateResponseDataList MockModelTypeSyncProxy::GetNthPendingUpdates(
-    size_t n) const {
-  DCHECK_LT(n, GetNumUpdateResponses());
-  return received_pending_updates_[n];
-}
-
 DataTypeState MockModelTypeSyncProxy::GetNthTypeStateReceivedInUpdateResponse(
     size_t n) const {
   DCHECK_LT(n, GetNumUpdateResponses());
@@ -189,10 +181,8 @@ void MockModelTypeSyncProxy::OnCommitCompletedImpl(
 
 void MockModelTypeSyncProxy::OnUpdateReceivedImpl(
     const DataTypeState& type_state,
-    const UpdateResponseDataList& response_list,
-    const UpdateResponseDataList& pending_updates) {
+    const UpdateResponseDataList& response_list) {
   received_update_responses_.push_back(response_list);
-  received_pending_updates_.push_back(pending_updates);
   type_states_received_on_update_.push_back(type_state);
   for (UpdateResponseDataList::const_iterator it = response_list.begin();
        it != response_list.end();

sync/test/engine/mock_model_type_sync_proxy.h

@@ -36,8 +36,7 @@ class MockModelTypeSyncProxy : public ModelTypeSyncProxy {
       const CommitResponseDataList& response_list) OVERRIDE;
   virtual void OnUpdateReceived(
       const DataTypeState& type_state,
-      const UpdateResponseDataList& response_list,
-      const UpdateResponseDataList& pending_updates) OVERRIDE;
+      const UpdateResponseDataList& response_list) OVERRIDE;
 
   // By default, this object behaves as if all messages are processed
   // immediately.  Sometimes it is useful to defer work until later, as might
@@ -66,7 +65,6 @@ class MockModelTypeSyncProxy : public ModelTypeSyncProxy {
   // Does not includes repsonses that are in pending tasks.
   size_t GetNumUpdateResponses() const;
   UpdateResponseDataList GetNthUpdateResponse(size_t n) const;
-  UpdateResponseDataList GetNthPendingUpdates(size_t n) const;
   DataTypeState GetNthTypeStateReceivedInUpdateResponse(size_t n) const;
 
   // Getters to access the log of received commit responses.
@@ -95,8 +93,7 @@ class MockModelTypeSyncProxy : public ModelTypeSyncProxy {
   //
   // Implemented as an Impl method so we can defer its execution in some cases.
   void OnUpdateReceivedImpl(const DataTypeState& type_state,
-                            const UpdateResponseDataList& response_list,
-                            const UpdateResponseDataList& pending_updates);
+                            const UpdateResponseDataList& response_list);
 
   // Getter and setter for per-item sequence number tracking.
   int64 GetCurrentSequenceNumber(const std::string& tag_hash) const;
@@ -119,7 +116,6 @@ class MockModelTypeSyncProxy : public ModelTypeSyncProxy {
   // A log of messages received by this object.
   std::vector<CommitResponseDataList> received_commit_responses_;
   std::vector<UpdateResponseDataList> received_update_responses_;
-  std::vector<UpdateResponseDataList> received_pending_updates_;
   std::vector<DataTypeState> type_states_received_on_update_;
   std::vector<DataTypeState> type_states_received_on_commit_;
 

sync/test/engine/mock_model_type_sync_worker.cc

@@ -94,8 +94,6 @@ UpdateResponseData MockModelTypeSyncWorker::UpdateFromServer(
   data.mtime = data.ctime + base::TimeDelta::FromSeconds(version);
   data.non_unique_name = specifics.preference().name();
 
-  data.encryption_key_name = server_encryption_key_name_;
-
   return data;
 }
 
@@ -120,8 +118,6 @@ UpdateResponseData MockModelTypeSyncWorker::TombstoneFromServer(
   data.mtime = data.ctime + base::TimeDelta::FromSeconds(version);
   data.non_unique_name = "Name Non Unique";
 
-  data.encryption_key_name = server_encryption_key_name_;
-
   return data;
 }
 
@@ -153,11 +149,6 @@ CommitResponseData MockModelTypeSyncWorker::SuccessfulCommitResponse(
   return response_data;
 }
 
-void MockModelTypeSyncWorker::SetServerEncryptionKey(
-    const std::string& key_name) {
-  server_encryption_key_name_ = key_name;
-}
-
 std::string MockModelTypeSyncWorker::GenerateId(const std::string& tag_hash) {
   return "FakeId:" + tag_hash;
 }

sync/test/engine/mock_model_type_sync_worker.h

@@ -57,11 +57,6 @@ class MockModelTypeSyncWorker : public ModelTypeSyncWorker {
   CommitResponseData SuccessfulCommitResponse(
       const CommitRequestData& request_data);
 
-  // Sets the encryption key name used for updates from the server.
-  // (ie. the key other clients are using to encrypt their commits.)
-  // The default value is an empty string, which indicates no encryption.
-  void SetServerEncryptionKey(const std::string& key_name);
-
  private:
   // Generate an ID string.
   static std::string GenerateId(const std::string& tag_hash);
@@ -77,9 +72,6 @@ class MockModelTypeSyncWorker : public ModelTypeSyncWorker {
   // This is an essential part of the mocked server state.
   std::map<const std::string, int64> server_versions_;
 
-  // Name of the encryption key in use on other clients.
-  std::string server_encryption_key_name_;
-
   DISALLOW_COPY_AND_ASSIGN(MockModelTypeSyncWorker);
 };
 

sync/util/cryptographer.cc

@@ -251,7 +251,7 @@ bool Cryptographer::DecryptPendingKeys(const KeyParams& params) {
 
 bool Cryptographer::GetBootstrapToken(std::string* token) const {
   DCHECK(token);
-  std::string unencrypted_token = GetDefaultNigoriKeyData();
+  std::string unencrypted_token = GetDefaultNigoriKey();
   if (unencrypted_token.empty())
     return false;
 
@@ -324,11 +324,7 @@ bool Cryptographer::KeybagIsStale(
   return false;
 }
 
-std::string Cryptographer::GetDefaultNigoriKeyName() const {
-  return default_nigori_name_;
-}
-
-std::string Cryptographer::GetDefaultNigoriKeyData() const {
+std::string Cryptographer::GetDefaultNigoriKey() const {
   if (!is_initialized())
     return std::string();
   NigoriMap::const_iterator iter = nigoris_.find(default_nigori_name_);

sync/util/cryptographer.h

@@ -176,12 +176,9 @@ class SYNC_EXPORT Cryptographer {
   // and/or has a different default key.
   bool KeybagIsStale(const sync_pb::EncryptedData& keybag) const;
 
-  // Returns the name of the Nigori key currently used for encryption.
-  std::string GetDefaultNigoriKeyName() const;
-
   // Returns a serialized sync_pb::NigoriKey version of current default
   // encryption key.
-  std::string GetDefaultNigoriKeyData() const;
+  std::string GetDefaultNigoriKey() const;
 
   // Generates a new Nigori from |serialized_nigori_key|, and if successful
   // installs the new nigori as the default key.