diff options
28 files changed, 221 insertions, 121 deletions
diff --git a/components/sync_driver/generic_change_processor_unittest.cc b/components/sync_driver/generic_change_processor_unittest.cc index 8d30500..bbd99e4 100644 --- a/components/sync_driver/generic_change_processor_unittest.cc +++ b/components/sync_driver/generic_change_processor_unittest.cc @@ -370,8 +370,8 @@ TEST_F(SyncGenericChangeProcessorTest, pref_specifics->set_name("test"); syncer::AttachmentIdList attachment_ids; - attachment_ids.push_back(syncer::AttachmentId::Create()); - attachment_ids.push_back(syncer::AttachmentId::Create()); + attachment_ids.push_back(syncer::AttachmentId::Create(0, 0)); + attachment_ids.push_back(syncer::AttachmentId::Create(0, 0)); // Add a SyncData with two attachments. syncer::SyncChangeList change_list; @@ -394,7 +394,7 @@ TEST_F(SyncGenericChangeProcessorTest, // Update the SyncData, replacing its two attachments with one new attachment. syncer::AttachmentIdList new_attachment_ids; - new_attachment_ids.push_back(syncer::AttachmentId::Create()); + new_attachment_ids.push_back(syncer::AttachmentId::Create(0, 0)); mock_attachment_service()->attachment_id_sets()->clear(); change_list.clear(); change_list.push_back( @@ -424,7 +424,7 @@ TEST_F(SyncGenericChangeProcessorTest, AttachmentUploaded) { pref_specifics->set_name("test"); syncer::AttachmentIdList attachment_ids; - attachment_ids.push_back(syncer::AttachmentId::Create()); + attachment_ids.push_back(syncer::AttachmentId::Create(0, 0)); // Add a SyncData with two attachments. syncer::SyncChangeList change_list; @@ -452,8 +452,8 @@ TEST_F(SyncGenericChangeProcessorTest, AttachmentUploaded) { // scheduled for upload. TEST_F(SyncGenericChangeProcessorTest, UploadAllAttachmentsNotOnServer) { // Create two attachment ids. id2 will be marked as "on server". - syncer::AttachmentId id1 = syncer::AttachmentId::Create(); - syncer::AttachmentId id2 = syncer::AttachmentId::Create(); + syncer::AttachmentId id1 = syncer::AttachmentId::Create(0, 0); + syncer::AttachmentId id2 = syncer::AttachmentId::Create(0, 0); { // Write an entry containing these two attachment ids. syncer::WriteTransaction trans(FROM_HERE, user_share()); 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 8519244..db83699 100644 --- a/sync/internal_api/attachments/attachment_service_impl_unittest.cc +++ b/sync/internal_api/attachments/attachment_service_impl_unittest.cc @@ -79,9 +79,7 @@ class MockAttachmentStoreBackend 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); @@ -137,9 +135,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, @@ -321,7 +317,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; @@ -339,10 +335,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()); @@ -409,7 +405,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()); RunLoop(); @@ -427,7 +423,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); @@ -461,7 +457,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()); @@ -478,8 +474,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); @@ -505,7 +501,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); @@ -529,7 +525,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()); @@ -541,7 +537,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); @@ -571,7 +567,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 0d14ca5..6292b01 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 ec6fc75..699e1f5 100644 --- a/sync/internal_api/attachments/on_disk_attachment_store.cc +++ b/sync/internal_api/attachments/on_disk_attachment_store.cc @@ -319,12 +319,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 4dcc8f8..2f45b20 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(); |