Previews: Check for data saver state at navigation start

This CL makes the data saver checks more resilient.

Moving the checks to the very beginning at the navigation start also
prevents recording of some of the Previews histograms for
non-DRP users.

Change-Id: I26c96d654180a2202eef5f0899c89c37bf4c6da5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1716121Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682836}

chrome/browser/chrome_content_browser_client.cc

@@ -5476,7 +5476,9 @@ ChromeContentBrowserClient::DetermineAllowedPreviewsWithoutHoldback(
   // If the profile does not support previews or Data Saver, do not turn on
   // Previews.
   if (!previews_service || !previews_service->previews_ui_service() ||
-      !data_reduction_proxy_settings) {
+      !data_reduction_proxy_settings || !browser_context ||
+      !data_reduction_proxy_settings->IsDataSaverEnabledByUser(
+          Profile::FromBrowserContext(browser_context)->GetPrefs())) {
     return content::PREVIEWS_OFF;
   }
 
@@ -5540,6 +5542,8 @@ ChromeContentBrowserClient::DetermineAllowedPreviewsWithoutHoldback(
     previews_state |=
         (previews_data->AllowedPreviewsState() & server_previews_enabled_state);
   } else {
+    DCHECK(data_reduction_proxy_settings->IsDataSaverEnabledByUser(
+        Profile::FromBrowserContext(browser_context)->GetPrefs()));
     if (previews_decider_impl->ShouldAllowPreviewAtNavigationStart(
             previews_data, current_navigation_url, is_reload,
             previews::PreviewsType::LITE_PAGE)) {
@@ -5548,10 +5552,11 @@ ChromeContentBrowserClient::DetermineAllowedPreviewsWithoutHoldback(
   }
 
   // Evaluate Offline, NoScript, and ResourceBlocking previews.
+  DCHECK(data_reduction_proxy_settings->IsDataSaverEnabledByUser(
+      Profile::FromBrowserContext(browser_context)->GetPrefs()));
   previews_state |= previews::DetermineAllowedClientPreviewsState(
       previews_data, previews_triggering_logic_already_ran,
-      data_reduction_proxy_settings->IsDataReductionProxyEnabled(),
-      previews_decider_impl, navigation_handle);
+      true /* is_data_saver_user */, previews_decider_impl, navigation_handle);
 
   if (previews_state & content::PREVIEWS_OFF) {
     previews_data->set_allowed_previews_state(content::PREVIEWS_OFF);

chrome/browser/previews/defer_all_script_browsertest.cc

@@ -23,6 +23,7 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_features.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h"
 #include "components/optimization_guide/hints_component_info.h"
 #include "components/optimization_guide/optimization_guide_features.h"
 #include "components/optimization_guide/optimization_guide_service.h"
@@ -66,6 +67,11 @@ void RetryForHistogramUntilCountReached(base::HistogramTester* histogram_tester,
   }
 }
 
+constexpr char kExpectedDeferScriptLogOrder[] =
+    "ScriptLog:_BodyEnd_InlineScript_SyncScript_DeveloperDeferScript_OnLoad";
+constexpr char kExpectedNormalScriptLogOrder[] =
+    "ScriptLog:_InlineScript_SyncScript_BodyEnd_DeveloperDeferScript_OnLoad";
+
 }  // namespace
 
 class DeferAllScriptBrowserTest : public InProcessBrowserTest {
@@ -101,7 +107,8 @@ class DeferAllScriptBrowserTest : public InProcessBrowserTest {
   }
 
   void SetUpCommandLine(base::CommandLine* cmd) override {
-    cmd->AppendSwitch("enable-spdy-proxy-auth");
+    cmd->AppendSwitch(
+        data_reduction_proxy::switches::kEnableDataReductionProxy);
 
     cmd->AppendSwitch("optimization-guide-disable-installer");
     cmd->AppendSwitch("purge_hint_cache_store");
@@ -214,9 +221,7 @@ IN_PROC_BROWSER_TEST_F(
       &histogram_tester, "PageLoad.DocumentTiming.NavigationToLoadEventFired",
       1);
 
-  EXPECT_EQ(
-      "ScriptLog:_BodyEnd_InlineScript_SyncScript_DeveloperDeferScript_OnLoad",
-      GetScriptLog());
+  EXPECT_EQ(kExpectedDeferScriptLogOrder, GetScriptLog());
 
   histogram_tester.ExpectBucketCount(
       "Previews.EligibilityReason.DeferAllScript",
@@ -259,9 +264,7 @@ IN_PROC_BROWSER_TEST_F(
       &histogram_tester, "PageLoad.DocumentTiming.NavigationToLoadEventFired",
       1);
 
-  EXPECT_EQ(
-      "ScriptLog:_InlineScript_SyncScript_BodyEnd_DeveloperDeferScript_OnLoad",
-      GetScriptLog());
+  EXPECT_EQ(kExpectedNormalScriptLogOrder, GetScriptLog());
 
   histogram_tester.ExpectBucketCount(
       "Previews.EligibilityReason.DeferAllScript",
@@ -298,9 +301,7 @@ IN_PROC_BROWSER_TEST_F(
       &histogram_tester, "PageLoad.DocumentTiming.NavigationToLoadEventFired",
       1);
 
-  EXPECT_EQ(
-      "ScriptLog:_InlineScript_SyncScript_BodyEnd_DeveloperDeferScript_OnLoad",
-      GetScriptLog());
+  EXPECT_EQ(kExpectedNormalScriptLogOrder, GetScriptLog());
 
   histogram_tester.ExpectBucketCount(
       "Previews.EligibilityReason.DeferAllScript",
@@ -319,3 +320,39 @@ IN_PROC_BROWSER_TEST_F(
   test_ukm_recorder.ExpectEntryMetric(entry, UkmEntry::kdefer_all_scriptName,
                                       true);
 }
+
+IN_PROC_BROWSER_TEST_F(
+    DeferAllScriptBrowserTest,
+    DISABLE_ON_WIN_MAC_CHROMESOS(
+        DeferAllScriptHttpsWhitelisted_DataSaver_Enabled_Disabled)) {
+  GURL url = https_url();
+
+  // Whitelist DeferAllScript for any path for the url's host.
+  SetDeferAllScriptHintWithPageWithPattern(url, "*");
+
+  base::HistogramTester histogram_tester;
+
+  // Navigate to |url| again. Verify that the defer triggered.
+  ui_test_utils::NavigateToURL(browser(), url);
+  RetryForHistogramUntilCountReached(
+      &histogram_tester, "PageLoad.DocumentTiming.NavigationToLoadEventFired",
+      1);
+  EXPECT_EQ(kExpectedDeferScriptLogOrder, GetScriptLog());
+
+  // Navigate to |url| again. Verify that the defer triggered.
+  ui_test_utils::NavigateToURL(browser(), url);
+  RetryForHistogramUntilCountReached(
+      &histogram_tester, "PageLoad.DocumentTiming.NavigationToLoadEventFired",
+      2);
+  EXPECT_EQ(kExpectedDeferScriptLogOrder, GetScriptLog());
+
+  // Disable data saver and navigate to |url| again. Verify that the defer is
+  // not triggered.
+  base::CommandLine::ForCurrentProcess()->RemoveSwitch(
+      data_reduction_proxy::switches::kEnableDataReductionProxy);
+  ui_test_utils::NavigateToURL(browser(), url);
+  RetryForHistogramUntilCountReached(
+      &histogram_tester, "PageLoad.DocumentTiming.NavigationToLoadEventFired",
+      3);
+  EXPECT_EQ(kExpectedNormalScriptLogOrder, GetScriptLog());
+}

chrome/browser/previews/previews_content_util.cc

@@ -217,6 +217,8 @@ content::PreviewsState DetermineAllowedClientPreviewsState(
     bool is_data_saver_user,
     previews::PreviewsDecider* previews_decider,
     content::NavigationHandle* navigation_handle) {
+  DCHECK(is_data_saver_user);
+
   content::PreviewsState previews_state = content::PREVIEWS_UNSPECIFIED;
 
   const GURL& url = navigation_handle->GetURL();

chrome/browser/previews/previews_content_util_unittest.cc

@@ -171,11 +171,11 @@ TEST_F(PreviewsContentUtilTest,
                 previews_triggering_logic_already_ran, is_data_saver_user,
                 enabled_previews_decider(), nullptr));
   is_data_saver_user = false;
-  EXPECT_EQ(content::PREVIEWS_UNSPECIFIED,
-            previews::CallDetermineAllowedClientPreviewsState(
-                &user_data, GURL("http://www.google.com"), is_reload,
-                previews_triggering_logic_already_ran, is_data_saver_user,
-                enabled_previews_decider(), nullptr));
+  EXPECT_DEATH(previews::CallDetermineAllowedClientPreviewsState(
+                   &user_data, GURL("http://www.google.com"), is_reload,
+                   previews_triggering_logic_already_ran, is_data_saver_user,
+                   enabled_previews_decider(), nullptr),
+               "Check failed");
 }
 
 TEST_F(PreviewsContentUtilTest,

chrome/browser/previews/previews_lite_page_browsertest.cc

@@ -447,6 +447,9 @@ class PreviewsLitePageServerBrowserTest
 
     previews::PreviewsUserData* previews_data =
         ui_tab_helper->previews_user_data();
+    // If |previews_data| is null, then no preview was shown.
+    if (!previews_data)
+      return;
     EXPECT_FALSE(previews_data->HasCommittedPreviewsType());
     EXPECT_NE(previews_data->CommittedPreviewsType(),
               previews::PreviewsType::LITE_PAGE_REDIRECT);