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

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/fetch/FetchHeaderList.h"
 
 #include "core/fetch/FetchUtils.h"
 #include "platform/network/HTTPParsers.h"
 #include "wtf/PtrUtil.h"
 #include <algorithm>
@@ skipping 113 matching lines @@
                                     AtomicString(m_headerList[i]->second)))
       return true;
   }
   return false;
 }
 
 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) {
   // "A name is a case-insensitive byte sequence that matches the field-name
   // token production."
   return isValidHTTPToken(name);
 }
 
 bool FetchHeaderList::isValidHeaderValue(const String& value) {
   // "A value is a byte sequence that matches the field-value token production
   // and contains no 0x0A or 0x0D bytes."
   return isValidHTTPHeaderValue(value);
 }
 
 }  // namespace blink