Enable persistent client hint feature by default

This simplifies the code a bit.

This has been enabled in-code as a "stable" setting since M-67
which has now hit the stable channel.

Change-Id: I8cfed718113bb990087e004f6c7d667a0b5d30ce
Bug: 852484
Reviewed-on: https://chromium-review.googlesource.com/1116077Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570664}

third_party/blink/public/platform/web_runtime_features.h

@@ -184,7 +184,6 @@ class WebRuntimeFeatures {
   BLINK_PLATFORM_EXPORT static void EnableMediaCastOverlayButton(bool);
   BLINK_PLATFORM_EXPORT static void EnableClientPlaceholdersForServerLoFi(bool);
   BLINK_PLATFORM_EXPORT static void EnableLazyInitializeMediaControls(bool);
-  BLINK_PLATFORM_EXPORT static void EnableClientHintsPersistent(bool);
   BLINK_PLATFORM_EXPORT static void EnableMediaEngagementBypassAutoplayPolicies(
       bool);
   BLINK_PLATFORM_EXPORT static void EnableV8ContextSnapshot(bool);

third_party/blink/renderer/core/html/parser/html_preload_scanner_test.cc

@@ -657,7 +657,6 @@ TEST_F(HTMLPreloadScannerTest, testMetaAcceptCHInsecureDocument) {
       all};
 
   // For an insecure document, client hint should not be attached.
-  WebRuntimeFeatures::EnableClientHintsPersistent(true);
   RunSetUp(kViewportDisabled, kPreloadEnabled, kReferrerPolicyDefault,
            false /* use_secure_document_url */);
   Test(expect_no_client_hint);
@@ -666,13 +665,6 @@ TEST_F(HTMLPreloadScannerTest, testMetaAcceptCHInsecureDocument) {
   RunSetUp(kViewportDisabled, kPreloadEnabled, kReferrerPolicyDefault,
            true /* use_secure_document_url */);
   Test(expect_client_hint);
-
-  // For an insecure document, client hint should be attached if the persistent
-  // client hints are not enabled.
-  WebRuntimeFeatures::EnableClientHintsPersistent(false);
-  RunSetUp(kViewportDisabled, kPreloadEnabled, kReferrerPolicyDefault,
-           false /* use_secure_document_url */);
-  Test(expect_client_hint);
 }
 
 TEST_F(HTMLPreloadScannerTest, testPreconnect) {

third_party/blink/renderer/core/loader/frame_fetch_context.cc

@@ -898,40 +898,37 @@ void FrameFetchContext::AddClientHintsIfNecessary(
 
   bool is_1p_origin = false;
 
-  if (blink::RuntimeEnabledFeatures::ClientHintsPersistentEnabled()) {
-    // If the feature is enabled, then client hints are allowed only on secure
-    // URLs.
-    if (!ClientHintsPreferences::IsClientHintsAllowed(request.Url()))
-      return;
+  // If the feature is enabled, then client hints are allowed only on secure
+  // URLs.
+  if (!ClientHintsPreferences::IsClientHintsAllowed(request.Url()))
+    return;
 
-    // Check if |url| is allowed to run JavaScript. If not, client hints are not
-    // attached to the requests that initiate on the render side.
-    if (!AllowScriptFromSourceWithoutNotifying(request.Url())) {
-      return;
-    }
+  // Check if |url| is allowed to run JavaScript. If not, client hints are not
+  // attached to the requests that initiate on the render side.
+  if (!AllowScriptFromSourceWithoutNotifying(request.Url()))
+    return;
 
-    if (IsDetached())
-      return;
+  if (IsDetached())
+    return;
 
-    is_1p_origin =
-        GetFrame()
-            ->Tree()
-            .Top()
-            .GetSecurityContext()
-            ->GetSecurityOrigin()
-            ->IsSameSchemeHostPort(SecurityOrigin::Create(request.Url()).get());
-
-    if (!base::FeatureList::IsEnabled(kAllowClientHintsToThirdParty) &&
-        !is_1p_origin) {
-      // No client hints for 3p origins.
-      return;
-    }
-    // Persisted client hints preferences should be read for only the first
-    // party origins.
-    if (is_1p_origin && GetContentSettingsClient()) {
-      GetContentSettingsClient()->GetAllowedClientHintsFromSource(
-          request.Url(), &enabled_hints);
-    }
+  is_1p_origin =
+      GetFrame()
+          ->Tree()
+          .Top()
+          .GetSecurityContext()
+          ->GetSecurityOrigin()
+          ->IsSameSchemeHostPort(SecurityOrigin::Create(request.Url()).get());
+
+  if (!base::FeatureList::IsEnabled(kAllowClientHintsToThirdParty) &&
+      !is_1p_origin) {
+    // No client hints for 3p origins.
+    return;
+  }
+  // Persisted client hints preferences should be read for only the first
+  // party origins.
+  if (is_1p_origin && GetContentSettingsClient()) {
+    GetContentSettingsClient()->GetAllowedClientHintsFromSource(request.Url(),
+                                                                &enabled_hints);
   }
 
   if (ShouldSendClientHint(mojom::WebClientHintsType::kDeviceMemory,

third_party/blink/renderer/core/loader/frame_fetch_context_test.cc

@@ -649,61 +649,20 @@ TEST_F(FrameFetchContextHintsTest, MonitorDeviceMemorySecureTransport) {
 #endif
 }
 
-// Verify that the client hints should be attached for subresources fetched
-// over secure transport. Tests when the persistent client hint feature is not
-// enabled.
-TEST_F(FrameFetchContextHintsTest,
-       MonitorDeviceMemorySecureTransportPersistentHintsDisabled) {
-  WebRuntimeFeatures::EnableClientHintsPersistent(false);
-  ExpectHeader("https://www.example.com/1.gif", "Device-Memory", false, "");
+// Verify that client hints are not attached when the resources do not belong to
+// a secure context.
+TEST_F(FrameFetchContextHintsTest, MonitorDeviceMemoryHintsInsecureContext) {
+  // Verify that client hints are not attached when the resources do not belong
+  // to a secure context and the persistent client hint features is enabled.
+  ExpectHeader("http://www.example.com/1.gif", "Device-Memory", false, "");
   ClientHintsPreferences preferences;
   preferences.SetShouldSendForTesting(mojom::WebClientHintsType::kDeviceMemory);
   document->GetClientHintsPreferences().UpdateFrom(preferences);
   ApproximatedDeviceMemory::SetPhysicalMemoryMBForTesting(4096);
-  ExpectHeader("https://www.example.com/1.gif", "Device-Memory", true, "4");
-  ExpectHeader("https://www.example.com/1.gif", "DPR", false, "");
-  ExpectHeader("https://www.example.com/1.gif", "Width", false, "");
-  ExpectHeader("https://www.example.com/1.gif", "Viewport-Width", false, "");
-  // The origin of the resource does not match the origin of the main frame
-  // resource. Client hint should be attached since the persisten client hint
-  // feature is not enabled.
-  ExpectHeader("https://www.someother-example.com/1.gif", "Device-Memory", true,
-               "4");
-}
-
-// Verify that client hints are not attched when the resources do not belong to
-// a secure context.
-TEST_F(FrameFetchContextHintsTest, MonitorDeviceMemoryHintsInsecureContext) {
-  WebRuntimeFeatures::EnableClientHintsPersistent(false);
   ExpectHeader("http://www.example.com/1.gif", "Device-Memory", false, "");
-
-  {
-    ClientHintsPreferences preferences;
-    preferences.SetShouldSendForTesting(
-        mojom::WebClientHintsType::kDeviceMemory);
-    document->GetClientHintsPreferences().UpdateFrom(preferences);
-    ApproximatedDeviceMemory::SetPhysicalMemoryMBForTesting(4096);
-    ExpectHeader("http://www.example.com/1.gif", "Device-Memory", true, "4");
-    ExpectHeader("http://www.example.com/1.gif", "DPR", false, "");
-    ExpectHeader("http://www.example.com/1.gif", "Width", false, "");
-    ExpectHeader("http://www.example.com/1.gif", "Viewport-Width", false, "");
-  }
-
-  {
-    // Verify that client hints are not attched when the resources do not belong
-    // to a secure context and the persistent client hint features is enabled.
-    WebRuntimeFeatures::EnableClientHintsPersistent(true);
-    ExpectHeader("http://www.example.com/1.gif", "Device-Memory", false, "");
-    ClientHintsPreferences preferences;
-    preferences.SetShouldSendForTesting(
-        mojom::WebClientHintsType::kDeviceMemory);
-    document->GetClientHintsPreferences().UpdateFrom(preferences);
-    ApproximatedDeviceMemory::SetPhysicalMemoryMBForTesting(4096);
-    ExpectHeader("http://www.example.com/1.gif", "Device-Memory", false, "");
-    ExpectHeader("http://www.example.com/1.gif", "DPR", false, "");
-    ExpectHeader("http://www.example.com/1.gif", "Width", false, "");
-    ExpectHeader("http://www.example.com/1.gif", "Viewport-Width", false, "");
-  }
+  ExpectHeader("http://www.example.com/1.gif", "DPR", false, "");
+  ExpectHeader("http://www.example.com/1.gif", "Width", false, "");
+  ExpectHeader("http://www.example.com/1.gif", "Viewport-Width", false, "");
 }
 
 // Verify that client hints are attched when the resources belong to a local
@@ -754,28 +713,11 @@ TEST_F(FrameFetchContextHintsTest, MonitorDPRHints) {
 }
 
 TEST_F(FrameFetchContextHintsTest, MonitorDPRHintsInsecureTransport) {
-  WebRuntimeFeatures::EnableClientHintsPersistent(false);
-  ExpectHeader("http://www.example.com/1.gif", "DPR", false, "");
-
-  {
-    ClientHintsPreferences preferences;
-    preferences.SetShouldSendForTesting(mojom::WebClientHintsType::kDpr);
-    document->GetClientHintsPreferences().UpdateFrom(preferences);
-    ExpectHeader("http://www.example.com/1.gif", "DPR", true, "1");
-    dummy_page_holder->GetPage().SetDeviceScaleFactorDeprecated(2.5);
-    ExpectHeader("http://www.example.com/1.gif", "DPR", true, "2.5");
-    ExpectHeader("http://www.example.com/1.gif", "Width", false, "");
-    ExpectHeader("http://www.example.com/1.gif", "Viewport-Width", false, "");
-  }
-
-  {
-    WebRuntimeFeatures::EnableClientHintsPersistent(true);
     ExpectHeader("http://www.example.com/1.gif", "DPR", false, "");
     dummy_page_holder->GetPage().SetDeviceScaleFactorDeprecated(2.5);
     ExpectHeader("http://www.example.com/1.gif", "DPR", false, "  ");
     ExpectHeader("http://www.example.com/1.gif", "Width", false, "");
     ExpectHeader("http://www.example.com/1.gif", "Viewport-Width", false, "");
-  }
 }
 
 TEST_F(FrameFetchContextHintsTest, MonitorResourceWidthHints) {

third_party/blink/renderer/platform/exported/web_runtime_features.cc

@@ -494,10 +494,6 @@ void WebRuntimeFeatures::EnableLazyInitializeMediaControls(bool enable) {
   RuntimeEnabledFeatures::SetLazyInitializeMediaControlsEnabled(enable);
 }
 
-void WebRuntimeFeatures::EnableClientHintsPersistent(bool enable) {
-  RuntimeEnabledFeatures::SetClientHintsPersistentEnabled(enable);
-}
-
 void WebRuntimeFeatures::EnableMediaEngagementBypassAutoplayPolicies(
     bool enable) {
   RuntimeEnabledFeatures::SetMediaEngagementBypassAutoplayPoliciesEnabled(

third_party/blink/renderer/platform/loader/fetch/client_hints_preferences.cc

@@ -72,12 +72,9 @@ void ClientHintsPreferences::UpdateFromAcceptClientHintsHeader(
   if (header_value.IsEmpty())
     return;
 
-  // If the persistent client hint feature is enabled, then client hints
-  // should be allowed only on secure URLs.
-  if (blink::RuntimeEnabledFeatures::ClientHintsPersistentEnabled() &&
-      !IsClientHintsAllowed(url)) {
+  // Client hints should be allowed only on secure URLs.
+  if (!IsClientHintsAllowed(url))
     return;
-  }
 
   WebEnabledClientHints new_enabled_types;
 
@@ -117,8 +114,7 @@ void ClientHintsPreferences::UpdatePersistentHintsFromHeaders(
   String accept_ch_lifetime_header_value =
       response.HttpHeaderField(HTTPNames::Accept_CH_Lifetime);
 
-  if (!RuntimeEnabledFeatures::ClientHintsPersistentEnabled() ||
-      accept_ch_header_value.IsEmpty() ||
+  if (accept_ch_header_value.IsEmpty() ||
       accept_ch_lifetime_header_value.IsEmpty()) {
     return;
   }

third_party/blink/renderer/platform/loader/fetch/client_hints_preferences_test.cc

@@ -96,22 +96,18 @@ TEST_F(ClientHintsPreferencesTest, Insecure) {
 
 TEST_F(ClientHintsPreferencesTest, PersistentHints) {
   struct TestCase {
-    bool enable_persistent_runtime_feature;
     const char* accept_ch_header_value;
     const char* accept_lifetime_header_value;
     int64_t expect_persist_duration_seconds;
   } test_cases[] = {
-      {true, "width, dpr, viewportWidth", "", 0},
-      {true, "width, dpr, viewportWidth", "-1000", 0},
-      {true, "width, dpr, viewportWidth", "1000s", 0},
-      {true, "width, dpr, viewportWidth", "1000.5", 0},
-      {false, "width, dpr, viewportWidth", "1000", 0},
-      {true, "width, dpr, rtt, downlink, ect", "1000", 1000},
+      {"width, dpr, viewportWidth", "", 0},
+      {"width, dpr, viewportWidth", "-1000", 0},
+      {"width, dpr, viewportWidth", "1000s", 0},
+      {"width, dpr, viewportWidth", "1000.5", 0},
+      {"width, dpr, rtt, downlink, ect", "1000", 1000},
   };
 
   for (const auto& test : test_cases) {
-    WebRuntimeFeatures::EnableClientHintsPersistent(
-        test.enable_persistent_runtime_feature);
     WebEnabledClientHints enabled_types;
     TimeDelta persist_duration;
 

third_party/blink/renderer/platform/runtime_enabled_features.json5

@@ -201,10 +201,6 @@
       name: "ClickRetargetting",
       status: "experimental",
     },
-    {
-      name: "ClientHintsPersistent",
-      status: "stable",
-    },
     {
       name: "ClientPlaceholdersForServerLoFi",
     },