Bug 1330699 part 2. Change the MozMap API and data storage to more what we want record<> to look like. r=qdot,smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 15 Feb 2017 00:00:00 -0500
changeset 484456 0a7450e715b5d045fe5f80a8995d1f52824dbbe8
parent 484455 eca50d428320783af941bbfde44886c317a30822
child 484457 cd59aa1d7844870c78f9ae60a951d48d95adf6dd
push id45482
push userbmo:kmckinley@mozilla.com
push dateWed, 15 Feb 2017 09:52:37 +0000
reviewersqdot, smaug
bugs1330699
milestone54.0a1
Bug 1330699 part 2. Change the MozMap API and data storage to more what we want record<> to look like. r=qdot,smaug
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/MozMap.h
dom/bindings/test/TestBindingHeader.h
dom/bindings/test/TestCodeGen.webidl
dom/fetch/InternalHeaders.cpp
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -2063,40 +2063,34 @@ public:
         T* ptr = &val;
         SequenceTracer<T>::TraceSequence(trc, ptr, ptr+1);
       }
     }
   }
 };
 
 template<typename T>
-static void
-TraceMozMapValue(T* aValue, void* aClosure)
-{
-  JSTracer* trc = static_cast<JSTracer*>(aClosure);
-  // Act like it's a one-element sequence to leverage all that infrastructure.
-  SequenceTracer<T>::TraceSequence(trc, aValue, aValue + 1);
-}
-
-template<typename T>
 void TraceMozMap(JSTracer* trc, MozMap<T>& map)
 {
-  map.EnumerateValues(TraceMozMapValue<T>, trc);
+  for (auto& entry : map.Entries()) {
+    // Act like it's a one-element sequence to leverage all that infrastructure.
+    SequenceTracer<T>::TraceSequence(trc, &entry.mValue, &entry.mValue + 1);
+  }
 }
 
 // sequence<MozMap>
 template<typename T>
 class SequenceTracer<MozMap<T>, false, false, false>
 {
   explicit SequenceTracer() = delete; // Should never be instantiated
 
 public:
   static void TraceSequence(JSTracer* trc, MozMap<T>* seqp, MozMap<T>* end) {
     for (; seqp != end; ++seqp) {
-      seqp->EnumerateValues(TraceMozMapValue<T>, trc);
+      TraceMozMap(trc, *seqp);
     }
   }
 };
 
 template<typename T>
 void DoTraceSequence(JSTracer* trc, FallibleTArray<T>& seq)
 {
   SequenceTracer<T>::TraceSequence(trc, seq.Elements(),
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -4868,53 +4868,53 @@ def getJSToNativeConversionInfo(type, de
             # interfaces, which use an internal holder for the
             # conversion even when forceOwningType ends up true.
             "holderName": "tempHolder",
             "passedToJSImpl": "${passedToJSImpl}"
         })
 
         templateBody = fill(
             """
-            ${mozMapType} &mozMap = ${mozMapRef};
+            auto& mozMapEntries = ${mozMapRef}.Entries();
 
             JS::Rooted<JSObject*> mozMapObj(cx, &$${val}.toObject());
             JS::Rooted<JS::IdVector> ids(cx, JS::IdVector(cx));
             if (!JS_Enumerate(cx, mozMapObj, &ids)) {
               $*{exceptionCode}
             }
+            if (!mozMapEntries.SetCapacity(ids.length(), mozilla::fallible)) {
+              JS_ReportOutOfMemory(cx);
+              $*{exceptionCode}
+            }
             JS::Rooted<JS::Value> propNameValue(cx);
             JS::Rooted<JS::Value> temp(cx);
             JS::Rooted<jsid> curId(cx);
             for (size_t i = 0; i < ids.length(); ++i) {
-              // Make sure we get the value before converting the name, since
-              // getting the value can trigger GC but our name is a dependent
-              // string.
               curId = ids[i];
               binding_detail::FakeString propName;
               bool isSymbol;
               if (!ConvertIdToString(cx, curId, propName, isSymbol) ||
                   (!isSymbol && !JS_GetPropertyById(cx, mozMapObj, curId, &temp))) {
                 $*{exceptionCode}
               }
               if (isSymbol) {
                 continue;
               }
 
-              ${valueType}* slotPtr = mozMap.AddEntry(propName);
-              if (!slotPtr) {
-                JS_ReportOutOfMemory(cx);
-                $*{exceptionCode}
-              }
-              ${valueType}& slot = *slotPtr;
+              // Safe to do an infallible append here, because we did a
+              // SetCapacity above to the right capacity.
+              ${mozMapType}::EntryType* entry = mozMapEntries.AppendElement();
+              entry->mKey = propName;
+              ${valueType}& slot = entry->mValue;
               $*{valueConversion}
             }
             """,
             exceptionCode=exceptionCode,
+            mozMapRef=mozMapRef,
             mozMapType=mozMapType,
-            mozMapRef=mozMapRef,
             valueType=valueInfo.declType.define(),
             valueConversion=valueConversion)
 
         templateBody = wrapObjectTemplate(templateBody, type,
                                           "${declName}.SetNull();\n",
                                           notMozMap)
 
         declType = typeName
@@ -6522,34 +6522,33 @@ def getWrapTemplateForType(type, descrip
                 'exceptionCode': exceptionCode,
                 'obj': "returnObj",
                 'typedArraysAreStructs': typedArraysAreStructs
             })
         mozMapWrapLevel -= 1
         code = fill(
             """
 
-            nsTArray<nsString> keys;
-            ${result}.GetKeys(keys);
             JS::Rooted<JSObject*> returnObj(cx, JS_NewPlainObject(cx));
             if (!returnObj) {
               $*{exceptionCode}
             }
             // Scope for 'tmp'
             {
               JS::Rooted<JS::Value> tmp(cx);
-              for (size_t idx = 0; idx < keys.Length(); ++idx) {
-                auto& ${valueName} = ${result}.Get(keys[idx]);
+              for (auto& entry : ${result}.Entries()) {
+                auto& ${valueName} = entry.mValue;
                 // Control block to let us common up the JS_DefineUCProperty calls when there
                 // are different ways to succeed at wrapping the value.
                 do {
                   $*{innerTemplate}
                 } while (0);
-                if (!JS_DefineUCProperty(cx, returnObj, keys[idx].get(),
-                                         keys[idx].Length(), tmp,
+                if (!JS_DefineUCProperty(cx, returnObj,
+                                         entry.mKey.BeginReading(),
+                                         entry.mKey.Length(), tmp,
                                          JSPROP_ENUMERATE)) {
                   $*{exceptionCode}
                 }
               }
             }
             $*{set}
             """,
             result=result,
@@ -7275,38 +7274,36 @@ def wrapTypeIntoCurrentCompartment(type,
                              pre=("for (uint32_t %s = 0; %s < %s.Length(); ++%s) {\n" %
                                   (index, index, value, index)),
                              post="}\n")
         if origType.nullable():
             wrapCode = CGIfWrapper(wrapCode, "!%s.IsNull()" % origValue)
         return wrapCode
 
     if type.isMozMap():
-        origValue = value
         origType = type
         if type.nullable():
             type = type.inner
-            value = "%s.Value()" % value
+            mozMapRef = "%s.Value()" % value
+        else:
+            mozMapRef = value
         global mapWrapLevel
-        key = "mapName%d" % mapWrapLevel
+        entryRef = "mapEntry%d" % mapWrapLevel
         mapWrapLevel += 1
         wrapElement = wrapTypeIntoCurrentCompartment(type.inner,
-                                                     "%s.Get(%sKeys[%sIndex])" % (value, key, key))
+                                                     "%s.mValue" % entryRef)
         mapWrapLevel -= 1
         if not wrapElement:
             return None
         wrapCode = CGWrapper(CGIndenter(wrapElement),
-                             pre=("""
-                                  nsTArray<nsString> %sKeys;
-                                  %s.GetKeys(%sKeys);
-                                  for (uint32_t %sIndex = 0; %sIndex < %sKeys.Length(); ++%sIndex) {
-                                  """ % (key, value, key, key, key, key, key)),
+                             pre=("for (auto& %s : %s.Entries()) {\n" %
+                                  (entryRef, mozMapRef)),
                              post="}\n")
         if origType.nullable():
