[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

Committed: https://crrev.com/23ae3128db0d84a6b1ffa640568a5ec90cfc8808
Cr-Commit-Position: refs/heads/master@{#319794}

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

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

9 files changed, 49 insertions, 33 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);