sync: add WIFI_CREDENTIALS protobuf, ModelType, and preference

sync: add WIFI_CREDENTIALS protobuf, ModelType, and preference

- add a protobuf for WiFi credentials
- add a ModelType for syncing WiFi credentials
- add UI for controlling whether or not WiFi credentials are synced
- add new data type to testserver

While there:
- update comment in sync_setup_overlay.js
- fix ordering of arguments to EXPECT_EQ in CheckBool
  (sync_setup_handler_unittest)
- move GetSelectableTypeNameMap from sync_setup_handler.cc to model_type.cc

BUG=chromium:422045
TEST=ProfileSync, SyncSetup, SyncModel, ModelType, NigoriUtil,
     ProtoEnumConversions, ProtoValueConversions,
     SyncEncryptionHandlerImpl

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

[mukesh agrawal, 2014-10-22 20:10:51]

quiche@chromium.org changed reviewers:
+ pavely@chromium.org, pstew@chromium.org, stevenjb@chromium.org, zea@google.com

[mukesh agrawal, 2014-10-22 20:10:51]

quiche@chromium.org changed required reviewers:
+ pavely@chromium.org, zea@google.com

[mukesh agrawal, 2014-10-22 20:15:24]

https://codereview.chromium.org/674633002/diff/1/sync/internal_api/public/syn...
File sync/internal_api/public/sync_encryption_handler.cc (right):

https://codereview.chromium.org/674633002/diff/1/sync/internal_api/public/syn...
sync/internal_api/public/sync_encryption_handler.cc:19:
types.Put(WIFI_CREDENTIALS);
kevinliu@ and I are still discussing the encryption story for WiFi credentials.
But I'd like to go with this for now (to dogfood), and revisit if necessary.

[stevenjb, 2014-10-24 17:35:18]

https://codereview.chromium.org/674633002/diff/1/sync/protocol/proto_enum_con...
File sync/protocol/proto_enum_conversions.h (right):

https://codereview.chromium.org/674633002/diff/1/sync/protocol/proto_enum_con...
sync/protocol/proto_enum_conversions.h:42: 
nit: extra blank line

https://codereview.chromium.org/674633002/diff/1/sync/protocol/wifi_credentia...
File sync/protocol/wifi_credential_specifics.proto (right):

https://codereview.chromium.org/674633002/diff/1/sync/protocol/wifi_credentia...
sync/protocol/wifi_credential_specifics.proto:17: // Properties of app list
objects.
s/app list/WiFi credential/

https://codereview.chromium.org/674633002/diff/1/sync/syncable/model_type.cc
File sync/syncable/model_type.cc (right):

https://codereview.chromium.org/674633002/diff/1/sync/syncable/model_type.cc#...
sync/syncable/model_type.cc:123: specifics->mutable_enhanced_bookmark();
break;

[mukesh agrawal, 2014-10-24 18:01:14]

On 2014/10/24 17:35:18, stevenjb wrote:
>
https://codereview.chromium.org/674633002/diff/1/sync/protocol/proto_enum_con...
> File sync/protocol/proto_enum_conversions.h (right):
> 
>
https://codereview.chromium.org/674633002/diff/1/sync/protocol/proto_enum_con...
> sync/protocol/proto_enum_conversions.h:42: 
> nit: extra blank line
> 
>
https://codereview.chromium.org/674633002/diff/1/sync/protocol/wifi_credentia...
> File sync/protocol/wifi_credential_specifics.proto (right):
> 
>
https://codereview.chromium.org/674633002/diff/1/sync/protocol/wifi_credentia...
> sync/protocol/wifi_credential_specifics.proto:17: // Properties of app list
> objects.
> s/app list/WiFi credential/
> 
> https://codereview.chromium.org/674633002/diff/1/sync/syncable/model_type.cc
> File sync/syncable/model_type.cc (right):
> 
>
https://codereview.chromium.org/674633002/diff/1/sync/syncable/model_type.cc#...
> sync/syncable/model_type.cc:123: specifics->mutable_enhanced_bookmark();
> break;

Thanks, Steven. PTAL.

[mukesh agrawal, 2014-10-24 18:01:55]

zea@, pavely@, I think this is now ready for you to review. Please let me know
if there's anything else I need to do before you review it.

[stevenjb, 2014-10-24 19:14:52]

lgtm

[pavely, 2014-10-24 23:00:34]

https://codereview.chromium.org/674633002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/sync_setup_overlay.js (right):

https://codereview.chromium.org/674633002/diff/20001/chrome/browser/resources...
chrome/browser/resources/sync_setup_overlay.js:309: //
sync_setup_handler.cc:GetConfiguration().
Thanks for updating comment.

https://codereview.chromium.org/674633002/diff/20001/chrome/browser/resources...
chrome/browser/resources/sync_setup_overlay.js:327:
$('wifiCredentials-checkbox').checked,
I think this should be 'wifi-credentials-checkbox'

