[Sync] Not having an autocomplete ModelTypeState is valid.

[Sync] Not having an autocomplete ModelTypeState is valid.

Previously, when there was no row for a ModelTypeState in
AutofillTable, it was returning false on its GetAllSyncMetadata call.
This then caused the AutocompleteSyncBridge to report a ModelError to
the processor. This caused the model type the fail initialization on a
first time sync. The in this CL changes AutofillTable's table to return
true and provide a default valued ModelTypeState this case. The
processor is fully capable of inspecting the ModelTypeState object and
determining that a first time sync has no occurred, and taking the
appropriate steps.

This potentially introduces a scenario where there are rows for
EntityMetadata but none of ModelTypeState. This is an invalid state,
but AutofillTable will not recognize this (nor should it). The right
place for handling this scenario is the processor. It should be quite
unlikely/difficult to get into this scenario, and the damage should be
low, some invalid/not relevant metadata may hang around indefinitely.
Added a DCHECK for this unexpected and unhanded case for now.

Also updated several includes for lint.

BUG=686172
Review-Url: https://codereview.chromium.org/2660673002
Cr-Commit-Position: refs/heads/master@{#447355}
Committed: https://chromium.googlesource.com/chromium/src/+/5f42ef10335277c152f30782723af6ee893d3f55

[skym, 2017-01-27 19:45:56]

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

[skym, 2017-01-27 19:46:09]

Description was changed from

==========
[Sync] Not having an autocomplete ModelTypeState is valid.

BUG=686172
==========

to

==========
[Sync] Not having an autocomplete ModelTypeState is valid.

TODO(skym): Words.

BUG=686172
==========

[skym, 2017-01-27 19:46:09]

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

[commit-bot: I haz the power, 2017-01-27 19:46:34]

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

[skym, 2017-01-27 20:33:57]

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

[commit-bot: I haz the power, 2017-01-27 20:34:59]

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

[skym, 2017-01-27 20:41:37]

Description was changed from

==========
[Sync] Not having an autocomplete ModelTypeState is valid.

TODO(skym): Words.

BUG=686172
==========

to

==========
[Sync] Not having an autocomplete ModelTypeState is valid.

Previously, when there was no row for a ModelTypeState in
AutofillTable, it was returning false on its GetAllSyncMetadata call.
This then caused the AutocompleteSyncBridge to report a ModelError to
the processor. This caused the model type the fail initialization on a
first time sync. The in this CL changes AutofillTable's table to return
true and provide a default valued ModelTypeState this case. The
processor is fully capable of inspecting the ModelTypeState object and
determining that a first time sync has no occurred, and taking the
appropriate steps.

This potentially introduces a scenario where there are rows for
EntityMetadata but none of ModelTypeState. This is an invalid state,
but AutofillTable will not recognize this (nor should it). The right
place for handling this scenario is the processor. It should be quite
unlikely/difficult to get into this scenario, and the damage should be
low, some invalid/not relevant metadata may hang around indefinitely.
Added a DCHECK for this unexpected and unhanded case for now.

Also updated several includes for lint.

BUG=686172
==========

[skym, 2017-01-27 20:41:53]

PTAL

[Patrick Noland, 2017-01-27 20:50:00]

https://codereview.chromium.org/2660673002/diff/20001/components/sync/model/r...
File components/sync/model/recording_model_type_change_processor.cc (right):

https://codereview.chromium.org/2660673002/diff/20001/components/sync/model/r...
components/sync/model/recording_model_type_change_processor.cc:7: #include
<utility>
is this for lint reasons?

https://codereview.chromium.org/2660673002/diff/20001/components/sync/model/r...
File components/sync/model/recording_model_type_change_processor.h (right):

https://codereview.chromium.org/2660673002/diff/20001/components/sync/model/r...
components/sync/model/recording_model_type_change_processor.h:42: MetadataBatch*
metadata() const { return metadata_.get(); }
why this change?

[skym, 2017-01-27 20:55:56]

https://codereview.chromium.org/2660673002/diff/20001/components/sync/model/r...
File components/sync/model/recording_model_type_change_processor.cc (right):

https://codereview.chromium.org/2660673002/diff/20001/components/sync/model/r...
components/sync/model/recording_model_type_change_processor.cc:7: #include
<utility>
On 2017/01/27 20:49:59, Patrick Noland wrote:
> is this for lint reasons?

Yes, std::swap on line 33.

https://codereview.chromium.org/2660673002/diff/20001/components/sync/model/r...
File components/sync/model/recording_model_type_change_processor.h (right):

https://codereview.chromium.org/2660673002/diff/20001/components/sync/model/r...
components/sync/model/recording_model_type_change_processor.h:42: MetadataBatch*
metadata() const { return metadata_.get(); }
On 2017/01/27 20:49:59, Patrick Noland wrote:
> why this change?

Because I want to call TakeAllMetadata() on it, which modifies |metadata_|.
There isn't a public const accessor on the interface.

[Patrick Noland, 2017-01-27 20:56:15]

lgtm

[commit-bot: I haz the power, 2017-01-27 21:49:48]

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

[commit-bot: I haz the power, 2017-01-27 21:49:49]

Dry run: This issue passed the CQ dry run.

[skym, 2017-01-30 22:34:48]

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

[skym, 2017-01-30 22:34:48]

Adding mathp@ for owners approval, PTAL

[Mathieu, 2017-01-31 15:43:17]

autofill lgtm

[skym, 2017-01-31 22:06:41]

The CQ bit was checked by skym@chromium.org

[commit-bot: I haz the power, 2017-01-31 22:07:33]

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-31 23:14:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485900401392330,
"parent_rev": "4a636520c5c1a351628ce7e469cc3603fcb939e8", "commit_rev":
"5f42ef10335277c152f30782723af6ee893d3f55"}

