Replace AttachmentStore's StoreAttachments with UploadAttachments.

Replace AttachmentStore's StoreAttachments with UploadAttachments.

This change is lays groundwork for making attachment upload operations
persistent.

Make GenericChangeProcessor responsible for writing attachments to the
AttachmentStore and calling UploadAttachments.  In a future CL, datatype
code will be responsible for writing attachments to the store.

Queue up attachments for upload inside of AttachmentService.  In a
future CL we'll add rate limiting, retry, and back-off logic.

Expose AttachmentService's AttachmentStore via GetStore method.

BUG=

Committed: https://crrev.com/5dde06853d1d5020796184c5fc749b9730ce8f07
Cr-Commit-Position: refs/heads/master@{#293536}

[maniscalco, 2014-09-02 18:56:29]

maniscalco@chromium.org changed reviewers:
+ pavely@chromium.org

[maniscalco, 2014-09-02 18:56:47]

Pavel, would you please review this change?

[pavely, 2014-09-04 17:44:37]

https://codereview.chromium.org/512413003/diff/80001/components/sync_driver/g...
File components/sync_driver/generic_change_processor_unittest.cc (right):

https://codereview.chromium.org/512413003/diff/80001/components/sync_driver/g...
components/sync_driver/generic_change_processor_unittest.cc:366:
EXPECT_THAT(attachments_added, testing::Contains(attachments[0].GetId()));
(not asking to change): I learned today about UnorderedElementsAre matcher.

https://codereview.chromium.org/512413003/diff/80001/sync/internal_api/attach...
File sync/internal_api/attachments/attachment_service_impl.cc (right):

https://codereview.chromium.org/512413003/diff/80001/sync/internal_api/attach...
sync/internal_api/attachments/attachment_service_impl.cc:266:
ids_in_queue_.erase(id);
This should go into UploadDone. We discussed that.

[maniscalco, 2014-09-04 22:39:38]

Thanks for the feedback.  All applied.  PTAL.

https://codereview.chromium.org/512413003/diff/80001/components/sync_driver/g...
File components/sync_driver/generic_change_processor_unittest.cc (right):

https://codereview.chromium.org/512413003/diff/80001/components/sync_driver/g...
components/sync_driver/generic_change_processor_unittest.cc:366:
EXPECT_THAT(attachments_added, testing::Contains(attachments[0].GetId()));
On 2014/09/04 17:44:37, pavely wrote:
> (not asking to change): I learned today about UnorderedElementsAre matcher.

Neat.  I hadn't heard of UnorderedElementsAre.  It seems like a better fit here
(no need to check size) so I'll switch.

https://codereview.chromium.org/512413003/diff/80001/sync/internal_api/attach...
File sync/internal_api/attachments/attachment_service_impl.cc (right):

https://codereview.chromium.org/512413003/diff/80001/sync/internal_api/attach...
sync/internal_api/attachments/attachment_service_impl.cc:266:
ids_in_queue_.erase(id);
On 2014/09/04 17:44:37, pavely wrote:
> This should go into UploadDone. We discussed that.

Good catch.  Done.  I also updated the data member comments to clarify.

[pavely, 2014-09-05 01:15:49]

lgtm

[maniscalco, 2014-09-05 15:41:25]

The CQ bit was checked by maniscalco@chromium.org

[commit-bot: I haz the power, 2014-09-05 15:43:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/maniscalco@chromium.org/512413003/100001

[commit-bot: I haz the power, 2014-09-05 16:46:31]

Committed patchset #6 (id:100001) as 15e22ff398406c253ca172047e493cadf5252c73

[Noam Samuel, 2014-09-05 17:10:07]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/538403004/ by noamsml@chromium.org.

The reason for reverting is: Chromium GPU build breakage:

http://build.chromium.org/p/chromium.gpu/builders/GPU%20Linux%20Builder/build....

[maniscalco, 2014-09-05 17:42:52]

On 2014/09/05 17:10:07, Noam Samuel wrote:
> A revert of this CL (patchset #6 id:100001) has been created in
> https://codereview.chromium.org/538403004/ by mailto:noamsml@chromium.org.
> 
> The reason for reverting is: Chromium GPU build breakage:
> 
>
http://build.chromium.org/p/chromium.gpu/builders/GPU%20Linux%20Builder/build....

Looks like the changed passed the trybots because there was an intervening
change
(https://chromium.googlesource.com/chromium/src/+/664a17fefa8bc4b5076376528103...)
that landed after the revision built by the trybots (but before this change
landed).

[maniscalco, 2014-09-08 21:59:08]

On 2014/09/05 17:42:52, maniscalco wrote:
> On 2014/09/05 17:10:07, Noam Samuel wrote:
> > A revert of this CL (patchset #6 id:100001) has been created in
> > https://codereview.chromium.org/538403004/ by mailto:noamsml@chromium.org.
> > 
> > The reason for reverting is: Chromium GPU build breakage:
> > 
> >
>
http://build.chromium.org/p/chromium.gpu/builders/GPU%20Linux%20Builder/build....
> 
> Looks like the changed passed the trybots because there was an intervening
> change
>
(https://chromium.googlesource.com/chromium/src/+/664a17fefa8bc4b5076376528103...)
> that landed after the revision built by the trybots (but before this change
> landed).

Relanded as https://codereview.chromium.org/548533003/

[commit-bot: I haz the power, 2014-09-10 03:40:06]

Patchset 6 (id:??) landed as
https://crrev.com/5dde06853d1d5020796184c5fc749b9730ce8f07
Cr-Commit-Position: refs/heads/master@{#293536}