Allows cacao hint-based previews to run on http scheme urls

Bug: 1022820
Change-Id: Icada0c443861e2c5a9b66a879abb3c976db2c2e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906311Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713974}

chrome/browser/previews/previews_browsertest.cc

@@ -360,9 +360,8 @@ IN_PROC_BROWSER_TEST_P(
   EXPECT_TRUE(noscript_js_requested());
 }
 
-IN_PROC_BROWSER_TEST_P(
-    PreviewsNoScriptBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(NoScriptPreviewsEnabledButHttpRequest)) {
+IN_PROC_BROWSER_TEST_P(PreviewsNoScriptBrowserTest,
+                       DISABLE_ON_WIN_MAC_CHROMEOS(NoScriptPreviewsForHttp)) {
   GURL url = http_url();
 
   // Whitelist NoScript for http_hint_setup_url() host.
@@ -370,9 +369,9 @@ IN_PROC_BROWSER_TEST_P(
 
   ui_test_utils::NavigateToURL(browser(), url);
 
-  // Verify loaded js resource but not css triggered by noscript tag.
-  EXPECT_TRUE(noscript_js_requested());
-  EXPECT_FALSE(noscript_css_requested());
+  // Verify loaded noscript tag triggered css resource but not js one.
+  EXPECT_TRUE(noscript_css_requested());
+  EXPECT_FALSE(noscript_js_requested());
 }
 
 IN_PROC_BROWSER_TEST_P(PreviewsNoScriptBrowserTest,

chrome/browser/previews/previews_content_util.cc

@@ -401,7 +401,6 @@ content::PreviewsState DetermineCommittedClientPreviewsState(
     content::PreviewsState previews_state,
     const previews::PreviewsDecider* previews_decider,
     content::NavigationHandle* navigation_handle) {
-  bool is_https = url.SchemeIs(url::kHttpsScheme);
 
   // Record whether the hint cache has a matching entry for this committed URL.
   previews_decider->LogHintCacheMatch(url, true /* is_committed */);
@@ -512,10 +511,9 @@ content::PreviewsState DetermineCommittedClientPreviewsState(
   if (previews_state & content::DEFER_ALL_SCRIPT_ON) {
     // DeferAllScript was allowed for the original URL but only continue with it
     // if the committed URL has HTTPS scheme and is allowed by decider.
-    if (is_https && previews_decider &&
-        previews_decider->ShouldCommitPreview(
-            previews_data, navigation_handle,
-            previews::PreviewsType::DEFER_ALL_SCRIPT)) {
+    if (previews_decider && previews_decider->ShouldCommitPreview(
+                                previews_data, navigation_handle,
+                                previews::PreviewsType::DEFER_ALL_SCRIPT)) {
       LogCommittedPreview(previews_data, PreviewsType::DEFER_ALL_SCRIPT);
       return content::DEFER_ALL_SCRIPT_ON;
     }
@@ -527,7 +525,7 @@ content::PreviewsState DetermineCommittedClientPreviewsState(
   if (previews_state & content::RESOURCE_LOADING_HINTS_ON) {
     // Resource loading hints was chosen for the original URL but only continue
     // with it if the committed URL has HTTPS scheme and is allowed by decider.
-    if (is_https && previews_decider &&
+    if (previews_decider &&
         previews_decider->ShouldCommitPreview(
             previews_data, navigation_handle,
             previews::PreviewsType::RESOURCE_LOADING_HINTS)) {
@@ -542,10 +540,9 @@ content::PreviewsState DetermineCommittedClientPreviewsState(
   if (previews_state & content::NOSCRIPT_ON) {
     // NoScript was chosen for the original URL but only continue with it
     // if the committed URL has HTTPS scheme and is allowed by decider.
-    if (is_https && previews_decider &&
-        previews_decider->ShouldCommitPreview(
-            previews_data, navigation_handle,
-            previews::PreviewsType::NOSCRIPT)) {
+    if (previews_decider && previews_decider->ShouldCommitPreview(
+                                previews_data, navigation_handle,
+                                previews::PreviewsType::NOSCRIPT)) {
       LogCommittedPreview(previews_data, PreviewsType::NOSCRIPT);
       return content::NOSCRIPT_ON;
     }

chrome/browser/previews/previews_content_util_unittest.cc

@@ -447,25 +447,26 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsStateForHttp) {
   user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_2G);
   base::HistogramTester histogram_tester;
 
-  // Verify that currently these previews do not commit on HTTP.
-  EXPECT_EQ(content::PREVIEWS_OFF,
+  // Verify that these previews do now commit on HTTP.
+  EXPECT_EQ(content::DEFER_ALL_SCRIPT_ON,
             previews::DetermineCommittedClientPreviewsState(
                 &user_data, GURL("http://www.google.com"),
                 content::NOSCRIPT_ON | content::RESOURCE_LOADING_HINTS_ON |
                     content::DEFER_ALL_SCRIPT_ON,
                 enabled_previews_decider(), nullptr));
   histogram_tester.ExpectTotalCount(
-      "Previews.Triggered.EffectiveConnectionType2", 0);
+      "Previews.Triggered.EffectiveConnectionType2", 1);
 
-  // Ensure one does commit on HTTPS (to confirm test setup).
-  EXPECT_NE(content::PREVIEWS_OFF,
+  EXPECT_EQ(content::RESOURCE_LOADING_HINTS_ON,
             previews::DetermineCommittedClientPreviewsState(
-                &user_data, GURL("https://www.google.com"),
-                content::NOSCRIPT_ON | content::RESOURCE_LOADING_HINTS_ON |
-                    content::DEFER_ALL_SCRIPT_ON,
+                &user_data, GURL("http://www.google.com"),
+                content::RESOURCE_LOADING_HINTS_ON, enabled_previews_decider(),
+                nullptr));
+
+  EXPECT_EQ(content::NOSCRIPT_ON,
+            previews::DetermineCommittedClientPreviewsState(
+                &user_data, GURL("http://www.google.com"), content::NOSCRIPT_ON,
                 enabled_previews_decider(), nullptr));
-  histogram_tester.ExpectTotalCount(
-      "Previews.Triggered.EffectiveConnectionType2", 1);
 }
 
 TEST_F(PreviewsContentUtilTest,

chrome/browser/previews/resource_loading_hints/resource_loading_hints_browsertest.cc

@@ -936,14 +936,15 @@ IN_PROC_BROWSER_TEST_P(
       "ResourceLoadingHints.CountBlockedSubresourcePatterns", 0);
 }
 
-IN_PROC_BROWSER_TEST_P(ResourceLoadingHintsBrowserTest,
-                       DISABLE_ON_WIN_MAC_CHROMEOS(ResourceLoadingHintsHttp)) {
+IN_PROC_BROWSER_TEST_P(
+    ResourceLoadingHintsBrowserTest,
+    DISABLE_ON_WIN_MAC_CHROMEOS(ResourceLoadingHintsForHttp)) {
   GURL url = http_url();
 
   // Whitelist resource loading hints for http_hint_setup_url()'s' host.
   SetDefaultOnlyResourceLoadingHints(http_hint_setup_url());
 
-  SetExpectedFooJpgRequest(true);
+  SetExpectedFooJpgRequest(false);
   SetExpectedBarJpgRequest(true);
 
   base::HistogramTester histogram_tester;
@@ -953,11 +954,7 @@ IN_PROC_BROWSER_TEST_P(ResourceLoadingHintsBrowserTest,
 
   histogram_tester.ExpectBucketCount(
       "Previews.EligibilityReason.ResourceLoadingHints",
-      static_cast<int>(previews::PreviewsEligibilityReason::ALLOWED), 1);
-  histogram_tester.ExpectTotalCount(
-      "Previews.PreviewShown.ResourceLoadingHints", 0);
-  histogram_tester.ExpectTotalCount(
-      "ResourceLoadingHints.CountBlockedSubresourcePatterns", 0);
+      static_cast<int>(previews::PreviewsEligibilityReason::COMMITTED), 1);
 }
 
 IN_PROC_BROWSER_TEST_P(ResourceLoadingHintsBrowserTest,