sync: Add interfaces for per-type sync

sync: Add interfaces for per-type sync

Puts the DirectoryCommitContributor and DirectoryUpdateHandler
classes behind interfaces.  These classes were always intended
to form the basis of an abstract interface.  Now we can start
to introduce alternate implementations of them.

This CL includes one such alternate implementation.  It adds
the MockUpdateHandler and uses it to remove some unnecessary
boilerplate from the download unit tests.

This CL also resolves an old TODO.  It renames the SyncDirectory*
classes to Directory*, since the 'Sync' part of the name is redundant.

This should have no effect on program behavior.

BUG=278484
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251969

[rlarocque, 2014-02-12 21:15:20]

Here's the long-awaited per-type sync interface.

Since this CL necessarily had to include lots of renaming and shuffling around
of gyp files, I decided to perform the s/SyncDirectory/Directory/ rename here,
too.

Please review.

[tim (not reviewing), 2014-02-18 19:15:01]

https://codereview.chromium.org/161253002/diff/40001/sync/engine/commit_contr...
File sync/engine/commit_contributor.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/commit_contr...
sync/engine/commit_contributor.h:27: virtual CommitContribution*
GetContribution(size_t max_entries) = 0;
Even though these are ultimately going into an STL map, return a scoped_ptr vs
raw pointer here in the interface to make ownership responsibility clear.

Also, explain the |max_entries| parameter with a comment.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/commit_proce...
File sync/engine/commit_processor.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/commit_proce...
sync/engine/commit_processor.h:36: explicit
CommitProcessor(CommitContributorMap* commit_contributor_map);
Comment on parameter (at least ownership / lifetime expectation).

https://codereview.chromium.org/161253002/diff/40001/sync/engine/commit_proce...
sync/engine/commit_processor.h:39: // Gathers a set of contributions to be used
to populate a commit message.
Explaining the parameters would help (e.g does |max_entries| truncate, or?)

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_co...
File sync/engine/directory_commit_contribution.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_co...
sync/engine/directory_commit_contribution.h:46: static
DirectoryCommitContribution* Build(
Unless there's a strong reason against it, better to return a scoped_ptr here to
force ownership.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_co...
File sync/engine/directory_commit_contributor.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_co...
sync/engine/directory_commit_contributor.h:37: ModelType type_;
DISALLOW_COPY_AND_ASSIGN?

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_up...
File sync/engine/directory_update_handler.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_up...
sync/engine/directory_update_handler.h:44: ModelType type,
nit - indent is off.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/update_handl...
File sync/engine/update_handler.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/update_handl...
sync/engine/update_handler.h:28: // sync server.
"...request, receive, and apply updates from the sync server".

https://codereview.chromium.org/161253002/diff/40001/sync/engine/update_handl...
sync/engine/update_handler.h:46: const SyncEntityList& applicable_updates,
describe what "applicable" means in this context.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/update_handl...
sync/engine/update_handler.h:48: 
nit - extra newline.

[rlarocque, 2014-02-18 20:01:13]

Patch updated.  PTAL.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/commit_contr...
File sync/engine/commit_contributor.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/commit_contr...
sync/engine/commit_contributor.h:27: virtual CommitContribution*
GetContribution(size_t max_entries) = 0;
On 2014/02/18 19:15:01, timsteele wrote:
> Even though these are ultimately going into an STL map, return a scoped_ptr vs
> raw pointer here in the interface to make ownership responsibility clear.
> 
> Also, explain the |max_entries| parameter with a comment.

Done.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/commit_proce...
File sync/engine/commit_processor.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/commit_proce...
sync/engine/commit_processor.h:36: explicit
CommitProcessor(CommitContributorMap* commit_contributor_map);
On 2014/02/18 19:15:01, timsteele wrote:
> Comment on parameter (at least ownership / lifetime expectation).

Done.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/commit_proce...
sync/engine/commit_processor.h:39: // Gathers a set of contributions to be used
to populate a commit message.
On 2014/02/18 19:15:01, timsteele wrote:
> Explaining the parameters would help (e.g does |max_entries| truncate, or?)

Done.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_co...
File sync/engine/directory_commit_contribution.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_co...
sync/engine/directory_commit_contribution.h:46: static
DirectoryCommitContribution* Build(
On 2014/02/18 19:15:01, timsteele wrote:
> Unless there's a strong reason against it, better to return a scoped_ptr here
to
> force ownership.

Done.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_co...
File sync/engine/directory_commit_contributor.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_co...
sync/engine/directory_commit_contributor.h:37: ModelType type_;
On 2014/02/18 19:15:01, timsteele wrote:
> DISALLOW_COPY_AND_ASSIGN?

Done.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_up...
File sync/engine/directory_update_handler.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/directory_up...
sync/engine/directory_update_handler.h:44: ModelType type,
On 2014/02/18 19:15:01, timsteele wrote:
> nit - indent is off. 

Done.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/update_handl...
File sync/engine/update_handler.h (right):

https://codereview.chromium.org/161253002/diff/40001/sync/engine/update_handl...
sync/engine/update_handler.h:28: // sync server.
On 2014/02/18 19:15:01, timsteele wrote:
> "...request, receive, and apply updates from the sync server".

Done.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/update_handl...
sync/engine/update_handler.h:46: const SyncEntityList& applicable_updates,
On 2014/02/18 19:15:01, timsteele wrote:
> describe what "applicable" means in this context.

Done.

https://codereview.chromium.org/161253002/diff/40001/sync/engine/update_handl...
sync/engine/update_handler.h:48: 
On 2014/02/18 19:15:01, timsteele wrote:
> nit - extra newline.

Done.

[tim (not reviewing), 2014-02-18 20:41:39]

LGTM

[rlarocque, 2014-02-18 20:42:33]

The CQ bit was checked by rlarocque@chromium.org

[commit-bot: I haz the power, 2014-02-18 20:44:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/161253002/400001

[commit-bot: I haz the power, 2014-02-18 21:25:14]

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

[commit-bot: I haz the power, 2014-02-18 21:25:15]

Retried try job too often on win_rel for step(s) app_list_unittests,
ash_unittests, aura_unittests, cacheinvalidation_unittests, cc_unittests,
check_deps, check_deps2git, chrome_elf_unittests, chromedriver_unittests,
components_unittests, compositor_unittests, content_browsertests,
content_unittests, crypto_unittests, events_unittests, google_apis_unittests,
gpu_unittests, installer_util_unittests, ipc_tests, jingle_unittests,
media_unittests, mini_installer_test, nacl_integration, ppapi_unittests,
printing_unittests, remoting_unittests, sql_unittests, sync_integration_tests,
sync_unit_tests, telemetry_perf_unittests, telemetry_unittests, views_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[rlarocque, 2014-02-18 23:24:14]

The CQ bit was checked by rlarocque@chromium.org

[commit-bot: I haz the power, 2014-02-18 23:25:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/161253002/620001

[rlarocque, 2014-02-19 00:53:53]

The CQ bit was checked by rlarocque@chromium.org

[commit-bot: I haz the power, 2014-02-19 00:55:00]

Failed to trigger a try job on win
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2014-02-19 00:55:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/161253002/190003

[commit-bot: I haz the power, 2014-02-19 02:03:13]

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

[commit-bot: I haz the power, 2014-02-19 02:03:13]

Retried try job too often on win_rel for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[rlarocque, 2014-02-19 02:06:47]

The CQ bit was checked by rlarocque@chromium.org

[commit-bot: I haz the power, 2014-02-19 02:09:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/161253002/190003

[commit-bot: I haz the power, 2014-02-19 06:28:18]

Change committed as 251969