[Suggestions] No longer check Variations for the enabling of SuggestionsService

The service is now on by default.

BUG=387105
TEST=SuggestionsServiceTest

Review URL: https://codereview.chromium.org/727573002

Cr-Commit-Position: refs/heads/master@{#304598}

3 files changed, 33 insertions, 106 deletions

diff --git a/components/suggestions/suggestions_service.cc b/components/suggestions/suggestions_service.cc
index 9af6087..44e173b 100644
--- a/components/suggestions/suggestions_service.cc
+++ b/components/suggestions/suggestions_service.cc
@@ -4,7 +4,6 @@
 
 #include "components/suggestions/suggestions_service.h"
 
-#include <sstream>
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
@@ -90,31 +89,18 @@ const int kBlacklistMaxDelaySec = 300;  // 5 minutes
 }  // namespace
 
 const char kSuggestionsFieldTrialName[] = "ChromeSuggestions";
-const char kSuggestionsFieldTrialURLParam[] = "url";
-const char kSuggestionsFieldTrialCommonParamsParam[] = "common_params";
-const char kSuggestionsFieldTrialBlacklistPathParam[] = "blacklist_path";
-const char kSuggestionsFieldTrialBlacklistUrlParam[] = "blacklist_url_param";
-const char kSuggestionsFieldTrialStateParam[] = "state";
 const char kSuggestionsFieldTrialControlParam[] = "control";
 const char kSuggestionsFieldTrialStateEnabled[] = "enabled";
 
+// TODO(mathp): Put this in TemplateURL.
+const char kSuggestionsURL[] = "https://www.google.com/chromesuggestions?t=2";
+const char kSuggestionsBlacklistURLPrefix[] =
+    "https://www.google.com/chromesuggestions/blacklist?t=2&url=";
+const char kSuggestionsBlacklistURLParam[] = "url";
+
 // The default expiry timeout is 72 hours.
 const int64 kDefaultExpiryUsec = 72 * base::Time::kMicrosecondsPerHour;
 
