Fix duplicate value of client hint headers on asynchronous validation requests

Fix issue where asynchronous validation requests (specifically,
revalidation on stale-while-revalidate requests) result in a duplicate
value for client hint headers (i.e. `Sec-CH-UA: Chromium 76, Chromium 76`
instead of `Sec-CH-UA: Chromium 76`)

Bug: 959611
Change-Id: Iac16b84b129108b9ebe1258e11722cdc9eaa52de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1645455
Commit-Queue: Aaron Tagliaboschi <aarontag@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694763}

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

@@ -523,7 +523,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
       result.Append(' ');
       result.Append(version.data());
     }
-    request.AddHttpHeaderField(
+    request.SetHttpHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kUA)],
         result.ToAtomicString());
@@ -561,7 +561,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
            resource_origin)) &&
       ShouldSendClientHint(mojom::WebClientHintsType::kDeviceMemory,
                            hints_preferences, enabled_hints)) {
-    request.AddHttpHeaderField(
+    request.SetHttpHeaderField(
         "Device-Memory",
         AtomicString(String::Number(
             ApproximatedDeviceMemory::GetApproximatedDeviceMemory())));
@@ -573,7 +573,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
            mojom::FeaturePolicyFeature::kClientHintDPR, resource_origin)) &&
       ShouldSendClientHint(mojom::WebClientHintsType::kDpr, hints_preferences,
                            enabled_hints)) {
-    request.AddHttpHeaderField("DPR", AtomicString(String::Number(dpr)));
+    request.SetHttpHeaderField("DPR", AtomicString(String::Number(dpr)));
   }
 
   if ((!RuntimeEnabledFeatures::FeaturePolicyForClientHintsEnabled() ||
@@ -583,7 +583,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
                            hints_preferences, enabled_hints)) {
     if (resource_width.is_set) {
       float physical_width = resource_width.width * dpr;
-      request.AddHttpHeaderField(
+      request.SetHttpHeaderField(
           "Width", AtomicString(String::Number(ceil(physical_width))));
     }
   }
@@ -595,7 +595,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
       ShouldSendClientHint(mojom::WebClientHintsType::kViewportWidth,
                            hints_preferences, enabled_hints) &&
       !GetResourceFetcherProperties().IsDetached() && GetFrame()->View()) {
-    request.AddHttpHeaderField(
+    request.SetHttpHeaderField(
         "Viewport-Width",
         AtomicString(String::Number(GetFrame()->View()->ViewportWidth())));
   }
@@ -630,7 +630,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
 
     uint32_t rtt =
         GetNetworkStateNotifier().RoundRtt(request.Url().Host(), http_rtt);
-    request.AddHttpHeaderField(
+    request.SetHttpHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kRtt)],
         AtomicString(String::Number(rtt)));
@@ -651,7 +651,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
 
     double mbps = GetNetworkStateNotifier().RoundMbps(request.Url().Host(),
                                                       throughput_mbps);
-    request.AddHttpHeaderField(
+    request.SetHttpHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kDownlink)],
         AtomicString(String::Number(mbps)));
@@ -668,7 +668,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
     if (!holdback_ect)
       holdback_ect = GetNetworkStateNotifier().EffectiveType();
 
-    request.AddHttpHeaderField(
+    request.SetHttpHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kEct)],
         AtomicString(NetworkStateNotifier::EffectiveConnectionTypeToString(
@@ -681,7 +681,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
             mojom::FeaturePolicyFeature::kClientHintLang, resource_origin))) &&
       ShouldSendClientHint(mojom::WebClientHintsType::kLang, hints_preferences,
                            enabled_hints)) {
-    request.AddHttpHeaderField(
+    request.SetHttpHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kLang)],
         GetFrame()
@@ -697,7 +697,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
             resource_origin))) &&
       ShouldSendClientHint(mojom::WebClientHintsType::kUAArch,
                            hints_preferences, enabled_hints)) {
-    request.AddHttpHeaderField(
+    request.SetHttpHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kUAArch)],
         AtomicString(ua.architecture.data()));
@@ -710,7 +710,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
             resource_origin))) &&
       ShouldSendClientHint(mojom::WebClientHintsType::kUAPlatform,
                            hints_preferences, enabled_hints)) {
-    request.AddHttpHeaderField(
+    request.SetHttpHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kUAPlatform)],
         AtomicString(ua.platform.data()));
