Bug 1396848 - Iterating a Header object returns sorted and combined v…

…alues, r=qdot
iaxat · Sep 17, 2017 · cd1638b · cd1638b
1 parent 65dd59b
commit cd1638b
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 27 deletions.
diff --git a/dom/fetch/InternalHeaders.cpp b/dom/fetch/InternalHeaders.cpp
@@ -22,12 +22,14 @@ InternalHeaders::InternalHeaders(const nsTArray<Entry>&& aHeaders,
                                  HeadersGuardEnum aGuard)
   : mGuard(aGuard)
   , mList(aHeaders)
+  , mListDirty(true)
 {
 }
 
 InternalHeaders::InternalHeaders(const nsTArray<HeadersEntry>& aHeadersEntryList,
                                  HeadersGuardEnum aGuard)
   : mGuard(aGuard)
+  , mListDirty(true)
 {
   for (const HeadersEntry& headersEntry : aHeadersEntryList) {
     mList.AppendElement(Entry(headersEntry.name(), headersEntry.value()));
@@ -59,6 +61,8 @@ InternalHeaders::Append(const nsACString& aName, const nsACString& aValue,
     return;
   }
 
+  SetListDirty();
+
   mList.AppendElement(Entry(lowerName, trimValue));
 }
 
@@ -72,6 +76,8 @@ InternalHeaders::Delete(const nsACString& aName, ErrorResult& aRv)
     return;
   }
 
+  SetListDirty();
+
   // remove in reverse order to minimize copying
   for (int32_t i = mList.Length() - 1; i >= 0; --i) {
     if (lowerName == mList[i].mName) {
@@ -160,6 +166,8 @@ InternalHeaders::Set(const nsACString& aName, const nsACString& aValue, ErrorRes
     return;
   }
 
+  SetListDirty();
+
   int32_t firstIndex = INT32_MAX;
 
   // remove in reverse order to minimize copying
@@ -182,6 +190,7 @@ InternalHeaders::Set(const nsACString& aName, const nsACString& aValue, ErrorRes
 void
 InternalHeaders::Clear()
 {
+  SetListDirty();
   mList.Clear();
 }
 
@@ -470,5 +479,54 @@ InternalHeaders::GetUnsafeHeaders(nsTArray<nsCString>& aNames) const
   }
 }
 
+void
+InternalHeaders::MaybeSortList()
+{
+  class Comparator {
+  public:
+    bool Equals(const Entry& aA, const Entry& aB) const
+    {
+       return aA.mName == aB.mName;
+    }
+
+    bool LessThan(const Entry& aA, const Entry& aB) const
+    {
+      return aA.mName < aB.mName;
+    }
+  };
+
+  if (!mListDirty) {
+    return;
+  }
+
+  mListDirty = false;
+
+  Comparator comparator;
+
+  mSortedList.Clear();
+  for (const Entry& entry : mList) {
+    bool found = false;
+    for (Entry& sortedEntry : mSortedList) {
+      if (sortedEntry.mName == entry.mName) {
+        sortedEntry.mValue += ", ";
+        sortedEntry.mValue += entry.mValue;
+        found = true;
+        break;
+      }
+    }
+
+    if (!found) {
+      mSortedList.InsertElementSorted(entry, comparator);
+    }
+  }
+}
+
+void
+InternalHeaders::SetListDirty()
+{
+  mSortedList.Clear();
+  mListDirty = true;
+}
+
 } // namespace dom
 } // namespace mozilla
diff --git a/dom/fetch/InternalHeaders.h b/dom/fetch/InternalHeaders.h
@@ -45,14 +45,23 @@ class InternalHeaders final
   HeadersGuardEnum mGuard;
   nsTArray<Entry> mList;
 
+  nsTArray<Entry> mSortedList;
+
+  // This boolean is set to true at any writing operation to mList. It's set to
+  // false when mSortedList is regenerated. This happens when the header is
+  // iterated.
+  bool mListDirty;
+
 public:
   explicit InternalHeaders(HeadersGuardEnum aGuard = HeadersGuardEnum::None)
     : mGuard(aGuard)
+    , mListDirty(false)
   {
   }
 
   explicit InternalHeaders(const InternalHeaders& aOther)
     : mGuard(HeadersGuardEnum::None)
