prerender: Reports the status of PreloadingHoldback to Devtools.

This CL moves PreloadingHoldback to common features namespace, so that content/ has access to the feature, and reports the status of PreloadingHoldback to Devtools to show the information to the developers. Bug: 1391411 Change-Id: Iff1b0c38b4e8046364eebcd69b766c2e8fca4336 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4113084 Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Commit-Queue: Huanpo Lin <robertlin@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org> Cr-Commit-Position: refs/heads/main@{#1084840}
chromium · Dec 19, 2022 · 6a1e41a · 6a1e41a
1 parent 8e25fd7
commit 6a1e41a
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 16 deletions.
diff --git a/chrome/browser/prefetch/prefetch_prefs.cc b/chrome/browser/prefetch/prefetch_prefs.cc
@@ -9,13 +9,9 @@
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/preloading.h"
+#include "content/public/common/content_features.h"
 
 namespace prefetch {
-
-BASE_FEATURE(kPreloadingHoldback,
-             "PreloadingHoldback",
-             base::FEATURE_DISABLED_BY_DEFAULT);
-
 void RegisterPredictionOptionsProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterIntegerPref(
@@ -68,7 +64,7 @@ content::PreloadingEligibility IsSomePreloadingEnabled(
   // Override kPreloadingHoldback when DevTools is opened to mitigate the cases
   // in which developers are affected by kPreloadingHoldback.
   if (!(web_contents && content::DevToolsAgentHost::HasFor(web_contents)) &&
-      base::FeatureList::IsEnabled(kPreloadingHoldback)) {
+      base::FeatureList::IsEnabled(features::kPreloadingHoldback)) {
     return content::PreloadingEligibility::kPreloadingDisabled;
   }
   return IsSomePreloadingEnabledIgnoringFinch(prefs);

diff --git a/chrome/browser/prefetch/prefetch_prefs.h b/chrome/browser/prefetch/prefetch_prefs.h
@@ -19,8 +19,6 @@ class PrefRegistrySyncable;
 class PrefService;
 
 namespace prefetch {
-BASE_DECLARE_FEATURE(kPreloadingHoldback);
-
 // Enum describing when to allow network predictions.  The numerical value is
 // stored in the prefs file, therefore the same enum with the same order must be
 // used by the platform-dependent components.

diff --git a/chrome/browser/prefetch/prefetch_prefs_unittest.cc b/chrome/browser/prefetch/prefetch_prefs_unittest.cc
@@ -10,6 +10,7 @@
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
+#include "content/public/common/content_features.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 TEST(PrefetchPrefsTest, GetPreloadPagesState) {
@@ -105,7 +106,7 @@ TEST(PrefetchPrefsTest, IsSomePreloadingEnabled) {
 
 TEST(PrefetchPrefsTest, IsSomePreloadingEnabled_PreloadingHoldback) {
   base::test::ScopedFeatureList features;
-  features.InitAndEnableFeature(prefetch::kPreloadingHoldback);
+  features.InitAndEnableFeature(features::kPreloadingHoldback);
   TestingPrefServiceSimple prefs;
   prefs.registry()->RegisterIntegerPref(
       prefs::kNetworkPredictionOptions,
@@ -139,7 +140,7 @@ TEST(PrefetchPrefsTest, IsSomePreloadingEnabled_PreloadingHoldback) {
 
 TEST(PrefetchPrefsTest, IsSomePreloadingEnabledIgnoringFinch) {
   base::test::ScopedFeatureList features;
-  features.InitAndEnableFeature(prefetch::kPreloadingHoldback);
+  features.InitAndEnableFeature(features::kPreloadingHoldback);
   TestingPrefServiceSimple prefs;
   prefs.registry()->RegisterIntegerPref(
       prefs::kNetworkPredictionOptions,

diff --git a/chrome/browser/preloading/prerender/prerender_browsertest.cc b/chrome/browser/preloading/prerender/prerender_browsertest.cc
@@ -16,6 +16,7 @@
 #include "chrome/test/base/chrome_test_utils.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/devtools_agent_host.h"
+#include "content/public/common/content_features.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/prerender_test_util.h"
@@ -79,7 +80,7 @@ class PrerenderBrowserTest : public PlatformBrowserTest {
 class PrerenderHoldbackBrowserTest : public PrerenderBrowserTest {
  public:
   PrerenderHoldbackBrowserTest() {
-    feature_list_.InitAndEnableFeature(prefetch::kPreloadingHoldback);
+    feature_list_.InitAndEnableFeature(features::kPreloadingHoldback);
   }
 
  private:

diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -254,6 +254,17 @@ class PrerenderHoldbackDevToolsProtocolTest
   base::test::ScopedFeatureList feature_list_;
 };
 
+class PreloadingHoldbackDevToolsProtocolTest
+    : public PrerenderDevToolsProtocolTest {
+ public:
+  PreloadingHoldbackDevToolsProtocolTest() {
+    feature_list_.InitAndEnableFeature(features::kPreloadingHoldback);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
 class MultiplePrerendersDevToolsProtocolTest
     : public PrerenderDevToolsProtocolTest {
  public:
@@ -3787,10 +3798,16 @@ IN_PROC_BROWSER_TEST_F(PrerenderDevToolsProtocolTest,
 IN_PROC_BROWSER_TEST_F(PrerenderDevToolsProtocolTest,
                        CheckReportedPrerenderFeatures) {
   AttachToBrowserTarget();
-  base::Value::Dict params;
-  params.Set("featureState", "PrerenderHoldback");
-  const base::Value::Dict* result =
-      SendCommand("SystemInfo.getFeatureState", std::move(params));
+  base::Value::Dict paramsPrerenderHoldback;
+  paramsPrerenderHoldback.Set("featureState", "PrerenderHoldback");
+  const base::Value::Dict* result = SendCommand(
+      "SystemInfo.getFeatureState", std::move(paramsPrerenderHoldback));
+  EXPECT_THAT(result->FindBool("featureEnabled"), false);
+
+  base::Value::Dict paramsPreloadingHolback;
+  paramsPreloadingHolback.Set("featureState", "PreloadingHoldback");
+  result = SendCommand("SystemInfo.getFeatureState",
+                       std::move(paramsPreloadingHolback));
   EXPECT_THAT(result->FindBool("featureEnabled"), false);
 }
 
@@ -3804,6 +3821,16 @@ IN_PROC_BROWSER_TEST_F(PrerenderHoldbackDevToolsProtocolTest,
   EXPECT_THAT(result->FindBool("featureEnabled"), true);
 }
 
+IN_PROC_BROWSER_TEST_F(PreloadingHoldbackDevToolsProtocolTest,
+                       CheckReportedPreloadingFeatures) {
+  AttachToBrowserTarget();
+  base::Value::Dict params;
+  params.Set("featureState", "PreloadingHoldback");
+  const base::Value::Dict* result =
+      SendCommand("SystemInfo.getFeatureState", std::move(params));
+  EXPECT_THAT(result->FindBool("featureEnabled"), true);
+}
+
 IN_PROC_BROWSER_TEST_F(PrerenderDevToolsProtocolTest,
                        RemoveStoredPrerenderActivationIfNavigateAway) {
   base::HistogramTester histogram_tester;

diff --git a/content/browser/devtools/protocol/system_info_handler.cc b/content/browser/devtools/protocol/system_info_handler.cc
@@ -432,9 +432,16 @@ Response SystemInfoHandler::GetFeatureState(const String& in_featureState,
                                             bool* featureEnabled) {
   if (in_featureState == "PrerenderHoldback") {
     *featureEnabled =
-        std::move(base::FeatureList::IsEnabled(features::kPrerender2Holdback));
+        base::FeatureList::IsEnabled(features::kPrerender2Holdback);
     return Response::Success();
   }
+
+  if (in_featureState == "PreloadingHoldback") {
+    *featureEnabled =
+        base::FeatureList::IsEnabled(features::kPreloadingHoldback);
+    return Response::Success();
+  }
+
   return Response::InvalidParams("Unknown feature");
 }
 

diff --git a/content/public/common/content_features.cc b/content/public/common/content_features.cc
@@ -820,6 +820,16 @@ BASE_FEATURE(kPrerender2Holdback,
              "Prerender2Holdback",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
+// Preloading holdback feature disables preloading (e.g., preconnect, prefetch,
+// and prerender) on all predictors. This is useful in comparing the impact of
+// blink::features::kPrerender2 experiment with and without them.
+
+// Please note this feature is only used for experimental purposes, please don't
+// enable this feature by default.
+BASE_FEATURE(kPreloadingHoldback,
+             "PreloadingHoldback",
+             base::FEATURE_DISABLED_BY_DEFAULT);
+
 // Enables exposure of ads APIs in the renderer: Attribution Reporting,
 // FLEDGE, Topics.
 BASE_FEATURE(kPrivacySandboxAdsAPIsOverride,

diff --git a/content/public/common/content_features.h b/content/public/common/content_features.h
@@ -178,6 +178,7 @@ CONTENT_EXPORT BASE_DECLARE_FEATURE(kPersistentOriginTrials);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kHighPriorityBeforeUnload);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kPreloadCookies);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kPrerender2Holdback);
+CONTENT_EXPORT BASE_DECLARE_FEATURE(kPreloadingHoldback);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kPrivacySandboxAdsAPIsOverride);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kPrivateNetworkAccessForWorkers);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kPrivateNetworkAccessForWorkersWarningOnly);