[Sync] Clean up ModelType code.

[Sync] Clean up ModelType code.

Various cleanup of ModelType usage and implementation. When enumerating
the types in a hard coded manner, we should always strive to order the
code to reflect model_type.h's order. ModelTypeSet's methods are
slightly more inline-able. Updated ModelType histogram and static
asserts.

BUG=

Review-Url: https://codereview.chromium.org/2701003002
Cr-Commit-Position: refs/heads/master@{#453616}
Committed: https://chromium.googlesource.com/chromium/src/+/e8c91968e771bd1877d957db4d564728b31184d0

[skym, 2017-02-17 17:57:34]

Description was changed from

==========
[Sync] Update SyncPrefs::GetPrefNameForDataType to reflect ModelType order,
remove default.

BUG=
==========

to

==========
[Sync] Update SyncPrefs::GetPrefNameForDataType to reflect ModelType order,
remove default.

BUG=
==========

[skym, 2017-02-17 17:57:34]

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

[skym, 2017-02-17 17:57:39]

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

[commit-bot: I haz the power, 2017-02-17 17:58:07]

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, 2017-02-17 19:00:54]

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

[commit-bot: I haz the power, 2017-02-17 19:00:56]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[skym, 2017-02-17 22:14:33]

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

[commit-bot: I haz the power, 2017-02-17 22:16:00]

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, 2017-02-17 23:29:00]

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

[commit-bot: I haz the power, 2017-02-17 23:29:00]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[skym, 2017-02-21 23:22:07]

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

[commit-bot: I haz the power, 2017-02-21 23:24:17]

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, 2017-02-22 00:52:25]

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

[commit-bot: I haz the power, 2017-02-22 00:52:26]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[skym, 2017-02-23 18:27:02]

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

[commit-bot: I haz the power, 2017-02-23 18:27:31]

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

[skym, 2017-02-23 19:15:15]

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

[commit-bot: I haz the power, 2017-02-23 19:16:07]

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

[skym, 2017-02-23 20:37:39]

Description was changed from

==========
[Sync] Update SyncPrefs::GetPrefNameForDataType to reflect ModelType order,
remove default.

BUG=
==========

to

==========
[Sync] Clean up ModelType code.

Various cleanup of ModelType usage and implementation. When enumerating
the types in a hard coded manner, we should always strive to order the
code to reflect model_type.h's order. ModelTypeSet's methods are
slightly more inline-able. Updated ModelType histogram and static
asserts.

BUG=
==========

[skym, 2017-02-23 20:38:12]

PTAL. While getting ready to add a new model type, I wanted to fix a few pain
points, and figured it was best to do that in a separate CL.

[commit-bot: I haz the power, 2017-02-23 20:39:27]

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

[commit-bot: I haz the power, 2017-02-23 20:39:27]

Dry run: This issue passed the CQ dry run.

[pavely, 2017-02-23 23:23:03]

lgtm

https://codereview.chromium.org/2701003002/diff/100001/components/browser_syn...
File components/browser_sync/profile_sync_service.cc (right):

https://codereview.chromium.org/2701003002/diff/100001/components/browser_syn...
components/browser_sync/profile_sync_service.cc:1521: "If adding a user
selectable type, updated "
updated => update

https://codereview.chromium.org/2701003002/diff/100001/components/sync/base/e...
File components/sync/base/enum_set.h (right):

https://codereview.chromium.org/2701003002/diff/100001/components/sync/base/e...
components/sync/base/enum_set.h:158: int endIndexInclusive = ToIndex(end);
Could you DCHECK that ToIndex(start) <= ToIndex(end).

https://codereview.chromium.org/2701003002/diff/100001/components/sync/base/m...
File components/sync/base/model_type.h (right):

https://codereview.chromium.org/2701003002/diff/100001/components/sync/base/m...
components/sync/base/model_type.h:94: // History delete directives.
Seems like git cl format messed up indentation.

https://codereview.chromium.org/2701003002/diff/100001/components/sync/driver...
File components/sync/driver/user_selectable_sync_type.h (right):

https://codereview.chromium.org/2701003002/diff/100001/components/sync/driver...
components/sync/driver/user_selectable_sync_type.h:38: // TODO(lgcheng)
ARC_PACKAGE would be selectable.
Maybe add comment here that mapping from user selectable types to full set of
synced types is controlled in SyncPrefs::RegisterPrefGroups?

https://codereview.chromium.org/2701003002/diff/100001/components/sync/syncab...
File components/sync/syncable/model_type.cc (right):

