Refactor client hints preferences code.

This separates out the code for parsing
accept-ch and accept-cl-lifetime header.

This makes it easier to parse http-equiv
accept-cl-lifetime header which will be
added in the next CL.

Previously, when only accept-ch header was present, it
was handled using a different code path then when
both accept-ch and accept-cl-lifetime headers
are present.

This CL changes it to handle the two headers separately.
That also makes it possible to unify the two code paths.

Change-Id: I1013419a55360249718855677678356f8446ec11
Bug: 852484
Reviewed-on: https://chromium-review.googlesource.com/1125083
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572495}

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

@@ -574,12 +574,6 @@ void FrameFetchContext::DispatchDidReceiveResponse(
       document_loader_ == document_loader_->GetFrame()
                               ->Loader()
                               .GetProvisionalDocumentLoader()) {
-    FrameClientHintsPreferencesContext hints_context(GetFrame());
-    document_loader_->GetClientHintsPreferences()
-        .UpdateFromAcceptClientHintsHeader(
-            response.HttpHeaderField(HTTPNames::Accept_CH), response.Url(),
-            &hints_context);
-
     // When response is received with a provisional docloader, the resource
     // haven't committed yet, and we cannot load resources, only preconnect.
     resource_loading_policy = LinkLoader::kDoNotLoadResources;
@@ -1329,16 +1323,26 @@ bool FrameFetchContext::ShouldSendClientHint(
 
 void FrameFetchContext::ParseAndPersistClientHints(
     const ResourceResponse& response) {
-  ClientHintsPreferences hints_preferences;
-  WebEnabledClientHints enabled_client_hints;
-  TimeDelta persist_duration;
   FrameClientHintsPreferencesContext hints_context(GetFrame());
-  hints_preferences.UpdatePersistentHintsFromHeaders(
-      response, &hints_context, enabled_client_hints, &persist_duration);
 
+  document_loader_->GetClientHintsPreferences()
+      .UpdateFromAcceptClientHintsLifetimeHeader(
+          response.HttpHeaderField(HTTPNames::Accept_CH_Lifetime),
+          response.Url(), &hints_context);
+
+  document_loader_->GetClientHintsPreferences()
+      .UpdateFromAcceptClientHintsHeader(
+          response.HttpHeaderField(HTTPNames::Accept_CH), response.Url(),
+          &hints_context);
+
+  // Notify content settings client of persistent client hints.
+  TimeDelta persist_duration =
+      document_loader_->GetClientHintsPreferences().GetPersistDuration();
   if (persist_duration.InSeconds() <= 0)
     return;
 
+  WebEnabledClientHints enabled_client_hints =
+      document_loader_->GetClientHintsPreferences().GetWebEnabledClientHints();
   if (!AllowScriptFromSourceWithoutNotifying(response.Url())) {
     // Do not persist client hint preferences if the JavaScript is disabled.
     return;

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

@@ -47,7 +47,7 @@ void HttpEquiv::Process(Document& document,
         kSecurityMessageSource, kErrorMessageLevel,
         "X-Frame-Options may only be set via an HTTP header sent along with a "
         "document. It may not be set inside <meta>."));
-  } else if (EqualIgnoringASCIICase(equiv, "accept-ch")) {
+  } else if (EqualIgnoringASCIICase(equiv, HTTPNames::Accept_CH)) {
     ProcessHttpEquivAcceptCH(document, content);
   } else if (EqualIgnoringASCIICase(equiv, "content-security-policy") ||
              EqualIgnoringASCIICase(equiv,

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

@@ -6,11 +6,11 @@
 
 #include "base/macros.h"
 #include "third_party/blink/public/common/client_hints/client_hints.h"
-#include "third_party/blink/renderer/platform/loader/fetch/resource_response.h"
 #include "third_party/blink/renderer/platform/network/http_names.h"
 #include "third_party/blink/renderer/platform/network/http_parsers.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 #include "third_party/blink/renderer/platform/weborigin/kurl.h"
+#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
 
 namespace blink {
 
@@ -98,42 +98,25 @@ void ClientHintsPreferences::UpdateFromAcceptClientHintsHeader(
   }
 }
 
-// static
-void ClientHintsPreferences::UpdatePersistentHintsFromHeaders(
-    const ResourceResponse& response,
-    Context* context,
-    WebEnabledClientHints& enabled_hints,
-    TimeDelta* persist_duration) {
-  *persist_duration = base::TimeDelta();
-
-  if (response.WasCached())
+void ClientHintsPreferences::UpdateFromAcceptClientHintsLifetimeHeader(
+    const String& header_value,
+    const KURL& url,
+    Context* context) {
+  if (header_value.IsEmpty())
     return;
 
-  String accept_ch_header_value =
-      response.HttpHeaderField(HTTPNames::Accept_CH);
-  String accept_ch_lifetime_header_value =
-      response.HttpHeaderField(HTTPNames::Accept_CH_Lifetime);
-
-  if (accept_ch_header_value.IsEmpty() ||
-      accept_ch_lifetime_header_value.IsEmpty()) {
-    return;
-  }
-
-  const KURL url = response.Url();
+  // Client hints should be allowed only on secure URLs.
   if (!IsClientHintsAllowed(url))
     return;
 
   bool conversion_ok = false;
-  int64_t persist_duration_seconds =
-      accept_ch_lifetime_header_value.ToInt64Strict(&conversion_ok);
+  int64_t persist_duration_seconds = header_value.ToInt64Strict(&conversion_ok);
   if (!conversion_ok || persist_duration_seconds <= 0)
     return;
 
-  *persist_duration = TimeDelta::FromSeconds(persist_duration_seconds);
+  persist_duration_ = TimeDelta::FromSeconds(persist_duration_seconds);
   if (context)
     context->CountPersistentClientHintHeaders();
-
-  ParseAcceptChHeader(accept_ch_header_value, enabled_hints);
 }
 
 // static
@@ -143,4 +126,12 @@ bool ClientHintsPreferences::IsClientHintsAllowed(const KURL& url) {
           SecurityOrigin::Create(url)->IsLocalhost());
 }
 
+WebEnabledClientHints ClientHintsPreferences::GetWebEnabledClientHints() const {
+  return enabled_hints_;
+}
+
+base::TimeDelta ClientHintsPreferences::GetPersistDuration() const {
+  return persist_duration_;
+}
+
 }  // namespace blink

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

@@ -14,7 +14,6 @@
 namespace blink {
 
 class KURL;
-class ResourceResponse;
 
 // TODO (tbansal): Remove PLATFORM_EXPORT, and pass WebClientHintsType
 // everywhere.
@@ -50,26 +49,25 @@ class PLATFORM_EXPORT ClientHintsPreferences {
     enabled_hints_.SetIsEnabled(type, true);
   }
 
-  // Parses the client hints headers, and populates |enabled_hints| with the
-  // client hint preferences that should be persisted for |persist_duration|.
-  // |persist_duration| should be non-null.
-  // If there are no client hints that need to be persisted,
-  // |persist_duration| is not set, otherwise it is set to the duration for
-  // which the client hint preferences should be persisted.
-  // UpdatePersistentHintsFromHeaders may be called for all responses
-  // received (including subresources). |context| may be null.
-  static void UpdatePersistentHintsFromHeaders(
-      const ResourceResponse&,
-      Context*,
-      WebEnabledClientHints& enabled_hints,
-      TimeDelta* persist_duration);
+  // Parses the accept-ch-lifetime header, and populates |this| with the client
+  // hints persistence duration. |url| is the URL of the resource whose response
+  // included the |header_value|. |context| may be null. If client hints are not
+  // allowed for |url|, then |this| would not be updated.
+  void UpdateFromAcceptClientHintsLifetimeHeader(const String& header_value,
+                                                 const KURL& url,
+                                                 Context* context);
 
   // Returns true if client hints are allowed for the provided KURL. Client
   // hints are allowed only on HTTP URLs that belong to secure contexts.
   static bool IsClientHintsAllowed(const KURL&);
 
+  WebEnabledClientHints GetWebEnabledClientHints() const;
+
+  base::TimeDelta GetPersistDuration() const;
+
  private:
   WebEnabledClientHints enabled_hints_;
+  base::TimeDelta persist_duration_;
 };
 
 }  // namespace blink

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

@@ -82,6 +82,58 @@ TEST_F(ClientHintsPreferencesTest, BasicSecure) {
   }
 }
 
+// Verify that the set of enabled client hints is updated every time Update*()
+// methods are called.
+TEST_F(ClientHintsPreferencesTest, SecureEnabledTypesAreUpdated) {
+  ClientHintsPreferences preferences;
+  const KURL kurl(String::FromUTF8("https://www.google.com/"));
+  preferences.UpdateFromAcceptClientHintsHeader("rtt, downlink", kurl, nullptr);
+
+  EXPECT_EQ(base::TimeDelta(), preferences.GetPersistDuration());
+  EXPECT_FALSE(
+      preferences.ShouldSend(mojom::WebClientHintsType::kResourceWidth));
+  EXPECT_FALSE(preferences.ShouldSend(mojom::WebClientHintsType::kDpr));
+  EXPECT_FALSE(
+      preferences.ShouldSend(mojom::WebClientHintsType::kViewportWidth));
+  EXPECT_TRUE(preferences.ShouldSend(mojom::WebClientHintsType::kRtt));
+  EXPECT_TRUE(preferences.ShouldSend(mojom::WebClientHintsType::kDownlink));
+  EXPECT_FALSE(preferences.ShouldSend(mojom::WebClientHintsType::kEct));
+
+  // Calling UpdateFromAcceptClientHintsHeader with empty header should have
+  // no impact on client hint preferences.
+  preferences.UpdateFromAcceptClientHintsHeader("", kurl, nullptr);
+  EXPECT_EQ(base::TimeDelta(), preferences.GetPersistDuration());
+  EXPECT_FALSE(
+      preferences.ShouldSend(mojom::WebClientHintsType::kResourceWidth));
+  EXPECT_TRUE(preferences.ShouldSend(mojom::WebClientHintsType::kRtt));
+  EXPECT_TRUE(preferences.ShouldSend(mojom::WebClientHintsType::kDownlink));
+  EXPECT_FALSE(preferences.ShouldSend(mojom::WebClientHintsType::kEct));
+
+  // Calling UpdateFromAcceptClientHintsHeader with an invalid header should
+  // have no impact on client hint preferences.
+  preferences.UpdateFromAcceptClientHintsHeader("foobar", kurl, nullptr);
+  EXPECT_EQ(base::TimeDelta(), preferences.GetPersistDuration());
+  EXPECT_FALSE(
+      preferences.ShouldSend(mojom::WebClientHintsType::kResourceWidth));
+  EXPECT_TRUE(preferences.ShouldSend(mojom::WebClientHintsType::kRtt));
+  EXPECT_TRUE(preferences.ShouldSend(mojom::WebClientHintsType::kDownlink));
+  EXPECT_FALSE(preferences.ShouldSend(mojom::WebClientHintsType::kEct));
+
+  // Calling UpdateFromAcceptClientHintsHeader with "width" header should
+  // have no impact on already enabled client hint preferences.
+  preferences.UpdateFromAcceptClientHintsHeader("width", kurl, nullptr);
+  EXPECT_EQ(base::TimeDelta(), preferences.GetPersistDuration());
+  EXPECT_TRUE(
+      preferences.ShouldSend(mojom::WebClientHintsType::kResourceWidth));
+  EXPECT_TRUE(preferences.ShouldSend(mojom::WebClientHintsType::kRtt));
+  EXPECT_TRUE(preferences.ShouldSend(mojom::WebClientHintsType::kDownlink));
+  EXPECT_FALSE(preferences.ShouldSend(mojom::WebClientHintsType::kEct));
+
+  preferences.UpdateFromAcceptClientHintsLifetimeHeader("1000", kurl, nullptr);
+  EXPECT_EQ(base::TimeDelta::FromSeconds(1000),
+            preferences.GetPersistDuration());
+}
+
 TEST_F(ClientHintsPreferencesTest, Insecure) {
   for (const auto& use_secure_url : {false, true}) {
     ClientHintsPreferences preferences;
@@ -94,60 +146,83 @@ TEST_F(ClientHintsPreferencesTest, Insecure) {
   }
 }
 
-TEST_F(ClientHintsPreferencesTest, PersistentHints) {
+// Verify that the client hints header and the lifetime header is parsed
+// correctly.
+TEST_F(ClientHintsPreferencesTest, ParseHeaders) {
   struct TestCase {
     const char* accept_ch_header_value;
     const char* accept_lifetime_header_value;
     int64_t expect_persist_duration_seconds;
+    bool expect_device_memory;
+    bool expect_width;
+    bool expect_dpr;
+    bool expect_viewport_width;
+    bool expect_rtt;
+    bool expect_downlink;
+    bool expect_ect;
   } test_cases[] = {
-      {"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},
+      {"width, dpr, viewportWidth", "", 0, false, true, true, false, false,
+       false, false},
+      {"width, dpr, viewportWidth", "-1000", 0, false, true, true, false, false,
+       false, false},
+      {"width, dpr, viewportWidth", "1000s", 0, false, true, true, false, false,
+       false, false},
+      {"width, dpr, viewportWidth", "1000.5", 0, false, true, true, false,
+       false, false, false},
+      {"width, dpr, rtt, downlink, ect", "1000", 1000, false, true, true, false,
+       true, true, true},
+      {"device-memory", "-1000", 0, true, false, false, false, false, false,
+       false},
+      {"dpr rtt", "1000", 1000, false, false, false, false, false, false,
+       false},
   };
 
   for (const auto& test : test_cases) {
-    WebEnabledClientHints enabled_types;
-    TimeDelta persist_duration;
+    ClientHintsPreferences preferences;
+    WebEnabledClientHints enabled_types =
+        preferences.GetWebEnabledClientHints();
+    EXPECT_FALSE(
+        enabled_types.IsEnabled(mojom::WebClientHintsType::kDeviceMemory));
+    EXPECT_FALSE(enabled_types.IsEnabled(mojom::WebClientHintsType::kDpr));
+    EXPECT_FALSE(
+        enabled_types.IsEnabled(mojom::WebClientHintsType::kResourceWidth));
+    EXPECT_FALSE(
+        enabled_types.IsEnabled(mojom::WebClientHintsType::kViewportWidth));
+    EXPECT_FALSE(enabled_types.IsEnabled(mojom::WebClientHintsType::kRtt));
+    EXPECT_FALSE(enabled_types.IsEnabled(mojom::WebClientHintsType::kDownlink));
+    EXPECT_FALSE(enabled_types.IsEnabled(mojom::WebClientHintsType::kEct));
+    TimeDelta persist_duration = preferences.GetPersistDuration();
+    EXPECT_EQ(base::TimeDelta(), persist_duration);
 
     const KURL kurl(String::FromUTF8("https://www.google.com/"));
+    preferences.UpdateFromAcceptClientHintsHeader(test.accept_ch_header_value,
+                                                  kurl, nullptr);
+    preferences.UpdateFromAcceptClientHintsLifetimeHeader(
+        test.accept_lifetime_header_value, kurl, nullptr);
 
-    ResourceResponse response(kurl);
-    response.SetHTTPHeaderField(HTTPNames::Accept_CH,
-                                test.accept_ch_header_value);
-    response.SetHTTPHeaderField(HTTPNames::Accept_CH_Lifetime,
-                                test.accept_lifetime_header_value);
+    enabled_types = preferences.GetWebEnabledClientHints();
+    persist_duration = preferences.GetPersistDuration();
 
-    ClientHintsPreferences::UpdatePersistentHintsFromHeaders(
-        response, nullptr, enabled_types, &persist_duration);
     EXPECT_EQ(test.expect_persist_duration_seconds,
               persist_duration.InSeconds());
-    if (test.expect_persist_duration_seconds > 0) {
-      EXPECT_FALSE(
-          enabled_types.IsEnabled(mojom::WebClientHintsType::kDeviceMemory));
-      EXPECT_TRUE(enabled_types.IsEnabled(mojom::WebClientHintsType::kDpr));
-      EXPECT_TRUE(
-          enabled_types.IsEnabled(mojom::WebClientHintsType::kResourceWidth));
-      EXPECT_FALSE(
-          enabled_types.IsEnabled(mojom::WebClientHintsType::kViewportWidth));
-      EXPECT_TRUE(enabled_types.IsEnabled(mojom::WebClientHintsType::kRtt));
-      EXPECT_TRUE(
-          enabled_types.IsEnabled(mojom::WebClientHintsType::kDownlink));
-      EXPECT_TRUE(enabled_types.IsEnabled(mojom::WebClientHintsType::kEct));
-    } else {
-      EXPECT_FALSE(
-          enabled_types.IsEnabled(mojom::WebClientHintsType::kDeviceMemory));
-      EXPECT_FALSE(enabled_types.IsEnabled(mojom::WebClientHintsType::kDpr));
-      EXPECT_FALSE(
-          enabled_types.IsEnabled(mojom::WebClientHintsType::kResourceWidth));
-      EXPECT_FALSE(
-          enabled_types.IsEnabled(mojom::WebClientHintsType::kViewportWidth));
-      EXPECT_FALSE(enabled_types.IsEnabled(mojom::WebClientHintsType::kRtt));
-      EXPECT_FALSE(
-          enabled_types.IsEnabled(mojom::WebClientHintsType::kDownlink));
-      EXPECT_FALSE(enabled_types.IsEnabled(mojom::WebClientHintsType::kEct));
-    }
+
+    EXPECT_EQ(
+        test.expect_device_memory,
+        enabled_types.IsEnabled(mojom::WebClientHintsType::kDeviceMemory));
+    EXPECT_EQ(test.expect_dpr,
+              enabled_types.IsEnabled(mojom::WebClientHintsType::kDpr));
+    EXPECT_EQ(
+        test.expect_width,
+        enabled_types.IsEnabled(mojom::WebClientHintsType::kResourceWidth));
+    EXPECT_EQ(
+        test.expect_viewport_width,
+        enabled_types.IsEnabled(mojom::WebClientHintsType::kViewportWidth));
+    EXPECT_EQ(test.expect_rtt,
+              enabled_types.IsEnabled(mojom::WebClientHintsType::kRtt));
+    EXPECT_EQ(test.expect_downlink,
+              enabled_types.IsEnabled(mojom::WebClientHintsType::kDownlink));
+    EXPECT_EQ(test.expect_ect,
+              enabled_types.IsEnabled(mojom::WebClientHintsType::kEct));
   }
 }