[Sync] Refactor AttachmentStore classes. Introduce concept of referrer.

[Sync] Refactor AttachmentStore classes. Introduce concept of referrer.

In this change:
- Move towards following class hierarchy: http://www.plantuml.com:80/plantuml/png/ZPBTJiCm38NlynIvv4TzWsaI525nCI5jkw-on1EZQHmvwQG9U7TC4-ZeKiITvNm-EiSExbv1Hxc6VOszYs24mDHQeG6xFNcGRqBAknYLVkd0nKr40l7nmqrU1deDeRUHYrfPkrEw3LmJx848YCkWqODfkECZBIOAZuHin9abWxUo9b0Hdjt38RGJyEhwZ4YZI9kJq_mmRp0pAPKnd7pGgG8t6usTHyVe7_FPtY3mbOth9ghGDjGxTnwlaEq-ySjv-KmCwZflxvVyE5bSIa4Mw7ZGyDHvAyHurPkgkhZgz9PuoNp75tDhAUZcJ68cwkATHyfXnFWn4_PFDsKuNLuKLrFodGS-0G00
- Remove AttachmenService::GetStore. Now attachment store is owned by
  model type, there is no need to get it from AttachmentService
- Introduce AttachmentReferrer. There is no functionality behind it yet
  and interface is not complete for it. These will come in the next change.

BUG=457735
R=maniscalco@chromium.org
TEST=Not exposed in chrome. Only unit tests available (sync_unit_tests)

