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

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

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

7 files changed, 225 insertions, 76 deletions

diff --git a/sync/internal_api/base_node.cc b/sync/internal_api/base_node.cc
index 0788395..57dbb22 100644
--- a/sync/internal_api/base_node.cc
+++ b/sync/internal_api/base_node.cc
@@ -291,10 +291,13 @@ ModelType BaseNode::GetModelType() const {
 }
 
 const syncer::AttachmentIdList BaseNode::GetAttachmentIds() const {
-  // 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();
+  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;
 }
 
 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
new file mode 100644
index 0000000..d09c83e
--- /dev/null
+++ b/sync/internal_api/public/base/attachment_id_proto.cc
@@ -0,0 +1,19 @@
+// 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
new file mode 100644
index 0000000..3332d81
--- /dev/null
+++ b/sync/internal_api/public/base/attachment_id_proto.h
@@ -0,0 +1,20 @@
+// 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 2f4fa3c..c75086e 100644
--- a/sync/internal_api/public/base_transaction.h
+++ b/sync/internal_api/public/base_transaction.h
@@ -24,6 +24,9 @@ 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 e41c966..5b47bd6 100644
--- a/sync/internal_api/public/write_node.h
+++ b/sync/internal_api/public/write_node.h
@@ -173,6 +173,10 @@ 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 a5c0c81..20b7685 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -210,22 +210,94 @@ 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, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     EXPECT_TRUE(trans.GetWrappedTrans());
   }
   {
-    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+    WriteTransaction trans(FROM_HERE, user_share());
     EXPECT_TRUE(trans.GetWrappedTrans());
   }
   {
     // No entries but root should exist
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     ReadNode node(&trans);
     // Metahandle 1 can be root, sanity check 2
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_NOT_GOOD, node.InitByIdLookup(2));
@@ -234,17 +306,16 @@ TEST_F(SyncApiTest, SanityCheckTest) {
 
 TEST_F(SyncApiTest, BasicTagWrite) {
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
     EXPECT_EQ(root_node.GetFirstChildId(), 0);
   }
 
-  ignore_result(MakeNode(test_user_share_.user_share(),
-                         BOOKMARKS, "testtag"));
+  ignore_result(MakeNode(user_share(), BOOKMARKS, "testtag"));
 
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               node.InitByClientTagLookup(BOOKMARKS, "testtag"));
@@ -258,21 +329,18 @@ TEST_F(SyncApiTest, BasicTagWrite) {
 
 TEST_F(SyncApiTest, ModelTypesSiloed) {
   {
-    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+    WriteTransaction trans(FROM_HERE, user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
     EXPECT_EQ(root_node.GetFirstChildId(), 0);
   }
 
-  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"));
+  ignore_result(MakeNode(user_share(), BOOKMARKS, "collideme"));
+  ignore_result(MakeNode(user_share(), PREFERENCES, "collideme"));
+  ignore_result(MakeNode(user_share(), AUTOFILL, "collideme"));
 
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
 
     ReadNode bookmarknode(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
@@ -297,14 +365,14 @@ TEST_F(SyncApiTest, ModelTypesSiloed) {
 
 TEST_F(SyncApiTest, ReadMissingTagsFails) {
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_NOT_GOOD,
               node.InitByClientTagLookup(BOOKMARKS,
                   "testtag"));
   }
   {
-    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+    WriteTransaction trans(FROM_HERE, user_share());
     WriteNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_NOT_GOOD,
               node.InitByClientTagLookup(BOOKMARKS,
@@ -320,7 +388,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
   std::string test_title("test1");
 
   {
-    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+    WriteTransaction trans(FROM_HERE, user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -341,7 +409,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
 
   // Ensure we can delete something with a tag.
   {
-    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+    WriteTransaction trans(FROM_HERE, user_share());
     WriteNode wnode(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               wnode.InitByClientTagLookup(BOOKMARKS,
@@ -355,7 +423,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, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_IS_DEL,
               node.InitByClientTagLookup(BOOKMARKS,
@@ -366,7 +434,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
   }
 
   {
-    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+    WriteTransaction trans(FROM_HERE, user_share());
     ReadNode folder_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK, folder_node.InitByIdLookup(folder_id));
 
@@ -384,7 +452,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
 
   // Now look up should work.
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     ReadNode node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               node.InitByClientTagLookup(BOOKMARKS,
@@ -397,11 +465,11 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
 TEST_F(SyncApiTest, WriteAndReadPassword) {
   KeyParams params = {"localhost", "username", "passphrase"};
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     trans.GetCryptographer()->AddKey(params);
   }
   {
-    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+    WriteTransaction trans(FROM_HERE, user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -415,7 +483,7 @@ TEST_F(SyncApiTest, WriteAndReadPassword) {
     password_node.SetPasswordSpecifics(data);
   }
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -431,13 +499,13 @@ TEST_F(SyncApiTest, WriteAndReadPassword) {
 TEST_F(SyncApiTest, WriteEncryptedTitle) {
   KeyParams params = {"localhost", "username", "passphrase"};
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     trans.GetCryptographer()->AddKey(params);
   }
-  test_user_share_.encryption_handler()->EnableEncryptEverything();
+  encryption_handler()->EnableEncryptEverything();
   int bookmark_id;
   {
-    WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+    WriteTransaction trans(FROM_HERE, user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -453,7 +521,7 @@ TEST_F(SyncApiTest, WriteEncryptedTitle) {
     pref_node.SetTitle("bar");
   }
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -472,9 +540,8 @@ TEST_F(SyncApiTest, WriteEncryptedTitle) {
 }
 
 TEST_F(SyncApiTest, BaseNodeSetSpecifics) {
-  int64 child_id = MakeNode(test_user_share_.user_share(),
-                            BOOKMARKS, "testtag");
-  WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+  int64 child_id = MakeNode(user_share(), BOOKMARKS, "testtag");
+  WriteTransaction trans(FROM_HERE, user_share());
   WriteNode node(&trans);
   EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(child_id));
 
@@ -489,9 +556,8 @@ TEST_F(SyncApiTest, BaseNodeSetSpecifics) {
 }
 
 TEST_F(SyncApiTest, BaseNodeSetSpecificsPreservesUnknownFields) {
-  int64 child_id = MakeNode(test_user_share_.user_share(),
-                            BOOKMARKS, "testtag");
-  WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+  int64 child_id = MakeNode(user_share(), BOOKMARKS, "testtag");
+  WriteTransaction trans(FROM_HERE, user_share());
   WriteNode node(&trans);
   EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(child_id));
   EXPECT_TRUE(node.GetEntitySpecifics().unknown_fields().empty());
@@ -508,7 +574,7 @@ TEST_F(SyncApiTest, BaseNodeSetSpecificsPreservesUnknownFields) {
 }
 
 TEST_F(SyncApiTest, EmptyTags) {
-  WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
+  WriteTransaction trans(FROM_HERE, user_share());
   ReadNode root_node(&trans);
   root_node.InitByRootLookup();
   WriteNode node(&trans);
@@ -522,10 +588,9 @@ TEST_F(SyncApiTest, EmptyTags) {
 
 // Test counting nodes when the type's root node has no children.
 TEST_F(SyncApiTest, GetTotalNodeCountEmpty) {
-  int64 type_root = MakeServerNodeForType(test_user_share_.user_share(),
-                                          BOOKMARKS);
+  int64 type_root = MakeServerNodeForType(user_share(), BOOKMARKS);
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     ReadNode type_root_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               type_root_node.InitByIdLookup(type_root));
@@ -535,14 +600,10 @@ 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(test_user_share_.user_share(),
-                                          BOOKMARKS);
-  int64 parent = MakeFolderWithParent(test_user_share_.user_share(),
-                                      BOOKMARKS,
-                                      type_root,
-                                      NULL);
+  int64 type_root = MakeServerNodeForType(user_share(), BOOKMARKS);
+  int64 parent = MakeFolderWithParent(user_share(), BOOKMARKS, type_root, NULL);
   {
-    ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    ReadTransaction trans(FROM_HERE, user_share());
     ReadNode type_root_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               type_root_node.InitByIdLookup(type_root));
@@ -557,32 +618,15 @@ 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(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());
+  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());
     ReadNode type_root_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
               type_root_node.InitByIdLookup(type_root));
@@ -594,6 +638,57 @@ 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 6a634eb..c3ef081 100644
--- a/sync/internal_api/write_node.cc
+++ b/sync/internal_api/write_node.cc
@@ -464,6 +464,11 @@ 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_;
 }