Fetch API: Stop lowercasing header names.

third_party/WebKit/Source/modules/fetch/FetchHeaderList.cpp

Index: third_party/WebKit/Source/modules/fetch/FetchHeaderList.cpp
diff --git a/third_party/WebKit/Source/modules/fetch/FetchHeaderList.cpp b/third_party/WebKit/Source/modules/fetch/FetchHeaderList.cpp
index ec34fc97fb1375921aa508e49ff6f38711b76ccb..d86a86a60c0697d73e50c6b148874e3a4b3cd3b6 100644
--- a/third_party/WebKit/Source/modules/fetch/FetchHeaderList.cpp
+++ b/third_party/WebKit/Source/modules/fetch/FetchHeaderList.cpp
@@ -4,10 +4,11 @@
 
 #include "modules/fetch/FetchHeaderList.h"
 
+#include <algorithm>
+#include <utility>
 #include "platform/loader/fetch/FetchUtils.h"
 #include "platform/network/HTTPParsers.h"
-#include "wtf/PtrUtil.h"
-#include <algorithm>
+#include "wtf/text/StringBuilder.h"
 
 namespace blink {
 
@@ -17,8 +18,8 @@ FetchHeaderList* FetchHeaderList::Create() {
 
 FetchHeaderList* FetchHeaderList::Clone() const {
   FetchHeaderList* list = Create();
-  for (size_t i = 0; i < header_list_.size(); ++i)
-    list->Append(header_list_[i]->first, header_list_[i]->second);
+  for (const auto& header : header_list_)
+    list->Append(header.first, header.second);
   return list;
 }
 
@@ -27,34 +28,36 @@ FetchHeaderList::FetchHeaderList() {}
 FetchHeaderList::~FetchHeaderList() {}
 
 void FetchHeaderList::Append(const String& name, const String& value) {
+  // https://fetch.spec.whatwg.org/#concept-header-list-append
   // "To append a name/value (|name|/|value|) pair to a header list (|list|),
-  // append a new header whose name is |name|, byte lowercased, and value is
-  // |value|, to |list|."
-  header_list_.push_back(WTF::WrapUnique(new Header(name.Lower(), value)));
+  // run these steps:
+  // 1. If |list| contains |name|, then set |name| to the first such header’s
+  //    name. This reuses the casing of the name of the header already in the
+  //    header list, if any. If there are multiple matched headers their names
+  //    will all be identical.
+  // 2. Append a new header whose name is |name| and |value| is |value| to
+  //    |list|."
+  const auto& header = header_list_.find(name);

[yhirano, 2017/04/11 14:03:05]

Using |auto| instead of |const auto&| is easier to understand in this case, I
think.

+  if (header != header_list_.end())
+    header_list_.insert(std::make_pair(header->first, value));
+  else
+    header_list_.insert(std::make_pair(name, value));
 }
 
 void FetchHeaderList::Set(const String& name, const String& value) {
+  // https://fetch.spec.whatwg.org/#concept-header-list-set
   // "To set a name/value (|name|/|value|) pair in a header list (|list|), run
   // these steps:
-  // 1. Byte lowercase |name|.
-  // 2. If there are any headers in |list| whose name is |name|, set the value
-  //    of the first such header to |value| and remove the others.
-  // 3. Otherwise, append a new header whose name is |name| and value is
-  //    |value|, to |list|."
-  const String lowercased_name = name.Lower();
-  for (size_t i = 0; i < header_list_.size(); ++i) {
-    if (header_list_[i]->first == lowercased_name) {
-      header_list_[i]->second = value;
-      for (size_t j = i + 1; j < header_list_.size();) {
-        if (header_list_[j]->first == lowercased_name)
-          header_list_.erase(j);
-        else
-          ++j;
-      }
-      return;
-    }
-  }
-  header_list_.push_back(WTF::MakeUnique<Header>(lowercased_name, value));
+  // 1. If |list| contains |name|, then set the value of the first such header
+  //    to |value| and remove the others.
+  // 2. Otherwise, append a new header whose name is |name| and value is
+  //    |value| to |list|."
+  const auto& existingHeader = header_list_.find(name);

[yhirano, 2017/04/11 14:03:05]

ditto

+  const FetchHeaderList::Header newHeader = std::make_pair(
+      existingHeader != header_list_.end() ? existingHeader->first : name,
+      value);
+  header_list_.erase(name);
+  header_list_.insert(newHeader);
 }
 
 String FetchHeaderList::ExtractMIMEType() const {
@@ -74,85 +77,87 @@ size_t FetchHeaderList::size() const {
 }
 
 void FetchHeaderList::Remove(const String& name) {
-  // "To delete a name (|name|) from a header list (|list|), remove all headers
-  // whose name is |name|, byte lowercased, from |list|."
-  const String lowercased_name = name.Lower();
-  for (size_t i = 0; i < header_list_.size();) {
-    if (header_list_[i]->first == lowercased_name)
-      header_list_.erase(i);
-    else
-      ++i;
-  }
+  // https://fetch.spec.whatwg.org/#concept-header-list-delete
+  // "To delete a name (name) from a header list (list), remove all headers
+  // whose name is a byte-case-insensitive match for name from list."
+  header_list_.erase(name);
 }
 
 bool FetchHeaderList::Get(const String& name, String& result) const {
-  const String lowercased_name = name.Lower();
+  // https://fetch.spec.whatwg.org/#concept-header-list-combine
+  // "To combine a name/value (|name|/|value|) pair in a header list (|list|),
+  // run these steps:
+  // 1. If |list| contains |name|, then set the value of the first such header
+  //    to its value, followed by 0x2C 0x20, followed by |value|.
+  // 2. Otherwise, append a new header whose name is |name| and value is
+  //    |value| to |list|."
+  StringBuilder resultBuilder;
   bool found = false;
-  for (const auto& header : header_list_) {
-    if (header->first == lowercased_name) {
-      if (!found) {
-        result = "";
-        result.Append(header->second);
-        found = true;
-      } else {
-        result.Append(",");
-        result.Append(header->second);
-      }
+  const auto& range = header_list_.equal_range(name);

[yhirano, 2017/04/11 14:03:05]

ditto

+  for (auto header = range.first; header != range.second; ++header) {
+    if (!found) {
+      resultBuilder.Append(header->second);
+      found = true;
+    } else {
+      // TODO(rakuco): This must be ", " instead. crbug.com/700434.
+      resultBuilder.Append(',');
+      resultBuilder.Append(header->second);
     }
   }
+  if (found)
+    result = resultBuilder.ToString();
   return found;
 }
 
+// This is going to be removed: see crbug.com/645492.
 void FetchHeaderList::GetAll(const String& name, Vector<String>& result) const {
-  const String lowercased_name = name.Lower();
   result.Clear();
-  for (size_t i = 0; i < header_list_.size(); ++i) {
-    if (header_list_[i]->first == lowercased_name)
-      result.push_back(header_list_[i]->second);
+  const auto& range = header_list_.equal_range(name);

[yhirano, 2017/04/11 14:03:05]

ditto

+  for (auto header = range.first; header != range.second; ++header) {
+    result.push_back(header->second);
   }
 }
 
 bool FetchHeaderList::Has(const String& name) const {
-  const String lowercased_name = name.Lower();
-  for (size_t i = 0; i < header_list_.size(); ++i) {
-    if (header_list_[i]->first == lowercased_name)
-      return true;
-  }
-  return false;
+  // https://fetch.spec.whatwg.org/#header-list-contains
+  // "A header list (|list|) contains a name (|name|) if |list| contains a
+  // header whose name is a byte-case-insensitive match for |name|."
+  return header_list_.find(name) != header_list_.end();
 }
 
 void FetchHeaderList::ClearList() {
-  header_list_.Clear();
+  header_list_.clear();
 }
 
 bool FetchHeaderList::ContainsNonSimpleHeader() const {
-  for (size_t i = 0; i < header_list_.size(); ++i) {
-    if (!FetchUtils::IsSimpleHeader(AtomicString(header_list_[i]->first),
-                                    AtomicString(header_list_[i]->second)))
-      return true;
-  }
-  return false;
+  return std::any_of(
+      header_list_.begin(), header_list_.end(), [](const Header& header) {
+        return !FetchUtils::IsSimpleHeader(AtomicString(header.first),
+                                           AtomicString(header.second));
+      });
 }
 
-void FetchHeaderList::SortAndCombine() {
+Vector<FetchHeaderList::Header> FetchHeaderList::SortAndCombine() const {
   // https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine
-  // "To sort and combine a header list..."
-  if (header_list_.IsEmpty())
-    return;
-
-  std::sort(
-      header_list_.begin(), header_list_.end(),
-      [](const std::unique_ptr<Header>& a, const std::unique_ptr<Header>& b) {
-        return WTF::CodePointCompareLessThan(a->first, b->first);
-      });
-
-  for (size_t index = header_list_.size() - 1; index > 0; --index) {
-    if (header_list_[index - 1]->first == header_list_[index]->first) {
-      header_list_[index - 1]->second.Append(",");
-      header_list_[index - 1]->second.Append(header_list_[index]->second);
-      header_list_.erase(index, 1);
-    }
+  // "To sort and combine a header list (|list|), run these steps:
+  // 1. Let |headers| be an empty list of name-value pairs with the key being
+  //    the name and value the value.
+  // 2. Let |names| be all the names of the headers in |list|, byte-lowercased,
+  //    with duplicates removed, and finally sorted lexicographically.
+  // 3. For each |name| in |names|, run these substeps:
+  //    1. Let |value| be the combined value given |name| and |list|.
+  //    2. Append |name-value| to |headers|.
+  // 4. Return |headers|."
+  Vector<FetchHeaderList::Header> ret;
+  for (auto it = header_list_.begin(); it != header_list_.end();) {
+    const String& headerName = it->first.Lower();
+    String combinedValue;
+    Get(headerName, combinedValue);
+    ret.emplace_back(std::make_pair(headerName, combinedValue));
+    // Skip to the next distinct key.
+    it = header_list_.upper_bound(headerName);
   }
+  return ret;
 }
 
 bool FetchHeaderList::IsValidHeaderName(const String& name) {