[commit-bot: I haz the power, 2017-01-31 23:14:41]

Description was changed from

==========
[Sync] Not having an autocomplete ModelTypeState is valid.

Previously, when there was no row for a ModelTypeState in
AutofillTable, it was returning false on its GetAllSyncMetadata call.
This then caused the AutocompleteSyncBridge to report a ModelError to
the processor. This caused the model type the fail initialization on a
first time sync. The in this CL changes AutofillTable's table to return
true and provide a default valued ModelTypeState this case. The
processor is fully capable of inspecting the ModelTypeState object and
determining that a first time sync has no occurred, and taking the
appropriate steps.

This potentially introduces a scenario where there are rows for
EntityMetadata but none of ModelTypeState. This is an invalid state,
but AutofillTable will not recognize this (nor should it). The right
place for handling this scenario is the processor. It should be quite
unlikely/difficult to get into this scenario, and the damage should be
low, some invalid/not relevant metadata may hang around indefinitely.
Added a DCHECK for this unexpected and unhanded case for now.

Also updated several includes for lint.

BUG=686172
==========

to

==========
[Sync] Not having an autocomplete ModelTypeState is valid.

Previously, when there was no row for a ModelTypeState in
AutofillTable, it was returning false on its GetAllSyncMetadata call.
This then caused the AutocompleteSyncBridge to report a ModelError to
the processor. This caused the model type the fail initialization on a
first time sync. The in this CL changes AutofillTable's table to return
true and provide a default valued ModelTypeState this case. The
processor is fully capable of inspecting the ModelTypeState object and
determining that a first time sync has no occurred, and taking the
appropriate steps.

This potentially introduces a scenario where there are rows for
EntityMetadata but none of ModelTypeState. This is an invalid state,
but AutofillTable will not recognize this (nor should it). The right
place for handling this scenario is the processor. It should be quite
unlikely/difficult to get into this scenario, and the damage should be
low, some invalid/not relevant metadata may hang around indefinitely.
Added a DCHECK for this unexpected and unhanded case for now.

Also updated several includes for lint.

BUG=686172

Review-Url: https://codereview.chromium.org/2660673002
Cr-Commit-Position: refs/heads/master@{#447355}
Committed:
https://chromium.googlesource.com/chromium/src/+/5f42ef10335277c152f30782723a...
==========

[commit-bot: I haz the power, 2017-01-31 23:14:42]

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