Pass AttachmentIdList instead of AttachmentList to SyncData

Pass AttachmentIdList instead of AttachmentList to SyncData

Datatype is responsible for writing attachments to AttachmentStore.
GenericChangeProcessor doesn't need attachment data in SyncData, only
list of attachment ids.

This change removes AttachmentList from SyncData and fixes corresponding
dependencies.

BUG=
R=maniscalco@chromium.org

Committed: https://crrev.com/28667de498d657951a9fb4858e2b7c1ddf7c3016
Cr-Commit-Position: refs/heads/master@{#294879}

[maniscalco, 2014-09-12 23:23:34]

Very nice, here's my feedback.

https://codereview.chromium.org/567053002/diff/1/components/sync_driver/gener...
File components/sync_driver/generic_change_processor.cc (right):

https://codereview.chromium.org/567053002/diff/1/components/sync_driver/gener...
components/sync_driver/generic_change_processor.cc:727: // Begin uploading the
attachments now that they are safe on disk.
This comment is now out of date.

https://codereview.chromium.org/567053002/diff/1/sync/api/sync_data.cc
File sync/api/sync_data.cc (right):

https://codereview.chromium.org/567053002/diff/1/sync/api/sync_data.cc#newcode30
sync/api/sync_data.cc:30: // Return true iff |attachments| contains one or more
elements with the same
|attachments| -> |attachment_ids|

https://codereview.chromium.org/567053002/diff/1/sync/api/sync_data.cc#newcode35
sync/api/sync_data.cc:35: id_set.insert(attachment_ids.begin(),
attachment_ids.end());
Nice!

https://codereview.chromium.org/567053002/diff/1/sync/api/sync_data.h
File sync/api/sync_data.h (right):

https://codereview.chromium.org/567053002/diff/1/sync/api/sync_data.h#newcode58
sync/api/sync_data.h:58: // For data with attachments: |attachments| must not
contain duplicates.
Can you update this comment?

[pavely, 2014-09-15 17:41:50]

PTAL

https://codereview.chromium.org/567053002/diff/1/components/sync_driver/gener...
File components/sync_driver/generic_change_processor.cc (right):

https://codereview.chromium.org/567053002/diff/1/components/sync_driver/gener...
components/sync_driver/generic_change_processor.cc:727: // Begin uploading the
attachments now that they are safe on disk.
I removed this comment. It now becomes datatype's responsibility to call
ProcessSyncChanges only after attachment is stored with AttachmentStore.
On 2014/09/12 23:23:33, maniscalco wrote:
> This comment is now out of date.

https://codereview.chromium.org/567053002/diff/1/sync/api/sync_data.cc
File sync/api/sync_data.cc (right):

https://codereview.chromium.org/567053002/diff/1/sync/api/sync_data.cc#newcode30
sync/api/sync_data.cc:30: // Return true iff |attachments| contains one or more
elements with the same
On 2014/09/12 23:23:34, maniscalco wrote:
> |attachments| -> |attachment_ids|

Done.

https://codereview.chromium.org/567053002/diff/1/sync/api/sync_data.h
File sync/api/sync_data.h (right):

https://codereview.chromium.org/567053002/diff/1/sync/api/sync_data.h#newcode58
sync/api/sync_data.h:58: // For data with attachments: |attachments| must not
contain duplicates.
On 2014/09/12 23:23:34, maniscalco wrote:
> Can you update this comment?

Done.

[maniscalco, 2014-09-15 18:40:25]

lgtm

[pavely, 2014-09-15 19:00:37]

The CQ bit was checked by pavely@chromium.org

[commit-bot: I haz the power, 2014-09-15 19:16:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/567053002/20001

[commit-bot: I haz the power, 2014-09-15 20:22:00]

Committed patchset #2 (id:20001) as 722529415776f449737f1dad73b3680f95053a5b

[commit-bot: I haz the power, 2014-09-15 20:24:38]

Patchset 2 (id:??) landed as
https://crrev.com/28667de498d657951a9fb4858e2b7c1ddf7c3016
Cr-Commit-Position: refs/heads/master@{#294879}