predictors: Move the configuration of speculative prefetch to field trials.

chrome/browser/predictors/resource_prefetch_common_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <memory>
 #include <string>
 
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "chrome/browser/net/prediction_options.h"
 #include "chrome/browser/predictors/resource_prefetch_common.h"
 #include "chrome/browser/predictors/resource_prefetch_predictor.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/prefs/pref_service.h"
+#include "components/variations/variations_params_manager.h"
 #include "content/public/test/test_browser_thread.h"
 #include "net/base/network_change_notifier.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using chrome_browser_net::NetworkPredictionOptions;
 using net::NetworkChangeNotifier;
 
 namespace {
 
 class MockNetworkChangeNotifierWIFI : public NetworkChangeNotifier {
@@ skipping 15 matching lines @@
 namespace predictors {
 
 class ResourcePrefetchCommonTest : public testing::Test {
  public:
   ResourcePrefetchCommonTest();
 
   void SetPreference(NetworkPredictionOptions value) {
     profile_->GetPrefs()->SetInteger(prefs::kNetworkPredictionOptions, value);
   }
 
-  void SetCommandLineValue(const std::string& value) {
-    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-        switches::kSpeculativeResourcePrefetching, value);
-  }
-
   void TestIsPrefetchEnabledForOrigin(
       const ResourcePrefetchPredictorConfig& config,
       PrefetchOrigin origin) {
     EXPECT_TRUE(config.IsLearningEnabled());
     EXPECT_TRUE(config.IsPrefetchingEnabledForOrigin(profile_.get(), origin));
   }
 
   void TestIsPrefetchLearning(const ResourcePrefetchPredictorConfig& config) {
     EXPECT_TRUE(config.IsLearningEnabled());
     EXPECT_FALSE(config.IsPrefetchingEnabledForOrigin(
@@ skipping 27 matching lines @@
 
   EXPECT_FALSE(config.IsLearningEnabled());
   EXPECT_FALSE(config.IsPrefetchingEnabledForOrigin(profile_.get(),
                                                     PrefetchOrigin::EXTERNAL));
   EXPECT_FALSE(config.IsLearningEnabled());
 
   TestIsDefaultExtraConfig(config);
 }
 
 TEST_F(ResourcePrefetchCommonTest, EnableLearning) {
-  SetCommandLineValue(switches::kSpeculativeResourcePrefetchingLearning);
+  variations::testing::VariationParamsManager params_manager(
+      "dummy-trial", {{kModeParamName, kLearningMode}},
+      {kSpeculativeResourcePrefetchingFeatureName});
+
   ResourcePrefetchPredictorConfig config;
   EXPECT_TRUE(IsSpeculativeResourcePrefetchingEnabled(profile_.get(), &config));
   TestIsPrefetchLearning(config);
   TestIsDefaultExtraConfig(config);
 }
 
 TEST_F(ResourcePrefetchCommonTest, EnablePrefetch) {
-  SetCommandLineValue(switches::kSpeculativeResourcePrefetchingEnabled);
+  variations::testing::VariationParamsManager params_manager(
+      "dummy-trial", {{kModeParamName, kPrefetchingMode}},
+      {kSpeculativeResourcePrefetchingFeatureName});
+
   ResourcePrefetchPredictorConfig config;
   EXPECT_TRUE(IsSpeculativeResourcePrefetchingEnabled(profile_.get(), &config));
   TestIsPrefetchEnabledForOrigin(config, PrefetchOrigin::EXTERNAL);
   TestIsPrefetchEnabledForOrigin(config, PrefetchOrigin::NAVIGATION);
   TestIsDefaultExtraConfig(config);
 }
 
 TEST_F(ResourcePrefetchCommonTest, EnablePrefetchExternalOnly) {
-  SetCommandLineValue(switches::kSpeculativeResourcePrefetchingEnabledExternal);
+  variations::testing::VariationParamsManager params_manager(
+      "dummy-trial", {{kModeParamName, kExternalPrefetchingMode}},
+      {kSpeculativeResourcePrefetchingFeatureName});
+
   ResourcePrefetchPredictorConfig config;
   EXPECT_TRUE(IsSpeculativeResourcePrefetchingEnabled(profile_.get(), &config));
   TestIsPrefetchEnabledForOrigin(config, PrefetchOrigin::EXTERNAL);
   EXPECT_FALSE(config.IsPrefetchingEnabledForOrigin(
       profile_.get(), PrefetchOrigin::NAVIGATION));
   TestIsDefaultExtraConfig(config);
 }
 

[droger, 2017/01/25 17:27:11]

Do we need a test with the feature disabled?

I think you can do something like:

base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(kSpeculativeResourcePrefetchingFeatureName);

[Benoit L, 2017/01/26 09:58:15]

There is 
 TEST_F(ResourcePrefetchCommonTest, IsDisabledByDefault)
above (was folded by the code review tool by default).

 // Verifies whether prefetching is disabled according to the network type. But
 // learning should not be disabled by network.
 TEST_F(ResourcePrefetchCommonTest, RespectsNetworkSettings) {
-  SetCommandLineValue(switches::kSpeculativeResourcePrefetchingEnabled);
+  variations::testing::VariationParamsManager params_manager(
+      "dummy-trial", {{kModeParamName, kPrefetchingMode}},
+      {kSpeculativeResourcePrefetchingFeatureName});
+
   ResourcePrefetchPredictorConfig config;
   EXPECT_TRUE(IsSpeculativeResourcePrefetchingEnabled(profile_.get(), &config));
   TestIsPrefetchEnabledForOrigin(config, PrefetchOrigin::EXTERNAL);
   TestIsDefaultExtraConfig(config);
 
   // Set preference to WIFI_ONLY: prefetch when not on cellular.
   SetPreference(NetworkPredictionOptions::NETWORK_PREDICTION_WIFI_ONLY);
   {
     std::unique_ptr<NetworkChangeNotifier> mock(
         new MockNetworkChangeNotifierWIFI);
@@ skipping 13 matching lines @@
     TestIsPrefetchLearning(config);
   }
   {
     std::unique_ptr<NetworkChangeNotifier> mock(
         new MockNetworkChangeNotifier4G);
     TestIsPrefetchLearning(config);
   }
 }
 
 }  // namespace predictors