https://codereview.chromium.org/2701003002/diff/100001/components/sync/syncab...
components/sync/syncable/model_type.cc:432: return ModelTypeSet(BOOKMARKS,
PREFERENCES, PASSWORDS, AUTOFILL, THEMES,
What if you put one type per line? Will it be more readable? Will git cl format
try to straighten it on every change in the file?

[skym, 2017-02-24 18:59:07]

https://codereview.chromium.org/2701003002/diff/100001/components/browser_syn...
File components/browser_sync/profile_sync_service.cc (right):

https://codereview.chromium.org/2701003002/diff/100001/components/browser_syn...
components/browser_sync/profile_sync_service.cc:1521: "If adding a user
selectable type, updated "
On 2017/02/23 23:23:03, pavely wrote:
> updated => update

Done.

https://codereview.chromium.org/2701003002/diff/100001/components/sync/base/e...
File components/sync/base/enum_set.h (right):

https://codereview.chromium.org/2701003002/diff/100001/components/sync/base/e...
components/sync/base/enum_set.h:158: int endIndexInclusive = ToIndex(end);
On 2017/02/23 23:23:03, pavely wrote:
> Could you DCHECK that ToIndex(start) <= ToIndex(end).

Done.

https://codereview.chromium.org/2701003002/diff/100001/components/sync/base/m...
File components/sync/base/model_type.h (right):

https://codereview.chromium.org/2701003002/diff/100001/components/sync/base/m...
components/sync/base/model_type.h:94: // History delete directives.
On 2017/02/23 23:23:03, pavely wrote:
> Seems like git cl format messed up indentation.

It does. I've moved the Deprecated text onto the lines above to avoid this.

https://codereview.chromium.org/2701003002/diff/100001/components/sync/driver...
File components/sync/driver/user_selectable_sync_type.h (right):

https://codereview.chromium.org/2701003002/diff/100001/components/sync/driver...
components/sync/driver/user_selectable_sync_type.h:38: // TODO(lgcheng)
ARC_PACKAGE would be selectable.
On 2017/02/23 23:23:03, pavely wrote:
> Maybe add comment here that mapping from user selectable types to full set of
> synced types is controlled in SyncPrefs::RegisterPrefGroups?

Done.

https://codereview.chromium.org/2701003002/diff/100001/components/sync/syncab...
File components/sync/syncable/model_type.cc (right):

https://codereview.chromium.org/2701003002/diff/100001/components/sync/syncab...
components/sync/syncable/model_type.cc:432: return ModelTypeSet(BOOKMARKS,
PREFERENCES, PASSWORDS, AUTOFILL, THEMES,
On 2017/02/23 23:23:03, pavely wrote:
> What if you put one type per line? Will it be more readable? Will git cl
format
> try to straighten it on every change in the file?

Yeah, format wants them to look like this.

I really wish we could do something like just return the keys from the map
created in RegisterPrefGroups() here, but things are not setup to facilitate
that right now.

[skym, 2017-02-24 18:59:10]

The CQ bit was checked by skym@chromium.org

[skym, 2017-02-24 18:59:11]

The patchset sent to the CQ was uploaded after l-g-t-m from pavely@chromium.org
Link to the patchset: https://codereview.chromium.org/2701003002/#ps120001
(title: "Updated for Pavel's comments.")

[commit-bot: I haz the power, 2017-02-24 19:00:08]

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-02-24 19:15:06]

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

[commit-bot: I haz the power, 2017-02-24 19:15:07]

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

[skym, 2017-02-24 21:00:32]

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

[skym, 2017-02-24 21:01:21]

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

[skym, 2017-02-24 21:01:22]

Adding rkaplow@ for histograms.xml, PTAL

[commit-bot: I haz the power, 2017-02-24 21:01:43]

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, 2017-02-24 22:34:13]

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

[commit-bot: I haz the power, 2017-02-24 22:34:14]

Dry run: This issue passed the CQ dry run.

[rkaplow, 2017-02-28 02:27:47]

lgtm

[skym, 2017-02-28 15:04:47]

The CQ bit was checked by skym@chromium.org

[skym, 2017-02-28 15:04:48]

The patchset sent to the CQ was uploaded after l-g-t-m from pavely@chromium.org
Link to the patchset: https://codereview.chromium.org/2701003002/#ps140001
(title: "Switched int to size_t to fix compile.")

[commit-bot: I haz the power, 2017-02-28 15:04:54]

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-02-28 16:31:20]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1488294287534080,
"parent_rev": "057d6501ebc769b7195d48e057fa951258475d41", "commit_rev":
"e8c91968e771bd1877d957db4d564728b31184d0"}

[commit-bot: I haz the power, 2017-02-28 16:32:04]

Description was changed from

==========
[Sync] Clean up ModelType code.

Various cleanup of ModelType usage and implementation. When enumerating
the types in a hard coded manner, we should always strive to order the
code to reflect model_type.h's order. ModelTypeSet's methods are
slightly more inline-able. Updated ModelType histogram and static
asserts.

BUG=
==========

to

==========
[Sync] Clean up ModelType code.

Various cleanup of ModelType usage and implementation. When enumerating
the types in a hard coded manner, we should always strive to order the
code to reflect model_type.h's order. ModelTypeSet's methods are
slightly more inline-able. Updated ModelType histogram and static
asserts.

BUG=

Review-Url: https://codereview.chromium.org/2701003002
Cr-Commit-Position: refs/heads/master@{#453616}
Committed:
https://chromium.googlesource.com/chromium/src/+/e8c91968e771bd1877d957db4d56...
==========

[commit-bot: I haz the power, 2017-02-28 16:32:05]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/e8c91968e771bd1877d957db4d56...