Prerender: omnibox and instant field trial parameters.

chrome/browser/prerender/prerender_unittest.cc

Index: chrome/browser/prerender/prerender_unittest.cc
diff --git a/chrome/browser/prerender/prerender_unittest.cc b/chrome/browser/prerender/prerender_unittest.cc
index f949fc83c4f325ecbd806c18f20696daf3358455..d889c5b9ff20cf1be2af3451b44fba43eee33782 100644
--- a/chrome/browser/prerender/prerender_unittest.cc
+++ b/chrome/browser/prerender/prerender_unittest.cc
@@ -15,6 +15,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/field_trial_param_associator.h"
+#include "base/metrics/field_trial_params.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
@@ -28,6 +29,7 @@
 #include "chrome/browser/prerender/prerender_link_manager.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_origin.h"
+#include "chrome/browser/prerender/prerender_test_utils.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/prerender_types.h"
@@ -35,7 +37,6 @@
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/common/pref_names.h"
 #include "components/prefs/pref_service.h"
-#include "components/variations/variations_associated_data.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/test/test_browser_thread.h"
 #include "net/base/network_change_notifier.h"
@@ -287,17 +288,6 @@ class UnitTestPrerenderManager : public PrerenderManager {
   bool is_low_end_device_;
 };
 
-class RestorePrerenderMode {
- public:
-  RestorePrerenderMode() : prev_mode_(PrerenderManager::GetMode()) {
-  }
-
-  ~RestorePrerenderMode() { PrerenderManager::SetMode(prev_mode_); }
-
- private:
-  PrerenderManager::PrerenderManagerMode prev_mode_;
-};
-
 class MockNetworkChangeNotifier4G : public net::NetworkChangeNotifier {
  public:
   ConnectionType GetCurrentConnectionType() const override {
@@ -359,6 +349,10 @@ class PrerenderTest : public testing::Test {
     prerender_manager_->Shutdown();
   }
 
+  void TearDown() override {
+    base::FieldTrialParamAssociator::GetInstance()->ClearAllParamsForTesting();
+  }
+
   UnitTestPrerenderManager* prerender_manager() {
     return prerender_manager_.get();
   }
@@ -425,6 +419,33 @@ class PrerenderTest : public testing::Test {
         chrome_browser_net::NETWORK_PREDICTION_ALWAYS);
   }
 
+  void SetUpFieldTrial(const std::map<std::string, std::string>& params,
+                       base::test::ScopedFeatureList* scoped_feature_list) {
+    // Set up the prerender mode through a field trial.
+    std::string kTrialName = "name";
+    std::string kTrialGroup = "group";
+    base::FieldTrial* trial =
+        base::FieldTrialList::CreateFieldTrial(kTrialName, kTrialGroup);
+    ASSERT_TRUE(

[pasko, 2017/03/17 17:33:33]

this would return from the function if the condition does not hold, do we really
want it? do we care?

[mattcary, 2017/03/20 10:20:31]

This returns false if, for example, one tries to associate multiple params with
a single trial group. This is why I had to add the tearDown() method. It was a
lot easier seeing this assert fail than to have the params for a test not get
applied and have a tricky test failure to debug.

[pasko, 2017/03/20 13:24:55]

Let me rephrase: ASSERT_* returns from the subroutine, we should use
ASSERT_NO_FATAL_FAILURE with them if we want exception-like propagation of
assertions in tests.

See:
https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid...

+        base::FieldTrialParamAssociator::GetInstance()
+            ->AssociateFieldTrialParams(kTrialName, kTrialGroup, params));
+
+    std::unique_ptr<base::FeatureList> feature_list =
+        base::MakeUnique<base::FeatureList>();
+    feature_list->RegisterFieldTrialOverride(
+        kNoStatePrefetchFeature.name,
+        base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial);
+
+    scoped_feature_list->InitWithFeatureList(std::move(feature_list));
+
+    EXPECT_EQ(base::FeatureList::GetFieldTrial(kNoStatePrefetchFeature), trial);

[pasko, 2017/03/17 17:33:33]

This is a utility function, not a test, I think it would be more readable to
perform these EXPECT_* checks in one of the tests
(PrerenderRespectsFieldTrialParameters?) and assume it always works.

[mattcary, 2017/03/20 10:20:31]

Changed to an assert, see previous comment.

+
+    std::map<std::string, std::string> actual_params;
+    EXPECT_TRUE(base::GetFieldTrialParamsByFeature(kNoStatePrefetchFeature,
+                                                   &actual_params));
+    EXPECT_EQ(params, actual_params);
+  }
+
   const base::HistogramTester& histogram_tester() { return histogram_tester_; }
 
  private:
@@ -436,66 +457,157 @@ class PrerenderTest : public testing::Test {
   std::unique_ptr<UnitTestPrerenderManager> prerender_manager_;
   std::unique_ptr<PrerenderLinkManager> prerender_link_manager_;
   int last_prerender_id_;
-  base::FieldTrialList field_trial_list_;
   base::HistogramTester histogram_tester_;
+
+  // An instance of base::FieldTrialList is necessary in order to initialize
+  // global state.
+  base::FieldTrialList field_trial_list_;
 };
 
 TEST_F(PrerenderTest, PrerenderRespectsDisableFlag) {
   RestorePrerenderMode restore_prerender_mode;
-  ASSERT_TRUE(PrerenderManager::IsPrerenderingPossible());
+  ASSERT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
   ASSERT_EQ(PrerenderManager::PRERENDER_MODE_ENABLED,
-            PrerenderManager::GetMode());
+            PrerenderManager::GetMode(ORIGIN_NONE));
 
   {
     base::test::ScopedFeatureList scoped_feature_list;
     scoped_feature_list.InitAndDisableFeature(kNoStatePrefetchFeature);
     prerender::ConfigurePrerender();
+    EXPECT_FALSE(PrerenderManager::IsAnyPrerenderingPossible());
+    EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
+              PrerenderManager::GetMode(ORIGIN_NONE));
     EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
-              PrerenderManager::GetMode());
-    EXPECT_FALSE(PrerenderManager::IsPrerenderingPossible());
+              PrerenderManager::GetMode(ORIGIN_OMNIBOX));
+    EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
+              PrerenderManager::GetMode(ORIGIN_INSTANT));
   }
 }
 
 TEST_F(PrerenderTest, PrerenderRespectsFieldTrialParameters) {
   RestorePrerenderMode restore_prerender_mode;
 
-  // Set up the prerender mode through a field trial.
-  std::string kTrialName = "name";
-  std::string kTrialGroup = "group";
-  base::FieldTrial* trial =
-      base::FieldTrialList::CreateFieldTrial(kTrialName, kTrialGroup);
-  std::map<std::string, std::string> params = {
-      {kNoStatePrefetchFeatureModeParameterName,
-       kNoStatePrefetchFeatureModeParameterSimpleLoad}};
-  ASSERT_TRUE(
-      variations::AssociateVariationParams(kTrialName, kTrialGroup, params));
-
-  std::unique_ptr<base::FeatureList> feature_list =
-      base::MakeUnique<base::FeatureList>();
-  feature_list->RegisterFieldTrialOverride(
-      kNoStatePrefetchFeature.name, base::FeatureList::OVERRIDE_ENABLE_FEATURE,
-      trial);
+  base::test::ScopedFeatureList scoped_feature_list;
+  SetUpFieldTrial({{kNoStatePrefetchFeatureModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterSimpleLoad}},
+                  &scoped_feature_list);
+
+  prerender::ConfigurePrerender();
+  EXPECT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_SIMPLE_LOAD_EXPERIMENT,
+            PrerenderManager::GetMode(ORIGIN_NONE));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_SIMPLE_LOAD_EXPERIMENT,
+            PrerenderManager::GetMode(ORIGIN_OMNIBOX));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_SIMPLE_LOAD_EXPERIMENT,
+            PrerenderManager::GetMode(ORIGIN_INSTANT));
+}
+
+TEST_F(PrerenderTest, PrerenderRespectsFieldTrialParametersPrefetchInstant) {
+  RestorePrerenderMode restore_prerender_mode;
 
   base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitWithFeatureList(std::move(feature_list));
+  SetUpFieldTrial({{kNoStatePrefetchFeatureModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterSimpleLoad},
+                   {kNoStatePrefetchFeatureInstantModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterPrefetch}},
+                  &scoped_feature_list);
+
+  prerender::ConfigurePrerender();
+  EXPECT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_SIMPLE_LOAD_EXPERIMENT,
+            PrerenderManager::GetMode(ORIGIN_NONE));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_SIMPLE_LOAD_EXPERIMENT,
+            PrerenderManager::GetMode(ORIGIN_OMNIBOX));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_NOSTATE_PREFETCH,
+            PrerenderManager::GetMode(ORIGIN_INSTANT));
+}
 
