Bug 1074963 - Constructor URLSearchParams() should preserve the order of pairs from passing argument, r=bz
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 01 Oct 2014 14:55:33 +0100
changeset 208182 f5ccdeb368435a8cb2fb0464ba9157112536fbb1
parent 208181 4247b6b396a5aa25b8b7b58dad27e6cb4bc83684
child 208183 f54ea093e8365a97cfa64099af0a7ef42748c700
push id49861
push useramarchesini@mozilla.com
push dateWed, 01 Oct 2014 13:55:51 +0000
treeherdermozilla-inbound@f5ccdeb36843 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1074963
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1074963 - Constructor URLSearchParams() should preserve the order of pairs from passing argument, r=bz
dom/base/URLSearchParams.cpp
dom/base/URLSearchParams.h
dom/base/test/test_urlSearchParams.html
--- a/dom/base/URLSearchParams.cpp
+++ b/dom/base/URLSearchParams.cpp
@@ -46,17 +46,17 @@ URLSearchParams::Constructor(const Globa
 }
 
 /* static */ already_AddRefed<URLSearchParams>
 URLSearchParams::Constructor(const GlobalObject& aGlobal,
                              URLSearchParams& aInit,
                              ErrorResult& aRv)
 {
   nsRefPtr<URLSearchParams> sp = new URLSearchParams();
-  aInit.mSearchParams.EnumerateRead(CopyEnumerator, sp);
+  sp->mSearchParams = aInit.mSearchParams;
   return sp.forget();
 }
 
 void
 URLSearchParams::ParseInput(const nsACString& aInput,
                             URLSearchParamsObserver* aObserver)
 {
   // Remove all the existing data before parsing a new input.
@@ -202,30 +202,16 @@ URLSearchParams::ConvertString(const nsA
     return;
   }
 
   if (newOutputLength < outputLength) {
     aOutput.Truncate(newOutputLength);
   }
 }
 
-/* static */ PLDHashOperator
-URLSearchParams::CopyEnumerator(const nsAString& aName,
-                                nsTArray<nsString>* aArray,
-                                void *userData)
-{
-  URLSearchParams* aSearchParams = static_cast<URLSearchParams*>(userData);
-
-  nsTArray<nsString>* newArray = new nsTArray<nsString>();
-  newArray->AppendElements(*aArray);
-
-  aSearchParams->mSearchParams.Put(aName, newArray);
-  return PL_DHASH_NEXT;
-}
-
 void
 URLSearchParams::AddObserver(URLSearchParamsObserver* aObserver)
 {
   MOZ_ASSERT(aObserver);
   MOZ_ASSERT(!mObservers.Contains(aObserver));
   mObservers.AppendElement(aObserver);
 }
 