+    , mListDirty(true)
   {
     ErrorResult result;
     Fill(aOther, result);
@@ -79,19 +88,22 @@ class InternalHeaders final
   bool Has(const nsACString& aName, ErrorResult& aRv) const;
   void Set(const nsACString& aName, const nsACString& aValue, ErrorResult& aRv);
 
-  uint32_t GetIterableLength() const
+  uint32_t GetIterableLength()
   {
-    return mList.Length();
+    MaybeSortList();
+    return mSortedList.Length();
   }
-  const NS_ConvertASCIItoUTF16 GetKeyAtIndex(unsigned aIndex) const
+  const NS_ConvertASCIItoUTF16 GetKeyAtIndex(unsigned aIndex)
   {
-    MOZ_ASSERT(aIndex < mList.Length());
-    return NS_ConvertASCIItoUTF16(mList[aIndex].mName);
+    MaybeSortList();
+    MOZ_ASSERT(aIndex < mSortedList.Length());
+    return NS_ConvertASCIItoUTF16(mSortedList[aIndex].mName);
   }
-  const NS_ConvertASCIItoUTF16 GetValueAtIndex(unsigned aIndex) const
+  const NS_ConvertASCIItoUTF16 GetValueAtIndex(unsigned aIndex)
   {
-    MOZ_ASSERT(aIndex < mList.Length());
-    return NS_ConvertASCIItoUTF16(mList[aIndex].mValue);
+    MaybeSortList();
+    MOZ_ASSERT(aIndex < mSortedList.Length());
+    return NS_ConvertASCIItoUTF16(mSortedList[aIndex].mValue);
   }
 
   void Clear();
@@ -153,6 +165,9 @@ class InternalHeaders final
                              const nsACString& aValue);
 
   static bool IsRevalidationHeader(const nsACString& aName);
+
+  void MaybeSortList();
+  void SetListDirty();
 };
 
 } // namespace dom

diff --git a/dom/tests/mochitest/fetch/test_headers_common.js b/dom/tests/mochitest/fetch/test_headers_common.js
@@ -240,12 +240,12 @@ function TestHeadersIterator() {
   var value_iter = headers.values();
   var entries_iter = headers.entries();
 
-  arrayEquals(iterate(key_iter), ["foo", "foo", "foo2"], "Correct key iterator");
-  arrayEquals(iterate(value_iter), ["bar", ehsanInflated, "baz2"], "Correct value iterator");
-  arrayEquals(iterate(entries_iter), [["foo", "bar"], ["foo", ehsanInflated], ["foo2", "baz2"]], "Correct entries iterator");
+  arrayEquals(iterate(key_iter), ["foo", "foo2"], "Correct key iterator");
+  arrayEquals(iterate(value_iter), ["bar, " + ehsanInflated, "baz2"], "Correct value iterator");
+  arrayEquals(iterate(entries_iter), [["foo", "bar, " + ehsanInflated], ["foo2", "baz2"]], "Correct entries iterator");
 
-  arrayEquals(iterateForOf(headers), [["foo", "bar"], ["foo", ehsanInflated], ["foo2", "baz2"]], "Correct entries iterator");
-  arrayEquals(iterateForOf(new Headers(headers)), [["foo", "bar"], ["foo", ehsanInflated], ["foo2", "baz2"]], "Correct entries iterator");
+  arrayEquals(iterateForOf(headers), [["foo", "bar, " + ehsanInflated], ["foo2", "baz2"]], "Correct entries iterator");
+  arrayEquals(iterateForOf(new Headers(headers)), [["foo", "bar, " + ehsanInflated], ["foo2", "baz2"]], "Correct entries iterator");
 }
 
 function runTest() {

diff --git a/testing/web-platform/meta/fetch/api/headers/headers-basic.html.ini b/testing/web-platform/meta/fetch/api/headers/headers-basic.html.ini
@@ -1,19 +1,5 @@
 [headers-basic.html]
   type: testharness
-  [Check keys method]
-    expected: FAIL
-
-  [Check values method]
-    expected: FAIL
-
-  [Check entries method]
-    expected: FAIL
-
-  [Check Symbol.iterator method]
-    expected: FAIL
-
-  [Check forEach method]
-    expected: FAIL
 
   [Create headers with existing headers with custom iterator]
     expected: FAIL