-            wrapCode = CGIfWrapper(wrapCode, "!%s.IsNull()" % origValue)
+            wrapCode = CGIfWrapper(wrapCode, "!%s.IsNull()" % value)
         return wrapCode
 
     if type.isDictionary():
         assert not type.nullable()
         myDict = type.inner
         memberWraps = []
         while myDict:
             for member in myDict.members:
--- a/dom/bindings/MozMap.h
+++ b/dom/bindings/MozMap.h
@@ -1,121 +1,81 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /**
- * Class for representing MozMap arguments.  This is an nsTHashtable
- * under the hood, but we don't want to leak that implementation
- * detail.
+ * Class for representing MozMap arguments.  Basically an array under the hood.
  */
 
 #ifndef mozilla_dom_MozMap_h
 #define mozilla_dom_MozMap_h
 
 #include "nsTHashtable.h"
 #include "nsHashKeys.h"
 #include "nsStringGlue.h"
 #include "nsTArray.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Move.h"
 
 namespace mozilla {
 namespace dom {
 
 namespace binding_detail {
-template<typename DataType>
-class MozMapEntry : public nsStringHashKey
+template<typename KeyType, typename ValueType>
+class MozMapEntry
 {
 public:
-  explicit MozMapEntry(const nsAString* aKeyTypePointer)
-    : nsStringHashKey(aKeyTypePointer)
+  MozMapEntry()
   {
   }
 
   // Move constructor so we can do MozMaps of MozMaps.
-  MozMapEntry(MozMapEntry<DataType>&& aOther)
-    : nsStringHashKey(aOther),
-      mData(Move(aOther.mData))
+  MozMapEntry(MozMapEntry<KeyType, ValueType>&& aOther)
+    : mKey(Move(aOther.mKey)),
+      mValue(Move(aOther.mValue))
   {
   }
 
-  DataType mData;
+  KeyType mKey;
+  ValueType mValue;
 };
 
 } // namespace binding_detail
 
-template<typename DataType>
-class MozMap : protected nsTHashtable<binding_detail::MozMapEntry<DataType>>
+template<typename ValueType>
+class MozMap
 {
 public:
-  typedef typename binding_detail::MozMapEntry<DataType> EntryType;
-  typedef nsTHashtable<EntryType> Base;
-  typedef MozMap<DataType> SelfType;
+  typedef nsString KeyType;
+  typedef typename binding_detail::MozMapEntry<KeyType, ValueType> EntryType;
+  typedef MozMap<ValueType> SelfType;
 
   MozMap()
   {
   }
 
   // Move constructor so we can do MozMap of MozMap.
   MozMap(SelfType&& aOther) :
-    Base(Move(aOther))
+    mEntries(Move(aOther.mEntries))
   {
   }
 
-  // The return value is only safe to use until an AddEntry call.
-  const DataType& Get(const nsAString& aKey) const
-  {
-    const EntryType* ent = this->GetEntry(aKey);
-    MOZ_ASSERT(ent, "Why are you using a key we didn't claim to have?");
-    return ent->mData;
-  }
-
-  DataType& Get(const nsAString& aKey)
+  const nsTArray<EntryType>& Entries() const
   {
-    EntryType* ent = this->GetEntry(aKey);
-    MOZ_ASSERT(ent, "Why are you using a key we didn't claim to have?");
-    return ent->mData;
-  }
-
-  // The return value is only safe to use until an AddEntry call.
-  const DataType* GetIfExists(const nsAString& aKey) const
-  {
-    const EntryType* ent = this->GetEntry(aKey);
-    if (!ent) {
-      return nullptr;
-    }
-    return &ent->mData;
+    return mEntries;
   }
 
-  void GetKeys(nsTArray<nsString>& aKeys) const {
-    for (auto iter = this->ConstIter(); !iter.Done(); iter.Next()) {
-      aKeys.AppendElement(iter.Get()->GetKey());
-    }
+  nsTArray<EntryType>& Entries()
+  {
+    return mEntries;
   }
 
-  // XXXbz we expose this generic enumerator for tracing.  Otherwise we'd end up
-  // with a dependency on BindingUtils.h here for the SequenceTracer bits.
-  typedef void (* Enumerator)(DataType* aValue, void* aClosure);
-  void EnumerateValues(Enumerator aEnumerator, void *aClosure)
-  {
-    for (auto iter = this->Iter(); !iter.Done(); iter.Next()) {
-      aEnumerator(&iter.Get()->mData, aClosure);
-    }
-  }
-
-  MOZ_MUST_USE
-  DataType* AddEntry(const nsAString& aKey)
-  {
-    EntryType* ent = this->PutEntry(aKey, fallible);
-    if (!ent) {
-      return nullptr;
-    }
-    return &ent->mData;
-  }
+private:
+  nsTArray<EntryType> mEntries;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_MozMap_h
--- a/dom/bindings/test/TestBindingHeader.h
+++ b/dom/bindings/test/TestBindingHeader.h
@@ -168,16 +168,31 @@ public:
                                         const Optional<JS::Handle<JSObject*> >&,
                                         ErrorResult&);
 
   static
   already_AddRefed<TestInterface> Test3(const GlobalObject&,
                                         const LongOrAnyMozMap&,
                                         ErrorResult&);
 
+  static
+  already_AddRefed<TestInterface> Test4(const GlobalObject&,
+                                        const MozMap<MozMap<JS::Value>>&,
+                                        ErrorResult&);
+
+  static
+  already_AddRefed<TestInterface> Test5(const GlobalObject&,
+                                        const MozMap<Sequence<MozMap<MozMap<Sequence<Sequence<JS::Value>>>>>>&,
+                                        ErrorResult&);
+
+  static
+  already_AddRefed<TestInterface> Test6(const GlobalObject&,
+                                        const Sequence<MozMap<Sequence<Sequence<MozMap<MozMap<JS::Value>>>>>>&,
+                                        ErrorResult&);
+
   // Integer types
   int8_t ReadonlyByte();
   int8_t WritableByte();
   void SetWritableByte(int8_t);
   void PassByte(int8_t);
   int8_t ReceiveByte();
   void PassOptionalByte(const Optional<int8_t>&);
   void PassOptionalByteBeforeRequired(const Optional<int8_t>&, int8_t);
--- a/dom/bindings/test/TestCodeGen.webidl
+++ b/dom/bindings/test/TestCodeGen.webidl
@@ -135,17 +135,20 @@ interface OnlyForUseInConstructor {
  Constructor(ArrayBuffer arrayBuf),
  Constructor(Uint8Array typedArr),
  // Constructor(long arg1, long arg2, (TestInterface or OnlyForUseInConstructor) arg3),
  NamedConstructor=Test,
  NamedConstructor=Test(DOMString str),
  NamedConstructor=Test2(DictForConstructor dict, any any1, object obj1,
                         object? obj2, sequence<Dict> seq, optional any any2,
                         optional object obj3, optional object? obj4),
- NamedConstructor=Test3((long or MozMap<any>) arg1)
+ NamedConstructor=Test3((long or MozMap<any>) arg1),
+ NamedConstructor=Test4(MozMap<MozMap<any>> arg1),
+ NamedConstructor=Test5(MozMap<sequence<MozMap<MozMap<sequence<sequence<any>>>>>> arg1),
+ NamedConstructor=Test6(sequence<MozMap<sequence<sequence<MozMap<MozMap<any>>>>>> arg1),
  ]
 interface TestInterface {
   // Integer types
   // XXXbz add tests for throwing versions of all the integer stuff
   readonly attribute byte readonlyByte;
   attribute byte writableByte;
   void passByte(byte arg);
   byte receiveByte();
--- a/dom/fetch/InternalHeaders.cpp
+++ b/dom/fetch/InternalHeaders.cpp
@@ -303,20 +303,21 @@ InternalHeaders::Fill(const Sequence<Seq
     }
     Append(tuple[0], tuple[1], aRv);
   }
 }
 
 void
 InternalHeaders::Fill(const MozMap<nsCString>& aInit, ErrorResult& aRv)
 {
-  nsTArray<nsString> keys;
-  aInit.GetKeys(keys);
-  for (uint32_t i = 0; i < keys.Length() && !aRv.Failed(); ++i) {
-    Append(NS_ConvertUTF16toUTF8(keys[i]), aInit.Get(keys[i]), aRv);
+  for (auto& entry : aInit.Entries()) {
+    Append(NS_ConvertUTF16toUTF8(entry.mKey), entry.mValue, aRv);
+    if (aRv.Failed()) {
+      return;
+    }
   }
 }
 
 namespace {
 
 class FillHeaders final : public nsIHttpHeaderVisitor
 {
   RefPtr<InternalHeaders> mInternalHeaders;