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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 394293003: Do not update AttachmentIds after uploading attachments to sync server. (Closed)

Created:
6 years, 5 months ago by maniscalco
Modified:
6 years, 5 months ago
Reviewers:
pavely
CC:
chromium-reviews, tim+watch_chromium.org, haitaol+watch_chromium.org, zea+watch_chromium.org, maniscalco+watch_chromium.org
Project:
chromium
Visibility:
Public.

Description

Do not update AttachmentIds after uploading attachments to sync server. In a previous design iteration we planned to have the sync server assign an attachments id after receiving an attachment upload. We have since changed the design so the client is responsible for assigning the attachment id before upload occurs. Remove comments related to updating attachment id after upload. Rename UpdateAttachmentIdWithServerInfo to MarkAttachmentAsOnServer. Rename UpdateEntriesWithAttachmentId to UpdateEntriesMarkAttachmentAsOnServer. BUG=371522 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283670

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -46 lines) Patch
M components/sync_driver/generic_change_processor.cc View 1 chunk +1 line, -1 line 0 comments Download
M sync/api/attachments/attachment_uploader.h View 1 chunk +2 lines, -2 lines 0 comments Download
M sync/internal_api/attachments/attachment_uploader_impl.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M sync/internal_api/attachments/attachment_uploader_impl_unittest.cc View 10 chunks +15 lines, -19 lines 0 comments Download
M sync/internal_api/attachments/fake_attachment_uploader.cc View 1 chunk +3 lines, -5 lines 0 comments Download
M sync/internal_api/public/write_transaction.h View 1 chunk +3 lines, -3 lines 0 comments Download
M sync/internal_api/write_transaction.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M sync/syncable/directory_unittest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M sync/syncable/mutable_entry.h View 1 chunk +4 lines, -5 lines 0 comments Download
M sync/syncable/mutable_entry.cc View 1 chunk +4 lines, -5 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
maniscalco
Pavel, would you please review this change?
6 years, 5 months ago (2014-07-16 21:27:03 UTC) #1
pavely
lgtm
6 years, 5 months ago (2014-07-16 21:57:45 UTC) #2
maniscalco
The CQ bit was checked by maniscalco@chromium.org
6 years, 5 months ago (2014-07-16 22:42:34 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/maniscalco@chromium.org/394293003/60001
6 years, 5 months ago (2014-07-16 22:45:41 UTC) #4
commit-bot: I haz the power
6 years, 5 months ago (2014-07-17 06:47:51 UTC) #5
Message was sent while issue was closed. 
Change committed as 283670

Powered by Google App Engine
This is Rietveld 408576698