[Sync] Add size and crc32c to AttachmentId.

The purpose of this change is to ensure that if a client knows about an
attachment (i.e. has an AttachmentId or AttachmentIdProto) it will know
the attachment's size even if the attachment has never been available on
the local device.

The idea is that by storing size locally, we can simplify remote storage
management.

Move crc32c out of Attachment now that it's part of AttachmentId.

BUG=464431

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

Cr-Commit-Position: refs/heads/master@{#319794}

27 files changed, 215 insertions, 115 deletions

diff --git a/sync/api/attachments/attachment.cc b/sync/api/attachments/attachment.cc
index b682520..32a514a 100644
--- a/sync/api/attachments/attachment.cc
+++ b/sync/api/attachments/attachment.cc
@@ -15,15 +15,14 @@ Attachment::~Attachment() {}
 Attachment Attachment::Create(
     const scoped_refptr<base::RefCountedMemory>& data) {
   uint32_t crc32c = ComputeCrc32c(data);
-  return CreateFromParts(AttachmentId::Create(), data, crc32c);
+  return CreateFromParts(AttachmentId::Create(data->size(), crc32c), data);
 }
 
 // Static.
 Attachment Attachment::CreateFromParts(
     const AttachmentId& id,
-    const scoped_refptr<base::RefCountedMemory>& data,
-    uint32_t crc32c) {
-  return Attachment(id, data, crc32c);
+    const scoped_refptr<base::RefCountedMemory>& data) {
+  return Attachment(id, data);
 }
 
 const AttachmentId& Attachment::GetId() const { return id_; }
@@ -32,12 +31,14 @@ const scoped_refptr<base::RefCountedMemory>& Attachment::GetData() const {
   return data_;
 }
 
-uint32_t Attachment::GetCrc32c() const { return crc32c_; }
+uint32_t Attachment::GetCrc32c() const {
+  return id_.GetCrc32c();
+}
 
 Attachment::Attachment(const AttachmentId& id,
-                       const scoped_refptr<base::RefCountedMemory>& data,
-                       uint32_t crc32c)
-    : id_(id), data_(data), crc32c_(crc32c) {
+                       const scoped_refptr<base::RefCountedMemory>& data)
+    : id_(id), data_(data) {
+  DCHECK_EQ(id.GetSize(), data->size());
   DCHECK(data.get());
 }
 
diff --git a/sync/api/attachments/attachment.h b/sync/api/attachments/attachment.h
index 51fe986..7999269 100644
--- a/sync/api/attachments/attachment.h
+++ b/sync/api/attachments/attachment.h
@@ -34,14 +34,13 @@ class SYNC_EXPORT Attachment {
   // Used when creating a brand new attachment.
   static Attachment Create(const scoped_refptr<base::RefCountedMemory>& data);
 
-  // Creates an attachment with the supplied id, data and crc32c.
+  // Creates an attachment with the supplied id and data.
   //
   // Used when you want to recreate a specific attachment. E.g. creating a local
   // copy of an attachment that already exists on the sync server.
   static Attachment CreateFromParts(
       const AttachmentId& id,
-      const scoped_refptr<base::RefCountedMemory>& data,
-      uint32_t crc32c);
+      const scoped_refptr<base::RefCountedMemory>& data);
 
   // Returns this attachment's id.
   const AttachmentId& GetId() const;
@@ -58,11 +57,9 @@ class SYNC_EXPORT Attachment {
  private:
   AttachmentId id_;
   scoped_refptr<base::RefCountedMemory> data_;
-  uint32_t crc32c_;
 
   Attachment(const AttachmentId& id,
-             const scoped_refptr<base::RefCountedMemory>& data,
-             uint32_t crc32c);
+             const scoped_refptr<base::RefCountedMemory>& data);
 };
 
 typedef std::vector<syncer::Attachment> AttachmentList;
diff --git a/sync/api/attachments/attachment_id.cc b/sync/api/attachments/attachment_id.cc
index e61705e..07f3410 100644
--- a/sync/api/attachments/attachment_id.cc
+++ b/sync/api/attachments/attachment_id.cc
@@ -52,8 +52,8 @@ bool AttachmentId::operator<(const AttachmentId& other) const {
 }
 
 // Static.
