[Sync] Use ModelError all the way through to ModelTypeController.

[Sync] Use ModelError all the way through to ModelTypeController.

ModelError is a simplified error object for use with USS types. This
change replaces all uses of SyncError between the
SharedModelTypeProcessor and the ModelTypeController with ModelError,
so the translation to SyncError happens only when it needs to and not
before, keeping the code simpler and cleaner.

BUG=673883
Review-Url: https://codereview.chromium.org/2620773002
Cr-Commit-Position: refs/heads/master@{#442466}
Committed: https://chromium.googlesource.com/chromium/src/+/8c8dd627152d4ff7704addbea1c769a56307a7c7

[maxbogue, 2017-01-09 22:25:42]

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

[commit-bot: I haz the power, 2017-01-09 22:26:30]

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

[maxbogue, 2017-01-09 23:12:33]

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

[maxbogue, 2017-01-09 23:12:33]

Sky, PTAL! ModelError part 2: plumbing through to MTC.

[maxbogue, 2017-01-09 23:24:58]

Description was changed from

==========
[Sync] Use ModelError all the way through to ModelTypeController.

BUG=673883
==========

to

==========
[Sync] Use ModelError all the way through to ModelTypeController.

ModelError is a simplified error object for use with USS types. This
change replaces all uses of SyncError between the
SharedModelTypeProcessor and the ModelTypeController with ModelError,
so the translation to SyncError happens only when it needs to and not
before, keeping the code simpler and cleaner.

BUG=673883
==========

[skym, 2017-01-09 23:35:09]

lgtm

https://codereview.chromium.org/2620773002/diff/1/components/sync/driver/mode...
File components/sync/driver/model_type_controller.cc (right):

https://codereview.chromium.org/2620773002/diff/1/components/sync/driver/mode...
components/sync/driver/model_type_controller.cc:65: base::Closure dump_stack,
Can these base::Closure and base::Callback params be const&?

https://codereview.chromium.org/2620773002/diff/1/components/sync/driver/mode...
components/sync/driver/model_type_controller.cc:250: void
ModelTypeController::ReportModelError(ModelError error) {
Can this error be const&?

https://codereview.chromium.org/2620773002/diff/1/components/sync/model/model...
File components/sync/model/model_type_sync_bridge.h (right):

https://codereview.chromium.org/2620773002/diff/1/components/sync/model/model...
components/sync/model/model_type_sync_bridge.h:115: void
OnSyncStarting(base::RepeatingCallback<void(ModelError)> error_handler,
const&?

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
File components/sync/model_impl/shared_model_type_processor.cc (right):

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
components/sync/model_impl/shared_model_type_processor.cc:103:
start_callback_.Reset();
You could also have if () {} else { } start_callback.Reset(); and never use an
early return here.

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
File components/sync/model_impl/shared_model_type_processor.h (right):

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
components/sync/model_impl/shared_model_type_processor.h:55: void
OnSyncStarting(base::RepeatingCallback<void(ModelError)> error_handler,
Can this be a const&?

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
File components/sync/model_impl/shared_model_type_processor_unittest.cc (right):

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
components/sync/model_impl/shared_model_type_processor_unittest.cc:260: void
ErrorReceived(ModelError error) {
I guess this doesn't really matter if we do the optional thing, but this should
verify that error.IsSet(), right?

[commit-bot: I haz the power, 2017-01-10 00:18:52]

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

[commit-bot: I haz the power, 2017-01-10 00:18:53]

Dry run: This issue passed the CQ dry run.

[maxbogue, 2017-01-10 00:34:34]

I've added ModelErrorHandler as a typedef to keep code clean with the addition
of const& everywhere.

https://codereview.chromium.org/2620773002/diff/1/components/sync/driver/mode...
File components/sync/driver/model_type_controller.cc (right):

https://codereview.chromium.org/2620773002/diff/1/components/sync/driver/mode...
components/sync/driver/model_type_controller.cc:65: base::Closure dump_stack,
On 2017/01/09 23:35:09, skym wrote:
> Can these base::Closure and base::Callback params be const&?

Done.

https://codereview.chromium.org/2620773002/diff/1/components/sync/driver/mode...
components/sync/driver/model_type_controller.cc:250: void
ModelTypeController::ReportModelError(ModelError error) {
On 2017/01/09 23:35:09, skym wrote:
> Can this error be const&?

Done.

https://codereview.chromium.org/2620773002/diff/1/components/sync/model/model...
File components/sync/model/model_type_sync_bridge.h (right):

https://codereview.chromium.org/2620773002/diff/1/components/sync/model/model...
components/sync/model/model_type_sync_bridge.h:115: void
OnSyncStarting(base::RepeatingCallback<void(ModelError)> error_handler,
On 2017/01/09 23:35:09, skym wrote:
> const&?

Done.

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
File components/sync/model_impl/shared_model_type_processor.cc (right):

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
components/sync/model_impl/shared_model_type_processor.cc:103:
start_callback_.Reset();
On 2017/01/09 23:35:09, skym wrote:
> You could also have if () {} else { } start_callback.Reset(); and never use an
> early return here.

I think once StartCallback is a OnceCallback the Reset in the lower branch won't
be necessary, so I'm going to leave this structure.

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
File components/sync/model_impl/shared_model_type_processor.h (right):

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
components/sync/model_impl/shared_model_type_processor.h:55: void
OnSyncStarting(base::RepeatingCallback<void(ModelError)> error_handler,
On 2017/01/09 23:35:09, skym wrote:
> Can this be a const&?

Done.

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
File components/sync/model_impl/shared_model_type_processor_unittest.cc (right):

https://codereview.chromium.org/2620773002/diff/1/components/sync/model_impl/...
components/sync/model_impl/shared_model_type_processor_unittest.cc:260: void
ErrorReceived(ModelError error) {
On 2017/01/09 23:35:09, skym wrote:
> I guess this doesn't really matter if we do the optional thing, but this
should
> verify that error.IsSet(), right?

Yeah n/a with the optional thing coming.

[maxbogue, 2017-01-10 00:36:16]

The CQ bit was checked by maxbogue@chromium.org

[maxbogue, 2017-01-10 00:36:17]

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/2620773002/#ps20001
(title: "const& everything and use a typedef.")

[commit-bot: I haz the power, 2017-01-10 00:37:13]

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, 2017-01-10 03:30:36]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484008576541070,
"parent_rev": "2a4032ecbad9f39e07e28d7466a0793381a24626", "commit_rev":
"8c8dd627152d4ff7704addbea1c769a56307a7c7"}

[commit-bot: I haz the power, 2017-01-10 03:31:13]

Description was changed from

==========
[Sync] Use ModelError all the way through to ModelTypeController.

ModelError is a simplified error object for use with USS types. This
change replaces all uses of SyncError between the
SharedModelTypeProcessor and the ModelTypeController with ModelError,
so the translation to SyncError happens only when it needs to and not
before, keeping the code simpler and cleaner.

BUG=673883
==========

to

==========
[Sync] Use ModelError all the way through to ModelTypeController.

ModelError is a simplified error object for use with USS types. This
change replaces all uses of SyncError between the
SharedModelTypeProcessor and the ModelTypeController with ModelError,
so the translation to SyncError happens only when it needs to and not
before, keeping the code simpler and cleaner.

BUG=673883

Review-Url: https://codereview.chromium.org/2620773002
Cr-Commit-Position: refs/heads/master@{#442466}
Committed:
https://chromium.googlesource.com/chromium/src/+/8c8dd627152d4ff7704addbea1c7...
==========

[commit-bot: I haz the power, 2017-01-10 03:31:14]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/8c8dd627152d4ff7704addbea1c7...