[Sync] Move data type logic from PSSA to PSS.

[Sync] Move data type logic from PSSA to PSS.

Now PSS::OnUserChoseDatatypes filters to only user selectable types,
and GetPreferredDataTypes() will always include the control types.

BUG=497518
Committed: https://crrev.com/0f5131e4f6ae03647aae0cac8154bf888b844e95
Cr-Commit-Position: refs/heads/master@{#350230}

[maxbogue, 2015-09-16 17:34:04]

Nicolas, PTAL; this is the change we discussed Monday.

[maxbogue, 2015-09-16 18:39:53]

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

[Nicolas Zea, 2015-09-16 21:30:26]

https://codereview.chromium.org/1304933010/diff/20001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_service.cc (right):

https://codereview.chromium.org/1304933010/diff/20001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_service.cc:1815:
chosen_types.RetainAll(syncer::UserSelectableTypes());
Should this be a DCHECK I wonder? If not, it would be good to understand why
OnUserChoseDatatypes is being passed non selectable types, so we can verify they
don't matter.

[maxbogue, 2015-09-18 23:47:52]

https://codereview.chromium.org/1304933010/diff/20001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_service.cc (right):

https://codereview.chromium.org/1304933010/diff/20001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_service.cc:1815:
chosen_types.RetainAll(syncer::UserSelectableTypes());
On 2015/09/16 21:30:25, Nicolas Zea wrote:
> Should this be a DCHECK I wonder? If not, it would be good to understand why
> OnUserChoseDatatypes is being passed non selectable types, so we can verify
they
> don't matter.

I tried converting to a DCHECK and there were lots of issues with tests. After
some investigation I think I would prefer to leave this as RetainAll for now and
do a larger inspection of all the data type methods and their relationships
later on.

[Nicolas Zea, 2015-09-19 00:02:13]

On 2015/09/18 23:47:52, maxbogue wrote:
>
https://codereview.chromium.org/1304933010/diff/20001/chrome/browser/sync/pro...
> File chrome/browser/sync/profile_sync_service.cc (right):
> 
>
https://codereview.chromium.org/1304933010/diff/20001/chrome/browser/sync/pro...
> chrome/browser/sync/profile_sync_service.cc:1815:
> chosen_types.RetainAll(syncer::UserSelectableTypes());
> On 2015/09/16 21:30:25, Nicolas Zea wrote:
> > Should this be a DCHECK I wonder? If not, it would be good to understand why
> > OnUserChoseDatatypes is being passed non selectable types, so we can verify
> they
> > don't matter.
> 
> I tried converting to a DCHECK and there were lots of issues with tests. After
> some investigation I think I would prefer to leave this as RetainAll for now
and
> do a larger inspection of all the data type methods and their relationships
> later on.

I guess my concern here is that if a DCHECK doesn't work, I wonder if it's
actually safe to do. It seems we need to better understand the cases this is
being called with non-user selectable types before this change can be made.

[maxbogue, 2015-09-22 17:17:30]

Ok, got the DCHECK working and tests all green! PTAL!

[Nicolas Zea, 2015-09-22 20:29:43]

LGTM

[maxbogue, 2015-09-22 20:49:38]

The CQ bit was checked by maxbogue@chromium.org

[commit-bot: I haz the power, 2015-09-22 20:50:18]

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

[commit-bot: I haz the power, 2015-09-22 21:16:24]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-09-22 21:52:52]

Patchset 6 (id:??) landed as
https://crrev.com/0f5131e4f6ae03647aae0cac8154bf888b844e95
Cr-Commit-Position: refs/heads/master@{#350230}

[jbudorick, 2015-09-23 03:57:57]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1363803002/ by jbudorick@chromium.org.

The reason for reverting is: broke sync instrumentation tests:
http://build.chromium.org/p/chromium.linux/builders/Android%20Tests/builds/22408.