@@ -723,7 +723,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
             resource_origin))) &&
       ShouldSendClientHint(mojom::WebClientHintsType::kUAModel,
                            hints_preferences, enabled_hints)) {
-    request.AddHttpHeaderField(
+    request.SetHttpHeaderField(
         blink::kClientHintsHeaderMapping[static_cast<size_t>(
             mojom::WebClientHintsType::kUAModel)],
         AtomicString(ua.model.data()));
@@ -740,6 +740,9 @@ void FrameFetchContext::PopulateResourceRequest(
 
   const ContentSecurityPolicy* csp = GetContentSecurityPolicy();
   if (csp && csp->ShouldSendCSPHeader(type))
+    // TODO(crbug.com/993769): Test if this header returns duplicated values
+    // (i.e. "CSP: active, active") on asynchronous "stale-while-revalidate"
+    // revalidation requests and if this is unexpected behavior.
     request.AddHttpHeaderField("CSP", "active");
 }
 

third_party/blink/web_tests/external/wpt/client-hints/accept-ch-cache-revalidation.https.html

+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Tests Stale While Revalidate is not executed for fetch API</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/common/utils.js"></script>
+<script>
+function wait25ms(test) {
+  return new Promise(resolve => {
+    test.step_timeout(() => {
+      resolve();
+    }, 25);
+  });
+}
+
+promise_test(async (test) => {
+  var request_token = token();
+  var client_hint_headers = [
+    "device-memory",
+    "dpr",
+    "width",
+    "viewport-width",
+    "rtt",
+    "downlink",
+    "ect",
+    "sec-ch-lang",
+    "sec-ch-ua",
+    "sec-ch-ua-arch",
+    "sec-ch-ua-platform",
+    "sec-ch-ua-model",
+  ];
+
+  const response = await fetch(`resources/stale-echo-client-hints.py?token=` + request_token);
+  const response2 = await fetch(`resources/stale-echo-client-hints.py?token=` + request_token);
+
+  assert_equals(response.headers.get('Unique-Id'), response2.headers.get('Unique-Id'));
+
+  while(true) {
+    const revalidation_check = await fetch(`resources/stale-echo-client-hints.py?query&token=` + request_token);
+    if (revalidation_check.headers.get('Count') == '2') {
+      client_hint_headers.forEach(header => {
+        assert_equals(revalidation_check.headers.get(header+"-recieved"), revalidation_check.headers.get(header+"-previous"));
+      });
+      break;
+    }
+    await wait25ms(test);
+  }
+}, 'Same headers sent for revalidation request');
+</script>
\ No newline at end of file

third_party/blink/web_tests/external/wpt/client-hints/accept-ch-cache-revalidation.https.html.headers

+Accept-CH: dpr,device-memory,viewport-width,rtt,downlink,ect,lang,ua,arch,platform,model
\ No newline at end of file

third_party/blink/web_tests/external/wpt/client-hints/resources/stale-echo-client-hints.py

+import random
+import string
+
+def id_token():
+   letters = string.ascii_lowercase
+   return ''.join(random.choice(letters) for i in range(20))
+
+def main(request, response):
+    client_hint_headers = [
+      "device-memory",
+      "dpr",
+      "width",
+      "viewport-width",
+      "rtt",
+      "downlink",
+      "ect",
+      "sec-ch-lang",
+      "sec-ch-ua",
+      "sec-ch-ua-arch",
+      "sec-ch-ua-platform",
+      "sec-ch-ua-model",
+    ]
+
+    client_hints_curr = {i:request.headers.get(i) for i in client_hint_headers}
+
+    token = request.GET.first("token", None)
+    is_query = request.GET.first("query", None) is not None
+    with request.server.stash.lock:
+      stash = request.server.stash.take(token)
+      if stash != None:
+        (value, client_hints_prev) = stash
+        count = int(value)
+      else:
+        count = 0
+        client_hints_prev = {}
+
+      if is_query:
+        if count < 2:
+          request.server.stash.put(token, (count, client_hints_curr))
+      else:
+        count = count + 1
+        request.server.stash.put(token, (count, client_hints_curr))
+
+    for header in client_hint_headers:
+      if client_hints_curr[header] is not None:
+        response.headers.set(header+"-recieved", client_hints_curr[header])
+      if (header in client_hints_prev) and (client_hints_prev[header] is not None):
+        response.headers.set(header+"-previous", client_hints_prev[header])
+
+    if is_query:
+      headers = [("Count", count)]
+      content = ""
+      return 200, headers, content
+    else:
+      unique_id = id_token()
+      headers = [("Content-Type", "text/html"),
+                 ("Cache-Control", "private, max-age=0, stale-while-revalidate=60"),
+                 ("Unique-Id", unique_id)]
+      content = "report('{}')".format(unique_id)
+      return 200, headers, content
\ No newline at end of file