https://codereview.chromium.org/674633002/diff/20001/chrome/browser/resources...
chrome/browser/resources/sync_setup_overlay.js:446:
dataTypeBoxes_['wifi-credentials-checkbox'] =
dataTypeBoxesChecked_[...] =

https://codereview.chromium.org/674633002/diff/20001/chrome/browser/resources...
chrome/browser/resources/sync_setup_overlay.js:447: args.wifiCredentialsSynced;
dataTypeBoxesDisabled_[...] = args.wifiCredentialsEnforced;

https://codereview.chromium.org/674633002/diff/20001/sync/util/data_type_hist...
File sync/util/data_type_histogram.h (right):

https://codereview.chromium.org/674633002/diff/20001/sync/util/data_type_hist...
sync/util/data_type_histogram.h:128: break; \
I think this change needs to be acompanied by change in
tools/metrics/histograms/histograms.xml

You can add histograms similar to Sync.FreqSyncedNotifications,
Sync.SyncedNotificationsAssociationTime and
Sync.SyncedNotificationsStartFailure.

[mukesh agrawal, 2014-10-25 00:39:43]

https://codereview.chromium.org/674633002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/sync_setup_overlay.js (right):

https://codereview.chromium.org/674633002/diff/20001/chrome/browser/resources...
chrome/browser/resources/sync_setup_overlay.js:327:
$('wifiCredentials-checkbox').checked,
On 2014/10/24 23:00:33, pavely wrote:
> I think this should be 'wifi-credentials-checkbox'

Done.

https://codereview.chromium.org/674633002/diff/20001/chrome/browser/resources...
chrome/browser/resources/sync_setup_overlay.js:446:
dataTypeBoxes_['wifi-credentials-checkbox'] =
On 2014/10/24 23:00:34, pavely wrote:
> dataTypeBoxesChecked_[...] = 

Done.

https://codereview.chromium.org/674633002/diff/20001/chrome/browser/resources...
chrome/browser/resources/sync_setup_overlay.js:447: args.wifiCredentialsSynced;
On 2014/10/24 23:00:33, pavely wrote:
> dataTypeBoxesDisabled_[...] = args.wifiCredentialsEnforced;

Done.

https://codereview.chromium.org/674633002/diff/20001/sync/util/data_type_hist...
File sync/util/data_type_histogram.h (right):

https://codereview.chromium.org/674633002/diff/20001/sync/util/data_type_hist...
sync/util/data_type_histogram.h:128: break; \
Done.

Note that I changed the prefix from WifiCredential to WifiCredentials. I think
that's more consistent with the existing datatypes. Please let me know if that's
incorrect.

[mukesh agrawal, 2014-10-25 00:42:06]

quiche@chromium.org changed reviewers:
+ jwd@chromium.org

[mukesh agrawal, 2014-10-25 00:42:06]

quiche@chromium.org changed required reviewers:
+ jwd@chromium.org

[mukesh agrawal, 2014-10-25 00:42:06]

jwd@, can you review the histograms.xml part of this change? Thanks.

[jwd, 2014-10-27 19:31:26]

https://codereview.chromium.org/674633002/diff/40001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/674633002/diff/40001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:34812: +    commit frequency.
Can you mention when this is logged? The other two seem more obvious.

[mukesh agrawal, 2014-10-27 19:36:47]

https://codereview.chromium.org/674633002/diff/40001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/674633002/diff/40001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:34812: +    commit frequency.
zea@, pavely@, do you have a succinct description of when this metric is
reported? (I didn't see one on the existing Sync.Freq histograms.)

On 2014/10/27 19:31:26, Jesse Doherty wrote:
> Can you mention when this is logged? The other two seem more obvious.

[Nicolas Zea, 2014-10-28 20:33:34]

zea@chromium.org changed reviewers:
+ zea@chromium.org
- zea@google.com

[Nicolas Zea, 2014-10-28 20:33:34]

zea@chromium.org changed required reviewers:
- zea@google.com

[Nicolas Zea, 2014-10-28 20:33:36]

FYI Pavel is reviewing this from the sync side of things, no need to wait on me

https://codereview.chromium.org/674633002/diff/40001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/674633002/diff/40001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:34812: +    commit frequency.
On 2014/10/27 19:36:47, mukesh agrawal wrote:
> zea@, pavely@, do you have a succinct description of when this metric is
> reported? (I didn't see one on the existing Sync.Freq histograms.)
> 
> On 2014/10/27 19:31:26, Jesse Doherty wrote:
> > Can you mention when this is logged? The other two seem more obvious.
> 

It is logged at the time a sync cycle is performed for that datatype.

[mukesh agrawal, 2014-10-28 20:49:05]

thanks, zea@

pavely@, does PS3 address your concerns, or is there more needed on your end?
jwd@, does PS4 address your concerns, or is there more needed?

[mukesh agrawal, 2014-10-28 20:49:58]

https://codereview.chromium.org/674633002/diff/40001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/674633002/diff/40001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:34812: +    commit frequency.
On 2014/10/27 19:31:26, Jesse Doherty wrote:
> Can you mention when this is logged? The other two seem more obvious.

Done.

[jwd, 2014-10-28 21:04:05]

lgtm

[pavely, 2014-10-28 21:58:18]

lgtm

[mukesh agrawal, 2014-10-28 22:08:01]

The CQ bit was checked by quiche@chromium.org

[commit-bot: I haz the power, 2014-10-28 22:08:41]

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

[commit-bot: I haz the power, 2014-10-28 22:13:59]

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

[commit-bot: I haz the power, 2014-10-28 22:14:00]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/74846)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/79886)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[mukesh agrawal, 2014-10-28 23:20:53]

