Change the default value of the fetching_requires_signin parameter

With the current default value fetching_requires_signin=true, users get
suboptimal UX after fresh install / after clearing app data. Because of
bug 632199, the server-side variation parameters are not applied in the
first run and sign-in is required. This is inconsistent with the second
run of Chrome, where the server sets fetching_requires_signin:=false
and no sign-in is required any more.

BUG=646759

Review-Url: https://codereview.chromium.org/2343583002
Cr-Commit-Position: refs/heads/master@{#419147}

components/ntp_snippets/ntp_snippets_status_service.cc

@@ -26,16 +26,16 @@ NTPSnippetsStatusService::NTPSnippetsStatusService(
     SigninManagerBase* signin_manager,
     PrefService* pref_service)
     : disabled_reason_(DisabledReason::EXPLICITLY_DISABLED),
-      require_signin_(true),
+      require_signin_(false),
       signin_manager_(signin_manager),
       pref_service_(pref_service),
       signin_observer_(this) {
   std::string param_value_str = variations::GetVariationParamValueByFeature(
       kArticleSuggestionsFeature, kFetchingRequiresSignin);
-  if (param_value_str == kFetchingRequiresSigninDisabled) {
-    require_signin_ = false;
+  if (param_value_str == kFetchingRequiresSigninEnabled) {
+    require_signin_ = true;
   } else if (!param_value_str.empty() &&
-             param_value_str != kFetchingRequiresSigninEnabled) {
+             param_value_str != kFetchingRequiresSigninDisabled) {
     DLOG(WARNING) << "Unknow value for the variations parameter "
                   << kFetchingRequiresSignin << ": " << param_value_str;
   }

components/ntp_snippets/ntp_snippets_status_service.h

@@ -43,8 +43,6 @@ class NTPSnippetsStatusService : public SigninManagerBase::Observer {
   void Init(const DisabledReasonChangeCallback& callback);
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(NTPSnippetsStatusServiceTest,
-                           SigninStateCompatibility);
   FRIEND_TEST_ALL_PREFIXES(NTPSnippetsStatusServiceTest, DisabledViaPref);
 
   // SigninManagerBase::Observer implementation

components/ntp_snippets/ntp_snippets_status_service_unittest.cc

@@ -37,29 +37,21 @@ class NTPSnippetsStatusServiceTest : public ::testing::Test {
   test::NTPSnippetsTestUtils utils_;
 };
 
-TEST_F(NTPSnippetsStatusServiceTest, SigninStateCompatibility) {
-  auto service = MakeService();
-
-  // The default test setup is signed out.
-  EXPECT_EQ(DisabledReason::SIGNED_OUT, service->GetDisabledReasonFromDeps());
-
-  // Once signed in, we should be in a compatible state.
-  utils_.fake_signin_manager()->SignIn("foo@bar.com");
-  EXPECT_EQ(DisabledReason::NONE, service->GetDisabledReasonFromDeps());
-}
-
+// TODO(jkrcal): Extend the ways to override variation parameters in unit-test
+// (bug 645447), and recover the SigninStateCompatibility test that sign-in is
+// required when the parameter is overriden.
 TEST_F(NTPSnippetsStatusServiceTest, DisabledViaPref) {
   auto service = MakeService();
 
-  // The default test setup is signed out.
-  ASSERT_EQ(DisabledReason::SIGNED_OUT, service->GetDisabledReasonFromDeps());
+  // The default test setup is signed out. The service is enabled.
+  ASSERT_EQ(DisabledReason::NONE, service->GetDisabledReasonFromDeps());
 
   // Once the enabled pref is set to false, we should be disabled.
   utils_.pref_service()->SetBoolean(prefs::kEnableSnippets, false);
   EXPECT_EQ(DisabledReason::EXPLICITLY_DISABLED,
             service->GetDisabledReasonFromDeps());
 
-  // The other dependencies shouldn't matter anymore.
+  // Signing-in shouldn't matter anymore.
   utils_.fake_signin_manager()->SignIn("foo@bar.com");
   EXPECT_EQ(DisabledReason::EXPLICITLY_DISABLED,
             service->GetDisabledReasonFromDeps());