[Fetch API] Implement combining of Headers with same keys.

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 abbd34f37ca60b78591f37db5f8b40c32a4f4b05..a9eba3377afd916ed5dd468939411fced6070b1c 100644
--- a/third_party/WebKit/Source/modules/fetch/FetchHeaderList.cpp
+++ b/third_party/WebKit/Source/modules/fetch/FetchHeaderList.cpp
@@ -131,13 +131,26 @@ void FetchHeaderList::sortAndCombine() {
   // https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine
   // "To sort and combine a header list..."
 
-  // TODO(jsbell): Implement the combining part - this currently just sorts.
-
   std::sort(
       m_headerList.begin(), m_headerList.end(),
       [](const std::unique_ptr<Header>& a, const std::unique_ptr<Header>& b) {
+        if (a->first == b->first) {

[jsbell, 2016/12/12 16:54:51]

nit: no {} needed here

+          return WTF::codePointCompareLessThan(a->second, b->second);

[jsbell, 2016/12/12 16:54:50]

Is the sorting here (by value) done to make the sort stable? It does not seem
necessary per spec, which only defines sorting by names.

This may require filing a spec issue.

+        }
         return WTF::codePointCompareLessThan(a->first, b->first);
       });
+
+  if (m_headerList.size() > 1) {

[jsbell, 2016/12/12 16:54:51]

This test could be inverted to become an early exit and moved to the top of the
method to avoid the indenting here.

+    for (size_t index = 1; index < m_headerList.size();) {
+      if (m_headerList[index - 1]->first == m_headerList[index]->first) {
+        m_headerList[index - 1]->second.append(",");
+        m_headerList[index - 1]->second.append(m_headerList[index]->second);
+        m_headerList.remove(index, 1);

[jsbell, 2016/12/12 16:54:51]

This algorithm could be reversed to start at the end of the vector, and avoid
shuffling the array down for each removal, but it's probably not worth it and
would be more confusing. 

+      } else {
+        index++;

[jsbell, 2016/12/12 16:54:51]

nit: prefer pre-increment when post-increment is not necessary

+      }
+    }
+  }
 }
 
 bool FetchHeaderList::isValidHeaderName(const String& name) {