-  EXPECT_EQ(base::FeatureList::GetFieldTrial(kNoStatePrefetchFeature), trial);
+TEST_F(PrerenderTest, PrerenderRespectsFieldTrialParametersPrefetchOmnibox) {
+  RestorePrerenderMode restore_prerender_mode;
 
-  std::map<std::string, std::string> actual_params;
-  EXPECT_TRUE(variations::GetVariationParamsByFeature(kNoStatePrefetchFeature,
-                                                      &actual_params));
-  EXPECT_EQ(params, actual_params);
+  base::test::ScopedFeatureList scoped_feature_list;
+  SetUpFieldTrial({{kNoStatePrefetchFeatureModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterSimpleLoad},
+                   {kNoStatePrefetchFeatureOmniboxModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterPrefetch}},
+                  &scoped_feature_list);
 
   prerender::ConfigurePrerender();
-  EXPECT_TRUE(PrerenderManager::IsPrerenderingPossible());
+  EXPECT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
   EXPECT_EQ(PrerenderManager::PRERENDER_MODE_SIMPLE_LOAD_EXPERIMENT,
-            PrerenderManager::GetMode());
+            PrerenderManager::GetMode(ORIGIN_NONE));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_NOSTATE_PREFETCH,
+            PrerenderManager::GetMode(ORIGIN_OMNIBOX));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_SIMPLE_LOAD_EXPERIMENT,
+            PrerenderManager::GetMode(ORIGIN_INSTANT));
+}
+
+TEST_F(PrerenderTest, PrerenderRespectsFieldTrialParametersNoneButInstant) {
+  RestorePrerenderMode restore_prerender_mode;
+
+  base::test::ScopedFeatureList scoped_feature_list;
+  SetUpFieldTrial({{kNoStatePrefetchFeatureModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterDisabled},
+                   {kNoStatePrefetchFeatureInstantModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterPrerender},
+                   {kNoStatePrefetchFeatureOmniboxModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterDisabled}},
+                  &scoped_feature_list);
+
+  prerender::ConfigurePrerender();
+  EXPECT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
+            PrerenderManager::GetMode(ORIGIN_NONE));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
+            PrerenderManager::GetMode(ORIGIN_OMNIBOX));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_ENABLED,
+            PrerenderManager::GetMode(ORIGIN_INSTANT));
+}
+
+TEST_F(PrerenderTest, PrerenderRespectsFieldTrialParametersNoneAtAll) {
+  RestorePrerenderMode restore_prerender_mode;
+
+  base::test::ScopedFeatureList scoped_feature_list;
+  SetUpFieldTrial({{kNoStatePrefetchFeatureModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterDisabled},
+                   {kNoStatePrefetchFeatureOmniboxModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterDisabled},
+                   {kNoStatePrefetchFeatureInstantModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterDisabled}},
+                  &scoped_feature_list);
+
+  prerender::ConfigurePrerender();
+  EXPECT_FALSE(PrerenderManager::IsAnyPrerenderingPossible());
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
+            PrerenderManager::GetMode(ORIGIN_NONE));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
+            PrerenderManager::GetMode(ORIGIN_OMNIBOX));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
+            PrerenderManager::GetMode(ORIGIN_INSTANT));
+}
+
+TEST_F(PrerenderTest, PrerenderRespectsFieldTrialParametersDefaultNone) {
+  RestorePrerenderMode restore_prerender_mode;
+
+  base::test::ScopedFeatureList scoped_feature_list;
+  SetUpFieldTrial({{kNoStatePrefetchFeatureModeParameterName,
+                    kNoStatePrefetchFeatureModeParameterDisabled}},
+                  &scoped_feature_list);
+
+  prerender::ConfigurePrerender();
+  EXPECT_FALSE(PrerenderManager::IsAnyPrerenderingPossible());
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
+            PrerenderManager::GetMode(ORIGIN_NONE));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
+            PrerenderManager::GetMode(ORIGIN_OMNIBOX));
+  EXPECT_EQ(PrerenderManager::PRERENDER_MODE_DISABLED,
+            PrerenderManager::GetMode(ORIGIN_INSTANT));
 }
 
 TEST_F(PrerenderTest, PrerenderRespectsThirdPartyCookiesPref) {
   GURL url("http://www.google.com/");
   RestorePrerenderMode restore_prerender_mode;
-  ASSERT_TRUE(PrerenderManager::IsPrerenderingPossible());
+  ASSERT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
 
   profile()->GetPrefs()->SetBoolean(prefs::kBlockThirdPartyCookies, true);
   EXPECT_FALSE(AddSimplePrerender(url));
@@ -506,7 +618,7 @@ TEST_F(PrerenderTest, PrerenderRespectsThirdPartyCookiesPref) {
 TEST_F(PrerenderTest, OfflinePrerenderIgnoresThirdPartyCookiesPref) {
   GURL url("http://www.google.com/");
   RestorePrerenderMode restore_prerender_mode;
-  ASSERT_TRUE(PrerenderManager::IsPrerenderingPossible());
+  ASSERT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
 
   profile()->GetPrefs()->SetBoolean(prefs::kBlockThirdPartyCookies, true);
   DummyPrerenderContents* prerender_contents =
@@ -528,29 +640,39 @@ TEST_F(PrerenderTest, PrerenderModePerOrigin) {
 
   prerender_manager()->SetMode(
       PrerenderManager::PRERENDER_MODE_NOSTATE_PREFETCH);
-  EXPECT_TRUE(PrerenderManager::IsPrerenderingPossible());
+  prerender_manager()->SetOmniboxMode(
+      PrerenderManager::PRERENDER_MODE_NOSTATE_PREFETCH);
+  EXPECT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
   EXPECT_FALSE(PrerenderManager::IsNoStatePrefetch(ORIGIN_OFFLINE));
   EXPECT_TRUE(PrerenderManager::IsNoStatePrefetch(ORIGIN_OMNIBOX));
+  EXPECT_FALSE(PrerenderManager::IsNoStatePrefetch(ORIGIN_INSTANT));
   EXPECT_FALSE(PrerenderManager::IsSimpleLoadExperiment(ORIGIN_OFFLINE));
   EXPECT_FALSE(PrerenderManager::IsSimpleLoadExperiment(ORIGIN_OMNIBOX));
 
   prerender_manager()->SetMode(PrerenderManager::PRERENDER_MODE_ENABLED);
-  EXPECT_TRUE(PrerenderManager::IsPrerenderingPossible());
+  EXPECT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
   EXPECT_FALSE(PrerenderManager::IsNoStatePrefetch(ORIGIN_OFFLINE));
-  EXPECT_FALSE(PrerenderManager::IsNoStatePrefetch(ORIGIN_OMNIBOX));
+  EXPECT_TRUE(PrerenderManager::IsNoStatePrefetch(ORIGIN_OMNIBOX));
   EXPECT_FALSE(PrerenderManager::IsSimpleLoadExperiment(ORIGIN_OFFLINE));
   EXPECT_FALSE(PrerenderManager::IsSimpleLoadExperiment(ORIGIN_OMNIBOX));
 
   prerender_manager()->SetMode(PrerenderManager::PRERENDER_MODE_DISABLED);
-  EXPECT_FALSE(PrerenderManager::IsPrerenderingPossible());
+  prerender_manager()->SetInstantMode(
+      PrerenderManager::PRERENDER_MODE_DISABLED);
+  prerender_manager()->SetOmniboxMode(
+      PrerenderManager::PRERENDER_MODE_DISABLED);
+  EXPECT_FALSE(PrerenderManager::IsAnyPrerenderingPossible());
 
   prerender_manager()->SetMode(
       PrerenderManager::PRERENDER_MODE_SIMPLE_LOAD_EXPERIMENT);
-  EXPECT_TRUE(PrerenderManager::IsPrerenderingPossible());
+  prerender_manager()->SetInstantMode(
+      PrerenderManager::PRERENDER_MODE_SIMPLE_LOAD_EXPERIMENT);
+  EXPECT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
   EXPECT_FALSE(PrerenderManager::IsNoStatePrefetch(ORIGIN_OFFLINE));
   EXPECT_FALSE(PrerenderManager::IsNoStatePrefetch(ORIGIN_OMNIBOX));
   EXPECT_FALSE(PrerenderManager::IsSimpleLoadExperiment(ORIGIN_OFFLINE));
-  EXPECT_TRUE(PrerenderManager::IsSimpleLoadExperiment(ORIGIN_OMNIBOX));
+  EXPECT_FALSE(PrerenderManager::IsSimpleLoadExperiment(ORIGIN_OMNIBOX));
+  EXPECT_TRUE(PrerenderManager::IsSimpleLoadExperiment(ORIGIN_INSTANT));
 }
 
 TEST_F(PrerenderTest, PrerenderRespectsPrerenderModeNoStatePrefetch) {
@@ -585,7 +707,7 @@ TEST_F(PrerenderTest, OfflinePrerenderIgnoresPrerenderMode) {
   prerender_manager()->SetMode(
       PrerenderManager::PRERENDER_MODE_NOSTATE_PREFETCH);
 
-  ASSERT_TRUE(PrerenderManager::IsPrerenderingPossible());
+  ASSERT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
 
   DummyPrerenderContents* prerender_contents =
       prerender_manager()->CreateNextPrerenderContents(
@@ -597,7 +719,7 @@ TEST_F(PrerenderTest, OfflinePrerenderIgnoresPrerenderMode) {
 
 TEST_F(PrerenderTest, PrerenderDisabledOnLowEndDevice) {
   GURL url("http://www.google.com/");
-  ASSERT_TRUE(PrerenderManager::IsPrerenderingPossible());
+  ASSERT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
   prerender_manager()->SetIsLowEndDevice(true);
   EXPECT_FALSE(AddSimplePrerender(url));
   histogram_tester().ExpectUniqueSample("Prerender.FinalStatus",
@@ -606,7 +728,7 @@ TEST_F(PrerenderTest, PrerenderDisabledOnLowEndDevice) {
 
 TEST_F(PrerenderTest, OfflinePrerenderPossibleOnLowEndDevice) {
   GURL url("http://www.google.com/");
-  ASSERT_TRUE(PrerenderManager::IsPrerenderingPossible());
+  ASSERT_TRUE(PrerenderManager::IsAnyPrerenderingPossible());
 
   prerender_manager()->SetIsLowEndDevice(true);
 
@@ -852,6 +974,8 @@ TEST_F(PrerenderTest, NoStatePrefetchDuplicate) {
   RestorePrerenderMode restore_prerender_mode;
   prerender_manager()->SetMode(
       PrerenderManager::PRERENDER_MODE_NOSTATE_PREFETCH);
+  prerender_manager()->SetOmniboxMode(
+      PrerenderManager::PRERENDER_MODE_NOSTATE_PREFETCH);
   base::SimpleTestTickClock* tick_clock =
       OverridePrerenderManagerTimeTicks(prerender_manager());