[Sync] Only pass user-selectable types to OnUserChoseDataTypes

[Sync] Only pass user-selectable types to OnUserChoseDataTypes

DCHECK in ProfileSyncService::OnUserChoseDataTypes was being triggered
when the user switches "Sync Everything" on or off on the Account Sync
settings page because GetDataTypes() returns non user-selectable
types and was being passed into OnUserChoseDataTypes as
chosen_types. Intersecting with syncer::UserSelectableTypes to
avoid this issue.

BUG=542935
Committed: https://crrev.com/a6e131b465246f71d7375a51ddefbc48a55fc984
Cr-Commit-Position: refs/heads/master@{#370234}

[Jackie Quinn, 2016-01-14 22:05:48]

jyquinn@chromium.org changed reviewers:
+ droger@chromium.org

[Jackie Quinn, 2016-01-14 22:05:48]

Hi David,

We're hitting this DCHECK when switching on and off "Sync Everything."
Continuing execution doesn't seem to have any negative implications, so I'm
wondering whether this DCHECK is necessary. OnUserChoseDataTypes is called from
SetSyncingAllDataTypes, using "GetDataTypes()" for chosen_types.

Let me know what you think.

[droger, 2016-01-15 10:07:50]

droger@chromium.org changed reviewers:
+ zea@chromium.org

[droger, 2016-01-15 10:07:51]

I don't really know about this.

+zea

[maxbogue, 2016-01-15 18:54:59]

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

[maxbogue, 2016-01-15 18:54:59]

Could you do the intersection with syncer::UserSelectableTypes() before calling
OnUserChoseDatatypes() instead of removing the DCHECK? I agree that it would be
nicer if the set and get methods for preferred types were symmetric but for now
they aren't.

[Nicolas Zea, 2016-01-15 21:24:23]

Yeah, I think I agree with Max that this method really is meant to only affect
user selectable types, so it's confusing to allow non-user selectable types in
here.

[Jackie Quinn, 2016-01-16 00:33:49]

On 2016/01/15 21:24:23, Nicolas Zea wrote:
> Yeah, I think I agree with Max that this method really is meant to only affect
> user selectable types, so it's confusing to allow non-user selectable types in
> here.

That sounds good to me. New patch uploaded!

[Jackie Quinn, 2016-01-16 00:36:15]

Description was changed from

==========
Remove DCHECK in ProfileSyncService::OnUserChoseDataTypes

DCHECK in ProfileSyncService::OnUserChoseDataTypes was being triggered
when the user switches "Sync Everything" on or off on the Account Sync
settings page. This DCHECK should be removed if not actually necessary.

BUG=542935
==========

to

==========
Only pass user-selectable types into ProfileSyncService::OnUserChoseDataTypes

DCHECK in ProfileSyncService::OnUserChoseDataTypes was being triggered
when the user switches "Sync Everything" on or off on the Account Sync
settings page because GetDataTypes() returns non user-selectable
types and was being passed into OnUserChoseDataTypes as 
chosen_types. Intersecting with syncer::UserSelectableTypes to
avoid this issue.

BUG=542935
==========

[Jackie Quinn, 2016-01-16 00:37:48]

Description was changed from

==========
Only pass user-selectable types into ProfileSyncService::OnUserChoseDataTypes

DCHECK in ProfileSyncService::OnUserChoseDataTypes was being triggered
when the user switches "Sync Everything" on or off on the Account Sync
settings page because GetDataTypes() returns non user-selectable
types and was being passed into OnUserChoseDataTypes as 
chosen_types. Intersecting with syncer::UserSelectableTypes to
avoid this issue.

BUG=542935
==========

to

==========
[Sync] Only pass user-selectable types to OnUserChoseDataTypes

DCHECK in ProfileSyncService::OnUserChoseDataTypes was being triggered
when the user switches "Sync Everything" on or off on the Account Sync
settings page because GetDataTypes() returns non user-selectable
types and was being passed into OnUserChoseDataTypes as 
chosen_types. Intersecting with syncer::UserSelectableTypes to
avoid this issue.

BUG=542935
==========

[Nicolas Zea, 2016-01-16 00:37:54]

lgtm

[Jackie Quinn, 2016-01-16 00:46:06]

The CQ bit was checked by jyquinn@chromium.org

[commit-bot: I haz the power, 2016-01-16 00:46:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1582183004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1582183004/20001

[commit-bot: I haz the power, 2016-01-16 00:55:15]

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

[commit-bot: I haz the power, 2016-01-16 00:55:16]

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

[Jackie Quinn, 2016-01-19 22:47:40]

droger, could I get an OWNERS review? Thanks!

[maxbogue, 2016-01-19 22:59:53]

On 2016/01/19 22:47:40, Jackie Quinn wrote:
> droger, could I get an OWNERS review? Thanks!

Nicolas is now an owner for that file as of http://crrev.com/1598483003.

[Jackie Quinn, 2016-01-19 23:00:49]

The CQ bit was checked by jyquinn@chromium.org

[commit-bot: I haz the power, 2016-01-19 23:03:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1582183004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1582183004/20001

[commit-bot: I haz the power, 2016-01-20 00:19:56]

Description was changed from

==========
[Sync] Only pass user-selectable types to OnUserChoseDataTypes

DCHECK in ProfileSyncService::OnUserChoseDataTypes was being triggered
when the user switches "Sync Everything" on or off on the Account Sync
settings page because GetDataTypes() returns non user-selectable
types and was being passed into OnUserChoseDataTypes as 
chosen_types. Intersecting with syncer::UserSelectableTypes to
avoid this issue.

BUG=542935
==========

to

==========
[Sync] Only pass user-selectable types to OnUserChoseDataTypes

DCHECK in ProfileSyncService::OnUserChoseDataTypes was being triggered
when the user switches "Sync Everything" on or off on the Account Sync
settings page because GetDataTypes() returns non user-selectable
types and was being passed into OnUserChoseDataTypes as 
chosen_types. Intersecting with syncer::UserSelectableTypes to
avoid this issue.

BUG=542935
==========

[commit-bot: I haz the power, 2016-01-20 00:19:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-20 00:21:06]

Description was changed from

==========
[Sync] Only pass user-selectable types to OnUserChoseDataTypes

DCHECK in ProfileSyncService::OnUserChoseDataTypes was being triggered
when the user switches "Sync Everything" on or off on the Account Sync
settings page because GetDataTypes() returns non user-selectable
types and was being passed into OnUserChoseDataTypes as 
chosen_types. Intersecting with syncer::UserSelectableTypes to
avoid this issue.

BUG=542935
==========

to

==========
[Sync] Only pass user-selectable types to OnUserChoseDataTypes

DCHECK in ProfileSyncService::OnUserChoseDataTypes was being triggered
when the user switches "Sync Everything" on or off on the Account Sync
settings page because GetDataTypes() returns non user-selectable
types and was being passed into OnUserChoseDataTypes as
chosen_types. Intersecting with syncer::UserSelectableTypes to
avoid this issue.

BUG=542935

Committed: https://crrev.com/a6e131b465246f71d7375a51ddefbc48a55fc984
Cr-Commit-Position: refs/heads/master@{#370234}
==========

[commit-bot: I haz the power, 2016-01-20 00:21:07]

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