Add feature param to skip ShouldShowPreview check

This is a simple check to allow us to be able to easily run a limit
study to understand the effectiveness of previews. By limit study, we
mean showing Previews on all page loads for a subset of the population.
The limit study will still utilize the CanApplyPreview logic (e.g.,
hints and blacklists).

Bug: 1031593
Change-Id: I4fd6aec779447615a0660f3d22167091441d4e12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1954710
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722684}

chrome/browser/previews/previews_browsertest.cc

@@ -97,12 +97,6 @@ class PreviewsBrowserTest : public InProcessBrowserTest {
     cmd->AppendSwitch(previews::switches::kIgnorePreviewsBlacklist);
   }
 
-  void TearDown() override {
-    scoped_feature_list_.Reset();
-
-    InProcessBrowserTest::TearDown();
-  }
-
   const GURL& https_url() const { return https_url_; }
   const GURL& https_no_transform_url() const { return https_no_transform_url_; }
   const GURL& https_hint_setup_url() const { return https_hint_setup_url_; }
@@ -160,7 +154,6 @@ class PreviewsBrowserTest : public InProcessBrowserTest {
     return std::move(response);
   }
 
-  base::test::ScopedFeatureList scoped_feature_list_;
   std::unique_ptr<net::EmbeddedTestServer> https_server_;
   std::unique_ptr<net::EmbeddedTestServer> http_server_;
   GURL https_url_;
@@ -192,20 +185,24 @@ IN_PROC_BROWSER_TEST_F(PreviewsBrowserTest, NoScriptPreviewsDisabled) {
   histogram_tester.ExpectTotalCount("Previews.PreviewShown.NoScript", 0);
 }
 