@@ -240,156 +226,153 @@ void
 URLSearchParams::RemoveObservers()
 {
   mObservers.Clear();
 }
 
 void
 URLSearchParams::Get(const nsAString& aName, nsString& aRetval)
 {
-  nsTArray<nsString>* array;
-  if (!mSearchParams.Get(aName, &array)) {
-    aRetval.Truncate();
-    return;
+  aRetval.Truncate();
+
+  for (uint32_t i = 0, len = mSearchParams.Length(); i < len; ++i) {
+    if (mSearchParams[i].mKey.Equals(aName)) {
+      aRetval.Assign(mSearchParams[i].mValue);
+      break;
+    }
   }
-
-  aRetval.Assign(array->ElementAt(0));
 }
 
 void
 URLSearchParams::GetAll(const nsAString& aName, nsTArray<nsString>& aRetval)
 {
-  nsTArray<nsString>* array;
-  if (!mSearchParams.Get(aName, &array)) {
-    return;
+  aRetval.Clear();
+
+  for (uint32_t i = 0, len = mSearchParams.Length(); i < len; ++i) {
+    if (mSearchParams[i].mKey.Equals(aName)) {
+      aRetval.AppendElement(mSearchParams[i].mValue);
+    }
   }
-
-  aRetval.AppendElements(*array);
 }
 
 void
 URLSearchParams::Set(const nsAString& aName, const nsAString& aValue)
 {
-  nsTArray<nsString>* array;
-  if (!mSearchParams.Get(aName, &array)) {
-    array = new nsTArray<nsString>();
-    array->AppendElement(aValue);
-    mSearchParams.Put(aName, array);
-  } else {
-    array->ElementAt(0) = aValue;
+  Param* param = nullptr;
+  for (uint32_t i = 0, len = mSearchParams.Length(); i < len; ++i) {
+    if (mSearchParams[i].mKey.Equals(aName)) {
+      param = &mSearchParams[i];
+      break;
+    }
   }
 
+  if (!param) {
+    param = mSearchParams.AppendElement();
+    param->mKey = aName;
+  }
+
+  param->mValue = aValue;
+
   NotifyObservers(nullptr);
 }
 
 void
 URLSearchParams::Append(const nsAString& aName, const nsAString& aValue)
 {
   AppendInternal(aName, aValue);
   NotifyObservers(nullptr);
 }
 
 void
 URLSearchParams::AppendInternal(const nsAString& aName, const nsAString& aValue)
 {
-  nsTArray<nsString>* array;
-  if (!mSearchParams.Get(aName, &array)) {
-    array = new nsTArray<nsString>();
-    mSearchParams.Put(aName, array);
-  }
-
-  array->AppendElement(aValue);
+  Param* param = mSearchParams.AppendElement();
+  param->mKey = aName;
+  param->mValue = aValue;
 }
 
 bool
 URLSearchParams::Has(const nsAString& aName)
 {
-  return mSearchParams.Get(aName, nullptr);
+  for (uint32_t i = 0, len = mSearchParams.Length(); i < len; ++i) {
+    if (mSearchParams[i].mKey.Equals(aName)) {
+      return true;
+    }
+  }
+
+  return false;
 }
 
 void
 URLSearchParams::Delete(const nsAString& aName)
 {
-  nsTArray<nsString>* array;
-  if (!mSearchParams.Get(aName, &array)) {
-    return;
+  bool found = false;
+  for (uint32_t i = 0; i < mSearchParams.Length();) {
+    if (mSearchParams[i].mKey.Equals(aName)) {
+      mSearchParams.RemoveElementAt(i);
+      found = true;
+    } else {
+      ++i;
+    }
   }
 
-  mSearchParams.Remove(aName);
-
-  NotifyObservers(nullptr);
+  if (found) {
+    NotifyObservers(nullptr);
+  }
 }
 
 void
 URLSearchParams::DeleteAll()
 {
   mSearchParams.Clear();
 }
 
-class MOZ_STACK_CLASS SerializeData
-{
-public:
-  SerializeData()
-    : mFirst(true)
-  {}
+namespace {
 
-  nsAutoString mValue;
-  bool mFirst;
-
-  void Serialize(const nsCString& aInput)
-  {
-    const unsigned char* p = (const unsigned char*) aInput.get();
+void SerializeString(const nsCString& aInput, nsAString& aValue)
+{
+  const unsigned char* p = (const unsigned char*) aInput.get();
 
-    while (p && *p) {
-      // ' ' to '+'
-      if (*p == 0x20) {
-        mValue.Append(0x2B);
-      // Percent Encode algorithm
-      } else if (*p == 0x2A || *p == 0x2D || *p == 0x2E ||
-                 (*p >= 0x30 && *p <= 0x39) ||
-                 (*p >= 0x41 && *p <= 0x5A) || *p == 0x5F ||
-                 (*p >= 0x61 && *p <= 0x7A)) {
-        mValue.Append(*p);
-      } else {
-        mValue.AppendPrintf("%%%.2X", *p);
-      }
+  while (p && *p) {
+    // ' ' to '+'
+    if (*p == 0x20) {
+      aValue.Append(0x2B);
+    // Percent Encode algorithm
+    } else if (*p == 0x2A || *p == 0x2D || *p == 0x2E ||
+               (*p >= 0x30 && *p <= 0x39) ||
+               (*p >= 0x41 && *p <= 0x5A) || *p == 0x5F ||
+               (*p >= 0x61 && *p <= 0x7A)) {
+      aValue.Append(*p);
+    } else {
+      aValue.AppendPrintf("%%%.2X", *p);
+    }
 
-      ++p;
-    }
+    ++p;
   }
-};
+}
+
+} // anonymous namespace
 
 void
 URLSearchParams::Serialize(nsAString& aValue) const
 {
-  SerializeData data;
-  mSearchParams.EnumerateRead(SerializeEnumerator, &data);
-  aValue.Assign(data.mValue);
-}
+  aValue.Truncate();
+  bool first = true;
 
-/* static */ PLDHashOperator
-URLSearchParams::SerializeEnumerator(const nsAString& aName,
-                                     nsTArray<nsString>* aArray,
-                                     void *userData)
-{
-  SerializeData* data = static_cast<SerializeData*>(userData);
-
-  for (uint32_t i = 0, len = aArray->Length(); i < len; ++i) {
-    if (data->mFirst) {
-      data->mFirst = false;
+  for (uint32_t i = 0, len = mSearchParams.Length(); i < len; ++i) {
+    if (first) {
+      first = false;
     } else {
-      data->mValue.Append('&');
+      aValue.Append('&');
     }
 
-    data->Serialize(NS_ConvertUTF16toUTF8(aName));
-    data->mValue.Append('=');
-    data->Serialize(NS_ConvertUTF16toUTF8(aArray->ElementAt(i)));
+    SerializeString(NS_ConvertUTF16toUTF8(mSearchParams[i].mKey), aValue);
+    aValue.Append('=');
+    SerializeString(NS_ConvertUTF16toUTF8(mSearchParams[i].mValue), aValue);
   }
-
-  return PL_DHASH_NEXT;
 }
 
 void
 URLSearchParams::NotifyObservers(URLSearchParamsObserver* aExceptObserver)
 {
   for (uint32_t i = 0; i < mObservers.Length(); ++i) {
     if (mObservers[i] != aExceptObserver) {
       mObservers[i]->URLSearchParamsUpdated(this);
--- a/dom/base/URLSearchParams.h
+++ b/dom/base/URLSearchParams.h
@@ -5,18 +5,16 @@
 
 #ifndef mozilla_dom_URLSearchParams_h
 #define mozilla_dom_URLSearchParams_h
 
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/ErrorResult.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsWrapperCache.h"
-#include "nsClassHashtable.h"
-#include "nsHashKeys.h"
 #include "nsISupports.h"
 #include "nsIUnicodeDecoder.h"
 
 namespace mozilla {
 namespace dom {
 
 class URLSearchParams;
 
@@ -87,25 +85,23 @@ private:
 
   void DeleteAll();
 
   void DecodeString(const nsACString& aInput, nsAString& aOutput);
   void ConvertString(const nsACString& aInput, nsAString& aOutput);
 
   void NotifyObservers(URLSearchParamsObserver* aExceptObserver);
 
-  static PLDHashOperator
-  CopyEnumerator(const nsAString& aName, nsTArray<nsString>* aArray,
-                 void *userData);
+  struct Param
+  {
+    nsString mKey;
+    nsString mValue;
+  };
 
-  static PLDHashOperator
-  SerializeEnumerator(const nsAString& aName, nsTArray<nsString>* aArray,
-                      void *userData);
-
-  nsClassHashtable<nsStringHashKey, nsTArray<nsString>> mSearchParams;
+  nsTArray<Param> mSearchParams;
 
   nsTArray<nsRefPtr<URLSearchParamsObserver>> mObservers;
   nsCOMPtr<nsIUnicodeDecoder> mDecoder;
 };
 
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/base/test/test_urlSearchParams.html
+++ b/dom/base/test/test_urlSearchParams.html
@@ -243,25 +243,54 @@ https://bugzilla.mozilla.org/show_bug.cg
     is(a.searchParams.get('bar'), 'foo', "a has bar=foo");
     is(b.searchParams.get('bar'), 'foo', "b has bar=foo");
     is(c.searchParams.get('bar'), 'foo', "c has bar=foo");
     is(d.searchParams.get('bar'), 'foo', "d has bar=foo");
 
     runTest();
   }
 
+  function testOrdering() {
+    var a = new URLSearchParams("a=1&a=2&b=3&c=4&c=5&a=6");
+    is(a.toString(), "a=1&a=2&b=3&c=4&c=5&a=6", "Order is correct");
+    is(a.getAll('a').length, 3, "Correct length of getAll()");
+
+    var b = new URLSearchParams();
+    b.append('a', '1');
+    b.append('b', '2');
+    b.append('a', '3');
+    is(b.toString(), "a=1&b=2&a=3", "Order is correct");
+    is(b.getAll('a').length, 2, "Correct length of getAll()");
+
+    runTest();
+  }
+
+  function testDelete() {
+    var a = new URLSearchParams("a=1&a=2&b=3&c=4&c=5&a=6");
+    is(a.toString(), "a=1&a=2&b=3&c=4&c=5&a=6", "Order is correct");
+    is(a.getAll('a').length, 3, "Correct length of getAll()");
+
+    a.delete('a');
+    is(a.getAll('a').length, 0, "Correct length of getAll()");
+    is(a.toString(), "b=3&c=4&c=5", "Order is correct");
+
+    runTest();
+  }
+
   var tests = [
     testSimpleURLSearchParams,
     testCopyURLSearchParams,
     testParserURLSearchParams,
     testURL,
     function() { testElement(document.getElementById('anchor')) },
     function() { testElement(document.getElementById('area')) },
     testEncoding,
-    testMultiURL
+    testMultiURL,
+    testOrdering,
+    testDelete
   ];
 
   function runTest() {
     if (!tests.length) {
       SimpleTest.finish();
       return;
     }