Oops. Looks like my rebase caused some additional changes. Let me clean those
up...

[mukesh agrawal, 2014-10-28 23:30:22]

Nevermind comment 26. After looking at the diff from base to PS5, I see that the
extra changes are all in base.

pavely@, would you like another look, or should I go ahead and re-check the
commit bit? (The conflict resolutions were basically changing some
COMPILE_ASSERT for number of datatypes.)

[pavely, 2014-10-28 23:34:55]

lgtm

[mukesh agrawal, 2014-10-29 00:30:53]

The CQ bit was checked by quiche@chromium.org

[commit-bot: I haz the power, 2014-10-29 00:33:11]

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

[commit-bot: I haz the power, 2014-10-29 00:42:22]

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

[commit-bot: I haz the power, 2014-10-29 00:42:24]

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

[mukesh agrawal, 2014-10-29 01:20:49]

quiche@chromium.org changed reviewers:
+ rogerta@chromium.org, tim@chromium.org

[mukesh agrawal, 2014-10-29 01:20:50]

quiche@chromium.org changed required reviewers:
+ rogerta@chromium.org, tim@chromium.org

[mukesh agrawal, 2014-10-29 01:26:53]

tim@, can you review sync_setup_overlay.html and sync_setup_overlay.js? (Or
point me to an alternate reviewer? You're listed as per-file for
sync_setup_overlay.*)

rogerta@, can you review webui/sync_setup_handler*? (Or point me to an alternate
reviewer? You're listed as per-file for sync_setup_handler*)

Thanks.

[Roger Tawa OOO till Jul 10th, 2014-10-29 13:55:03]

lgtm with comments below.

https://codereview.chromium.org/674633002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/sync_setup_overlay.js (right):

https://codereview.chromium.org/674633002/diff/80001/chrome/browser/resources...
chrome/browser/resources/sync_setup_overlay.js:449:
args.wifiCredentialsEnforced;
Indent 447 and 449 two more spaces.

https://codereview.chromium.org/674633002/diff/80001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/sync_setup_handler.cc (right):

https://codereview.chromium.org/674633002/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/sync_setup_handler.cc:124: }
Seems like this function should be moved to model_type.h|cc.

[mukesh agrawal, 2014-10-29 18:17:11]

https://codereview.chromium.org/674633002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/sync_setup_overlay.js (right):

https://codereview.chromium.org/674633002/diff/80001/chrome/browser/resources...
chrome/browser/resources/sync_setup_overlay.js:449:
args.wifiCredentialsEnforced;
On 2014/10/29 13:55:03, Roger Tawa wrote:
> Indent 447 and 449 two more spaces.

Done.

https://codereview.chromium.org/674633002/diff/80001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/sync_setup_handler.cc (right):

https://codereview.chromium.org/674633002/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/sync_setup_handler.cc:124: }
On 2014/10/29 13:55:03, Roger Tawa wrote:
> Seems like this function should be moved to model_type.h|cc.

Done.

[mukesh agrawal, 2014-10-29 18:20:43]

pavely@, can I trouble you for another look? Just want to make sure you're happy
with moving GetSelectableTypeNameMap over to model_type.cc.

[pavely, 2014-10-29 21:54:44]

On 2014/10/29 18:20:43, mukesh agrawal wrote:
> pavely@, can I trouble you for another look? Just want to make sure you're
happy
> with moving GetSelectableTypeNameMap over to model_type.cc.

LGTM.

After this change it is less clear where selectable type names are used and what
will break of one of these names change, but the benefit is that type set and
corresponding names are closer together, harder to get out of sync.

[tim (not reviewing), 2014-10-30 18:55:43]

rubber stamp owner LGTM

[mukesh agrawal, 2014-10-30 19:35:08]

The CQ bit was checked by quiche@chromium.org

[commit-bot: I haz the power, 2014-10-30 19:36:23]

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

[commit-bot: I haz the power, 2014-10-30 21:32:48]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2014-10-30 21:33:21]

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