-AttachmentId AttachmentId::Create() {
-  sync_pb::AttachmentIdProto proto = CreateAttachmentIdProto();
+AttachmentId AttachmentId::Create(size_t size, uint32_t crc32c) {
+  sync_pb::AttachmentIdProto proto = CreateAttachmentIdProto(size, crc32c);
   return AttachmentId(&proto);
 }
 
@@ -71,4 +71,12 @@ const sync_pb::AttachmentIdProto& AttachmentId::GetProto() const {
 AttachmentId::AttachmentId(sync_pb::AttachmentIdProto* proto)
     : proto_(proto) {}
 
+size_t AttachmentId::GetSize() const {
+  return proto_.Get().size_bytes();
+}
+
+uint32_t AttachmentId::GetCrc32c() const {
+  return proto_.Get().crc32c();
+}
+
 }  // namespace syncer
diff --git a/sync/api/attachments/attachment_id.h b/sync/api/attachments/attachment_id.h
index 6ce331a..1c53355 100644
--- a/sync/api/attachments/attachment_id.h
+++ b/sync/api/attachments/attachment_id.h
@@ -35,14 +35,24 @@ class SYNC_EXPORT AttachmentId {
   // Needed for using AttachmentId as key in std::map.
   bool operator<(const AttachmentId& other) const;
 
-  // Creates a unique attachment id.
-  static AttachmentId Create();
+  // Creates a unique id for an attachment.
+  //
+  // |size| is the attachment's size in bytes.
+  //
+  // |crc32c| is the attachment's crc32c.
+  static AttachmentId Create(size_t size, uint32_t crc32c);
 
   // Creates an attachment id from an initialized proto.
   static AttachmentId CreateFromProto(const sync_pb::AttachmentIdProto& proto);
 
   const sync_pb::AttachmentIdProto& GetProto() const;
 
+  // Returns the size (in bytes) the attachment.
+  size_t GetSize() const;
+
+  // Returns the crc32c the attachment.
+  uint32_t GetCrc32c() const;
+
  private:
   // Necessary since we forward-declare sync_pb::AttachmentIdProto; see comments
   // in immutable.h.
diff --git a/sync/api/attachments/attachment_id_unittest.cc b/sync/api/attachments/attachment_id_unittest.cc
index 3e6bb0a..dae5182 100644
--- a/sync/api/attachments/attachment_id_unittest.cc
+++ b/sync/api/attachments/attachment_id_unittest.cc
@@ -10,29 +10,25 @@
 
 namespace syncer {
 
-namespace {
-
-}  // namespace
-
 class AttachmentIdTest : public testing::Test {};
 
 TEST_F(AttachmentIdTest, Create_IsUnique) {
-  AttachmentId id1 = AttachmentId::Create();
-  AttachmentId id2 = AttachmentId::Create();
+  AttachmentId id1 = AttachmentId::Create(0, 0);
+  AttachmentId id2 = AttachmentId::Create(0, 0);
   EXPECT_NE(id1, id2);
 }
 
 TEST_F(AttachmentIdTest, OperatorEqual) {
-  AttachmentId id1 = AttachmentId::Create();
+  AttachmentId id1 = AttachmentId::Create(0, 0);
   AttachmentId id2(id1);
   EXPECT_EQ(id1, id2);
 }
 
 TEST_F(AttachmentIdTest, OperatorLess) {
-  AttachmentId id1 = AttachmentId::Create();
+  AttachmentId id1 = AttachmentId::Create(0, 0);
   EXPECT_FALSE(id1 < id1);
 
-  AttachmentId id2 = AttachmentId::Create();
+  AttachmentId id2 = AttachmentId::Create(0, 0);
   EXPECT_FALSE(id1 < id1);
 
   EXPECT_NE(id1, id2);
diff --git a/sync/api/attachments/attachment_metadata.h b/sync/api/attachments/attachment_metadata.h
index 10761f8..6eef6c6 100644
--- a/sync/api/attachments/attachment_metadata.h
+++ b/sync/api/attachments/attachment_metadata.h
@@ -30,6 +30,9 @@ class SYNC_EXPORT AttachmentMetadata {
   size_t GetSize() const;
 
  private:
+  // TODO(maniscalco): Reconcile AttachmentMetadata and
+  // AttachmentId. AttachmentId knows the size of the attachment so
+  // AttachmentMetadata may not be necessary (crbug/465375).
   AttachmentId id_;
   size_t size_;
 };
diff --git a/sync/api/attachments/attachment_metadata_unittest.cc b/sync/api/attachments/attachment_metadata_unittest.cc
index 06477d0..a736b7e 100644
--- a/sync/api/attachments/attachment_metadata_unittest.cc
+++ b/sync/api/attachments/attachment_metadata_unittest.cc
@@ -11,8 +11,9 @@ namespace syncer {
 class AttachmentMetadataTest : public testing::Test {};
 
 TEST_F(AttachmentMetadataTest, Create) {
-  AttachmentId id = AttachmentId::Create();
   size_t size = 42;
+  uint32_t crc32c = 2349829;
+  AttachmentId id = AttachmentId::Create(size, crc32c);
   AttachmentMetadata metadata(id, size);
   EXPECT_EQ(metadata.GetId(), id);
   EXPECT_EQ(metadata.GetSize(), size);
diff --git a/sync/api/attachments/attachment_unittest.cc b/sync/api/attachments/attachment_unittest.cc
index 49b4cab..cd1e3e4 100644
--- a/sync/api/attachments/attachment_unittest.cc
+++ b/sync/api/attachments/attachment_unittest.cc
@@ -38,11 +38,11 @@ TEST_F(AttachmentTest, Create_WithEmptyData) {
 }
 
 TEST_F(AttachmentTest, CreateFromParts_HappyCase) {
-  AttachmentId id = AttachmentId::Create();
   scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString);
   some_data->data() = kAttachmentData;
   uint32_t crc32c = ComputeCrc32c(some_data);
-  Attachment a = Attachment::CreateFromParts(id, some_data, crc32c);
+  AttachmentId id = AttachmentId::Create(some_data->size(), crc32c);
+  Attachment a = Attachment::CreateFromParts(id, some_data);
   EXPECT_EQ(id, a.GetId());
   EXPECT_EQ(some_data, a.GetData());
 }
diff --git a/sync/api/sync_data_unittest.cc b/sync/api/sync_data_unittest.cc
index 94c3f4c..9d45149 100644
--- a/sync/api/sync_data_unittest.cc
+++ b/sync/api/sync_data_unittest.cc
@@ -72,9 +72,9 @@ TEST_F(SyncDataTest, CreateLocalData) {
 TEST_F(SyncDataTest, CreateLocalDataWithAttachments) {
   specifics.mutable_preference();
   AttachmentIdList attachment_ids;
-  attachment_ids.push_back(AttachmentId::Create());
-  attachment_ids.push_back(AttachmentId::Create());
-  attachment_ids.push_back(AttachmentId::Create());
+  attachment_ids.push_back(AttachmentId::Create(0, 0));
+  attachment_ids.push_back(AttachmentId::Create(0, 0));
+  attachment_ids.push_back(AttachmentId::Create(0, 0));
 
   SyncData data = SyncData::CreateLocalDataWithAttachments(
       kSyncTag, kNonUniqueTitle, specifics, attachment_ids);
diff --git a/sync/engine/directory_commit_contribution_unittest.cc b/sync/engine/directory_commit_contribution_unittest.cc
index b5612b6..720f088 100644
--- a/sync/engine/directory_commit_contribution_unittest.cc
+++ b/sync/engine/directory_commit_contribution_unittest.cc
@@ -350,7 +350,7 @@ TEST_F(DirectoryCommitContributionTest, HierarchySupport_Preferences) {
 
 void AddAttachment(sync_pb::AttachmentMetadata* metadata, bool is_on_server) {
   sync_pb::AttachmentMetadataRecord record;
-  *record.mutable_id() = CreateAttachmentIdProto();
+  *record.mutable_id() = CreateAttachmentIdProto(0, 0);
   record.set_is_on_server(is_on_server);
   *metadata->add_record() = record;
 }
diff --git a/sync/engine/directory_update_handler_unittest.cc b/sync/engine/directory_update_handler_unittest.cc
index 46e2860..2311d0f 100644
--- a/sync/engine/directory_update_handler_unittest.cc
+++ b/sync/engine/directory_update_handler_unittest.cc
@@ -401,7 +401,7 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest,
   scoped_ptr<sync_pb::SyncEntity> e1 = CreateUpdate(
       SyncableIdToProto(Id::CreateFromServerId("e1")), "", ARTICLES);
   sync_pb::AttachmentIdProto* attachment_id = e1->add_attachment_id();
-  *attachment_id = CreateAttachmentIdProto();
+  *attachment_id = CreateAttachmentIdProto(0, 0);
 
   SyncEntityList updates;
   updates.push_back(e1.get());
@@ -1080,8 +1080,10 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest,
   *specifics.mutable_article() = sync_pb::ArticleSpecifics();
   int64 handle = entry_factory()->CreateSyncedItem("art1", ARTICLES, is_folder);
 
-  sync_pb::AttachmentIdProto local_attachment_id = CreateAttachmentIdProto();
-  sync_pb::AttachmentIdProto server_attachment_id = CreateAttachmentIdProto();
+  sync_pb::AttachmentIdProto local_attachment_id =
+      CreateAttachmentIdProto(0, 0);
+  sync_pb::AttachmentIdProto server_attachment_id =
+      CreateAttachmentIdProto(0, 0);
 
   // Add an attachment to the local attachment metadata.
   sync_pb::AttachmentMetadata local_metadata;
@@ -1122,8 +1124,8 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest,
   *specifics.mutable_article() = sync_pb::ArticleSpecifics();
   int64 handle = entry_factory()->CreateSyncedItem("art1", ARTICLES, is_folder);
 
-  sync_pb::AttachmentIdProto id1 = CreateAttachmentIdProto();
-  sync_pb::AttachmentIdProto id2 = CreateAttachmentIdProto();
+  sync_pb::AttachmentIdProto id1 = CreateAttachmentIdProto(0, 0);
+  sync_pb::AttachmentIdProto id2 = CreateAttachmentIdProto(0, 0);
 
   // Add id1, then id2 to the local attachment metadata.
   sync_pb::AttachmentMetadata local_metadata;
diff --git a/sync/internal_api/attachments/attachment_downloader_impl.cc b/sync/internal_api/attachments/attachment_downloader_impl.cc
index 56a2c34..f2e6a00 100644
--- a/sync/internal_api/attachments/attachment_downloader_impl.cc
+++ b/sync/internal_api/attachments/attachment_downloader_impl.cc
@@ -132,7 +132,7 @@ void AttachmentDownloaderImpl::OnGetTokenFailure(
     DownloadState* download_state = *iter;
     scoped_refptr<base::RefCountedString> null_attachment_data;
     ReportResult(*download_state, DOWNLOAD_TRANSIENT_ERROR,
-                 null_attachment_data, 0);
+                 null_attachment_data);
     DCHECK(state_map_.find(download_state->attachment_url) != state_map_.end());
     state_map_.erase(download_state->attachment_url);
   }
@@ -175,7 +175,13 @@ void AttachmentDownloaderImpl::OnURLFetchComplete(
       // locally calculated one will be stored and used for further checks.
       result = DOWNLOAD_TRANSIENT_ERROR;
     } else {
-      result = DOWNLOAD_SUCCESS;
+      // If the id's crc32c doesn't match that of the downloaded attachment,
+      // then we're stuck and retrying is unlikely to help.
+      if (attachment_crc32c != download_state.attachment_id.GetCrc32c()) {
+        result = DOWNLOAD_UNSPECIFIED_ERROR;
+      } else {
+        result = DOWNLOAD_SUCCESS;
+      }
     }
     UMA_HISTOGRAM_BOOLEAN("Sync.Attachments.DownloadChecksumResult",
                           result == DOWNLOAD_SUCCESS);
@@ -193,7 +199,7 @@ void AttachmentDownloaderImpl::OnURLFetchComplete(
   } else if (response_code == net::URLFetcher::RESPONSE_CODE_INVALID) {
     result = DOWNLOAD_TRANSIENT_ERROR;
   }
-  ReportResult(download_state, result, attachment_data, attachment_crc32c);
+  ReportResult(download_state, result, attachment_data);
   state_map_.erase(iter);
 }
 
@@ -221,8 +227,7 @@ void AttachmentDownloaderImpl::RequestAccessToken(
 void AttachmentDownloaderImpl::ReportResult(
     const DownloadState& download_state,
     const DownloadResult& result,
-    const scoped_refptr<base::RefCountedString>& attachment_data,
-    uint32_t attachment_crc32c) {
+    const scoped_refptr<base::RefCountedString>& attachment_data) {
   std::vector<DownloadCallback>::const_iterator iter;
   for (iter = download_state.user_callbacks.begin();
        iter != download_state.user_callbacks.end();
@@ -230,7 +235,7 @@ void AttachmentDownloaderImpl::ReportResult(
     scoped_ptr<Attachment> attachment;
     if (result == DOWNLOAD_SUCCESS) {
       attachment.reset(new Attachment(Attachment::CreateFromParts(
-          download_state.attachment_id, attachment_data, attachment_crc32c)));
+          download_state.attachment_id, attachment_data)));
     }
 
     base::MessageLoop::current()->PostTask(
diff --git a/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc b/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
index 1d026eb..0cae099 100644
--- a/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
@@ -152,7 +152,12 @@ class AttachmentDownloaderImplTest : public testing::Test {
     HASH_HEADER_INVALID
   };
 
-  AttachmentDownloaderImplTest() : num_completed_downloads_(0) {}
+  AttachmentDownloaderImplTest()
+      : num_completed_downloads_(0),
+        attachment_id_(
+            Attachment::Create(new base::RefCountedStaticMemory(
+                                   kAttachmentContent,
+                                   strlen(kAttachmentContent))).GetId()) {}
 
   void SetUp() override;
   void TearDown() override;
@@ -163,6 +168,8 @@ class AttachmentDownloaderImplTest : public testing::Test {
 
   int num_completed_downloads() { return num_completed_downloads_; }
 
+  const AttachmentId attachment_id() const { return attachment_id_; }
+
   AttachmentDownloader::DownloadCallback download_callback(
       const AttachmentId& id) {
     return base::Bind(&AttachmentDownloaderImplTest::DownloadDone,
@@ -193,6 +200,7 @@ class AttachmentDownloaderImplTest : public testing::Test {
   scoped_ptr<AttachmentDownloader> attachment_downloader_;
   ResultsMap download_results_;
   int num_completed_downloads_;
+  const AttachmentId attachment_id_;
 };
 
 void AttachmentDownloaderImplTest::SetUp() {
@@ -294,7 +302,7 @@ void AttachmentDownloaderImplTest::AddHashHeader(
 }
 
 TEST_F(AttachmentDownloaderImplTest, HappyCase) {
-  AttachmentId id1 = AttachmentId::Create();
+  AttachmentId id1 = attachment_id();
   // DownloadAttachment should trigger RequestAccessToken.
   downloader()->DownloadAttachment(id1, download_callback(id1));
   RunMessageLoop();
@@ -314,7 +322,7 @@ TEST_F(AttachmentDownloaderImplTest, HappyCase) {
 }
 
 TEST_F(AttachmentDownloaderImplTest, SameIdMultipleDownloads) {
-  AttachmentId id1 = AttachmentId::Create();
+  AttachmentId id1 = attachment_id();
   base::HistogramTester histogram_tester;
   // Call DownloadAttachment two times for the same id.
   downloader()->DownloadAttachment(id1, download_callback(id1));
@@ -351,8 +359,8 @@ TEST_F(AttachmentDownloaderImplTest, SameIdMultipleDownloads) {
 }
 
 TEST_F(AttachmentDownloaderImplTest, RequestAccessTokenFails) {
-  AttachmentId id1 = AttachmentId::Create();
-  AttachmentId id2 = AttachmentId::Create();
+  AttachmentId id1 = attachment_id();
+  AttachmentId id2 = AttachmentId::Create(id1.GetSize(), id1.GetCrc32c());
   // Trigger first RequestAccessToken.
   downloader()->DownloadAttachment(id1, download_callback(id1));
   RunMessageLoop();
@@ -375,7 +383,7 @@ TEST_F(AttachmentDownloaderImplTest, RequestAccessTokenFails) {
 }
 
 TEST_F(AttachmentDownloaderImplTest, URLFetcher_BadToken) {
-  AttachmentId id1 = AttachmentId::Create();
+  AttachmentId id1 = attachment_id();
   downloader()->DownloadAttachment(id1, download_callback(id1));
   RunMessageLoop();
   // Return valid access token.
@@ -393,7 +401,7 @@ TEST_F(AttachmentDownloaderImplTest, URLFetcher_BadToken) {
 }
 
 TEST_F(AttachmentDownloaderImplTest, URLFetcher_ServiceUnavailable) {
-  AttachmentId id1 = AttachmentId::Create();
+  AttachmentId id1 = attachment_id();
   downloader()->DownloadAttachment(id1, download_callback(id1));
   RunMessageLoop();
   // Return valid access token.
@@ -413,7 +421,7 @@ TEST_F(AttachmentDownloaderImplTest, URLFetcher_ServiceUnavailable) {
 // Verify that if no hash is present on the response the downloader accepts the
 // received attachment.
 TEST_F(AttachmentDownloaderImplTest, NoHash) {
-  AttachmentId id1 = AttachmentId::Create();
+  AttachmentId id1 = attachment_id();
   downloader()->DownloadAttachment(id1, download_callback(id1));
   RunMessageLoop();
   token_service()->RespondToAccessTokenRequest(
@@ -426,7 +434,7 @@ TEST_F(AttachmentDownloaderImplTest, NoHash) {
 // Verify that if an invalid hash is present on the response the downloader
 // treats it as a transient error.
 TEST_F(AttachmentDownloaderImplTest, InvalidHash) {
-  AttachmentId id1 = AttachmentId::Create();
+  AttachmentId id1 = attachment_id();
   downloader()->DownloadAttachment(id1, download_callback(id1));
   RunMessageLoop();
   token_service()->RespondToAccessTokenRequest(
@@ -436,6 +444,21 @@ TEST_F(AttachmentDownloaderImplTest, InvalidHash) {
   VerifyDownloadResult(id1, AttachmentDownloader::DOWNLOAD_TRANSIENT_ERROR);
 }
 
+// Verify that when the hash from the attachment id does not match the one on
+// the response the result is an unspecified error.
+TEST_F(AttachmentDownloaderImplTest, IdHashDoesNotMatch) {
+  // id1 has the wrong crc32c.
+  AttachmentId id1 = AttachmentId::Create(attachment_id().GetSize(), 12345);
+  downloader()->DownloadAttachment(id1, download_callback(id1));
+  RunMessageLoop();
+  token_service()->RespondToAccessTokenRequest(
+      GoogleServiceAuthError::AuthErrorNone());
+  RunMessageLoop();
+  CompleteDownload(net::HTTP_OK, HASH_HEADER_VALID);
+  VerifyDownloadResult(id1, AttachmentDownloader::DOWNLOAD_UNSPECIFIED_ERROR);
+}
+
+
 // Verify that extract fails when there is no headers object.
 TEST_F(AttachmentDownloaderImplTest, ExtractCrc32c_NoHeaders) {
   uint32_t extracted;
diff --git a/sync/internal_api/attachments/attachment_service_impl_unittest.cc b/sync/internal_api/attachments/attachment_service_impl_unittest.cc
index e72bb8f..c90bcd9 100644
--- a/sync/internal_api/attachments/attachment_service_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_service_impl_unittest.cc
@@ -68,9 +68,7 @@ class MockAttachmentStore : public AttachmentStore,
     for (AttachmentIdList::const_iterator iter = ids.begin(); iter != ids.end();
          ++iter) {
       if (local_attachments.find(*iter) != local_attachments.end()) {
-        uint32_t crc32c = ComputeCrc32c(data);
-        Attachment attachment =
-            Attachment::CreateFromParts(*iter, data, crc32c);
+        Attachment attachment = Attachment::CreateFromParts(*iter, data);
         attachments->insert(std::make_pair(*iter, attachment));
       } else {
         unavailable_attachments->push_back(*iter);
@@ -127,9 +125,7 @@ class MockAttachmentDownloader
     scoped_ptr<Attachment> attachment;
     if (result == DOWNLOAD_SUCCESS) {
       scoped_refptr<base::RefCountedString> data = new base::RefCountedString();
-      uint32_t crc32c = ComputeCrc32c(data);
-      attachment.reset(
-          new Attachment(Attachment::CreateFromParts(id, data, crc32c)));
+      attachment.reset(new Attachment(Attachment::CreateFromParts(id, data)));
     }
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
@@ -309,7 +305,7 @@ TEST_F(AttachmentServiceImplTest, GetOrDownload_EmptyAttachmentList) {
 
 TEST_F(AttachmentServiceImplTest, GetOrDownload_Local) {
   AttachmentIdList attachment_ids;
-  attachment_ids.push_back(AttachmentId::Create());
+  attachment_ids.push_back(AttachmentId::Create(0, 0));
   attachment_service()->GetOrDownloadAttachments(attachment_ids,
                                                  download_callback());
   AttachmentIdSet local_attachments;
@@ -326,10 +322,10 @@ TEST_F(AttachmentServiceImplTest, GetOrDownload_Local) {
 TEST_F(AttachmentServiceImplTest, GetOrDownload_LocalRemoteUnavailable) {
   // Create attachment list with 4 ids.
   AttachmentIdList attachment_ids;
-  attachment_ids.push_back(AttachmentId::Create());
-  attachment_ids.push_back(AttachmentId::Create());
-  attachment_ids.push_back(AttachmentId::Create());
-  attachment_ids.push_back(AttachmentId::Create());
+  attachment_ids.push_back(AttachmentId::Create(0, 0));
+  attachment_ids.push_back(AttachmentId::Create(0, 0));
+  attachment_ids.push_back(AttachmentId::Create(0, 0));
+  attachment_ids.push_back(AttachmentId::Create(0, 0));
   // Call attachment service.
   attachment_service()->GetOrDownloadAttachments(attachment_ids,
                                                  download_callback());
@@ -395,7 +391,7 @@ TEST_F(AttachmentServiceImplTest, GetOrDownload_NoDownloader) {
       this);
 
   AttachmentIdList attachment_ids;
-  attachment_ids.push_back(AttachmentId::Create());
+  attachment_ids.push_back(AttachmentId::Create(0, 0));
   attachment_service()->GetOrDownloadAttachments(attachment_ids,
                                                  download_callback());
   EXPECT_FALSE(store()->read_ids.empty());
@@ -412,7 +408,7 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_Success) {
   AttachmentIdSet attachment_ids;
   const unsigned num_attachments = 3;
   for (unsigned i = 0; i < num_attachments; ++i) {
-    attachment_ids.insert(AttachmentId::Create());
+    attachment_ids.insert(AttachmentId::Create(0, 0));
   }
   attachment_service()->UploadAttachments(attachment_ids);
 
@@ -446,7 +442,7 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_Success_NoDelegate) {
                               NULL);  // No delegate.
 
   AttachmentIdSet attachment_ids;
-  attachment_ids.insert(AttachmentId::Create());
+  attachment_ids.insert(AttachmentId::Create(0, 0));
   attachment_service()->UploadAttachments(attachment_ids);
   RunLoopAndFireTimer();
   ASSERT_EQ(1U, store()->read_ids.size());
@@ -463,8 +459,8 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_Success_NoDelegate) {
 
 TEST_F(AttachmentServiceImplTest, UploadAttachments_SomeMissingFromStore) {
   AttachmentIdSet attachment_ids;
-  attachment_ids.insert(AttachmentId::Create());
-  attachment_ids.insert(AttachmentId::Create());
+  attachment_ids.insert(AttachmentId::Create(0, 0));
+  attachment_ids.insert(AttachmentId::Create(0, 0));
   attachment_service()->UploadAttachments(attachment_ids);
   RunLoopAndFireTimer();
   ASSERT_GE(store()->read_ids.size(), 1U);
@@ -490,7 +486,7 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_AllMissingFromStore) {
   AttachmentIdSet attachment_ids;
   const unsigned num_attachments = 2;
   for (unsigned i = 0; i < num_attachments; ++i) {
-    attachment_ids.insert(AttachmentId::Create());
+    attachment_ids.insert(AttachmentId::Create(0, 0));
   }
   attachment_service()->UploadAttachments(attachment_ids);
 
@@ -514,7 +510,7 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_NoUploader) {
                               this);
 
   AttachmentIdSet attachment_ids;
-  attachment_ids.insert(AttachmentId::Create());
+  attachment_ids.insert(AttachmentId::Create(0, 0));
   attachment_service()->UploadAttachments(attachment_ids);
   RunLoop();
   EXPECT_EQ(0U, store()->read_ids.size());
@@ -526,7 +522,7 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_OneUploadFails) {
   AttachmentIdSet attachment_ids;
   const unsigned num_attachments = 3;
   for (unsigned i = 0; i < num_attachments; ++i) {
-    attachment_ids.insert(AttachmentId::Create());
+    attachment_ids.insert(AttachmentId::Create(0, 0));
   }
   attachment_service()->UploadAttachments(attachment_ids);
 
@@ -556,7 +552,7 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_OneUploadFails) {
 TEST_F(AttachmentServiceImplTest,
        UploadAttachments_ResetBackoffAfterNetworkChange) {
   AttachmentIdSet attachment_ids;
-  attachment_ids.insert(AttachmentId::Create());
+  attachment_ids.insert(AttachmentId::Create(0, 0));
   attachment_service()->UploadAttachments(attachment_ids);
 
   RunLoopAndFireTimer();
diff --git a/sync/internal_api/attachments/attachment_store_test_template.h b/sync/internal_api/attachments/attachment_store_test_template.h
index 290ef22..8e40f64 100644
--- a/sync/internal_api/attachments/attachment_store_test_template.h
+++ b/sync/internal_api/attachments/attachment_store_test_template.h
@@ -133,9 +133,8 @@ TYPED_TEST_CASE_P(AttachmentStoreTest);
 TYPED_TEST_P(AttachmentStoreTest, Write_NoOverwriteNoError) {
   // Create two attachments with the same id but different data.
   Attachment attachment1 = Attachment::Create(this->some_data1);
-  uint32_t crc32c = ComputeCrc32c(this->some_data2);
-  Attachment attachment2 = Attachment::CreateFromParts(
-      attachment1.GetId(), this->some_data2, crc32c);
+  Attachment attachment2 =
+      Attachment::CreateFromParts(attachment1.GetId(), this->some_data2);
 
   // Write the first one.
   AttachmentList some_attachments;
diff --git a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
index 4af6cd8..527b00e 100644
--- a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
@@ -409,7 +409,7 @@ net::HttpStatusCode RequestHandler::GetStatusCode() const {
 }
 
 TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_NoPath) {
-  AttachmentId id = AttachmentId::Create();
+  AttachmentId id = AttachmentId::Create(0, 0);
   std::string unique_id = id.GetProto().unique_id();
   GURL sync_service_url("https://example.com");
   EXPECT_EQ("https://example.com/attachments/" + unique_id,
@@ -418,7 +418,7 @@ TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_NoPath) {
 }
 
 TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_JustSlash) {
-  AttachmentId id = AttachmentId::Create();
+  AttachmentId id = AttachmentId::Create(0, 0);
   std::string unique_id = id.GetProto().unique_id();
   GURL sync_service_url("https://example.com/");
   EXPECT_EQ("https://example.com/attachments/" + unique_id,
@@ -427,7 +427,7 @@ TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_JustSlash) {
 }
 
 TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_Path) {
-  AttachmentId id = AttachmentId::Create();
+  AttachmentId id = AttachmentId::Create(0, 0);
   std::string unique_id = id.GetProto().unique_id();
   GURL sync_service_url("https://example.com/service");
   EXPECT_EQ("https://example.com/service/attachments/" + unique_id,
@@ -436,7 +436,7 @@ TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_Path) {
 }
 
 TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_PathAndSlash) {
-  AttachmentId id = AttachmentId::Create();
+  AttachmentId id = AttachmentId::Create(0, 0);
   std::string unique_id = id.GetProto().unique_id();
   GURL sync_service_url("https://example.com/service/");
   EXPECT_EQ("https://example.com/service/attachments/" + unique_id,
diff --git a/sync/internal_api/attachments/fake_attachment_downloader.cc b/sync/internal_api/attachments/fake_attachment_downloader.cc
index 53abc61..299c418 100644
--- a/sync/internal_api/attachments/fake_attachment_downloader.cc
+++ b/sync/internal_api/attachments/fake_attachment_downloader.cc
@@ -25,9 +25,8 @@ void FakeAttachmentDownloader::DownloadAttachment(
   // attachment.
   scoped_refptr<base::RefCountedMemory> data(new base::RefCountedBytes());
   scoped_ptr<Attachment> attachment;
-  const uint32_t crc32c = ComputeCrc32c(data);
   attachment.reset(
-      new Attachment(Attachment::CreateFromParts(attachment_id, data, crc32c)));
+      new Attachment(Attachment::CreateFromParts(attachment_id, data)));
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(callback, DOWNLOAD_SUCCESS, base::Passed(&attachment)));
diff --git a/sync/internal_api/attachments/fake_attachment_downloader_unittest.cc b/sync/internal_api/attachments/fake_attachment_downloader_unittest.cc
index 1436a49..762da44 100644
--- a/sync/internal_api/attachments/fake_attachment_downloader_unittest.cc
+++ b/sync/internal_api/attachments/fake_attachment_downloader_unittest.cc
@@ -50,7 +50,7 @@ class FakeAttachmentDownloaderTest : public testing::Test {
 };
 
 TEST_F(FakeAttachmentDownloaderTest, DownloadAttachment) {
-  AttachmentId attachment_id = AttachmentId::Create();
+  AttachmentId attachment_id = AttachmentId::Create(0, 0);
   downloader()->DownloadAttachment(attachment_id, download_callback());
   RunMessageLoop();
   EXPECT_EQ(1U, download_results().size());
diff --git a/sync/internal_api/attachments/on_disk_attachment_store.cc b/sync/internal_api/attachments/on_disk_attachment_store.cc
index 7b715e0..b054a6e 100644
--- a/sync/internal_api/attachments/on_disk_attachment_store.cc
+++ b/sync/internal_api/attachments/on_disk_attachment_store.cc
@@ -316,12 +316,18 @@ scoped_ptr<Attachment> OnDiskAttachmentStore::ReadSingleAttachment(
   scoped_refptr<base::RefCountedMemory> data =
       base::RefCountedString::TakeString(&data_str);
   uint32_t crc32c = ComputeCrc32c(data);
-  if (record_metadata.has_crc32c() && record_metadata.crc32c() != crc32c) {
-    DVLOG(1) << "Attachment crc does not match";
-    return attachment.Pass();
+  if (record_metadata.has_crc32c()) {
+    if (record_metadata.crc32c() != crc32c) {
+      DVLOG(1) << "Attachment crc32c does not match value read from store";
+      return attachment.Pass();
+    }
+    if (record_metadata.crc32c() != attachment_id.GetCrc32c()) {
+      DVLOG(1) << "Attachment crc32c does not match value in AttachmentId";
+      return attachment.Pass();
+    }
   }
   attachment.reset(
-      new Attachment(Attachment::CreateFromParts(attachment_id, data, crc32c)));
+      new Attachment(Attachment::CreateFromParts(attachment_id, data)));
   return attachment.Pass();
 }
 
diff --git a/sync/internal_api/attachments/on_disk_attachment_store_unittest.cc b/sync/internal_api/attachments/on_disk_attachment_store_unittest.cc
index e46f060..231ad96 100644
--- a/sync/internal_api/attachments/on_disk_attachment_store_unittest.cc
+++ b/sync/internal_api/attachments/on_disk_attachment_store_unittest.cc
@@ -311,8 +311,9 @@ TEST_F(OnDiskAttachmentStoreSpecificTest, RecordMetadata) {
   VerifyAttachmentRecordsPresent(attachments[1].GetId(), true);
 }
 
-// Ensure that attachment store fails to load attachment with mismatched crc.
-TEST_F(OnDiskAttachmentStoreSpecificTest, MismatchedCrc) {
+// Ensure that attachment store fails to load attachment if the crc in the store
+// does not match the data.
+TEST_F(OnDiskAttachmentStoreSpecificTest, MismatchedCrcInStore) {
   // Create attachment store.
   AttachmentStore::Result create_result = AttachmentStore::UNSPECIFIED_ERROR;
   store_ = AttachmentStore::CreateOnDiskStore(
@@ -322,10 +323,12 @@ TEST_F(OnDiskAttachmentStoreSpecificTest, MismatchedCrc) {
   // Write attachment with incorrect crc32c.
   AttachmentStore::Result write_result = AttachmentStore::UNSPECIFIED_ERROR;
   const uint32_t intentionally_wrong_crc32c = 0;
-  std::string some_data("data1");
+
+  scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString());
+  some_data->data() = "data1";
   Attachment attachment = Attachment::CreateFromParts(
-      AttachmentId::Create(), base::RefCountedString::TakeString(&some_data),
-      intentionally_wrong_crc32c);
+      AttachmentId::Create(some_data->size(), intentionally_wrong_crc32c),
+      some_data);
   AttachmentList attachments;
   attachments.push_back(attachment);
   store_->Write(attachments,
@@ -348,6 +351,44 @@ TEST_F(OnDiskAttachmentStoreSpecificTest, MismatchedCrc) {
   EXPECT_THAT(failed_attachment_ids, testing::ElementsAre(attachment.GetId()));
 }
 
+// Ensure that attachment store fails to load attachment if the crc in the id
+// does not match the data.
+TEST_F(OnDiskAttachmentStoreSpecificTest, MismatchedCrcInId) {
+  // Create attachment store.
+  AttachmentStore::Result create_result = AttachmentStore::UNSPECIFIED_ERROR;
+  store_ = AttachmentStore::CreateOnDiskStore(
+      temp_dir_.path(), base::ThreadTaskRunnerHandle::Get(),
+      base::Bind(&AttachmentStoreCreated, &create_result));
+
+  AttachmentStore::Result write_result = AttachmentStore::UNSPECIFIED_ERROR;
+  scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString());
+  some_data->data() = "data1";
+  Attachment attachment = Attachment::Create(some_data);
+  AttachmentList attachments;
+  attachments.push_back(attachment);
+  store_->Write(attachments,
+                base::Bind(&OnDiskAttachmentStoreSpecificTest::CopyResult,
+                           base::Unretained(this), &write_result));
+
+  // Read, but with the wrong crc32c in the id.
+  AttachmentStore::Result read_result = AttachmentStore::SUCCESS;
+
+  AttachmentId id_with_bad_crc32c =
+      AttachmentId::Create(attachment.GetId().GetSize(), 12345);
+  AttachmentIdList attachment_ids;
+  attachment_ids.push_back(id_with_bad_crc32c);
+  AttachmentIdList failed_attachment_ids;
+  store_->Read(
+      attachment_ids,
+      base::Bind(&OnDiskAttachmentStoreSpecificTest::CopyResultAttachments,
+                 base::Unretained(this), &read_result, &failed_attachment_ids));
+  RunLoop();
+  EXPECT_EQ(AttachmentStore::SUCCESS, create_result);
+  EXPECT_EQ(AttachmentStore::SUCCESS, write_result);
+  EXPECT_EQ(AttachmentStore::UNSPECIFIED_ERROR, read_result);
+  EXPECT_THAT(failed_attachment_ids, testing::ElementsAre(id_with_bad_crc32c));
+}
+
 // Ensure that after store initialization failure ReadWrite/Drop operations fail
 // with correct error.
 TEST_F(OnDiskAttachmentStoreSpecificTest, OpsAfterInitializationFailed) {
@@ -365,7 +406,10 @@ TEST_F(OnDiskAttachmentStoreSpecificTest, OpsAfterInitializationFailed) {
   // STORE_INITIALIZATION_FAILED.
   AttachmentStore::Result read_result = AttachmentStore::SUCCESS;
   AttachmentIdList attachment_ids;
-  attachment_ids.push_back(AttachmentId::Create());
+  std::string some_data("data1");
+  Attachment attachment =
+      Attachment::Create(base::RefCountedString::TakeString(&some_data));
+  attachment_ids.push_back(attachment.GetId());
   AttachmentIdList failed_attachment_ids;
   store_->Read(
       attachment_ids,
@@ -382,11 +426,8 @@ TEST_F(OnDiskAttachmentStoreSpecificTest, OpsAfterInitializationFailed) {
   // Writing to uninitialized store should result in
   // STORE_INITIALIZATION_FAILED.
   AttachmentStore::Result write_result = AttachmentStore::SUCCESS;
-  std::string some_data;
   AttachmentList attachments;
-  some_data = "data1";
-  attachments.push_back(
-      Attachment::Create(base::RefCountedString::TakeString(&some_data)));
+  attachments.push_back(attachment);
   store_->Write(attachments,
                 base::Bind(&OnDiskAttachmentStoreSpecificTest::CopyResult,
                            base::Unretained(this), &write_result));
diff --git a/sync/internal_api/public/attachments/attachment_downloader_impl.h b/sync/internal_api/public/attachments/attachment_downloader_impl.h
index 4e68f27..83bb77e 100644
--- a/sync/internal_api/public/attachments/attachment_downloader_impl.h
+++ b/sync/internal_api/public/attachments/attachment_downloader_impl.h
@@ -88,8 +88,7 @@ class AttachmentDownloaderImpl : public AttachmentDownloader,
   void ReportResult(
       const DownloadState& download_state,
       const DownloadResult& result,
-      const scoped_refptr<base::RefCountedString>& attachment_data,
-      uint32_t attachment_crc32c);
+      const scoped_refptr<base::RefCountedString>& attachment_data);
 
   // Extract the crc32c from an X-Goog-Hash header in |headers|.
   //
diff --git a/sync/internal_api/public/base/attachment_id_proto.cc b/sync/internal_api/public/base/attachment_id_proto.cc
index 52728cd..6692ab7 100644
--- a/sync/internal_api/public/base/attachment_id_proto.cc
+++ b/sync/internal_api/public/base/attachment_id_proto.cc
@@ -10,13 +10,16 @@
 
 namespace syncer {
 
-sync_pb::AttachmentIdProto CreateAttachmentIdProto() {
+sync_pb::AttachmentIdProto CreateAttachmentIdProto(size_t size,
+                                                   uint32_t crc32c) {
   sync_pb::AttachmentIdProto proto;
   std::string guid = base::StringToLowerASCII(base::GenerateGUID());
   DCHECK(!guid.empty());
   // Requirements are that this id must be a unique RFC4122 UUID, formatted in
   // lower case.
   proto.set_unique_id(guid);
+  proto.set_size_bytes(size);
+  proto.set_crc32c(crc32c);
   return proto;
 }
 
diff --git a/sync/internal_api/public/base/attachment_id_proto.h b/sync/internal_api/public/base/attachment_id_proto.h
index a47f46e..9af3a986 100644
--- a/sync/internal_api/public/base/attachment_id_proto.h
+++ b/sync/internal_api/public/base/attachment_id_proto.h
@@ -5,6 +5,7 @@
 #ifndef SYNC_INTERNAL_API_PUBLIC_BASE_ATTACHMENT_ID_PROTO_H_
 #define SYNC_INTERNAL_API_PUBLIC_BASE_ATTACHMENT_ID_PROTO_H_
 
+#include "base/basictypes.h"
 #include "sync/base/sync_export.h"
 #include "sync/protocol/sync.pb.h"
 
@@ -13,7 +14,13 @@ namespace syncer {
 // Helper functions that are logically part of sync_pb::AttachmentIdProto.
 
 // Creates a unique AttachmentIdProto.
-SYNC_EXPORT_PRIVATE sync_pb::AttachmentIdProto CreateAttachmentIdProto();
+//
+// |size| is the size in bytes of the attachment identified by this proto.
+//
+// |crc32c| is the crc32c of the attachment identified by this proto.
+SYNC_EXPORT_PRIVATE sync_pb::AttachmentIdProto CreateAttachmentIdProto(
+    size_t size,
+    uint32_t crc32c);
 
 // Creates an AttachmentMetadata object from a repeated field of
 // AttachmentIdProto objects.
diff --git a/sync/internal_api/public/base/attachment_id_proto_unittest.cc b/sync/internal_api/public/base/attachment_id_proto_unittest.cc
index 5321e2b..2f9d3f0 100644
--- a/sync/internal_api/public/base/attachment_id_proto_unittest.cc
+++ b/sync/internal_api/public/base/attachment_id_proto_unittest.cc
@@ -15,7 +15,7 @@ typedef testing::Test AttachmentIdProtoTest;
 // Verify that that we generate a proto with a properly formatted unique_id
 // field.
 TEST(AttachmentIdProtoTest, UniqueIdFormat) {
-  sync_pb::AttachmentIdProto id_proto = CreateAttachmentIdProto();
+  sync_pb::AttachmentIdProto id_proto = CreateAttachmentIdProto(0, 0);
   ASSERT_TRUE(id_proto.has_unique_id());
   // gtest's regular expression support is pretty poor so we cannot test as
   // closely as we would like.
@@ -35,9 +35,9 @@ TEST(AttachmentIdProtoTest, CreateAttachmentMetadata_Empty) {
 
 TEST(AttachmentIdProtoTest, CreateAttachmentMetadata_NonEmpty) {
   google::protobuf::RepeatedPtrField<sync_pb::AttachmentIdProto> ids;
-  *ids.Add() = CreateAttachmentIdProto();
-  *ids.Add() = CreateAttachmentIdProto();
-  *ids.Add() = CreateAttachmentIdProto();
+  *ids.Add() = CreateAttachmentIdProto(0, 0);
+  *ids.Add() = CreateAttachmentIdProto(0, 0);
+  *ids.Add() = CreateAttachmentIdProto(0, 0);
   sync_pb::AttachmentMetadata metadata = CreateAttachmentMetadata(ids);
   ASSERT_EQ(3, metadata.record_size());
   for (int i = 0; i < metadata.record_size(); ++i) {
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 5e69ec5..75fbe10 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -676,7 +676,7 @@ TEST_F(SyncApiTest, GetTotalNodeCountMultipleChildren) {
 TEST_F(SyncApiTest, AttachmentLinking) {
   // Add an entry with an attachment.
   std::string tag1("some tag");
-  syncer::AttachmentId attachment_id(syncer::AttachmentId::Create());
+  syncer::AttachmentId attachment_id(syncer::AttachmentId::Create(0, 0));
   sync_pb::AttachmentMetadata attachment_metadata;
   sync_pb::AttachmentMetadataRecord* record = attachment_metadata.add_record();
   *record->mutable_id() = attachment_id.GetProto();
@@ -3112,7 +3112,7 @@ TEST_F(SyncManagerChangeProcessingTest, AttachmentMetadataOnlyChanges) {
         FROM_HERE, syncable::SYNCER, share()->directory.get());
     syncable::MutableEntry article(&trans, syncable::GET_BY_HANDLE, article_id);
     sync_pb::AttachmentMetadata metadata;
-    *metadata.add_record()->mutable_id() = CreateAttachmentIdProto();
+    *metadata.add_record()->mutable_id() = CreateAttachmentIdProto(0, 0);
     article.PutAttachmentMetadata(metadata);
   }
   ASSERT_EQ(1UL, GetChangeListSize());
@@ -3126,7 +3126,7 @@ TEST_F(SyncManagerChangeProcessingTest, AttachmentMetadataOnlyChanges) {
         FROM_HERE, syncable::SYNCER, share()->directory.get());
     syncable::MutableEntry article(&trans, syncable::GET_BY_HANDLE, article_id);
     sync_pb::AttachmentMetadata metadata = article.GetAttachmentMetadata();
-    *metadata.add_record()->mutable_id() = CreateAttachmentIdProto();
+    *metadata.add_record()->mutable_id() = CreateAttachmentIdProto(0, 0);
     article.PutAttachmentMetadata(metadata);
   }
   ASSERT_EQ(1UL, GetChangeListSize());
diff --git a/sync/protocol/attachments.proto b/sync/protocol/attachments.proto
index e08e2b0..986e29f 100644
--- a/sync/protocol/attachments.proto
+++ b/sync/protocol/attachments.proto
@@ -19,6 +19,10 @@ message AttachmentIdProto {
   // Uniquely identifies the attachment. Two attachments with the same unique_id
   // are considered equivalent.
   optional string unique_id = 1;
+  // Size of the attachment data in bytes.
+  optional uint64 size_bytes = 2;
+  // Crc32c of the attachment data.
+  optional uint32 crc32c = 3;
 }
 
 // Metadata for a single attachment.
diff --git a/sync/syncable/directory_unittest.cc b/sync/syncable/directory_unittest.cc
index 64f3b57..5f8cbed 100644
--- a/sync/syncable/directory_unittest.cc
+++ b/sync/syncable/directory_unittest.cc
@@ -1639,7 +1639,7 @@ 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();
+      syncer::CreateAttachmentIdProto(0, 0);
   *record->mutable_id() = attachment_id_proto;
   ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id_proto));
   {
@@ -1683,8 +1683,8 @@ TEST_F(SyncableDirectoryTest, MutableEntry_UpdateAttachmentId) {
   sync_pb::AttachmentMetadata attachment_metadata;
   sync_pb::AttachmentMetadataRecord* r1 = attachment_metadata.add_record();
   sync_pb::AttachmentMetadataRecord* r2 = attachment_metadata.add_record();
-  *r1->mutable_id() = syncer::CreateAttachmentIdProto();
-  *r2->mutable_id() = syncer::CreateAttachmentIdProto();
+  *r1->mutable_id() = syncer::CreateAttachmentIdProto(0, 0);
+  *r2->mutable_id() = syncer::CreateAttachmentIdProto(0, 0);
   sync_pb::AttachmentIdProto attachment_id_proto = r1->id();
 
   WriteTransaction trans(FROM_HERE, UNITTEST, dir().get());
@@ -1713,7 +1713,7 @@ 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();
+      syncer::CreateAttachmentIdProto(0, 0);
   *record->mutable_id() = attachment_id_proto;
   ASSERT_FALSE(dir()->IsAttachmentLinked(attachment_id_proto));
   const Id id = TestIdFactory::FromNumber(-1);
@@ -1742,7 +1742,7 @@ TEST_F(SyncableDirectoryTest, Directory_LastReferenceUnlinksAttachments) {
   sync_pb::AttachmentMetadata attachment_metadata;
   sync_pb::AttachmentMetadataRecord* record = attachment_metadata.add_record();
   sync_pb::AttachmentIdProto attachment_id_proto =
-      syncer::CreateAttachmentIdProto();
+      syncer::CreateAttachmentIdProto(0, 0);
   *record->mutable_id() = attachment_id_proto;
 
   // Create two entries, each referencing the attachment.
@@ -1771,7 +1771,7 @@ TEST_F(SyncableDirectoryTest, Directory_LastReferenceUnlinksAttachments) {
 
 TEST_F(SyncableDirectoryTest, Directory_GetAttachmentIdsToUpload) {
   // Create one attachment, referenced by two entries.
-  AttachmentId attachment_id = AttachmentId::Create();
+  AttachmentId attachment_id = AttachmentId::Create(0, 0);
   sync_pb::AttachmentIdProto attachment_id_proto = attachment_id.GetProto();
   sync_pb::AttachmentMetadata attachment_metadata;
   sync_pb::AttachmentMetadataRecord* record = attachment_metadata.add_record();