Revert of Keep track of which attachments are referenced by which sync entries. (https://codereview.chromium.org/247983002/)

Reason for revert:
Looks like this may have introduced a memory leak:

http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28valgrind%29%282%29/builds/37232

Reverting.  Will investigate and reland when appropriate.

Original issue's description:
> Keep track of which attachments are referenced by which sync entries.
> 
> PutAttachmentMetadata on MutableEntry now notifies the Directory when
> the attachments associated with an entry change.
> 
> Give Directory::InitializeIndices a ScopedKernelLock to be consistent
> and make it easier to ensure we are locking when we need to.
> 
> GenericChangeProcess passes new attachments to AttachmentService for
> storage and upload.
> 
> Add an output parameter to GenericChangeProcessor's HandleActionAdd and
> HandleActionUpdate methods so they can keep track of potentially new
> attachments and pass them to AttachmentService for storage/upload.
> 
> Convert AttachmentService's OnSyncDataAdd to StoreAttachments.
> 
> Implement HasAttachmentNotOnServer.
> 
> BUG=348625,353303,354530,356266
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=267617

TBR=pavely@chromium.org,tim@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=348625,353303,354530,356266

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

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

27 files changed, 130 insertions, 640 deletions

diff --git a/sync/api/attachments/attachment_id.cc b/sync/api/attachments/attachment_id.cc
index e61705e..6f3e0a2 100644
--- a/sync/api/attachments/attachment_id.cc
+++ b/sync/api/attachments/attachment_id.cc
@@ -5,7 +5,7 @@
 #include "sync/api/attachments/attachment_id.h"
 
 #include "base/logging.h"
-#include "sync/internal_api/public/base/attachment_id_proto.h"
+#include "base/rand_util.h"
 #include "sync/protocol/sync.pb.h"
 
 namespace syncer {
@@ -53,7 +53,10 @@ bool AttachmentId::operator<(const AttachmentId& other) const {
 
 // Static.
 AttachmentId AttachmentId::Create() {
-  sync_pb::AttachmentIdProto proto = CreateAttachmentIdProto();
+  // Only requirement here is that this id must be globally unique.
+  // TODO(maniscalco): Consider making this base64 encoded.
+  sync_pb::AttachmentIdProto proto;
+  proto.set_unique_id(base::RandBytesAsString(16));
   return AttachmentId(&proto);
 }
 
diff --git a/sync/api/attachments/attachment_service.h b/sync/api/attachments/attachment_service.h
index 9e5e25d..81a1a63 100644
--- a/sync/api/attachments/attachment_service.h
+++ b/sync/api/attachments/attachment_service.h
@@ -38,22 +38,11 @@ class SYNC_EXPORT AttachmentService {
   // The result of a DropAttachments operation.
   enum DropResult {
     DROP_SUCCESS,            // No error, all attachments dropped.
-    DROP_UNSPECIFIED_ERROR,  // An unspecified error occurred. Some or all
-                             // attachments may not have been dropped.
+    DROP_UNSPECIFIED_ERROR,  // An unspecified error occurred.
   };
 
   typedef base::Callback<void(const DropResult&)> DropCallback;
 
-  // The result of a StoreAttachments operation.
-  enum StoreResult {
-    STORE_SUCCESS,            // No error, all attachments stored (at least
-                              // locally).
-    STORE_UNSPECIFIED_ERROR,  // An unspecified error occurred. Some or all
-                              // attachments may not have been stored.
-  };
-
-  typedef base::Callback<void(const StoreResult&)> StoreCallback;
-
   AttachmentService();
   virtual ~AttachmentService();
 
@@ -66,12 +55,10 @@ class SYNC_EXPORT AttachmentService {
   virtual void DropAttachments(const AttachmentIdList& attachment_ids,
                                const DropCallback& callback) = 0;
 
-  // Store |attachments| on device and (eventually) upload them to the server.
-  //
-  // Invokes |callback| once the attachments have been written to device
-  // storage.
-  virtual void StoreAttachments(const AttachmentList& attachments,
-                                const StoreCallback& callback) = 0;
+  // This method should be called when a SyncData is about to be added to the
+  // sync database so we have a chance to persist the Attachment locally and
+  // schedule it for upload to the sync server.
+  virtual void OnSyncDataAdd(const SyncData& sync_data) = 0;
 
   // This method should be called when a SyncData is about to be deleted from
   // the sync database so we can remove any unreferenced attachments from local
diff --git a/sync/api/attachments/attachment_service_proxy.cc b/sync/api/attachments/attachment_service_proxy.cc
index 6590033..9bc40e5 100644
--- a/sync/api/attachments/attachment_service_proxy.cc
+++ b/sync/api/attachments/attachment_service_proxy.cc
@@ -35,15 +35,6 @@ void ProxyDropCallback(
   task_runner->PostTask(FROM_HERE, base::Bind(callback, result));
 }
 
-// Invokes |callback| with |result| and |attachments| in the |task_runner|
-// thread.
-void ProxyStoreCallback(
-    const scoped_refptr<base::SequencedTaskRunner>& task_runner,
-    const AttachmentService::StoreCallback& callback,
-    const AttachmentService::StoreResult& result) {
-  task_runner->PostTask(FROM_HERE, base::Bind(callback, result));
-}
-
 }  // namespace
 
 AttachmentServiceProxy::AttachmentServiceProxy() {
@@ -94,17 +85,11 @@ void AttachmentServiceProxy::DropAttachments(
                                             proxy_callback));
 }
 
-void AttachmentServiceProxy::StoreAttachments(const AttachmentList& attachments,
-                                              const StoreCallback& callback) {
+void AttachmentServiceProxy::OnSyncDataAdd(const SyncData& sync_data) {
   DCHECK(wrapped_task_runner_);
-  StoreCallback proxy_callback = base::Bind(
-      &ProxyStoreCallback, base::MessageLoopProxy::current(), callback);
   wrapped_task_runner_->PostTask(
       FROM_HERE,
-      base::Bind(&AttachmentService::StoreAttachments,
-                 core_,
-                 attachments,
-                 proxy_callback));
+      base::Bind(&AttachmentService::OnSyncDataAdd, core_, sync_data));
 }
 
 void AttachmentServiceProxy::OnSyncDataDelete(const SyncData& sync_data) {
@@ -152,13 +137,11 @@ void AttachmentServiceProxy::Core::DropAttachments(
   wrapped_->DropAttachments(attachment_ids, callback);
 }
 
-void AttachmentServiceProxy::Core::StoreAttachments(
-    const AttachmentList& attachments,
-    const StoreCallback& callback) {
+void AttachmentServiceProxy::Core::OnSyncDataAdd(const SyncData& sync_data) {
   if (!wrapped_) {
     return;
   }
-  wrapped_->StoreAttachments(attachments, callback);
+  wrapped_->OnSyncDataAdd(sync_data);
 }
 
 void AttachmentServiceProxy::Core::OnSyncDataDelete(const SyncData& sync_data) {
diff --git a/sync/api/attachments/attachment_service_proxy.h b/sync/api/attachments/attachment_service_proxy.h
index 462cdda..7085625 100644
--- a/sync/api/attachments/attachment_service_proxy.h
+++ b/sync/api/attachments/attachment_service_proxy.h
@@ -58,8 +58,7 @@ class SYNC_EXPORT AttachmentServiceProxy : public AttachmentService {
       const GetOrDownloadCallback& callback) OVERRIDE;
   virtual void DropAttachments(const AttachmentIdList& attachment_ids,
                                const DropCallback& callback) OVERRIDE;
-  virtual void StoreAttachments(const AttachmentList& attachment,
-                                const StoreCallback& callback) OVERRIDE;
+  virtual void OnSyncDataAdd(const SyncData& sync_data) OVERRIDE;
   virtual void OnSyncDataDelete(const SyncData& sync_data) OVERRIDE;
   virtual void OnSyncDataUpdate(const AttachmentIdList& old_attachment_ids,
                                 const SyncData& updated_sync_data) OVERRIDE;
@@ -90,8 +89,7 @@ class SYNC_EXPORT AttachmentServiceProxy : public AttachmentService {
         const GetOrDownloadCallback& callback) OVERRIDE;
     virtual void DropAttachments(const AttachmentIdList& attachment_ids,
                                  const DropCallback& callback) OVERRIDE;
-    virtual void StoreAttachments(const AttachmentList& attachment,
-                                  const StoreCallback& callback) OVERRIDE;
+    virtual void OnSyncDataAdd(const SyncData& sync_data) OVERRIDE;
     virtual void OnSyncDataDelete(const SyncData& sync_data) OVERRIDE;
     virtual void OnSyncDataUpdate(const AttachmentIdList& old_attachment_ids,
                                   const SyncData& updated_sync_data) OVERRIDE;
diff --git a/sync/api/attachments/attachment_service_proxy_unittest.cc b/sync/api/attachments/attachment_service_proxy_unittest.cc
index 08c499e..f6e3502 100644
--- a/sync/api/attachments/attachment_service_proxy_unittest.cc
+++ b/sync/api/attachments/attachment_service_proxy_unittest.cc
@@ -56,12 +56,9 @@ class StubAttachmentService : public AttachmentService,
         FROM_HERE, base::Bind(callback, AttachmentService::DROP_SUCCESS));
   }
 
-  virtual void StoreAttachments(const AttachmentList& attachments,
-                                const StoreCallback& callback) OVERRIDE {
+  virtual void OnSyncDataAdd(const SyncData& sync_data) OVERRIDE {
     CalledOnValidThread();
     Increment();
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE, base::Bind(callback, AttachmentService::STORE_SUCCESS));
   }
 
   virtual void OnSyncDataDelete(const SyncData& sync_data) OVERRIDE {
@@ -122,11 +119,8 @@ class AttachmentServiceProxyTest : public testing::Test,
                    base::Unretained(this));
     callback_drop = base::Bind(&AttachmentServiceProxyTest::IncrementDrop,
                                base::Unretained(this));
-    callback_store = base::Bind(&AttachmentServiceProxyTest::IncrementStore,
-                                base::Unretained(this));
     count_callback_get_or_download = 0;
     count_callback_drop = 0;
-    count_callback_store = 0;
   }
 
   virtual void TearDown()
@@ -153,12 +147,6 @@ class AttachmentServiceProxyTest : public testing::Test,
     ++count_callback_drop;
   }
 
-  // a StoreCallback
-  void IncrementStore(const AttachmentService::StoreResult&) {
-    CalledOnValidThread();
-    ++count_callback_store;
-  }
-
   void WaitForStubThread() {
     base::WaitableEvent done(false, false);
     stub_thread->message_loop()->PostTask(
@@ -177,23 +165,21 @@ class AttachmentServiceProxyTest : public testing::Test,
 
   AttachmentService::GetOrDownloadCallback callback_get_or_download;
   AttachmentService::DropCallback callback_drop;
-  AttachmentService::StoreCallback callback_store;
 
   // number of times callback_get_or_download was invoked
   int count_callback_get_or_download;
   // number of times callback_drop was invoked
   int count_callback_drop;
-  // number of times callback_store was invoked
-  int count_callback_store;
 };
 
 // Verify that each of AttachmentServiceProxy's regular methods (those that
 // don't take callbacks) are invoked on the stub.
 TEST_F(AttachmentServiceProxyTest, MethodsAreProxied) {
+  proxy->OnSyncDataAdd(sync_data);
   proxy->OnSyncDataDelete(sync_data_delete);
   proxy->OnSyncDataUpdate(AttachmentIdList(), sync_data);
   WaitForStubThread();
-  EXPECT_EQ(2, stub->GetCallCount());
+  EXPECT_EQ(3, stub->GetCallCount());
 }
 
 // Verify that each of AttachmentServiceProxy's callback methods (those that
@@ -202,10 +188,9 @@ TEST_F(AttachmentServiceProxyTest, MethodsAreProxied) {
 TEST_F(AttachmentServiceProxyTest, MethodsWithCallbacksAreProxied) {
   proxy->GetOrDownloadAttachments(AttachmentIdList(), callback_get_or_download);
   proxy->DropAttachments(AttachmentIdList(), callback_drop);
-  proxy->StoreAttachments(AttachmentList(), callback_store);
   // Wait for the posted calls to execute in the stub thread.
   WaitForStubThread();
-  EXPECT_EQ(3, stub->GetCallCount());
+  EXPECT_EQ(2, stub->GetCallCount());
   // At this point the stub thread has finished executed the calls. However, the
   // result callbacks it has posted may not have executed yet. Wait a second
   // time to ensure the stub thread has executed the posted result callbacks.
@@ -214,7 +199,6 @@ TEST_F(AttachmentServiceProxyTest, MethodsWithCallbacksAreProxied) {
   loop.RunUntilIdle();
   EXPECT_EQ(1, count_callback_get_or_download);
   EXPECT_EQ(1, count_callback_drop);
-  EXPECT_EQ(1, count_callback_store);
 }
 
 // Verify that it's safe to use an AttachmentServiceProxy even after its wrapped
diff --git a/sync/api/attachments/fake_attachment_service.cc b/sync/api/attachments/fake_attachment_service.cc
index 5cb01e5..b9eb88e 100644
--- a/sync/api/attachments/fake_attachment_service.cc
+++ b/sync/api/attachments/fake_attachment_service.cc
@@ -51,15 +51,10 @@ void FakeAttachmentService::DropAttachments(
                                      callback));
 }
 
-void FakeAttachmentService::StoreAttachments(const AttachmentList& attachments,
-                                             const StoreCallback& callback) {
+void FakeAttachmentService::OnSyncDataAdd(const SyncData& sync_data) {
   DCHECK(CalledOnValidThread());
-  attachment_store_->Write(attachments,
-                           base::Bind(&FakeAttachmentService::WriteDone,
-                                      weak_ptr_factory_.GetWeakPtr(),
-                                      callback));
-  // TODO(maniscalco): Ensure the linked attachments are schedule for upload to
-  // the server (bug 356351).
+  // TODO(maniscalco): Ensure the linked attachments get persisted in local
+  // storage and schedule them for upload to the server (bug 356351).
 }
 
 void FakeAttachmentService::OnSyncDataDelete(const SyncData& sync_data) {
@@ -104,16 +99,4 @@ void FakeAttachmentService::DropDone(const DropCallback& callback,
                                          base::Bind(callback, drop_result));
 }
 
-void FakeAttachmentService::WriteDone(const StoreCallback& callback,
-                                      const AttachmentStore::Result& result) {
-  AttachmentService::StoreResult store_result =
-      AttachmentService::STORE_UNSPECIFIED_ERROR;
-  if (result == AttachmentStore::SUCCESS) {
-    store_result = AttachmentService::STORE_SUCCESS;
-  }
-  // TODO(maniscalco): Deal with case where an error occurred (bug 361251).
-  base::MessageLoop::current()->PostTask(FROM_HERE,
-                                         base::Bind(callback, store_result));
-}
-
 }  // namespace syncer
diff --git a/sync/api/attachments/fake_attachment_service.h b/sync/api/attachments/fake_attachment_service.h
index 5bc443a..eb97c52 100644
--- a/sync/api/attachments/fake_attachment_service.h
+++ b/sync/api/attachments/fake_attachment_service.h
@@ -29,8 +29,7 @@ class SYNC_EXPORT FakeAttachmentService : public AttachmentService,
       OVERRIDE;
   virtual void DropAttachments(const AttachmentIdList& attachment_ids,
                                const DropCallback& callback) OVERRIDE;
-  virtual void StoreAttachments(const AttachmentList& attachments,
-                                const StoreCallback& callback) OVERRIDE;
+  virtual void OnSyncDataAdd(const SyncData& sync_data) OVERRIDE;
   virtual void OnSyncDataDelete(const SyncData& sync_data) OVERRIDE;
   virtual void OnSyncDataUpdate(const AttachmentIdList& old_attachment_ids,
                                 const SyncData& updated_sync_data) OVERRIDE;
@@ -41,8 +40,6 @@ class SYNC_EXPORT FakeAttachmentService : public AttachmentService,
                 scoped_ptr<AttachmentMap> attachments);
   void DropDone(const DropCallback& callback,
                 const AttachmentStore::Result& result);
-  void WriteDone(const StoreCallback& callback,
-                 const AttachmentStore::Result& result);
 
   const scoped_ptr<AttachmentStore> attachment_store_;
   // Must be last data member.
diff --git a/sync/api/sync_data.cc b/sync/api/sync_data.cc
index 35a21f9..fa48ea5 100644
--- a/sync/api/sync_data.cc
+++ b/sync/api/sync_data.cc
@@ -36,21 +36,6 @@ syncer::AttachmentId ProtoToId(const sync_pb::AttachmentIdProto& proto) {
   return syncer::AttachmentId::CreateFromProto(proto);
 }
 
-// Return true iff |attachments| contains one or more elements with the same
-// AttachmentId.
-bool ContainsDuplicateAttachments(const syncer::AttachmentList& attachments) {
-  std::set<syncer::AttachmentId> id_set;
-  AttachmentList::const_iterator iter = attachments.begin();
-  AttachmentList::const_iterator end = attachments.end();
-  for (; iter != end; ++iter) {
-    if (id_set.find(iter->GetId()) != id_set.end()) {
-      return true;
-    }
-    id_set.insert(iter->GetId());
-  }
-  return false;
-}
-
 }  // namespace
 
 namespace syncer {
@@ -111,13 +96,15 @@ SyncData SyncData::CreateLocalData(const std::string& sync_tag,
       sync_tag, non_unique_title, specifics, attachments);
 }
 
+// TODO(maniscalco): What should happen if the same Attachment appears in the
+// list twice? Document the behavior and add a test case.
+//
 // Static.
 SyncData SyncData::CreateLocalDataWithAttachments(
     const std::string& sync_tag,
     const std::string& non_unique_title,
     const sync_pb::EntitySpecifics& specifics,
     const AttachmentList& attachments) {
-  DCHECK(!ContainsDuplicateAttachments(attachments));
   sync_pb::SyncEntity entity;
   entity.set_client_defined_unique_tag(sync_tag);
   entity.set_non_unique_name(non_unique_title);
@@ -126,6 +113,9 @@ SyncData SyncData::CreateLocalDataWithAttachments(
                  attachments.end(),
                  RepeatedFieldBackInserter(entity.mutable_attachment_id()),
                  AttachmentToProto);
+  // TODO(maniscalco): Actually pass the attachments to the ctor and make them
+  // available to the AttachmentService once this SyncData gets passed into
+  // GenericChangeProcesso::ProcessSyncChanges (bug 354530).
   AttachmentList copy_of_attachments(attachments);
   return SyncData(kInvalidId,
                   &entity,
diff --git a/sync/api/sync_data.h b/sync/api/sync_data.h
index 8795141..bb99b74 100644
--- a/sync/api/sync_data.h
+++ b/sync/api/sync_data.h
@@ -43,19 +43,14 @@ class SYNC_EXPORT SyncData {
   // Default copy and assign welcome.
 
   // Helper methods for creating SyncData objects for local data.
-  //
-  // |sync_tag| Must be a string unique to this datatype and is used as a node
+  // The sync tag must be a string unique to this datatype and is used as a node
   // identifier server-side.
-  //
   // For deletes: |datatype| must specify the datatype who node is being
   // deleted.
-  //
-  // For adds/updates: |specifics| must be valid and |non_unique_title| (can be
-  // the same as |sync_tag|) must be specfied.  Note: |non_unique_title| is
-  // primarily for debug purposes, and will be overwritten if the datatype is
-  // encrypted.
-  //
-  // For data with attachments: |attachments| must not contain duplicates.
+  // For adds/updates: the specifics must be valid and the non-unique title (can
+  // be the same as sync tag) must be specfied.
+  // Note: the non_unique_title is primarily for debug purposes, and will be
+  // overwritten if the datatype is encrypted.
   static SyncData CreateLocalDelete(
       const std::string& sync_tag,
       ModelType datatype);
diff --git a/sync/engine/get_commit_ids.cc b/sync/engine/get_commit_ids.cc
index 5f91915..80222ee 100644
--- a/sync/engine/get_commit_ids.cc
+++ b/sync/engine/get_commit_ids.cc
@@ -108,13 +108,9 @@ bool IsEntryInConflict(const syncable::Entry& entry) {
 // Return true if this entry has any attachments that haven't yet been uploaded
 // to the server.
 bool HasAttachmentNotOnServer(const syncable::Entry& entry) {
-  // TODO(maniscalco): Add test case (bug 356266).
-  const sync_pb::AttachmentMetadata& metadata = entry.GetAttachmentMetadata();
-  for (int i = 0; i < metadata.record_size(); ++i) {
-    if (!metadata.record(i).is_on_server()) {
-      return true;
-    }
-  }
+  // TODO(maniscalco): Once AttachmentMetadata is fleshed out, implement this
+  // function to return true if any of the attachments haven't been uploaded to
+  // the server. Add test case (bug 356266).
   return false;
 }
 
diff --git a/sync/internal_api/base_node.cc b/sync/internal_api/base_node.cc
index 57dbb22..0788395 100644
--- a/sync/internal_api/base_node.cc
+++ b/sync/internal_api/base_node.cc
@@ -291,13 +291,10 @@ ModelType BaseNode::GetModelType() const {
 }
 
 const syncer::AttachmentIdList BaseNode::GetAttachmentIds() const {
-  AttachmentIdList result;
-  const sync_pb::AttachmentMetadata& metadata =
-      GetEntry()->GetAttachmentMetadata();
-  for (int i = 0; i < metadata.record_size(); ++i) {
-    result.push_back(AttachmentId::CreateFromProto(metadata.record(i).id()));
-  }
-  return result;
+  // TODO(maniscalco): Once EntryKernel is capable of storing AttachmentIds,
+  // update this method to retrieve the list of AttachmentIds from read_node and
+  // pass it to CreateRemoteData (bug 348625).
+  return AttachmentIdList();
 }
 
 void BaseNode::SetUnencryptedSpecifics(
diff --git a/sync/internal_api/public/base/attachment_id_proto.cc b/sync/internal_api/public/base/attachment_id_proto.cc
deleted file mode 100644
index d09c83e..0000000
--- a/sync/internal_api/public/base/attachment_id_proto.cc
+++ /dev/null
@@ -1,19 +0,0 @@
-// Copyright 2014 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/public/base/attachment_id_proto.h"
-
-#include "base/rand_util.h"
-
-namespace syncer {
-
-sync_pb::AttachmentIdProto CreateAttachmentIdProto() {
-  // Only requirement here is that this id must be globally unique.
-  // TODO(maniscalco): Consider making this base64 encoded.
-  sync_pb::AttachmentIdProto proto;
-  proto.set_unique_id(base::RandBytesAsString(16));
-  return proto;
-}
-
-}  // namespace syncer
diff --git a/sync/internal_api/public/base/attachment_id_proto.h b/sync/internal_api/public/base/attachment_id_proto.h
deleted file mode 100644
index 3332d81..0000000
--- a/sync/internal_api/public/base/attachment_id_proto.h
+++ /dev/null
@@ -1,20 +0,0 @@
-// Copyright 2014 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.
-
-#ifndef SYNC_INTERNAL_API_PUBLIC_BASE_ATTACHMENT_ID_PROTO_H_
-#define SYNC_INTERNAL_API_PUBLIC_BASE_ATTACHMENT_ID_PROTO_H_
-
-#include "sync/base/sync_export.h"
-#include "sync/protocol/sync.pb.h"
-
-namespace syncer {
-
-// Helper functions that are logically part of sync_pb::AttachmentIdProto.
-
-// Creates a unique AttachmentIdProto.
-SYNC_EXPORT_PRIVATE sync_pb::AttachmentIdProto CreateAttachmentIdProto();
-
-}  // namespace syncer
-
-#endif  // SYNC_INTERNAL_API_PUBLIC_BASE_ATTACHMENT_ID_PROTO_H_
diff --git a/sync/internal_api/public/base_transaction.h b/sync/internal_api/public/base_transaction.h
index c75086e..2f4fa3c 100644
--- a/sync/internal_api/public/base_transaction.h
+++ b/sync/internal_api/public/base_transaction.h
@@ -24,9 +24,6 @@ class Directory;
 // syncable, and are used in a similar way. Unlike syncable::BaseTransaction,
 // whose construction requires an explicit syncable::Directory, a sync
 // API BaseTransaction is created from a UserShare object.
-//
-// Note, these transactions are not atomic. Individual operations can
-// fail. There is no built-in rollback or undo mechanism.
 class SYNC_EXPORT BaseTransaction {
  public:
   // Provide access to the underlying syncable objects from BaseNode.
diff --git a/sync/internal_api/public/write_node.h b/sync/internal_api/public/write_node.h
index 5b47bd6..e41c966 100644
--- a/sync/internal_api/public/write_node.h
+++ b/sync/internal_api/public/write_node.h
@@ -173,10 +173,6 @@ class SYNC_EXPORT WriteNode : public BaseNode {
   void SetPriorityPreferenceSpecifics(
       const sync_pb::PriorityPreferenceSpecifics& specifics);
 
-  // Set the attachment metadata.
-  void SetAttachmentMetadata(
-      const sync_pb::AttachmentMetadata& attachment_metadata);
-
   // Implementation of BaseNode's abstract virtual accessors.
   virtual const syncable::Entry* GetEntry() const OVERRIDE;
 
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 20b7685..a5c0c81 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -210,94 +210,22 @@ class SyncApiTest : public testing::Test {
   }
 
  protected:
-  // Create an entry with the given |model_type|, |client_tag| and
-  // |attachment_metadata|.
-  void CreateEntryWithAttachmentMetadata(
-      const ModelType& model_type,
-      const std::string& client_tag,
-      const sync_pb::AttachmentMetadata& attachment_metadata);
-
-  // Attempts to load the entry specified by |model_type| and |client_tag| and
-  // returns the lookup result code.
-  BaseNode::InitByLookupResult LookupEntryByClientTag(
-      const ModelType& model_type,
-      const std::string& client_tag);
-
-  // Replace the entry specified by |model_type| and |client_tag| with a
-  // tombstone.
-  void ReplaceWithTombstone(const ModelType& model_type,
-                            const std::string& client_tag);
-
-  // Save changes to the Directory, destroy it then reload it.
-  bool ReloadDir();
-
-  UserShare* user_share();
-  syncable::Directory* dir();
-  SyncEncryptionHandler* encryption_handler();
-
- private:
   base::MessageLoop message_loop_;
   TestUserShare test_user_share_;
 };
 
-UserShare* SyncApiTest::user_share() {
-  return test_user_share_.user_share();
-}
-
-syncable::Directory* SyncApiTest::dir() {
-  return test_user_share_.user_share()->directory.get();
-}
-
-SyncEncryptionHandler* SyncApiTest::encryption_handler() {
-  return test_user_share_.encryption_handler();
-}
-
-bool SyncApiTest::ReloadDir() {
-  return test_user_share_.Reload();
-}
-
-void SyncApiTest::CreateEntryWithAttachmentMetadata(
-    const ModelType& model_type,
-    const std::string& client_tag,
-    const sync_pb::AttachmentMetadata& attachment_metadata) {
-  syncer::WriteTransaction trans(FROM_HERE, user_share());
-  syncer::ReadNode root_node(&trans);
-  root_node.InitByRootLookup();
-  syncer::WriteNode node(&trans);
-  ASSERT_EQ(node.InitUniqueByCreation(model_type, root_node, client_tag),
-            syncer::WriteNode::INIT_SUCCESS);
-  node.SetAttachmentMetadata(attachment_metadata);
-}
-
-BaseNode::InitByLookupResult SyncApiTest::LookupEntryByClientTag(
-    const ModelType& model_type,
-    const std::string& client_tag) {
-  syncer::ReadTransaction trans(FROM_HERE, user_share());
-  syncer::ReadNode node(&trans);
-  return node.InitByClientTagLookup(model_type, client_tag);
-}
-
-void SyncApiTest::ReplaceWithTombstone(const ModelType& model_type,
-                                       const std::string& client_tag) {
-  syncer::WriteTransaction trans(FROM_HERE, user_share());
-  syncer::WriteNode node(&trans);
-  ASSERT_EQ(node.InitByClientTagLookup(model_type, client_tag),
-            syncer::WriteNode::INIT_OK);
-  node.Tombstone();
-}
-
 TEST_F(SyncApiTest, SanityCheckTest) {
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     EXPECT_TRUE(trans.GetWrappedTrans());
   }
   {
-    WriteTransaction trans(FROM_HERE, user_share());
+    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     EXPECT_TRUE(trans.GetWrappedTrans());
   }
   {
     // No entries but root should exist
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode node(&trans);
     // Metahandle 1 can be root, sanity check 2
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_NOT_GOOD, node.InitByIdLookup(2));
@@ -306,16 +234,17 @@ TEST_F(SyncApiTest, SanityCheckTest) {
 
 TEST_F(SyncApiTest, BasicTagWrite) {
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
     EXPECT_EQ(root_node.GetFirstChildId(), 0);
   }
 
-  ignore_result(MakeNode(user_share(), BOOKMARKS, "testtag"));
+  ignore_result(MakeNode(test_user_share_.user_share(),
+                         BOOKMARKS, "testtag"));
 
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               node.InitByClientTagLookup(BOOKMARKS, "testtag"));
@@ -329,18 +258,21 @@ TEST_F(SyncApiTest, BasicTagWrite) {
 
 TEST_F(SyncApiTest, ModelTypesSiloed) {
   {
-    WriteTransaction trans(FROM_HERE, user_share());
+    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
     EXPECT_EQ(root_node.GetFirstChildId(), 0);
   }
 
-  ignore_result(MakeNode(user_share(), BOOKMARKS, "collideme"));
-  ignore_result(MakeNode(user_share(), PREFERENCES, "collideme"));
-  ignore_result(MakeNode(user_share(), AUTOFILL, "collideme"));
+  ignore_result(MakeNode(test_user_share_.user_share(),
+                         BOOKMARKS, "collideme"));
+  ignore_result(MakeNode(test_user_share_.user_share(),
+                         PREFERENCES, "collideme"));
+  ignore_result(MakeNode(test_user_share_.user_share(),
+                         AUTOFILL, "collideme"));
 
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
 
     ReadNode bookmarknode(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
@@ -365,14 +297,14 @@ TEST_F(SyncApiTest, ModelTypesSiloed) {
 
 TEST_F(SyncApiTest, ReadMissingTagsFails) {
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_NOT_GOOD,
               node.InitByClientTagLookup(BOOKMARKS,
                   "testtag"));
   }
   {
-    WriteTransaction trans(FROM_HERE, user_share());
+    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     WriteNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_NOT_GOOD,
               node.InitByClientTagLookup(BOOKMARKS,
@@ -388,7 +320,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
   std::string test_title("test1");
 
   {
-    WriteTransaction trans(FROM_HERE, user_share());
+    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -409,7 +341,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
 
   // Ensure we can delete something with a tag.
   {
-    WriteTransaction trans(FROM_HERE, user_share());
+    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     WriteNode wnode(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               wnode.InitByClientTagLookup(BOOKMARKS,
@@ -423,7 +355,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
   // Lookup of a node which was deleted should return failure,
   // but have found some data about the node.
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_IS_DEL,
               node.InitByClientTagLookup(BOOKMARKS,
@@ -434,7 +366,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
   }
 
   {
-    WriteTransaction trans(FROM_HERE, user_share());
+    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode folder_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK, folder_node.InitByIdLookup(folder_id));
 
@@ -452,7 +384,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
 
   // Now look up should work.
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               node.InitByClientTagLookup(BOOKMARKS,
@@ -465,11 +397,11 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
 TEST_F(SyncApiTest, WriteAndReadPassword) {
   KeyParams params = {"localhost", "username", "passphrase"};
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     trans.GetCryptographer()->AddKey(params);
   }
   {
-    WriteTransaction trans(FROM_HERE, user_share());
+    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -483,7 +415,7 @@ TEST_F(SyncApiTest, WriteAndReadPassword) {
     password_node.SetPasswordSpecifics(data);
   }
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -499,13 +431,13 @@ TEST_F(SyncApiTest, WriteAndReadPassword) {
 TEST_F(SyncApiTest, WriteEncryptedTitle) {
   KeyParams params = {"localhost", "username", "passphrase"};
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     trans.GetCryptographer()->AddKey(params);
   }
-  encryption_handler()->EnableEncryptEverything();
+  test_user_share_.encryption_handler()->EnableEncryptEverything();
   int bookmark_id;
   {
-    WriteTransaction trans(FROM_HERE, user_share());
+    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -521,7 +453,7 @@ TEST_F(SyncApiTest, WriteEncryptedTitle) {
     pref_node.SetTitle("bar");
   }
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -540,8 +472,9 @@ TEST_F(SyncApiTest, WriteEncryptedTitle) {
 }
 
 TEST_F(SyncApiTest, BaseNodeSetSpecifics) {
-  int64 child_id = MakeNode(user_share(), BOOKMARKS, "testtag");
-  WriteTransaction trans(FROM_HERE, user_share());
+  int64 child_id = MakeNode(test_user_share_.user_share(),
+                            BOOKMARKS, "testtag");
+  WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
   WriteNode node(&trans);
   EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(child_id));
 
@@ -556,8 +489,9 @@ TEST_F(SyncApiTest, BaseNodeSetSpecifics) {
 }
 
 TEST_F(SyncApiTest, BaseNodeSetSpecificsPreservesUnknownFields) {
-  int64 child_id = MakeNode(user_share(), BOOKMARKS, "testtag");
-  WriteTransaction trans(FROM_HERE, user_share());
+  int64 child_id = MakeNode(test_user_share_.user_share(),
+                            BOOKMARKS, "testtag");
+  WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
   WriteNode node(&trans);
   EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(child_id));
   EXPECT_TRUE(node.GetEntitySpecifics().unknown_fields().empty());
@@ -574,7 +508,7 @@ TEST_F(SyncApiTest, BaseNodeSetSpecificsPreservesUnknownFields) {
 }
 
 TEST_F(SyncApiTest, EmptyTags) {
-  WriteTransaction trans(FROM_HERE, user_share());
+  WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
   ReadNode root_node(&trans);
   root_node.InitByRootLookup();
   WriteNode node(&trans);
@@ -588,9 +522,10 @@ TEST_F(SyncApiTest, EmptyTags) {
 
 // Test counting nodes when the type's root node has no children.
 TEST_F(SyncApiTest, GetTotalNodeCountEmpty) {
-  int64 type_root = MakeServerNodeForType(user_share(), BOOKMARKS);
+  int64 type_root = MakeServerNodeForType(test_user_share_.user_share(),
+                                          BOOKMARKS);
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode type_root_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               type_root_node.InitByIdLookup(type_root));
@@ -600,10 +535,14 @@ TEST_F(SyncApiTest, GetTotalNodeCountEmpty) {
 
 // Test counting nodes when there is one child beneath the type's root.
 TEST_F(SyncApiTest, GetTotalNodeCountOneChild) {
-  int64 type_root = MakeServerNodeForType(user_share(), BOOKMARKS);
-  int64 parent = MakeFolderWithParent(user_share(), BOOKMARKS, type_root, NULL);
+  int64 type_root = MakeServerNodeForType(test_user_share_.user_share(),
+                                          BOOKMARKS);
+  int64 parent = MakeFolderWithParent(test_user_share_.user_share(),
+                                      BOOKMARKS,
+                                      type_root,
+                                      NULL);
   {
-    ReadTransaction trans(FROM_HERE, user_share());
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode type_root_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               type_root_node.InitByIdLookup(type_root));
@@ -618,15 +557,32 @@ TEST_F(SyncApiTest, GetTotalNodeCountOneChild) {
 // Test counting nodes when there are multiple children beneath the type root,
 // and one of those children has children of its own.
 TEST_F(SyncApiTest, GetTotalNodeCountMultipleChildren) {
-  int64 type_root = MakeServerNodeForType(user_share(), BOOKMARKS);
-  int64 parent = MakeFolderWithParent(user_share(), BOOKMARKS, type_root, NULL);
-  ignore_result(MakeFolderWithParent(user_share(), BOOKMARKS, type_root, NULL));
-  int64 child1 = MakeFolderWithParent(user_share(), BOOKMARKS, parent, NULL);
-  ignore_result(MakeBookmarkWithParent(user_share(), parent, NULL));
-  ignore_result(MakeBookmarkWithParent(user_share(), child1, NULL));
-
-  {
-    ReadTransaction trans(FROM_HERE, user_share());
+  int64 type_root = MakeServerNodeForType(test_user_share_.user_share(),
+                                          BOOKMARKS);
+  int64 parent = MakeFolderWithParent(test_user_share_.user_share(),
+                                      BOOKMARKS,
+                                      type_root,
+                                      NULL);
+  ignore_result(MakeFolderWithParent(test_user_share_.user_share(),
+                                     BOOKMARKS,
+                                     type_root,
+                                     NULL));
+  int64 child1 = MakeFolderWithParent(
+      test_user_share_.user_share(),
+      BOOKMARKS,
+      parent,
+      NULL);
+  ignore_result(MakeBookmarkWithParent(
+      test_user_share_.user_share(),
+      parent,
+      NULL));
+  ignore_result(MakeBookmarkWithParent(
+      test_user_share_.user_share(),
+      child1,
+      NULL));
+
+  {
+    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
     ReadNode type_root_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               type_root_node.InitByIdLookup(type_root));
@@ -638,57 +594,6 @@ TEST_F(SyncApiTest, GetTotalNodeCountMultipleChildren) {
   }
 }
 
-// Verify that Directory keeps track of which attachments are referenced by
-// which entries.
-TEST_F(SyncApiTest, AttachmentLinking) {
-  // Add an entry with an attachment.
-  std::string tag1("some tag");
-  syncer::AttachmentId attachment_id(syncer::AttachmentId::Create());
-  sync_pb::AttachmentMetadata attachment_metadata;
-  sync_pb::AttachmentMetadataRecord* record = attachment_metadata.add_record();
-  *record->mutable_id() = attachment_id.GetProto();
-  ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id.GetProto()));
-  CreateEntryWithAttachmentMetadata(PREFERENCES, tag1, attachment_metadata);
-
-  // See that the directory knows it's linked.
-  ASSERT_TRUE(dir()->IsAttachmentLinked(attachment_id.GetProto()));
-
-  // Add a second entry referencing the same attachment.
-  std::string tag2("some other tag");
-  CreateEntryWithAttachmentMetadata(PREFERENCES, tag2, attachment_metadata);
-
-  // See that the directory knows it's still linked.
-  ASSERT_TRUE(dir()->IsAttachmentLinked(attachment_id.GetProto()));
-
-  // Tombstone the first entry.
-  ReplaceWithTombstone(syncer::PREFERENCES, tag1);
-
-  // See that the attachment is still considered linked because the entry hasn't
-  // been purged from the Directory.
-  ASSERT_TRUE(dir()->IsAttachmentLinked(attachment_id.GetProto()));
-
-  // Save changes and see that the entry is truly gone.
-  ASSERT_TRUE(dir()->SaveChanges());
-  ASSERT_EQ(LookupEntryByClientTag(PREFERENCES, tag1),
-            syncer::WriteNode::INIT_FAILED_ENTRY_NOT_GOOD);
-
-  // However, the attachment is still linked.
-  ASSERT_TRUE(dir()->IsAttachmentLinked(attachment_id.GetProto()));
-
-  // Save, destroy, and recreate the directory.  See that it's still linked.
-  ASSERT_TRUE(ReloadDir());
-  ASSERT_TRUE(dir()->IsAttachmentLinked(attachment_id.GetProto()));
-
-  // Tombstone the second entry, save changes, see that it's truly gone.
-  ReplaceWithTombstone(syncer::PREFERENCES, tag2);
-  ASSERT_TRUE(dir()->SaveChanges());
-  ASSERT_EQ(LookupEntryByClientTag(PREFERENCES, tag2),
-            syncer::WriteNode::INIT_FAILED_ENTRY_NOT_GOOD);
-
-  // Finally, the attachment is no longer linked.
-  ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id.GetProto()));
-}
-
 namespace {
 
 class TestHttpPostProviderInterface : public HttpPostProviderInterface {
diff --git a/sync/internal_api/write_node.cc b/sync/internal_api/write_node.cc
index c3ef081..6a634eb 100644
--- a/sync/internal_api/write_node.cc
+++ b/sync/internal_api/write_node.cc
@@ -464,11 +464,6 @@ bool WriteNode::SetPosition(const BaseNode& new_parent,
   return PutPredecessor(predecessor);
 }
 
-void WriteNode::SetAttachmentMetadata(
-    const sync_pb::AttachmentMetadata& attachment_metadata) {
-  entry_->PutAttachmentMetadata(attachment_metadata);
-}
-
 const syncable::Entry* WriteNode::GetEntry() const {
   return entry_;
 }
diff --git a/sync/protocol/attachments.proto b/sync/protocol/attachments.proto
index e08e2b0..50815d3 100644
--- a/sync/protocol/attachments.proto
+++ b/sync/protocol/attachments.proto
@@ -21,16 +21,7 @@ message AttachmentIdProto {
   optional string unique_id = 1;
 }
 
-// Metadata for a single attachment.
-message AttachmentMetadataRecord {
-  optional AttachmentIdProto id = 1;
-  // Indicates we know this attachment exists on the server.
-  optional bool is_on_server = 2;
-}
-
 // A collection of attachment metadata. This proto is part of EntryKernel's "on
 // disk" representation. Private to sync.
 message AttachmentMetadata {
-  // One record per attachment.
-  repeated AttachmentMetadataRecord record = 1;
 }
diff --git a/sync/sync_internal_api.gypi b/sync/sync_internal_api.gypi
index 6f9af0328..6078dd8 100644
--- a/sync/sync_internal_api.gypi
+++ b/sync/sync_internal_api.gypi
@@ -45,8 +45,6 @@
     'internal_api/protocol_event_buffer.h',
     'internal_api/public/base/ack_handle.cc',
     'internal_api/public/base/ack_handle.h',
-    'internal_api/public/base/attachment_id_proto.cc',
-    'internal_api/public/base/attachment_id_proto.h',
     'internal_api/public/base/cancelation_observer.cc',
     'internal_api/public/base/cancelation_observer.h',
     'internal_api/public/base/cancelation_signal.cc',
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index cebb2d4..75c1bec 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -10,7 +10,6 @@
 #include "base/debug/trace_event.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
-#include "sync/internal_api/public/base/attachment_id_proto.h"
 #include "sync/internal_api/public/base/unique_position.h"
 #include "sync/internal_api/public/util/unrecoverable_error_handler.h"
 #include "sync/syncable/entry.h"
@@ -124,7 +123,6 @@ DirOpenResult Directory::Open(
 }
 
 void Directory::InitializeIndices(MetahandlesMap* handles_map) {
-  ScopedKernelLock lock(this);
   kernel_->metahandles_map.swap(*handles_map);
   for (MetahandlesMap::const_iterator it = kernel_->metahandles_map.begin();
        it != kernel_->metahandles_map.end(); ++it) {
@@ -154,7 +152,6 @@ void Directory::InitializeIndices(MetahandlesMap* handles_map) {
            kernel_->ids_map.end()) << "Unexpected duplicate use of ID";
     kernel_->ids_map[entry->ref(ID).value()] = entry;
     DCHECK(!entry->is_dirty());
-    AddToAttachmentIndex(metahandle, entry->ref(ATTACHMENT_METADATA), lock);
   }
 }
 
@@ -365,8 +362,6 @@ bool Directory::InsertEntry(BaseWriteTransaction* trans,
       return false;
     }
   }
-  AddToAttachmentIndex(
-      entry->ref(META_HANDLE), entry->ref(ATTACHMENT_METADATA), *lock);
 
   // Should NEVER be created with a client tag or server tag.
   if (!SyncAssert(entry->ref(UNIQUE_SERVER_TAG).empty(), FROM_HERE,
@@ -413,51 +408,6 @@ bool Directory::ReindexParentId(BaseWriteTransaction* trans,
   return true;
 }
 
-void Directory::RemoveFromAttachmentIndex(
-    const int64 metahandle,
-    const sync_pb::AttachmentMetadata& attachment_metadata,
-    const ScopedKernelLock& lock) {
-  for (int i = 0; i < attachment_metadata.record_size(); ++i) {
-    AttachmentIdUniqueId unique_id =
-        attachment_metadata.record(i).id().unique_id();
-    IndexByAttachmentId::iterator iter =
-        kernel_->index_by_attachment_id.find(unique_id);
-    if (iter != kernel_->index_by_attachment_id.end()) {
-      iter->second.erase(metahandle);
-      if (iter->second.empty()) {
-        kernel_->index_by_attachment_id.erase(iter);
-      }
-    }
-  }
-}
-
-void Directory::AddToAttachmentIndex(
-    const int64 metahandle,
-    const sync_pb::AttachmentMetadata& attachment_metadata,
-    const ScopedKernelLock& lock) {
-  for (int i = 0; i < attachment_metadata.record_size(); ++i) {
-    AttachmentIdUniqueId unique_id =
-        attachment_metadata.record(i).id().unique_id();
-    IndexByAttachmentId::iterator iter =
-        kernel_->index_by_attachment_id.find(unique_id);
-    if (iter == kernel_->index_by_attachment_id.end()) {
-      iter = kernel_->index_by_attachment_id.insert(std::make_pair(
-                                                        unique_id,
-                                                        MetahandleSet())).first;
-    }
-    iter->second.insert(metahandle);
-  }
-}
-
-void Directory::UpdateAttachmentIndex(
-    const int64 metahandle,
-    const sync_pb::AttachmentMetadata& old_metadata,
-    const sync_pb::AttachmentMetadata& new_metadata) {
-  ScopedKernelLock lock(this);
-  RemoveFromAttachmentIndex(metahandle, old_metadata, lock);
-  AddToAttachmentIndex(metahandle, new_metadata, lock);
-}
-
 bool Directory::unrecoverable_error_set(const BaseTransaction* trans) const {
   DCHECK(trans != NULL);
   return unrecoverable_error_set_;
@@ -598,9 +548,6 @@ bool Directory::VacuumAfterSaveChanges(const SaveChangesSnapshot& snapshot) {
                       "Deleted entry still present",
                       (&trans)))
         return false;
-      RemoveFromAttachmentIndex(
-          entry->ref(META_HANDLE), entry->ref(ATTACHMENT_METADATA), lock);
-
       delete entry;
     }
     if (trans.unrecoverable_error_set())
@@ -660,8 +607,7 @@ void Directory::UnapplyEntry(EntryKernel* entry) {
 
 void Directory::DeleteEntry(bool save_to_journal,
                             EntryKernel* entry,
-                            EntryKernelSet* entries_to_journal,
-                            const ScopedKernelLock& lock) {
+                            EntryKernelSet* entries_to_journal) {
   int64 handle = entry->ref(META_HANDLE);
   ModelType server_type = GetModelTypeFromSpecifics(
       entry->ref(SERVER_SPECIFICS));
@@ -691,7 +637,6 @@ void Directory::DeleteEntry(bool save_to_journal,
         kernel_->server_tags_map.erase(entry->ref(UNIQUE_SERVER_TAG));
     DCHECK_EQ(1u, num_erased);
   }
-  RemoveFromAttachmentIndex(handle, entry->ref(ATTACHMENT_METADATA), lock);
 
   if (save_to_journal) {
     entries_to_journal->insert(entry);
@@ -759,7 +704,7 @@ bool Directory::PurgeEntriesWithTypeIn(ModelTypeSet disabled_types,
                types_to_journal.Has(server_type)) &&
               (delete_journal_->IsDeleteJournalEnabled(local_type) ||
                delete_journal_->IsDeleteJournalEnabled(server_type));
-          DeleteEntry(save_to_journal, entry, &entries_to_journal, lock);
+          DeleteEntry(save_to_journal, entry, &entries_to_journal);
         }
       }
 
@@ -817,17 +762,6 @@ bool Directory::ResetVersionsForType(BaseWriteTransaction* trans,
   return true;
 }
 
-bool Directory::IsAttachmentLinked(
-    const sync_pb::AttachmentIdProto& attachment_id_proto) const {
-  ScopedKernelLock lock(this);
-  IndexByAttachmentId::const_iterator iter =
-      kernel_->index_by_attachment_id.find(attachment_id_proto.unique_id());
-  if (iter != kernel_->index_by_attachment_id.end() && !iter->second.empty()) {
-    return true;
-  }
-  return false;
-}
-
 void Directory::HandleSaveChangesFailure(const SaveChangesSnapshot& snapshot) {
   WriteTransaction trans(FROM_HERE, HANDLE_SAVE_FAILURE, this);
   ScopedKernelLock lock(this);
diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h
index 5d26bbd..fdac9b4 100644
--- a/sync/syncable/directory.h
+++ b/sync/syncable/directory.h
@@ -47,9 +47,6 @@ enum InvariantCheckLevel {
   FULL_DB_VERIFICATION = 2 // Check every entry.  This can be expensive.
 };
 
-// Directory stores and manages EntryKernels.
-//
-// This class is tightly coupled to several other classes (see friends).
 class SYNC_EXPORT Directory {
   friend class BaseTransaction;
   friend class Entry;
@@ -83,9 +80,6 @@ class SYNC_EXPORT Directory {
   typedef base::hash_map<int64, EntryKernel*> MetahandlesMap;
   typedef base::hash_map<std::string, EntryKernel*> IdsMap;
   typedef base::hash_map<std::string, EntryKernel*> TagsMap;
-  typedef std::string AttachmentIdUniqueId;
-  typedef base::hash_map<AttachmentIdUniqueId, MetahandleSet>
-      IndexByAttachmentId;
 
   static const base::FilePath::CharType kSyncDatabaseFilename[];
 
@@ -389,14 +383,6 @@ class SYNC_EXPORT Directory {
   // WARNING! This can be slow, as it iterates over all entries for a type.
   bool ResetVersionsForType(BaseWriteTransaction* trans, ModelType type);
 
-  // Returns true iff the attachment identified by |attachment_id_proto| is
-  // linked to an entry.
-  //
-  // An attachment linked to a deleted entry is still considered linked if the
-  // entry hasn't yet been purged.
-  bool IsAttachmentLinked(
-      const sync_pb::AttachmentIdProto& attachment_id_proto) const;
-
  protected:  // for friends, mainly used by Entry constructors
   virtual EntryKernel* GetEntryByHandle(int64 handle);
   virtual EntryKernel* GetEntryByHandle(int64 metahandle,
@@ -408,11 +394,6 @@ class SYNC_EXPORT Directory {
                  const Id& new_id);
   bool ReindexParentId(BaseWriteTransaction* trans, EntryKernel* const entry,
                        const Id& new_parent_id);
-  // Update the attachment index for |metahandle| removing it from the index
-  // under |old_metadata| entries and add it under |new_metadata| entries.
-  void UpdateAttachmentIndex(const int64 metahandle,
-                             const sync_pb::AttachmentMetadata& old_metadata,
-                             const sync_pb::AttachmentMetadata& new_metadata);
   void ClearDirtyMetahandles();
 
   DirOpenResult OpenImpl(
@@ -470,17 +451,6 @@ class SYNC_EXPORT Directory {
     // within parent.  Protected by the ScopedKernelLock.
     ParentChildIndex parent_child_index;
 
-    // This index keeps track of which metahandles refer to a given attachment.
-    // Think of it as the inverse of EntryKernel's AttachmentMetadata Records.
-    //
-    // Because entries can be undeleted (e.g. PutIsDel(false)), entries should
-    // not removed from the index until they are actually deleted from memory.
-    //
-    // All access should go through IsAttachmentLinked,
-    // RemoveFromAttachmentIndex, AddToAttachmentIndex, and
-    // UpdateAttachmentIndex methods to avoid iterator invalidation errors.
-    IndexByAttachmentId index_by_attachment_id;
-
     // 3 in-memory indices on bits used extremely frequently by the syncer.
     // |unapplied_update_metahandles| is keyed by the server model type.
     MetahandleSet unapplied_update_metahandles[MODEL_TYPE_COUNT];
@@ -573,19 +543,7 @@ class SYNC_EXPORT Directory {
   void UnapplyEntry(EntryKernel* entry);
   void DeleteEntry(bool save_to_journal,
                    EntryKernel* entry,
-                   EntryKernelSet* entries_to_journal,
-                   const ScopedKernelLock& lock);
-
-  // Remove each of |metahandle|'s attachment ids from index_by_attachment_id.
-  void RemoveFromAttachmentIndex(
-      const int64 metahandle,
-      const sync_pb::AttachmentMetadata& attachment_metadata,
-      const ScopedKernelLock& lock);
-  // Add each of |metahandle|'s attachment ids to the index_by_attachment_id.
-  void AddToAttachmentIndex(
-      const int64 metahandle,
-      const sync_pb::AttachmentMetadata& attachment_metadata,
-      const ScopedKernelLock& lock);
+                   EntryKernelSet* entries_to_journal);
 
   Kernel* kernel_;
 
diff --git a/sync/syncable/directory_backing_store.cc b/sync/syncable/directory_backing_store.cc
index ec28a53..e466548 100644
--- a/sync/syncable/directory_backing_store.cc
+++ b/sync/syncable/directory_backing_store.cc
@@ -70,11 +70,6 @@ void BindFields(const EntryKernel& entry,
     entry.ref(static_cast<UniquePositionField>(i)).SerializeToString(&temp);
     statement->BindBlob(index++, temp.data(), temp.length());
   }
-  for (; i < ATTACHMENT_METADATA_FIELDS_END; ++i) {
-    std::string temp;
-    entry.ref(static_cast<AttachmentMetadataField>(i)).SerializeToString(&temp);
-    statement->BindBlob(index++, temp.data(), temp.length());
-  }
 }
 
 // The caller owns the returned EntryKernel*.  Assumes the statement currently
@@ -119,10 +114,6 @@ scoped_ptr<EntryKernel> UnpackEntry(sql::Statement* statement) {
     kernel->mutable_ref(static_cast<UniquePositionField>(i)) =
         UniquePosition::FromProto(proto);
   }
-  for (; i < ATTACHMENT_METADATA_FIELDS_END; ++i) {
-    kernel->mutable_ref(static_cast<AttachmentMetadataField>(i)).ParseFromArray(
-        statement->ColumnBlob(i), statement->ColumnByteLength(i));
-  }
   return kernel.Pass();
 }
 
diff --git a/sync/syncable/directory_unittest.cc b/sync/syncable/directory_unittest.cc
index f58f54f..b2bff36 100644
--- a/sync/syncable/directory_unittest.cc
+++ b/sync/syncable/directory_unittest.cc
@@ -6,7 +6,6 @@
 
 #include "base/strings/stringprintf.h"
 #include "base/test/values_test_util.h"
-#include "sync/internal_api/public/base/attachment_id_proto.h"
 #include "sync/syncable/syncable_proto_util.h"
 #include "sync/syncable/syncable_util.h"
 #include "sync/syncable/syncable_write_transaction.h"
@@ -85,45 +84,23 @@ DirOpenResult SyncableDirectoryTest::ReopenDirectory() {
 }
 
 // Creates an empty entry and sets the ID field to a default one.
-void SyncableDirectoryTest::CreateEntry(const ModelType& model_type,
-                                        const std::string& entryname) {
-  CreateEntry(model_type, entryname, TestIdFactory::FromNumber(-99));
+void SyncableDirectoryTest::CreateEntry(const std::string& entryname) {
+  CreateEntry(entryname, TestIdFactory::FromNumber(-99));
 }
 
 // Creates an empty entry and sets the ID field to id.
-void SyncableDirectoryTest::CreateEntry(const ModelType& model_type,
-                                        const std::string& entryname,
+void SyncableDirectoryTest::CreateEntry(const std::string& entryname,
                                         const int id) {
-  CreateEntry(model_type, entryname, TestIdFactory::FromNumber(id));
+  CreateEntry(entryname, TestIdFactory::FromNumber(id));
 }
-
-void SyncableDirectoryTest::CreateEntry(const ModelType& model_type,
-                                        const std::string& entryname,
-                                        const Id& id) {
-  CreateEntryWithAttachmentMetadata(
-      model_type, entryname, id, sync_pb::AttachmentMetadata());
-}
-
-void SyncableDirectoryTest::CreateEntryWithAttachmentMetadata(
-    const ModelType& model_type,
-    const std::string& entryname,
-    const Id& id,
-    const sync_pb::AttachmentMetadata& attachment_metadata) {
+void SyncableDirectoryTest::CreateEntry(const std::string& entryname, Id id) {
   WriteTransaction wtrans(FROM_HERE, UNITTEST, dir_.get());
-  MutableEntry me(&wtrans, CREATE, model_type, wtrans.root_id(), entryname);
+  MutableEntry me(&wtrans, CREATE, BOOKMARKS, wtrans.root_id(), entryname);
   ASSERT_TRUE(me.good());
   me.PutId(id);
-  me.PutAttachmentMetadata(attachment_metadata);
   me.PutIsUnsynced(true);
 }
 
-void SyncableDirectoryTest::DeleteEntry(const Id& id) {
-  WriteTransaction trans(FROM_HERE, UNITTEST, dir().get());
-  MutableEntry entry(&trans, GET_BY_ID, id);
-  ASSERT_TRUE(entry.good());
-  entry.PutIsDel(true);
-}
-
 DirOpenResult SyncableDirectoryTest::SimulateSaveAndReloadDir() {
   if (!dir_->SaveChanges())
     return FAILED_IN_UNITTEST;
@@ -434,8 +411,8 @@ TEST_F(SyncableDirectoryTest, ManageDeleteJournals) {
   int64 handle2 = 0;
   {
     // Create two bookmark entries and save in database.
-    CreateEntry(BOOKMARKS, "item1", id1);
-    CreateEntry(BOOKMARKS, "item2", id2);
+    CreateEntry("item1", id1);
+    CreateEntry("item2", id2);
     {
       WriteTransaction trans(FROM_HERE, UNITTEST, dir().get());
       MutableEntry item1(&trans, GET_BY_ID, id1);
@@ -552,7 +529,7 @@ TEST_F(SyncableDirectoryTest, TestBasicLookupNonExistantID) {
 }
 
 TEST_F(SyncableDirectoryTest, TestBasicLookupValidID) {
-  CreateEntry(BOOKMARKS, "rtc");
+  CreateEntry("rtc");
   ReadTransaction rtrans(FROM_HERE, dir().get());
   Entry e(&rtrans, GET_BY_ID, TestIdFactory::FromNumber(-99));
   ASSERT_TRUE(e.good());
@@ -1544,100 +1521,6 @@ TEST_F(SyncableDirectoryTest, StressTransactions) {
   }
 }
 
-// Verify that Directory is notifed when a MutableEntry's AttachmentMetadata
-// changes.
-TEST_F(SyncableDirectoryTest, MutableEntry_PutAttachmentMetadata) {
-  sync_pb::AttachmentMetadata attachment_metadata;
-  sync_pb::AttachmentMetadataRecord* record = attachment_metadata.add_record();
-  sync_pb::AttachmentIdProto attachment_id_proto =
-      syncer::CreateAttachmentIdProto();
-  *record->mutable_id() = attachment_id_proto;
-  ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id_proto));
-  {
-    WriteTransaction trans(FROM_HERE, UNITTEST, dir().get());
-
-    // Create an entry with attachment metadata and see that the attachment id
-    // is not linked.
-    MutableEntry entry(
-        &trans, CREATE, PREFERENCES, trans.root_id(), "some entry");
-    entry.PutId(TestIdFactory::FromNumber(-1));
-    entry.PutIsUnsynced(true);
-    ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id_proto));
-
-    // Now add the attachment metadata and see that Directory believes it is
-    // linked.
-    entry.PutAttachmentMetadata(attachment_metadata);
-    ASSERT_TRUE(dir()->IsAttachmentLinked(attachment_id_proto));
-
-    // Clear out the attachment metadata and see that it's no longer linked.
-    sync_pb::AttachmentMetadata empty_attachment_metadata;
-    entry.PutAttachmentMetadata(empty_attachment_metadata);
-    ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id_proto));
-  }
-  ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id_proto));
-}
-
-// Verify that deleted entries with attachments will retain the attachments.
-TEST_F(SyncableDirectoryTest, Directory_DeleteDoesNotUnlinkAttachments) {
-  sync_pb::AttachmentMetadata attachment_metadata;
-  sync_pb::AttachmentMetadataRecord* record = attachment_metadata.add_record();
-  sync_pb::AttachmentIdProto attachment_id_proto =
-      syncer::CreateAttachmentIdProto();
-  *record->mutable_id() = attachment_id_proto;
-  ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id_proto));
-  const Id id = TestIdFactory::FromNumber(-1);
-
-  // Create an entry with attachment metadata and see that the attachment id
-  // is linked.
-  CreateEntryWithAttachmentMetadata(
-      PREFERENCES, "some entry", id, attachment_metadata);
-  ASSERT_TRUE(dir()->IsAttachmentLinked(attachment_id_proto));
-
-  // Delete the entry and see that it's still linked because the entry hasn't
-  // yet been purged.
-  DeleteEntry(id);
-  ASSERT_TRUE(dir()->IsAttachmentLinked(attachment_id_proto));
-
-  // Reload the Directory, purging the deleted entry, and see that the
-  // attachment is no longer linked.
-  SimulateSaveAndReloadDir();
-  ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id_proto));
-}
-
-// Verify that a given attachment can be referenced by multiple entries and that
-// any one of the references is sufficient to ensure it remains linked.
-TEST_F(SyncableDirectoryTest, Directory_LastReferenceUnlinksAttachments) {
-  // Create one attachment.
-  sync_pb::AttachmentMetadata attachment_metadata;
-  sync_pb::AttachmentMetadataRecord* record = attachment_metadata.add_record();
-  sync_pb::AttachmentIdProto attachment_id_proto =
-      syncer::CreateAttachmentIdProto();
-  *record->mutable_id() = attachment_id_proto;
-
-  // Create two entries, each referencing the attachment.
-  const Id id1 = TestIdFactory::FromNumber(-1);
-  const Id id2 = TestIdFactory::FromNumber(-2);
-  CreateEntryWithAttachmentMetadata(
-      PREFERENCES, "some entry", id1, attachment_metadata);
-  CreateEntryWithAttachmentMetadata(
-      PREFERENCES, "some other entry", id2, attachment_metadata);
-
-  // See that the attachment is considered linked.
-  ASSERT_TRUE(dir()->IsAttachmentLinked(attachment_id_proto));
-
-  // Delete the first entry, reload the Directory, see that the attachment is
-  // still linked.
-  DeleteEntry(id1);
-  SimulateSaveAndReloadDir();
-  ASSERT_TRUE(dir()->IsAttachmentLinked(attachment_id_proto));
-
-  // Delete the second entry, reload the Directory, see that the attachment is
-  // no loner linked.
-  DeleteEntry(id2);
-  SimulateSaveAndReloadDir();
-  ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id_proto));
-}
-
 }  // namespace syncable
 
 }  // namespace syncer
diff --git a/sync/syncable/directory_unittest.h b/sync/syncable/directory_unittest.h
index 81e3888..14e1a99 100644
--- a/sync/syncable/directory_unittest.h
+++ b/sync/syncable/directory_unittest.h
@@ -31,6 +31,7 @@ class BaseTransaction;
 //
 // Serves as base class for several other test fixtures.
 class SyncableDirectoryTest : public testing::Test {
+ private:
  protected:
   static const char kDirectoryName[];
 
@@ -46,24 +47,12 @@ class SyncableDirectoryTest : public testing::Test {
   DirOpenResult ReopenDirectory();
 
   // Creates an empty entry and sets the ID field to a default one.
-  void CreateEntry(const ModelType& model_type, const std::string& entryname);
+  void CreateEntry(const std::string& entryname);
 
   // Creates an empty entry and sets the ID field to id.
-  void CreateEntry(const ModelType& model_type,
-                   const std::string& entryname,
-                   const int id);
-
-  void CreateEntry(const ModelType& model_type,
-                   const std::string& entryname,
-                   const Id& id);
-
-  void CreateEntryWithAttachmentMetadata(
-      const ModelType& model_type,
-      const std::string& entryname,
-      const Id& id,
-      const sync_pb::AttachmentMetadata& attachment_metadata);
+  void CreateEntry(const std::string& entryname, const int id);
 
-  void DeleteEntry(const Id& id);
+  void CreateEntry(const std::string& entryname, Id id);
 
   // When a directory is saved then loaded from disk, it will pass through
   // DropDeletedEntries().  This will remove some entries from the directory.
diff --git a/sync/syncable/entry_kernel.h b/sync/syncable/entry_kernel.h
index 6380420..b6bc2ab 100644
--- a/sync/syncable/entry_kernel.h
+++ b/sync/syncable/entry_kernel.h
@@ -333,10 +333,6 @@ struct SYNC_EXPORT_PRIVATE EntryKernel {
   inline UniquePosition& mutable_ref(UniquePositionField field) {
     return unique_position_fields[field - UNIQUE_POSITION_FIELDS_BEGIN];
   }
-  inline sync_pb::AttachmentMetadata& mutable_ref(
-      AttachmentMetadataField field) {
-    return attachment_metadata_fields[field - ATTACHMENT_METADATA_FIELDS_BEGIN];
-  }
 
   ModelType GetModelType() const;
   ModelType GetServerModelType() const;
diff --git a/sync/syncable/mutable_entry.cc b/sync/syncable/mutable_entry.cc
index d9d5daa..863e65b 100644
--- a/sync/syncable/mutable_entry.cc
+++ b/sync/syncable/mutable_entry.cc
@@ -233,20 +233,6 @@ bool MutableEntry::PutPredecessor(const Id& predecessor_id) {
   return true;
 }
 
-void MutableEntry::PutAttachmentMetadata(
-    const sync_pb::AttachmentMetadata& attachment_metadata) {
-  DCHECK(kernel_);
-  write_transaction()->TrackChangesTo(kernel_);
-  if (kernel_->ref(ATTACHMENT_METADATA).SerializeAsString() !=
-      attachment_metadata.SerializeAsString()) {
-    dir()->UpdateAttachmentIndex(GetMetahandle(),
-                                 kernel_->ref(ATTACHMENT_METADATA),
-                                 attachment_metadata);
-    kernel_->put(ATTACHMENT_METADATA, attachment_metadata);
-    kernel_->mark_dirty(&dir()->kernel_->dirty_metahandles);
-  }
-}
-
 // This function sets only the flags needed to get this entry to sync.
 bool MarkForSyncing(MutableEntry* e) {
   DCHECK_NE(static_cast<MutableEntry*>(NULL), e);
diff --git a/sync/syncable/mutable_entry.h b/sync/syncable/mutable_entry.h
index c064358..8c2f2ab 100644
--- a/sync/syncable/mutable_entry.h
+++ b/sync/syncable/mutable_entry.h
@@ -59,9 +59,6 @@ class SYNC_EXPORT_PRIVATE MutableEntry : public ModelNeutralMutableEntry {
   // ID to put the node in first position.
   bool PutPredecessor(const Id& predecessor_id);
 
-  void PutAttachmentMetadata(
-      const sync_pb::AttachmentMetadata& attachment_metadata);
-
  private:
   // Kind of redundant. We should reduce the number of pointers
   // floating around if at all possible. Could we store this in Directory?