[Sync] Device info should be enabled by default

[Sync] Device info should be enabled by default

Device info is always enabled, regardless of sync everything state.

BUG=429217
Committed: https://crrev.com/e2e772518116be2bcadcbf1f43612f67d0136509
Cr-Commit-Position: refs/heads/master@{#302487}

[Nicolas Zea, 2014-10-31 17:55:42]

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

[Nicolas Zea, 2014-10-31 17:55:43]

+Stan, PTAL

[stanisc, 2014-10-31 18:53:11]

On 2014/10/31 17:55:43, Nicolas Zea wrote:
> +Stan, PTAL

The change looks fine. However
ProfileSyncComponentsFactoryImpl::RegisterCommonDataTypes doesn't currently
check whether DEVICE_INFO is disabled. See TODO in the beginning of the
function. I am not sure if that is related to that change or not.

[Nicolas Zea, 2014-10-31 20:01:53]

On 2014/10/31 18:53:11, stanisc wrote:
> On 2014/10/31 17:55:43, Nicolas Zea wrote:
> > +Stan, PTAL
> 
> The change looks fine. However
> ProfileSyncComponentsFactoryImpl::RegisterCommonDataTypes doesn't currently
> check whether DEVICE_INFO is disabled. See TODO in the beginning of the
> function. I am not sure if that is related to that change or not.

I don't think that's related. We could theoretically add the ability to
conditionally disable this type via command line flag, but considering it
couldn't be disabled previously I don't think it's that important. There's a
fair amount of functionality that depends on the device info being synced, so
I'd rather just leave it as it is.

PTAL, I also had to fix an integration test I missed previously.

[stanisc, 2014-10-31 21:24:16]

On 2014/10/31 20:01:53, Nicolas Zea wrote:
> On 2014/10/31 18:53:11, stanisc wrote:
> > On 2014/10/31 17:55:43, Nicolas Zea wrote:
> > > +Stan, PTAL
> > 
> > The change looks fine. However
> > ProfileSyncComponentsFactoryImpl::RegisterCommonDataTypes doesn't currently
> > check whether DEVICE_INFO is disabled. See TODO in the beginning of the
> > function. I am not sure if that is related to that change or not.
> 
> I don't think that's related. We could theoretically add the ability to
> conditionally disable this type via command line flag, but considering it
> couldn't be disabled previously I don't think it's that important. There's a
> fair amount of functionality that depends on the device info being synced, so
> I'd rather just leave it as it is.
> 
> PTAL, I also had to fix an integration test I missed previously.

LGTM

[Nicolas Zea, 2014-10-31 21:26:03]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2014-10-31 21:27:03]

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

[commit-bot: I haz the power, 2014-10-31 21:27:05]

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

[commit-bot: I haz the power, 2014-10-31 21:27:07]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Nicolas Zea, 2014-11-03 19:08:38]

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

[Nicolas Zea, 2014-11-03 19:08:38]

+Pavely for owners review (Stan's committer status isn't enabled yet)

[pavely, 2014-11-03 20:02:23]

lgtm

[Nicolas Zea, 2014-11-03 20:36:27]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2014-11-03 20:38:24]

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

[commit-bot: I haz the power, 2014-11-03 21:38:31]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-11-03 21:40:10]

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