[MIGRATED] Make deprecation warnings on XHR's setRequestHeader more accurate

Source/core/xmlhttprequest/XMLHttpRequest.cpp

Index: Source/core/xmlhttprequest/XMLHttpRequest.cpp
diff --git a/Source/core/xmlhttprequest/XMLHttpRequest.cpp b/Source/core/xmlhttprequest/XMLHttpRequest.cpp
index a16849cf16b11ad3f65b6b6b2bca4833990b8aae..90ca488c648abe71275d2b9a3f5ece9d4a4f8585 100644
--- a/Source/core/xmlhttprequest/XMLHttpRequest.cpp
+++ b/Source/core/xmlhttprequest/XMLHttpRequest.cpp
@@ -1183,10 +1183,6 @@ void XMLHttpRequest::setRequestHeader(const AtomicString& name, const AtomicStri
         return;
     }
 
-    // Show deprecation warnings and count occurrences of such deprecated header values.
-    if (!value.isEmpty() && !isValidHTTPFieldContentRFC7230(value))
-        UseCounter::countDeprecation(executionContext(), UseCounter::HeaderValueNotMatchingRFC7230);
-
     // No script (privileged or not) can set unsafe headers.
     if (FetchUtils::isForbiddenHeaderName(name)) {

[tyoshino (SeeGerritForStatus), 2015/09/25 13:01:35]

Let's move this to the right place so that we won't forget to apply this check
on the result of normalization when we do the deprecation.

[hiroshige, 2015/09/29 05:19:09]

This check is on the header name, not the header value, so thus is not related
to header value update issues.

[tyoshino (SeeGerritForStatus), 2015/09/29 08:43:28]

Ouch. Never mind.

         logConsoleError(executionContext(), "Refused to set unsafe header \"" + name + "\"");
@@ -1198,9 +1194,32 @@ void XMLHttpRequest::setRequestHeader(const AtomicString& name, const AtomicStri
 
 void XMLHttpRequest::setRequestHeaderInternal(const AtomicString& name, const AtomicString& value)
 {
+    // We show deprecation warnings if |value| is still invalid header value
+    // after normalization (i.e. contains invalid octets).
+    String normalizedValue = FetchUtils::normalizeHeaderValue(value);
+    if (!normalizedValue.isEmpty() && !isValidHTTPFieldContentRFC7230(normalizedValue))
+        UseCounter::countDeprecation(executionContext(), UseCounter::HeaderValueNotMatchingRFC7230);
+
     HTTPHeaderMap::AddResult result = m_requestHeaders.add(name, value);
-    if (!result.isNewEntry)
-        result.storedValue->value = result.storedValue->value + ", " + value;
+    if (!result.isNewEntry) {

[tyoshino (SeeGerritForStatus), 2015/09/25 13:01:35]

if (result.isNewEntry)
    return;

....

[hiroshige, 2015/09/29 05:19:09]

Done.

+        AtomicString newValue = result.storedValue->value + ", " + value;
+
+        // We show deprecation warnings if this call to setRequestHeader() is
+        // affected by header value normalization.
+        // Without normalization at XHR level here, the actual header value
+        // sent to the network is |newValue| with leading/trailing whitespaces
+        // stripped (i.e. |normalizeHeaderValue(newValue)|).
+        // With normalization at XHR level here as the spec requires, the
+        // actual header value sent to the network is |normalizedNewValue|.
+        // If these two are different, introducing normalization here affects
+        // the header value sent to the network so we show warnings.
+        String normalizedNewValue = FetchUtils::normalizeHeaderValue(result.storedValue->value) + ", " + FetchUtils::normalizeHeaderValue(value);
+        if (FetchUtils::normalizeHeaderValue(newValue) != normalizedNewValue) {

[tyoshino (SeeGerritForStatus), 2015/09/25 13:01:35]

omit {}

[hiroshige, 2015/09/29 05:19:09]

Done.

+            UseCounter::countDeprecation(executionContext(), UseCounter::XHRSetRequestHeaderAffectedByNormalize);
+        }
+
+        result.storedValue->value = newValue;
+    }
 }
 
 const AtomicString& XMLHttpRequest::getRequestHeader(const AtomicString& name) const