-namespace {
-
-std::string GetBlacklistUrlPrefix() {
-  std::stringstream blacklist_url_prefix_stream;
-  blacklist_url_prefix_stream
-      << GetExperimentParam(kSuggestionsFieldTrialURLParam)
-      << GetExperimentParam(kSuggestionsFieldTrialBlacklistPathParam) << "?"
-      << GetExperimentParam(kSuggestionsFieldTrialCommonParamsParam) << "&"
-      << GetExperimentParam(kSuggestionsFieldTrialBlacklistUrlParam) << "=";
-  return blacklist_url_prefix_stream.str();
-}
-
-}  // namespace
-
 SuggestionsService::SuggestionsService(
     net::URLRequestContextGetter* url_request_context,
     scoped_ptr<SuggestionsStore> suggestions_store,
@@ -125,23 +111,13 @@ SuggestionsService::SuggestionsService(
       thumbnail_manager_(thumbnail_manager.Pass()),
       blacklist_store_(blacklist_store.Pass()),
       blacklist_delay_sec_(kBlacklistDefaultDelaySec),
-      weak_ptr_factory_(this) {
-  // Obtain various parameters from Variations.
-  suggestions_url_ =
-      GURL(GetExperimentParam(kSuggestionsFieldTrialURLParam) + "?" +
-           GetExperimentParam(kSuggestionsFieldTrialCommonParamsParam));
-  blacklist_url_prefix_ = GetBlacklistUrlPrefix();
-}
+      suggestions_url_(kSuggestionsURL),
+      blacklist_url_prefix_(kSuggestionsBlacklistURLPrefix),
+      weak_ptr_factory_(this) {}
 
 SuggestionsService::~SuggestionsService() {}
 
 // static
-bool SuggestionsService::IsEnabled() {
-  return GetExperimentParam(kSuggestionsFieldTrialStateParam) ==
-         kSuggestionsFieldTrialStateEnabled;
-}
-
-// static
 bool SuggestionsService::IsControlGroup() {
   return GetExperimentParam(kSuggestionsFieldTrialControlParam) ==
          kSuggestionsFieldTrialStateEnabled;
@@ -159,7 +135,6 @@ void SuggestionsService::FetchSuggestionsData(
     DispatchRequestsAndClear(SuggestionsProfile(), &waiting_requestors_);
   } else if (sync_state == INITIALIZED_ENABLED_HISTORY ||
              sync_state == NOT_INITIALIZED_ENABLED) {
-
     // Sync is enabled. Serve previously cached suggestions if available, else
     // an empty set of suggestions.
     ServeFromCache();
@@ -200,16 +175,18 @@ void SuggestionsService::BlacklistURL(
 bool SuggestionsService::GetBlacklistedUrl(const net::URLFetcher& request,
                                            GURL* url) {
   bool is_blacklist_request = StartsWithASCII(request.GetOriginalURL().spec(),
-                                              GetBlacklistUrlPrefix(), true);
+                                              kSuggestionsBlacklistURLPrefix,
+                                              true);
   if (!is_blacklist_request) return false;
 
   // Extract the blacklisted URL from the blacklist request.
   std::string blacklisted;
   if (!net::GetValueForKeyInQuery(
           request.GetOriginalURL(),
-          GetExperimentParam(kSuggestionsFieldTrialBlacklistUrlParam),
-          &blacklisted))
+          kSuggestionsBlacklistURLParam,
+          &blacklisted)) {
     return false;
+  }
 
   GURL blacklisted_url(blacklisted);
   blacklisted_url.Swap(url);
diff --git a/components/suggestions/suggestions_service.h b/components/suggestions/suggestions_service.h
index db8ab87..75d3275 100644
--- a/components/suggestions/suggestions_service.h
+++ b/components/suggestions/suggestions_service.h
@@ -38,13 +38,12 @@ class BlacklistStore;
 class SuggestionsStore;
 
 extern const char kSuggestionsFieldTrialName[];
-extern const char kSuggestionsFieldTrialURLParam[];
-extern const char kSuggestionsFieldTrialCommonParamsParam[];
-extern const char kSuggestionsFieldTrialBlacklistPathParam[];
-extern const char kSuggestionsFieldTrialBlacklistUrlParam[];
-extern const char kSuggestionsFieldTrialStateParam[];
 extern const char kSuggestionsFieldTrialControlParam[];
 extern const char kSuggestionsFieldTrialStateEnabled[];
+
+extern const char kSuggestionsURL[];
+extern const char kSuggestionsBlacklistURLPrefix[];
+extern const char kSuggestionsBlacklistURLParam[];
 extern const int64 kDefaultExpiryUsec;
 
 // An interface to fetch server suggestions asynchronously.
@@ -61,9 +60,6 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
       scoped_ptr<BlacklistStore> blacklist_store);
   ~SuggestionsService() override;
 
-  // Whether this service is enabled.
-  static bool IsEnabled();
-
   // Whether the user is part of a control group.
   static bool IsControlGroup();
 
diff --git a/components/suggestions/suggestions_service_unittest.cc b/components/suggestions/suggestions_service_unittest.cc
index a66700a..0f79b9c 100644
--- a/components/suggestions/suggestions_service_unittest.cc
+++ b/components/suggestions/suggestions_service_unittest.cc
@@ -38,11 +38,6 @@ using ::testing::_;
 
 namespace {
 
-const char kFakeSuggestionsURL[] = "https://mysuggestions.com/proto";
-const char kFakeSuggestionsCommonParams[] = "foo=bar";
-const char kFakeBlacklistPath[] = "/blacklist";
-const char kFakeBlacklistUrlParam[] = "baz";
-
 const char kTestTitle[] = "a title";
 const char kTestUrl[] = "http://go.com";
 const char kBlacklistUrl[] = "http://blacklist.com";
@@ -67,9 +62,8 @@ scoped_ptr<net::FakeURLFetcher> CreateURLFetcher(
 
 std::string GetExpectedBlacklistRequestUrl(const GURL& blacklist_url) {
   std::stringstream request_url;
-  request_url << kFakeSuggestionsURL << kFakeBlacklistPath << "?"
-              << kFakeSuggestionsCommonParams << "&" << kFakeBlacklistUrlParam
-              << "=" << net::EscapeQueryParamValue(blacklist_url.spec(), true);
+  request_url << "https://www.google.com/chromesuggestions/blacklist?t=2&url="
+              << net::EscapeQueryParamValue(blacklist_url.spec(), true);
   return request_url.str();
 }
 
@@ -118,19 +112,19 @@ class TestSuggestionsStore : public suggestions::SuggestionsStore {
     cached_suggestions = CreateSuggestionsProfile();
   }
   virtual ~TestSuggestionsStore() {}
-  virtual bool LoadSuggestions(SuggestionsProfile* suggestions) override {
+  bool LoadSuggestions(SuggestionsProfile* suggestions) override {
     if (cached_suggestions.suggestions_size()) {
       *suggestions = cached_suggestions;
       return true;
     }
     return false;
   }
-  virtual bool StoreSuggestions(const SuggestionsProfile& suggestions)
+  bool StoreSuggestions(const SuggestionsProfile& suggestions)
       override {
     cached_suggestions = suggestions;
     return true;
   }
-  virtual void ClearSuggestions() override {
+  void ClearSuggestions() override {
     cached_suggestions = SuggestionsProfile();
   }
 
@@ -188,12 +182,8 @@ class SuggestionsServiceTest : public testing::Test {
         io_message_loop_.message_loop_proxy());
   }
 
-  // Enables the "ChromeSuggestions.Group1" field trial.
-  void EnableFieldTrial(const std::string& url,
-                        const std::string& common_params,
-                        const std::string& blacklist_path,
-                        const std::string& blacklist_url_param,
-                        bool control_group) {
+  // Enables the control group in the "ChromeSuggestions.Group1" field trial.
+  void EnableFieldTrialControlGroup() {
     // Clear the existing |field_trial_list_| to avoid firing a DCHECK.
     field_trial_list_.reset(NULL);
     field_trial_list_.reset(
@@ -201,16 +191,8 @@ class SuggestionsServiceTest : public testing::Test {
 
     variations::testing::ClearAllVariationParams();
     std::map<std::string, std::string> params;
-    params[kSuggestionsFieldTrialStateParam] =
+    params[kSuggestionsFieldTrialControlParam] =
         kSuggestionsFieldTrialStateEnabled;
-    if (control_group) {
-      params[kSuggestionsFieldTrialControlParam] =
-          kSuggestionsFieldTrialStateEnabled;
-    }
-    params[kSuggestionsFieldTrialURLParam] = url;
-    params[kSuggestionsFieldTrialCommonParamsParam] = common_params;
-    params[kSuggestionsFieldTrialBlacklistPathParam] = blacklist_path;
-    params[kSuggestionsFieldTrialBlacklistUrlParam] = blacklist_url_param;
     variations::AssociateVariationParams(kSuggestionsFieldTrialName, "Group1",
                                          params);
     field_trial_ = base::FieldTrialList::CreateFieldTrial(
@@ -219,9 +201,6 @@ class SuggestionsServiceTest : public testing::Test {
   }
 
   void FetchSuggestionsDataHelper(SyncState sync_state) {
-    // Field trial enabled with a specific suggestions URL.
-    EnableFieldTrial(kFakeSuggestionsURL, kFakeSuggestionsCommonParams,
-                     kFakeBlacklistPath, kFakeBlacklistUrlParam, false);
     scoped_ptr<SuggestionsService> suggestions_service(
         CreateSuggestionsServiceWithMocks());
     EXPECT_TRUE(suggestions_service != NULL);
@@ -232,9 +211,7 @@ class SuggestionsServiceTest : public testing::Test {
     test_suggestions_store_->LoadSuggestions(&suggestions_profile);
 
     // Set up net::FakeURLFetcherFactory.
-    std::string expected_url =
-        (std::string(kFakeSuggestionsURL) + "?") + kFakeSuggestionsCommonParams;
-    factory_.SetFakeResponse(GURL(expected_url),
+    factory_.SetFakeResponse(GURL(kSuggestionsURL),
                              suggestions_profile.SerializeAsString(),
                              net::HTTP_OK, net::URLRequestStatus::SUCCESS);
 
@@ -292,11 +269,9 @@ class SuggestionsServiceTest : public testing::Test {
 };
 
 TEST_F(SuggestionsServiceTest, IsControlGroup) {
-  // Field trial enabled.
-  EnableFieldTrial("", "", "", "", false);
   EXPECT_FALSE(SuggestionsService::IsControlGroup());
 
-  EnableFieldTrial("", "", "", "", true);
+  EnableFieldTrialControlGroup();
   EXPECT_TRUE(SuggestionsService::IsControlGroup());
 }
 
@@ -309,9 +284,6 @@ TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncNotInitializedEnabled) {
 }
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncDisabled) {
-  // Field trial enabled with a specific suggestions URL.
-  EnableFieldTrial(kFakeSuggestionsURL, kFakeSuggestionsCommonParams,
-                   kFakeBlacklistPath, kFakeBlacklistUrlParam, false);
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
   EXPECT_TRUE(suggestions_service != NULL);
@@ -330,33 +302,25 @@ TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncDisabled) {
 }
 
 TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingError) {
-  // Field trial enabled with a specific suggestions URL.
-  EnableFieldTrial(kFakeSuggestionsURL, kFakeSuggestionsCommonParams,
-                   kFakeBlacklistPath, kFakeBlacklistUrlParam, false);
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
   EXPECT_TRUE(suggestions_service != NULL);
 
   // Fake a request error.
-  std::string expected_url =
-      (std::string(kFakeSuggestionsURL) + "?") + kFakeSuggestionsCommonParams;
-  factory_.SetFakeResponse(GURL(expected_url), "irrelevant", net::HTTP_OK,
+  factory_.SetFakeResponse(GURL(kSuggestionsURL), "irrelevant", net::HTTP_OK,
                            net::URLRequestStatus::FAILED);
 
   EXPECT_CALL(*mock_blacklist_store_, GetFirstUrlFromBlacklist(_))
       .WillOnce(Return(false));
 
   // Send the request. Empty data will be returned to the callback.
-  suggestions_service->IssueRequestIfNoneOngoing(GURL(expected_url));
+  suggestions_service->IssueRequestIfNoneOngoing(GURL(kSuggestionsURL));
 
   // (Testing only) wait until suggestion fetch is complete.
   io_message_loop_.RunUntilIdle();
 }
 
 TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingResponseNotOK) {
-  // Field trial enabled with a specific suggestions URL.
-  EnableFieldTrial(kFakeSuggestionsURL, kFakeSuggestionsCommonParams,
-                   kFakeBlacklistPath, kFakeBlacklistUrlParam, false);
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
   EXPECT_TRUE(suggestions_service != NULL);
@@ -365,9 +329,7 @@ TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingResponseNotOK) {
   FillSuggestionsStore();
 
   // Fake a non-200 response code.
-  std::string expected_url =
-      (std::string(kFakeSuggestionsURL) + "?") + kFakeSuggestionsCommonParams;
-  factory_.SetFakeResponse(GURL(expected_url), "irrelevant",
+  factory_.SetFakeResponse(GURL(kSuggestionsURL), "irrelevant",
                            net::HTTP_BAD_REQUEST,
                            net::URLRequestStatus::SUCCESS);
 
@@ -376,7 +338,7 @@ TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingResponseNotOK) {
       .WillOnce(Return(false));
 
   // Send the request. Empty data will be returned to the callback.
-  suggestions_service->IssueRequestIfNoneOngoing(GURL(expected_url));
+  suggestions_service->IssueRequestIfNoneOngoing(GURL(kSuggestionsURL));
 
   // (Testing only) wait until suggestion fetch is complete.
   io_message_loop_.RunUntilIdle();
@@ -387,8 +349,6 @@ TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingResponseNotOK) {
 }
 
 TEST_F(SuggestionsServiceTest, BlacklistURL) {
-  EnableFieldTrial(kFakeSuggestionsURL, kFakeSuggestionsCommonParams,
-                   kFakeBlacklistPath, kFakeBlacklistUrlParam, false);
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
   EXPECT_TRUE(suggestions_service != NULL);
@@ -427,8 +387,6 @@ TEST_F(SuggestionsServiceTest, BlacklistURL) {
 // Initial blacklist request fails, triggering a scheduled upload which
 // succeeds.
 TEST_F(SuggestionsServiceTest, BlacklistURLFails) {
-  EnableFieldTrial(kFakeSuggestionsURL, kFakeSuggestionsCommonParams,
-                   kFakeBlacklistPath, kFakeBlacklistUrlParam, false);
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
   EXPECT_TRUE(suggestions_service != NULL);
@@ -480,9 +438,6 @@ TEST_F(SuggestionsServiceTest, BlacklistURLFails) {
 }
 
 TEST_F(SuggestionsServiceTest, GetBlacklistedUrl) {
-  EnableFieldTrial(kFakeSuggestionsURL, kFakeSuggestionsCommonParams,
-                   kFakeBlacklistPath, kFakeBlacklistUrlParam, false);
-
   scoped_ptr<GURL> request_url;
   scoped_ptr<net::FakeURLFetcher> fetcher;
   GURL retrieved_url;
@@ -497,8 +452,7 @@ TEST_F(SuggestionsServiceTest, GetBlacklistedUrl) {
   string blacklisted_url = "http://blacklisted.com/a?b=c&d=e";
   string encoded_blacklisted_url =
       "http%3A%2F%2Fblacklisted.com%2Fa%3Fb%3Dc%26d%3De";
-  string blacklist_request_prefix =
-      "https://mysuggestions.com/proto/blacklist?foo=bar&baz=";
+  string blacklist_request_prefix(kSuggestionsBlacklistURLPrefix);
   request_url.reset(
       new GURL(blacklist_request_prefix + encoded_blacklisted_url));
   fetcher.reset();