-// This test class enables NoScriptPreviews but without OptimizationHints.
-class PreviewsNoScriptBrowserTest : public PreviewsBrowserTest {
+// This test class enables NoScriptPreviews and with OptimizationHints.
+class PreviewsNoScriptBrowserTest : public ::testing::WithParamInterface<bool>,
+                                    public PreviewsBrowserTest {
  public:
   PreviewsNoScriptBrowserTest() {}
 
   ~PreviewsNoScriptBrowserTest() override {}
 
   void SetUp() override {
-    scoped_feature_list_.InitWithFeatures(
-        {previews::features::kPreviews,
-         optimization_guide::features::kOptimizationHints,
-         previews::features::kNoScriptPreviews,
-         data_reduction_proxy::features::
-             kDataReductionProxyEnabledWithNetworkService},
+    scoped_feature_list_.InitWithFeaturesAndParameters(
+        {{previews::features::kPreviews,
+          {{"override_should_show_preview_check",
+            GetParam() ? "true" : "false"}}},
+         {optimization_guide::features::kOptimizationHints, {}},
+         {previews::features::kNoScriptPreviews, {}},
+         {data_reduction_proxy::features::
+              kDataReductionProxyEnabledWithNetworkService,
+          {}}},
         {});
     PreviewsBrowserTest::SetUp();
   }
@@ -246,14 +243,9 @@ class PreviewsNoScriptBrowserTest : public PreviewsBrowserTest {
         1);
   }
 
-  void TearDown() override {
-    // Make sure to reset the other feature list first, otherwise we hit a
-    // DCHECK where the feature lists aren't reset in the same order they are
-    // set up.
-    PreviewsBrowserTest::TearDown();
-
-    scoped_feature_list_.Reset();
-  }
+  // Returns whether the ShouldShowPreview check should have been overridden for
+  // the test case.
+  bool ShouldOverrideShouldShowPreviewCheck() const { return GetParam(); }
 
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
@@ -261,10 +253,14 @@ class PreviewsNoScriptBrowserTest : public PreviewsBrowserTest {
       test_hints_component_creator_;
 };
 
+INSTANTIATE_TEST_SUITE_P(ShouldSkipPreview,
+                         PreviewsNoScriptBrowserTest,
+                         ::testing::Bool());
+
 // Loads a webpage that has both script and noscript tags and also requests
 // a script resource. Verifies that the noscript tag is evaluated and the
 // script resource is not loaded.
-IN_PROC_BROWSER_TEST_F(PreviewsNoScriptBrowserTest,
+IN_PROC_BROWSER_TEST_P(PreviewsNoScriptBrowserTest,
                        DISABLE_ON_WIN_MAC_CHROMEOS(NoScriptPreviewsEnabled)) {
   GURL url = https_url();
 
@@ -283,7 +279,7 @@ IN_PROC_BROWSER_TEST_F(PreviewsNoScriptBrowserTest,
                                      "Previews.PreviewShown.NoScript", 1);
 }
 
-IN_PROC_BROWSER_TEST_F(
+IN_PROC_BROWSER_TEST_P(
     PreviewsNoScriptBrowserTest,
     DISABLE_ON_WIN_MAC_CHROMEOS(NoScriptPreviewsEnabled_Incognito)) {
   GURL url = https_url();
@@ -303,7 +299,7 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_TRUE(noscript_js_requested());
 }
 
-IN_PROC_BROWSER_TEST_F(PreviewsNoScriptBrowserTest,
+IN_PROC_BROWSER_TEST_P(PreviewsNoScriptBrowserTest,
                        DISABLE_ON_WIN_MAC_CHROMEOS(NoScriptPreviewsForHttp)) {
   GURL url = http_url();
 
@@ -317,7 +313,7 @@ IN_PROC_BROWSER_TEST_F(PreviewsNoScriptBrowserTest,
   EXPECT_FALSE(noscript_js_requested());
 }
 
-IN_PROC_BROWSER_TEST_F(PreviewsNoScriptBrowserTest,
+IN_PROC_BROWSER_TEST_P(PreviewsNoScriptBrowserTest,
                        DISABLE_ON_WIN_MAC_CHROMEOS(
                            NoScriptPreviewsEnabledButNoTransformDirective)) {
   GURL url = https_no_transform_url();
@@ -336,7 +332,7 @@ IN_PROC_BROWSER_TEST_F(PreviewsNoScriptBrowserTest,
       "Previews.CacheControlNoTransform.BlockedPreview", 5 /* NoScript */, 1);
 }
 
-IN_PROC_BROWSER_TEST_F(
+IN_PROC_BROWSER_TEST_P(
     PreviewsNoScriptBrowserTest,
     DISABLE_ON_WIN_MAC_CHROMEOS(NoScriptPreviewsEnabledHttpRedirectToHttps)) {
   GURL url = redirect_url();
@@ -356,7 +352,7 @@ IN_PROC_BROWSER_TEST_F(
                                      "Previews.PreviewShown.NoScript", 1);
 }
 
-IN_PROC_BROWSER_TEST_F(
+IN_PROC_BROWSER_TEST_P(
     PreviewsNoScriptBrowserTest,
     DISABLE_ON_WIN_MAC_CHROMEOS(NoScriptPreviewsRecordsOptOut)) {
   GURL url = redirect_url();
@@ -383,7 +379,7 @@ IN_PROC_BROWSER_TEST_F(
                                      1);
 }
 
-IN_PROC_BROWSER_TEST_F(
+IN_PROC_BROWSER_TEST_P(
     PreviewsNoScriptBrowserTest,
     DISABLE_ON_WIN_MAC_CHROMEOS(NoScriptPreviewsEnabledByWhitelist)) {
   GURL url = https_url();
@@ -398,7 +394,7 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_FALSE(noscript_js_requested());
 }
 
-IN_PROC_BROWSER_TEST_F(
+IN_PROC_BROWSER_TEST_P(
     PreviewsNoScriptBrowserTest,
     DISABLE_ON_WIN_MAC_CHROMEOS(NoScriptPreviewsNotEnabledByWhitelist)) {
   GURL url = https_url();
@@ -412,3 +408,34 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_TRUE(noscript_js_requested());
   EXPECT_FALSE(noscript_css_requested());
 }
+
+IN_PROC_BROWSER_TEST_P(PreviewsNoScriptBrowserTest,
+                       DISABLE_ON_WIN_MAC_CHROMEOS(
+                           NoScriptPreviewsEnabledShouldSkipPreviewCheck)) {
+  // Set ECT to 4G so that the Preview should not be shown in the regular case.
+  g_browser_process->network_quality_tracker()
+      ->ReportEffectiveConnectionTypeForTesting(
+          net::EFFECTIVE_CONNECTION_TYPE_4G);
+
+  GURL url = https_url();
+
+  // Whitelist NoScript for https_hint_setup_url()'s' host.
+  SetUpNoScriptWhitelist(https_hint_setup_url());
+
+  base::HistogramTester histogram_tester;
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  if (ShouldOverrideShouldShowPreviewCheck()) {
+    // Verify loaded noscript tag triggered css resource but not js one.
+    EXPECT_TRUE(noscript_css_requested());
+    EXPECT_FALSE(noscript_js_requested());
+
+    // Verify info bar presented via histogram check.
+    RetryForHistogramUntilCountReached(&histogram_tester,
+                                       "Previews.PreviewShown.NoScript", 1);
+  } else {
+    // Verify loaded js resource but not css triggered by noscript tag.
+    EXPECT_TRUE(noscript_js_requested());
+    EXPECT_FALSE(noscript_css_requested());
+  }
+}

components/previews/content/previews_optimization_guide.cc

@@ -100,6 +100,10 @@ PreviewsOptimizationGuide::~PreviewsOptimizationGuide() = default;
 
 bool PreviewsOptimizationGuide::ShouldShowPreview(
     content::NavigationHandle* navigation_handle) {
+  // See if we should override the optimization guide and always show a preview.
+  if (params::OverrideShouldShowPreviewCheck())
+    return true;
+
   optimization_guide::OptimizationGuideDecision decision =
       optimization_guide_decider_->ShouldTargetNavigation(
           navigation_handle,

components/previews/content/previews_optimization_guide_unittest.cc

@@ -362,6 +362,46 @@ TEST_F(PreviewsOptimizationGuideTest,
       /*previews_data=*/nullptr, &navigation_handle, PreviewsType::NOSCRIPT));
 }
 
+TEST_F(PreviewsOptimizationGuideTest,
+       ShouldShowPreviewWithOverrideFeatureEnabled) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeaturesAndParameters(
+      {{previews::features::kPreviews,
+        {{"override_should_show_preview_check", "true"}}}},
+      {});
+
+  // Set ECT to 4G so that if feature wasn't on then we would return false.
+  optimization_guide_decider()->ReportEffectiveConnectionType(
+      net::EFFECTIVE_CONNECTION_TYPE_4G);
+
+  PreviewsOptimizationGuide guide(optimization_guide_decider());
+
+  content::MockNavigationHandle navigation_handle;
+  navigation_handle.set_url(GURL("doesntmatter"));
+
+  EXPECT_TRUE(guide.ShouldShowPreview(&navigation_handle));
+}
+
+TEST_F(PreviewsOptimizationGuideTest,
+       ShouldShowPreviewWithOverrideFeatureDisabled) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeaturesAndParameters(
+      {{previews::features::kPreviews,
+        {{"override_should_show_preview_check", "false"}}}},
+      {});
+
+  // Set ECT to 4G so that if feature wasn't on then we would return false.
+  optimization_guide_decider()->ReportEffectiveConnectionType(
+      net::EFFECTIVE_CONNECTION_TYPE_4G);
+
+  PreviewsOptimizationGuide guide(optimization_guide_decider());
+
+  content::MockNavigationHandle navigation_handle;
+  navigation_handle.set_url(GURL("doesntmatter"));
+
+  EXPECT_FALSE(guide.ShouldShowPreview(&navigation_handle));
+}
+
 TEST_F(PreviewsOptimizationGuideTest,
        ShouldShowPreviewWithTrueDecisionReturnsTrue) {
   optimization_guide_decider()->ReportEffectiveConnectionType(

components/previews/core/previews_experiments.cc

@@ -553,6 +553,11 @@ bool DetectDeferRedirectLoopsUsingCache() {
                                            true);
 }
 
+bool OverrideShouldShowPreviewCheck() {
+  return base::GetFieldTrialParamByFeatureAsBool(
+      features::kPreviews, "override_should_show_preview_check", false);
+}
+
 }  // namespace params
 
 std::string GetStringNameForType(PreviewsType type) {

components/previews/core/previews_experiments.h

@@ -238,6 +238,10 @@ bool ShouldExcludeMediaSuffix(const GURL& url);
 // preview using a cache is enabled.
 bool DetectDeferRedirectLoopsUsingCache();
 
+// Returns true if the checks to show a preview for the navigation should be
+// overridden.
+bool OverrideShouldShowPreviewCheck();
+
 }  // namespace params
 
 }  // namespace previews

components/previews/core/previews_experiments_unittest.cc

@@ -384,6 +384,15 @@ TEST(PreviewsExperimentsTest, TestDeferAllScriptPreviewsCoinFlipExperiment) {
   EXPECT_FALSE(params::IsLitePageServerPreviewsEnabled());
 }
 
+TEST(PreviewsExperimentsTest, TestOverrideShouldShowPreviewCheck) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeaturesAndParameters(
+      {{features::kPreviews, {{"override_should_show_preview_check", "true"}}}},
+      {});
+
+  EXPECT_TRUE(params::OverrideShouldShowPreviewCheck());
+}
+
 }  // namespace
 
 }  // namespace previews