[Remote suggestions] Clean up variation params in the fetcher.

components/ntp_snippets/remote/remote_suggestions_fetcher.cc

Index: components/ntp_snippets/remote/remote_suggestions_fetcher.cc
diff --git a/components/ntp_snippets/remote/remote_suggestions_fetcher.cc b/components/ntp_snippets/remote/remote_suggestions_fetcher.cc
index b4afcee5cc6b80f32ce28f592e41a69e34d7a979..efd768988d9c7c7d93b147eb091c490993d7815c 100644
--- a/components/ntp_snippets/remote/remote_suggestions_fetcher.cc
+++ b/components/ntp_snippets/remote/remote_suggestions_fetcher.cc
@@ -52,17 +52,9 @@ const char kContentSuggestionsApiScope[] =
 const char kSnippetsServerNonAuthorizedFormat[] = "%s?key=%s";
 const char kAuthorizationRequestHeaderFormat[] = "Bearer %s";
 
-// Variation parameter for personalizing fetching of suggestions.
-const char kPersonalizationName[] = "fetching_personalization";
-
 // Variation parameter for chrome-content-suggestions backend.
 const char kContentSuggestionsBackend[] = "content_suggestions_backend";
 
-// Constants for possible values of the "fetching_personalization" parameter.
-const char kPersonalizationPersonalString[] = "personal";
-const char kPersonalizationNonPersonalString[] = "non_personal";
-const char kPersonalizationBothString[] = "both";  // the default value
-
 const int kFetchTimeHistogramResolution = 5;
 
 std::string FetchResultToString(FetchResult result) {
@@ -119,8 +111,8 @@ Status FetchResultToStatus(FetchResult result) {
 }
 
 std::string GetFetchEndpoint() {
-  std::string endpoint = variations::GetVariationParamValue(
-      ntp_snippets::kStudyName, kContentSuggestionsBackend);
+  std::string endpoint = variations::GetVariationParamValueByFeature(
+      ntp_snippets::kArticleSuggestionsFeature, kContentSuggestionsBackend);

[Marc Treib, 2017/01/30 17:02:57]

This will break the current configs, right?

Should we maybe try looking it up by feature, and if that fails, fall back to
study name? Or handle it all server-side in the configs? WDYT?

[jkrcal, 2017/01/30 17:11:01]

I would handle it in the configs, server-side.

After the new config lands, it will affect the Canary/Dev config. As the config
anyway specifies the default end-point, it will not break anything. I will adapt
the canary/dev config afterwards and move it into the NTPArticle... file.

We only need to keep it in mind when preparing configs for M58 Beta / Stable. I
will add comments to the respective finch config files to make sure we do not
forget.

Does it make sense?

[Marc Treib, 2017/01/31 09:01:43]

Actually on second thought, since the ZineServer endpoint is the default now
anyway (I think already in M57), we should just remove the param from the
Dev/Canary configs.

   return endpoint.empty() ? kContentSuggestionsServer : endpoint;
 }
 
@@ -284,21 +276,7 @@ RemoteSuggestionsFetcher::RemoteSuggestionsFetcher(
           pref_service,
           RequestThrottler::RequestType::
               CONTENT_SUGGESTION_FETCHER_ACTIVE_SUGGESTIONS_CONSUMER),
-      weak_ptr_factory_(this) {
-  std::string personalization = variations::GetVariationParamValue(
-      ntp_snippets::kStudyName, kPersonalizationName);
-  if (personalization == kPersonalizationNonPersonalString) {
-    personalization_ = Personalization::kNonPersonal;
-  } else if (personalization == kPersonalizationPersonalString) {
-    personalization_ = Personalization::kPersonal;
-  } else {
-    personalization_ = Personalization::kBoth;
-    LOG_IF(WARNING, !personalization.empty() &&
-                        personalization != kPersonalizationBothString)
-        << "Unknown value for " << kPersonalizationName << ": "
-        << personalization;
-  }
-}
+      weak_ptr_factory_(this) {}
 
 RemoteSuggestionsFetcher::~RemoteSuggestionsFetcher() {
   if (waiting_for_refresh_token_) {
@@ -333,17 +311,16 @@ void RemoteSuggestionsFetcher::FetchSnippets(
       .SetLanguageModel(language_model_)
       .SetParams(params)
       .SetParseJsonCallback(parse_json_callback_)
-      .SetPersonalization(personalization_)
       .SetTickClock(tick_clock_.get())
       .SetUrlRequestContextGetter(url_request_context_getter_)
       .SetUserClassifier(*user_classifier_);
 
-  if (NeedsAuthentication() && signin_manager_->IsAuthenticated()) {
+  if (signin_manager_->IsAuthenticated()) {
     // Signed-in: get OAuth token --> fetch suggestions.
     oauth_token_retried_ = false;
     pending_requests_.emplace(std::move(builder), std::move(callback));
     StartTokenRequest();
-  } else if (NeedsAuthentication() && signin_manager_->AuthInProgress()) {
+  } else if (signin_manager_->AuthInProgress()) {
     // Currently signing in: wait for auth to finish (the refresh token) -->
     //     get OAuth token --> fetch suggestions.
     pending_requests_.emplace(std::move(builder), std::move(callback));
@@ -607,25 +584,4 @@ bool RemoteSuggestionsFetcher::DemandQuotaForRequest(bool interactive_request) {
   return false;
 }
 
-bool RemoteSuggestionsFetcher::NeedsAuthentication() const {
-  return (personalization_ == Personalization::kPersonal ||
-          personalization_ == Personalization::kBoth);
-}
-
-std::string RemoteSuggestionsFetcher::PersonalizationModeString() const {
-  switch (personalization_) {
-    case Personalization::kPersonal:
-      return "Only personalized";
-      break;
-    case Personalization::kBoth:
-      return "Both personalized and non-personalized";
-      break;
-    case Personalization::kNonPersonal:
-      return "Only non-personalized";
-      break;
-  }
-  NOTREACHED();
-  return std::string();
-}
-
 }  // namespace ntp_snippets