[Sync] Move attachments code out of core/.

[Sync] Move attachments code out of core/.

Things moved to...

api/attachments/
  attachment_service
  attachment_service_proxy

api_impl/attachments/
  attachment_service_impl
  task_queue

engine/attachments/
  attachment_downloader
  attachment_store_backend
  attachment_store_frontend
  attachment_uploader
  attachment_util
  fake_attachment_downloader
  fake_attachment_uploader
  in_memory_attachment_store
  on_disk_attachment_store

engine_impl/attachments/
  attachment_downloader_impl
  attachment_uploader_impl
  proto/

I'm unfamiliar with the attachments code so this was as best as I could tell
where they should go... The store things in particular are a bit weird and
hard to place.

BUG=631271
TBR=thakis,sdefresne

Committed: https://crrev.com/7ed9ccefbec84b7e228a415449876a9bb5b3e105
Cr-Commit-Position: refs/heads/master@{#423954}

[maxbogue, 2016-10-07 02:59:08]

The CQ bit was checked by maxbogue@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-07 02:59:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-07 04:36:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-07 04:36:33]

Dry run: This issue passed the CQ dry run.

[maxbogue, 2016-10-07 17:47:21]

The CQ bit was checked by maxbogue@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-07 17:47:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[maxbogue, 2016-10-07 17:50:23]

maxbogue@chromium.org changed reviewers:
+ skym@chromium.org

[maxbogue, 2016-10-07 17:50:25]

Sky, PTAL!

[skym, 2016-10-07 18:22:16]

lgtm

https://codereview.chromium.org/2399953002/diff/40001/components/sync/BUILD.gn
File components/sync/BUILD.gn (right):

https://codereview.chromium.org/2399953002/diff/40001/components/sync/BUILD.g...
components/sync/BUILD.gn:64: "api_impl/attachments/task_queue.cc",
I'm still skeptical this is needed.

https://codereview.chromium.org/2399953002/diff/40001/components/sync/BUILD.g...
components/sync/BUILD.gn:541: "//components/sync/engine_impl/attachments/proto",
...Shouldn't the proto get moved out of an *_impl area?

https://codereview.chromium.org/2399953002/diff/40001/components/sync/api/att...
File components/sync/api/attachments/attachment_service.cc (right):

https://codereview.chromium.org/2399953002/diff/40001/components/sync/api/att...
components/sync/api/attachments/attachment_service.cc:24:
std::move(attachment_store), std::move(attachment_uploader),
Shouldn't you have #include <utility> for these std::move calls? Did you run
lint?

https://codereview.chromium.org/2399953002/diff/40001/components/sync/api/att...
components/sync/api/attachments/attachment_service.cc:35:
base::MakeUnique<FakeAttachmentUploader>(),
Shouldn't you have #include "base/memory/ptr_util.h" for these base::MakeUnique
calls?

https://codereview.chromium.org/2399953002/diff/40001/components/sync/driver/...
File components/sync/driver/generic_change_processor_unittest.cc (right):

