Connecting UserClassifier to NtpSnippetsFetcher

components/ntp_snippets/user_classifier.cc

Index: components/ntp_snippets/user_classifier.cc
diff --git a/components/ntp_snippets/user_classifier.cc b/components/ntp_snippets/user_classifier.cc
index a9122343a1ee3e2977c2416e02121463455b04e1..d25cb2256a1d36bc822d2529f99bf62738eee321 100644
--- a/components/ntp_snippets/user_classifier.cc
+++ b/components/ntp_snippets/user_classifier.cc
@@ -267,6 +267,10 @@ double UserClassifier::GetEstimatedAvgTime(Metric metric) const {
 }
 
 UserClassifier::UserClass UserClassifier::GetUserClass() const {
+  // The pref_service_ can be null in tests.
+  if (!pref_service_)

[Marc Treib, 2016/10/06 14:54:37]

Is this necessary? All things being equal, I prefer to avoid "null in tests".

[jkrcal, 2016/10/06 15:18:41]

If the pref_service is null, we anyway get some default value (based on default
values deeper in the calls), it is just very unclear what it is. If you insist,
I can remove it.

[Marc Treib, 2016/10/06 15:27:26]

Ah, I just noticed that there's lots of other null checks for the pref_service_
in this class... no, then one more doesn't matter. For the future, it'd be nice
if we could get rid of them all. It's fairly simple to pass in a fake pref
service in tests.

[jkrcal, 2016/10/06 18:25:38]

Makes sense...

+    return UserClass::ACTIVE_NTP_USER;
+
   if (GetEstimatedAvgTime(Metric::NTP_OPENED) >=
       rare_user_opens_ntp_at_most_once_per_hours_) {
     return UserClass::RARE_NTP_USER;