Committed: https://crrev.com/6adcada8a799057c31c0f17550c9e2747a8df847
Cr-Commit-Position: refs/heads/master@{#319733}

Committed: https://crrev.com/be7169cf3591e428d0b4056a2c23c8ace14f6169
Cr-Commit-Position: refs/heads/master@{#320015}

[pavely, 2015-03-06 21:34:24]

PTAL.

The number of changed lines is misleading. Most of it is renaming
AttachmentStoreHandle and moving AttachmentStoreBackend into separate file.

[maniscalco, 2015-03-06 23:15:04]

Nice work.  Here's my feedback.  Mostly nits.

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
File sync/api/attachments/attachment_store.h (right):

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
sync/api/attachments/attachment_store.h:124: scoped_ptr<AttachmentStore>
CreateAttachmentStoreForSync();
This method can't be made const, can it?

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
sync/api/attachments/attachment_store.h:154: // of |referref_|.
typo: referrer -> referrer

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
sync/api/attachments/attachment_store.h:155: AttachmentReferrer referrer_;
Can be made const (I think).

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
File sync/api/attachments/attachment_store_backend.h (right):

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
sync/api/attachments/attachment_store_backend.h:30: // them , there is no need
to declare separate set.
" , " -> ", "

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

https://codereview.chromium.org/986743004/diff/1/sync/api/syncable_service.h#...
sync/api/syncable_service.h:77: virtual AttachmentStore* GetAttachmentStore();
As we talked about in person, it would be great if SyncableService could create
the AttachmentStoreForSync and return it by scoped_ptr.  That way, we'd never
have to worry about the lifetime of the object returned by GetAttachmentStore()
[Should I free it?  Can I hold on to the pointer and use it later?  How much
later?]

https://codereview.chromium.org/986743004/diff/1/sync/internal_api/attachment...
File sync/internal_api/attachments/attachment_service_impl.cc (right):

https://codereview.chromium.org/986743004/diff/1/sync/internal_api/attachment...
sync/internal_api/attachments/attachment_service_impl.cc:14: #include
"sync/internal_api/public/attachments/attachment_store_frontend.h"
Maybe I overlooked something, but is this include necessary?

[pavely, 2015-03-09 18:09:29]

I've made changes based on your feedback and fixed components_unittests. PTAL.

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
File sync/api/attachments/attachment_store.h (right):

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
sync/api/attachments/attachment_store.h:124: scoped_ptr<AttachmentStore>
CreateAttachmentStoreForSync();
On 2015/03/06 23:15:03, maniscalco wrote:
> This method can't be made const, can it?

Done.

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
sync/api/attachments/attachment_store.h:154: // of |referref_|.
On 2015/03/06 23:15:03, maniscalco wrote:
> typo: referrer -> referrer

Done.

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
sync/api/attachments/attachment_store.h:155: AttachmentReferrer referrer_;
On 2015/03/06 23:15:03, maniscalco wrote:
> Can be made const (I think).

Done.

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
File sync/api/attachments/attachment_store_backend.h (right):

https://codereview.chromium.org/986743004/diff/1/sync/api/attachments/attachm...
sync/api/attachments/attachment_store_backend.h:30: // them , there is no need
to declare separate set.
On 2015/03/06 23:15:03, maniscalco wrote:
> " , " -> ", "

Done.

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

https://codereview.chromium.org/986743004/diff/1/sync/api/syncable_service.h#...
sync/api/syncable_service.h:77: virtual AttachmentStore* GetAttachmentStore();
On 2015/03/06 23:15:03, maniscalco wrote:
> As we talked about in person, it would be great if SyncableService could
create
> the AttachmentStoreForSync and return it by scoped_ptr.  That way, we'd never
> have to worry about the lifetime of the object returned by
GetAttachmentStore()
> [Should I free it?  Can I hold on to the pointer and use it later?  How much
> later?]

Done.

https://codereview.chromium.org/986743004/diff/1/sync/internal_api/attachment...
File sync/internal_api/attachments/attachment_service_impl.cc (right):

https://codereview.chromium.org/986743004/diff/1/sync/internal_api/attachment...
sync/internal_api/attachments/attachment_service_impl.cc:14: #include
"sync/internal_api/public/attachments/attachment_store_frontend.h"
On 2015/03/06 23:15:03, maniscalco wrote:
> Maybe I overlooked something, but is this include necessary?

Done.

[maniscalco, 2015-03-09 20:01:05]

LGTM

[pavely, 2015-03-09 20:10:40]

The CQ bit was checked by pavely@chromium.org

[commit-bot: I haz the power, 2015-03-09 20:10:55]

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

[commit-bot: I haz the power, 2015-03-09 20:34:08]

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

[commit-bot: I haz the power, 2015-03-09 20:34:09]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[pavely, 2015-03-09 20:54:06]

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

[pavely, 2015-03-09 20:55:19]

Chris, 
There is a small change in dom_distiller_store.h. Could you approve it?

[cjhopman, 2015-03-09 20:59:10]

lgtm

[pavely, 2015-03-09 20:59:28]

The CQ bit was checked by pavely@chromium.org

[commit-bot: I haz the power, 2015-03-09 20:59:48]

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

[commit-bot: I haz the power, 2015-03-09 21:14:42]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-09 21:16:18]

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

[Bence, 2015-03-09 23:08:49]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/996473005/ by bnc@chromium.org.

The reason for reverting is: I think this CL is responsible for tree closing
compile failure
https://build.chromium.org/p/chromium.mac/builders/iOS_Device/builds/26672/st....

[pavely, 2015-03-10 23:11:00]

I'm gonna try relanding this CL.
I didn't find anything wrong with this change and iOS_Device builder has been
green for some time.

[pavely, 2015-03-10 23:17:06]

The CQ bit was checked by pavely@chromium.org

[pavely, 2015-03-10 23:17:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
cjhopman@chromium.org, maniscalco@chromium.org
Link to the patchset: https://codereview.chromium.org/986743004/#ps40001 (title:
"Rebase")

[commit-bot: I haz the power, 2015-03-10 23:18:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/986743004/40001

[commit-bot: I haz the power, 2015-03-11 01:19:24]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-11 01:20:19]

Patchset 3 (id:??) landed as
https://crrev.com/be7169cf3591e428d0b4056a2c23c8ace14f6169
Cr-Commit-Position: refs/heads/master@{#320015}