https://codereview.chromium.org/2399953002/diff/40001/components/sync/driver/...
components/sync/driver/generic_change_processor_unittest.cc:102:
std::unique_ptr<MockAttachmentService> attachment_service(
base::MakeUnique()?

[maxbogue, 2016-10-07 19:21:50]

https://codereview.chromium.org/2399953002/diff/40001/components/sync/BUILD.gn
File components/sync/BUILD.gn (right):

https://codereview.chromium.org/2399953002/diff/40001/components/sync/BUILD.g...
components/sync/BUILD.gn:64: "api_impl/attachments/task_queue.cc",
On 2016/10/07 18:22:16, skym wrote:
> I'm still skeptical this is needed.

Damn it you're right, it worked this time. Maybe I hadn't actually saved the
BUILD.gn change or something last time -_-

https://codereview.chromium.org/2399953002/diff/40001/components/sync/BUILD.g...
components/sync/BUILD.gn:541: "//components/sync/engine_impl/attachments/proto",
On 2016/10/07 18:22:16, skym wrote:
> ...Shouldn't the proto get moved out of an *_impl area?

It's a proto only used for persisting to disk. It's not part of any interfaces.

https://codereview.chromium.org/2399953002/diff/40001/components/sync/api/att...
File components/sync/api/attachments/attachment_service.cc (right):

https://codereview.chromium.org/2399953002/diff/40001/components/sync/api/att...
components/sync/api/attachments/attachment_service.cc:24:
std::move(attachment_store), std::move(attachment_uploader),
On 2016/10/07 18:22:16, skym wrote:
> Shouldn't you have #include <utility> for these std::move calls? Did you run
> lint?

Done. Apparently I hadn't... added <memory> in attachment_service_proxy.cc as
well.

https://codereview.chromium.org/2399953002/diff/40001/components/sync/api/att...
components/sync/api/attachments/attachment_service.cc:35:
base::MakeUnique<FakeAttachmentUploader>(),
On 2016/10/07 18:22:16, skym wrote:
> Shouldn't you have #include "base/memory/ptr_util.h" for these
base::MakeUnique
> calls?

It's there!

https://codereview.chromium.org/2399953002/diff/40001/components/sync/driver/...
File components/sync/driver/generic_change_processor_unittest.cc (right):

https://codereview.chromium.org/2399953002/diff/40001/components/sync/driver/...
components/sync/driver/generic_change_processor_unittest.cc:102:
std::unique_ptr<MockAttachmentService> attachment_service(
On 2016/10/07 18:22:16, skym wrote:
> base::MakeUnique()?

Done.

[maxbogue, 2016-10-07 19:22:46]

maxbogue@chromium.org changed reviewers:
+ sdefresne@chromium.org, thakis@chromium.org

[maxbogue, 2016-10-07 19:22:47]

TBR thakis@ for chrome/ and sdefresne@ for components/ since this is a simple
refactor.

[maxbogue, 2016-10-07 19:23:02]

Description was changed from

==========
[Sync] Move attachments code out of core/.

Things moved to...

api/attachments/
  attachment_service
  attachment_service_proxy

api_impl/attachments/
  attachment_service_impl
  task_queue

engine/attachments/
  attachment_downloader
  attachment_store_backend
  attachment_store_frontend
  attachment_uploader
  attachment_util
  fake_attachment_downloader
  fake_attachment_uploader
  in_memory_attachment_store
  on_disk_attachment_store

engine_impl/attachments/
  attachment_downloader_impl
  attachment_uploader_impl
  proto/

I'm unfamiliar with the attachments code so this was as best as I could tell
where they should go... The store things in particular are a bit weird and
hard to place.

BUG=631271
==========

to

==========
[Sync] Move attachments code out of core/.

Things moved to...

api/attachments/
  attachment_service
  attachment_service_proxy

api_impl/attachments/
  attachment_service_impl
  task_queue

engine/attachments/
  attachment_downloader
  attachment_store_backend
  attachment_store_frontend
  attachment_uploader
  attachment_util
  fake_attachment_downloader
  fake_attachment_uploader
  in_memory_attachment_store
  on_disk_attachment_store

engine_impl/attachments/
  attachment_downloader_impl
  attachment_uploader_impl
  proto/

I'm unfamiliar with the attachments code so this was as best as I could tell
where they should go... The store things in particular are a bit weird and
hard to place.

BUG=631271
TBR=thakis,sdefresne
==========

[maxbogue, 2016-10-07 19:23:10]

The CQ bit was checked by maxbogue@chromium.org

[maxbogue, 2016-10-07 19:23:11]

The patchset sent to the CQ was uploaded after l-g-t-m from skym@chromium.org
Link to the patchset: https://codereview.chromium.org/2399953002/#ps60001
(title: "Address comments.")

[commit-bot: I haz the power, 2016-10-07 19:23:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nico, 2016-10-07 19:41:10]

chrome/ lgtm

[commit-bot: I haz the power, 2016-10-07 20:26:18]

Description was changed from

==========
[Sync] Move attachments code out of core/.

Things moved to...

api/attachments/
  attachment_service
  attachment_service_proxy

api_impl/attachments/
  attachment_service_impl
  task_queue

engine/attachments/
  attachment_downloader
  attachment_store_backend
  attachment_store_frontend
  attachment_uploader
  attachment_util
  fake_attachment_downloader
  fake_attachment_uploader
  in_memory_attachment_store
  on_disk_attachment_store

engine_impl/attachments/
  attachment_downloader_impl
  attachment_uploader_impl
  proto/

I'm unfamiliar with the attachments code so this was as best as I could tell
where they should go... The store things in particular are a bit weird and
hard to place.

BUG=631271
TBR=thakis,sdefresne
==========

to

==========
[Sync] Move attachments code out of core/.

Things moved to...

api/attachments/
  attachment_service
  attachment_service_proxy

api_impl/attachments/
  attachment_service_impl
  task_queue

engine/attachments/
  attachment_downloader
  attachment_store_backend
  attachment_store_frontend
  attachment_uploader
  attachment_util
  fake_attachment_downloader
  fake_attachment_uploader
  in_memory_attachment_store
  on_disk_attachment_store

engine_impl/attachments/
  attachment_downloader_impl
  attachment_uploader_impl
  proto/

I'm unfamiliar with the attachments code so this was as best as I could tell
where they should go... The store things in particular are a bit weird and
hard to place.

BUG=631271
TBR=thakis,sdefresne
==========

[commit-bot: I haz the power, 2016-10-07 20:26:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-07 20:30:01]

Description was changed from

==========
[Sync] Move attachments code out of core/.

Things moved to...

api/attachments/
  attachment_service
  attachment_service_proxy

api_impl/attachments/
  attachment_service_impl
  task_queue

engine/attachments/
  attachment_downloader
  attachment_store_backend
  attachment_store_frontend
  attachment_uploader
  attachment_util
  fake_attachment_downloader
  fake_attachment_uploader
  in_memory_attachment_store
  on_disk_attachment_store

engine_impl/attachments/
  attachment_downloader_impl
  attachment_uploader_impl
  proto/

I'm unfamiliar with the attachments code so this was as best as I could tell
where they should go... The store things in particular are a bit weird and
hard to place.

BUG=631271
TBR=thakis,sdefresne
==========

to

==========
[Sync] Move attachments code out of core/.

Things moved to...

api/attachments/
  attachment_service
  attachment_service_proxy

api_impl/attachments/
  attachment_service_impl
  task_queue

engine/attachments/
  attachment_downloader
  attachment_store_backend
  attachment_store_frontend
  attachment_uploader
  attachment_util
  fake_attachment_downloader
  fake_attachment_uploader
  in_memory_attachment_store
  on_disk_attachment_store

engine_impl/attachments/
  attachment_downloader_impl
  attachment_uploader_impl
  proto/

I'm unfamiliar with the attachments code so this was as best as I could tell
where they should go... The store things in particular are a bit weird and
hard to place.

BUG=631271
TBR=thakis,sdefresne

Committed: https://crrev.com/7ed9ccefbec84b7e228a415449876a9bb5b3e105
Cr-Commit-Position: refs/heads/master@{#423954}
==========

[commit-bot: I haz the power, 2016-10-07 20:30:03]

Patchset 4 (id:??) landed as
https://crrev.com/7ed9ccefbec84b7e228a415449876a9bb5b3e105
Cr-Commit-Position: refs/heads/master@{#423954}

[skym, 2016-10-07 20:43:23]

https://codereview.chromium.org/2399953002/diff/40001/components/sync/BUILD.gn
File components/sync/BUILD.gn (right):

https://codereview.chromium.org/2399953002/diff/40001/components/sync/BUILD.g...
components/sync/BUILD.gn:541: "//components/sync/engine_impl/attachments/proto",
On 2016/10/07 19:21:50, maxbogue wrote:
> On 2016/10/07 18:22:16, skym wrote:
> > ...Shouldn't the proto get moved out of an *_impl area?
> 
> It's a proto only used for persisting to disk. It's not part of any
interfaces.

Why is it in public_deps then?