Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4)

Issue 995683002: Revert of [Sync] Add size and crc32c to AttachmentId. (Closed)

Created:
5 years, 9 months ago by dgrogan
Modified:
5 years, 9 months ago
Reviewers:
maniscalco, pavely
CC:
chromium-reviews, tim+watch_chromium.org, zea+watch_chromium.org, maxbogue+watch_chromium.org, pvalenzuela+watch_chromium.org, albertb+watch_chromium.org, maniscalco+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of [Sync] Add size and crc32c to AttachmentId. (patchset #8 id:140001 of https://codereview.chromium.org/982883002/) Reason for revert: Broke build on iOS_Device http://build.chromium.org/p/chromium.mac/builders/iOS_Device/builds/26684/steps/compile/logs/stdio Undefined symbols for architecture armv7: "syncer::Attachment::CreateFromParts(syncer::AttachmentId const&, scoped_refptr<base::RefCountedMemory> const&, unsigned int)", referenced from: syncer::(anonymous namespace)::MockAttachmentStore::RespondToRead(std::__1::set<syncer::AttachmentId, std::__1::less<syncer::AttachmentId>, std::__1::allocator<syncer::AttachmentId> > const&) in attachment_service_impl_unittest.o syncer::(anonymous namespace)::MockAttachmentDownloader::RespondToDownload(syncer::AttachmentId const&, syncer::AttachmentDownloader::DownloadResult const&) in attachment_service_impl_unittest.o syncer::gtest_case_AttachmentStoreTest_::Write_NoOverwriteNoError<syncer::InMemoryAttachmentStoreFactory>::TestBody() in in_memory_attachment_store_unittest.o syncer::OnDiskAttachmentStoreSpecificTest_MismatchedCrc_Test::TestBody() in on_disk_attachment_store_unittest.o syncer::gtest_case_AttachmentStoreTest_::Write_NoOverwriteNoError<syncer::OnDiskAttachmentStoreFactory>::TestBody() in on_disk_attachment_store_unittest.o "syncer::CreateAttachmentIdProto()", referenced from: syncer::AddAttachment(sync_pb::AttachmentMetadata*, bool) in directory_commit_contribution_unittest.o syncer::DirectoryUpdateHandlerProcessUpdateTest_ProcessAndApplyUpdatesWithAttachments_Test::TestBody() in directory_update_handler_unittest.o syncer::DirectoryUpdateHandlerApplyUpdateTest_SimpleConflictDifferentAttachmentMetadata_Test::TestBody() in directory_update_handler_unittest.o syncer::DirectoryUpdateHandlerApplyUpdateTest_SimpleConflictSameAttachmentMetadataDifferentOrder_Test::TestBody() in directory_update_handler_unittest.o syncer::AttachmentIdProtoTest_UniqueIdFormat_Test::TestBody() in attachment_id_proto_unittest.o syncer::AttachmentIdProtoTest_CreateAttachmentMetadata_NonEmpty_Test::TestBody() in attachment_id_proto_unittest.o syncer::SyncManagerChangeProcessingTest_AttachmentMetadataOnlyChanges_Test::TestBody() in sync_manager_impl_unittest.o ... "syncer::AttachmentId::Create()", referenced from: syncer::(anonymous namespace)::SyncDataTest_CreateLocalDataWithAttachments_Test::TestBody() in sync_data_unittest.o syncer::AttachmentDownloaderImplTest_HappyCase_Test::TestBody() in attachment_downloader_impl_unittest.o syncer::AttachmentDownloaderImplTest_SameIdMultipleDownloads_Test::TestBody() in attachment_downloader_impl_unittest.o syncer::AttachmentDownloaderImplTest_RequestAccessTokenFails_Test::TestBody() in attachment_downloader_impl_unittest.o syncer::AttachmentDownloaderImplTest_URLFetcher_BadToken_Test::TestBody() in attachment_downloader_impl_unittest.o syncer::AttachmentDownloaderImplTest_URLFetcher_ServiceUnavailable_Test::TestBody() in attachment_downloader_impl_unittest.o syncer::AttachmentDownloaderImplTest_NoHash_Test::TestBody() in attachment_downloader_impl_unittest.o ... ld: symbol(s) not found for architecture armv7 clang:error: linker command failed with exit code 1 (use -v to see invocation) Original issue's description: > [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} TBR=pavely@chromium.org,maniscalco@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=464431 Committed: https://crrev.com/eb83b6cff47a33585e4d315f1517eaff14132bb5 Cr-Commit-Position: refs/heads/master@{#319807}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+121 lines, -221 lines) Patch
M components/sync_driver/generic_change_processor_unittest.cc View 4 chunks +6 lines, -6 lines 0 comments Download
M sync/api/attachments/attachment.h View 2 chunks +6 lines, -3 lines 0 comments Download
M sync/api/attachments/attachment.cc View 2 chunks +8 lines, -9 lines 0 comments Download
M sync/api/attachments/attachment_id.h View 1 chunk +2 lines, -12 lines 0 comments Download
M sync/api/attachments/attachment_id.cc View 2 chunks +2 lines, -10 lines 0 comments Download
M sync/api/attachments/attachment_id_unittest.cc View 1 chunk +9 lines, -5 lines 0 comments Download
M sync/api/attachments/attachment_metadata.h View 1 chunk +0 lines, -3 lines 0 comments Download
M sync/api/attachments/attachment_metadata_unittest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M sync/api/attachments/attachment_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M sync/api/sync_data_unittest.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M sync/engine/directory_commit_contribution_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M sync/engine/directory_update_handler_unittest.cc View 3 chunks +5 lines, -7 lines 0 comments Download
M sync/internal_api/attachments/attachment_downloader_impl.cc View 5 chunks +6 lines, -11 lines 0 comments Download
M sync/internal_api/attachments/attachment_downloader_impl_unittest.cc View 11 chunks +9 lines, -32 lines 0 comments Download
M sync/internal_api/attachments/attachment_service_impl_unittest.cc View 12 chunks +20 lines, -16 lines 0 comments Download
M sync/internal_api/attachments/attachment_store_test_template.h View 1 chunk +3 lines, -2 lines 0 comments Download
M sync/internal_api/attachments/attachment_uploader_impl_unittest.cc View 4 chunks +4 lines, -4 lines 0 comments Download
M sync/internal_api/attachments/fake_attachment_downloader.cc View 1 chunk +2 lines, -1 line 0 comments Download
M sync/internal_api/attachments/fake_attachment_downloader_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M sync/internal_api/attachments/on_disk_attachment_store.cc View 1 chunk +4 lines, -10 lines 0 comments Download
M sync/internal_api/attachments/on_disk_attachment_store_unittest.cc View 5 chunks +10 lines, -51 lines 0 comments Download
M sync/internal_api/public/attachments/attachment_downloader_impl.h View 1 chunk +2 lines, -1 line 0 comments Download
M sync/internal_api/public/base/attachment_id_proto.h View 2 chunks +1 line, -8 lines 0 comments Download
M sync/internal_api/public/base/attachment_id_proto.cc View 1 chunk +1 line, -4 lines 0 comments Download
M sync/internal_api/public/base/attachment_id_proto_unittest.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M sync/internal_api/sync_manager_impl_unittest.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M sync/protocol/attachments.proto View 1 chunk +0 lines, -4 lines 0 comments Download
M sync/syncable/directory_unittest.cc View 5 chunks +6 lines, -6 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
dgrogan
Created Revert of [Sync] Add size and crc32c to AttachmentId.
5 years, 9 months ago (2015-03-10 01:32:17 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/995683002/1
5 years, 9 months ago (2015-03-10 01:32:57 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 9 months ago (2015-03-10 01:34:32 UTC) #3
commit-bot: I haz the power
5 years, 9 months ago (2015-03-10 01:35:29 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/eb83b6cff47a33585e4d315f1517eaff14132bb5
Cr-Commit-Position: refs/heads/master@{#319807}

Powered by Google App Engine
This is Rietveld 408576698