Add another unittest for configuring sign-in dependency.

Add another unittest for configuring sign-in dependency.

This test is made possible by adding feature associations to
VariationParamsManager.

BUG=645447
Committed: https://crrev.com/2cd568f54d8de0e9daab1c25cfe0dc5056e0f234
Cr-Commit-Position: refs/heads/master@{#436573}

[jkrcal, 2016-12-06 07:33:18]

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

[commit-bot: I haz the power, 2016-12-06 07:33:33]

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, 2016-12-06 08:17:05]

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

[commit-bot: I haz the power, 2016-12-06 08:17:06]

Dry run: This issue passed the CQ dry run.

[jkrcal, 2016-12-06 08:25:33]

jkrcal@chromium.org changed reviewers:
+ treib@chromium.org

[jkrcal, 2016-12-06 08:25:34]

Marc, could you PTAL?

BTW, why is there still
components/ntp_snippets/remote/ntp_snippets_status_service.h? Not reference from
anywhere outside...

[Marc Treib, 2016-12-06 10:16:33]

On 2016/12/06 08:25:34, jkrcal wrote:
> Marc, could you PTAL?
> 
> BTW, why is there still
> components/ntp_snippets/remote/ntp_snippets_status_service.h? Not reference
from
> anywhere outside...

It's there because you added it back in
https://codereview.chromium.org/2321273003  ;P
(probably a bad merge/rebase)

[jkrcal, 2016-12-06 10:19:39]

On 2016/12/06 10:16:33, Marc Treib wrote:
> On 2016/12/06 08:25:34, jkrcal wrote:
> > Marc, could you PTAL?
> > 
> > BTW, why is there still
> > components/ntp_snippets/remote/ntp_snippets_status_service.h? Not reference
> from
> > anywhere outside...
> 
> It's there because you added it back in
> https://codereview.chromium.org/2321273003  ;P
> (probably a bad merge/rebase)

Oh :O
Ok, removed here.

[Marc Treib, 2016-12-06 10:21:09]

LGTM with some nits. Nice!

https://codereview.chromium.org/2556543003/diff/20001/components/ntp_snippets...
File
components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc
(right):

https://codereview.chromium.org/2556543003/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc:43:
// Specify by the parameter that no signin is required.
I think the "no" shouldn't be here?

https://codereview.chromium.org/2556543003/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc:57:
service->GetStatusFromDeps());
Would be nice to rewrite this tests in terms of the public interface and remove
the FRIEND_TESTs.
Not for this CL, but maybe add a TODO somewhere here?

https://codereview.chromium.org/2556543003/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc:63:
// Indeed, no signin is required.
s/Indeed/By default/
?

https://codereview.chromium.org/2556543003/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc:67:
// Once can still sign in.
s/Once/One/
?

[jkrcal, 2016-12-06 10:32:44]

Oh, oh, high ratio of typos in comments vs. no of lines touched ;)

Thanks!

https://codereview.chromium.org/2556543003/diff/20001/components/ntp_snippets...
File
components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc
(right):

https://codereview.chromium.org/2556543003/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc:43:
// Specify by the parameter that no signin is required.
On 2016/12/06 10:21:09, Marc Treib wrote:
> I think the "no" shouldn't be here?

Done.

https://codereview.chromium.org/2556543003/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc:57:
service->GetStatusFromDeps());
On 2016/12/06 10:21:09, Marc Treib wrote:
> Would be nice to rewrite this tests in terms of the public interface and
remove
> the FRIEND_TESTs.
> Not for this CL, but maybe add a TODO somewhere here?

Done.

https://codereview.chromium.org/2556543003/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc:63:
// Indeed, no signin is required.
On 2016/12/06 10:21:09, Marc Treib wrote:
> s/Indeed/By default/
> ?

Done.

https://codereview.chromium.org/2556543003/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc:67:
// Once can still sign in.
On 2016/12/06 10:21:09, Marc Treib wrote:
> s/Once/One/
> ?

Done.

[jkrcal, 2016-12-06 10:32:52]

The CQ bit was checked by jkrcal@chromium.org

[jkrcal, 2016-12-06 10:32:52]

The patchset sent to the CQ was uploaded after l-g-t-m from treib@chromium.org
Link to the patchset: https://codereview.chromium.org/2556543003/#ps60001
(title: "Marc's comments")

[commit-bot: I haz the power, 2016-12-06 10:33:01]

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, 2016-12-06 12:00:38]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481020372008410,
"parent_rev": "10b837a671050c924a42736a0dbc2aa93af25c52", "commit_rev":
"81ce2c5af1df02e186526bd2b4c6e3857425bdbf"}

[commit-bot: I haz the power, 2016-12-06 12:01:04]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-06 12:03:32]

Description was changed from

==========
Add another unittest for configuring sign-in dependency.

This test is made possible by adding feature associations to
VariationParamsManager.

BUG=645447
==========

to

==========
Add another unittest for configuring sign-in dependency.

This test is made possible by adding feature associations to
VariationParamsManager.

BUG=645447

Committed: https://crrev.com/2cd568f54d8de0e9daab1c25cfe0dc5056e0f234
Cr-Commit-Position: refs/heads/master@{#436573}
==========

[commit-bot: I haz the power, 2016-12-06 12:03:32]

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