Bug 1215559. Create separate iterator native types for one-type and two-type iterators, so we don't force the one-type case to implement GetKeyAtIndex. r=qdot

dom/bindings/Codegen.py

@@ -12,7 +12,7 @@ import textwrap
 import functools
 
 from WebIDL import BuiltinTypes, IDLBuiltinType, IDLNullValue, IDLSequenceType, IDLType, IDLAttribute, IDLInterfaceMember, IDLUndefinedValue, IDLEmptySequenceValue, IDLDictionary
-from Configuration import NoSuchDescriptorError, getTypesFromDescriptor, getTypesFromDictionary, getTypesFromCallback, getAllTypes, Descriptor, MemberIsUnforgeable
+from Configuration import NoSuchDescriptorError, getTypesFromDescriptor, getTypesFromDictionary, getTypesFromCallback, getAllTypes, Descriptor, MemberIsUnforgeable, iteratorNativeType
 
 AUTOGENERATED_WARNING_COMMENT = \
     "/* THIS FILE IS AUTOGENERATED BY Codegen.py - DO NOT EDIT */\n\n"
@@ -15472,12 +15472,12 @@ class CGIterableMethodGenerator(CGGeneric):
     def __init__(self, descriptor, iterable, methodName):
         CGGeneric.__init__(self, fill(
             """
-            typedef IterableIterator<${nativeType}> itrType;
+            typedef ${iterClass} itrType;
             nsRefPtr<itrType> result(new itrType(self,
                                                  itrType::IterableIteratorType::${itrMethod},
                                                  &${ifaceName}IteratorBinding::Wrap));
             """,
-            nativeType=descriptor.nativeType,
+            iterClass=iteratorNativeType(descriptor),
             ifaceName=descriptor.interface.identifier.name,
             itrMethod=methodName.title()))
 

dom/bindings/Configuration.py

@@ -360,8 +360,7 @@ class Descriptor(DescriptorProvider):
         if self.interface.isIteratorInterface():
             itrName = self.interface.iterableInterface.identifier.name
             itrDesc = self.getDescriptor(itrName)
-            nativeTypeDefault = ("mozilla::dom::IterableIterator<%s>"
-                                 % itrDesc.nativeType)
+            nativeTypeDefault = iteratorNativeType(itrDesc)
 
         elif self.interface.isExternal():
             assert not self.workers
@@ -890,3 +889,12 @@ def getAllTypes(descriptors, dictionaries, callbacks):
     for callback in callbacks:
         for t in getTypesFromCallback(callback):
             yield (t, None, None)
+
+def iteratorNativeType(descriptor):
+    assert descriptor.interface.isIterable()
+    iterableDecl = descriptor.interface.maplikeOrSetlikeOrIterable
+    if iterableDecl.valueType is None:
+        iterClass = "OneTypeIterableIterator"
+    else:
+        iterClass = "TwoTypeIterableIterator"
+    return "mozilla::dom::%s<%s>" % (iterClass, descriptor.nativeType)

dom/bindings/IterableIterator.h

@@ -9,16 +9,18 @@
  * iterable<> member defined. It handles the ES6 Iterator-like functions that
  * are generated for the iterable interface.
  *
- * For iterable interfaces, the implementation class will need to contain three
- * functions:
+ * For iterable interfaces, the implementation class will need to
+ * implement these two functions:
  *
  * - size_t GetIterableLength()
  *   - Returns the number of elements available to iterate over
+ * - [type] GetValueAtIndex(size_t index)
+ *   - Returns the value at the requested index.
+ *
+ * If this is a two-type iterator, then the implementation class will also need to implement:
+ *
  * - [type] GetKeyAtIndex(size_t index)
  *   - Returns the key at the requested index
- * - [type] GetValueAtIndex(size_t index)
- *   - Returns the value at the requested index, or the key again if this is
- *     a single type iterator.
  *
  * Examples of iterable interface implementations can be found in the bindings
  * test directory.
@@ -58,25 +60,17 @@ protected:
 };
 
 template <typename T>
-class IterableIterator final : public IterableIteratorBase
+class IterableIterator : public IterableIteratorBase
 {
 public:
-
-  typedef bool (*WrapFunc)(JSContext* aCx,
-                           mozilla::dom::IterableIterator<T>* aObject,
-                           JS::Handle<JSObject*> aGivenProto,
-                           JS::MutableHandle<JSObject*> aReflector);
-  IterableIterator(T* aIterableObj, IterableIteratorType aIteratorType, WrapFunc aWrapFunc)
-    : mIteratorType(aIteratorType)
-    , mIterableObj(aIterableObj)
-    , mIndex(0)
-    , mWrapFunc(aWrapFunc)
+  IterableIterator(T* aIterableObj)
+    : mIterableObj(aIterableObj)
   {
     MOZ_ASSERT(mIterableObj);
-    MOZ_ASSERT(mWrapFunc);
   }
 
-  void
+protected:
+  static void
   DictReturn(JSContext* aCx, JS::MutableHandle<JSObject*> aResult,
              bool aDone, JS::Handle<JS::Value> aValue, ErrorResult& aRv)
   {
@@ -91,86 +85,33 @@ public:
     aResult.set(&dictValue.toObject());
   }
 
-  void
-  Next(JSContext* aCx, JS::MutableHandle<JSObject*> aResult, ErrorResult& aRv)
+  static void
+  KeyAndValueReturn(JSContext* aCx, JS::Handle<JS::Value> aKey,
+                    JS::Handle<JS::Value> aValue,
+                    JS::MutableHandle<JSObject*> aResult, ErrorResult& aRv)
   {
-    JS::Rooted<JS::Value> value(aCx, JS::UndefinedValue());
-    if (mIndex >= mIterableObj->GetIterableLength()) {
-      DictReturn(aCx, aResult, true, value, aRv);
+    RootedDictionary<IterableKeyAndValueResult> dict(aCx);
+    dict.mDone = false;
+    // Dictionary values are a Sequence, which is a FallibleTArray, so we need
+    // to check returns when appending.
+    if (!dict.mValue.AppendElement(aKey, mozilla::fallible)) {
+      aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
       return;
     }
-    switch (mIteratorType) {
-    case IterableIteratorType::Keys:
-    {
-      if (!ToJSValue(aCx, mIterableObj->GetKeyAtIndex(mIndex), &value)) {
-        aRv.Throw(NS_ERROR_FAILURE);
-        return;
-      }
-      DictReturn(aCx, aResult, false, value, aRv);
-      break;
+    if (!dict.mValue.AppendElement(aValue, mozilla::fallible)) {
+      aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
+      return;
     }
-    case IterableIteratorType::Values:
-    {
-      if (!ToJSValue(aCx, mIterableObj->GetValueAtIndex(mIndex), &value)) {
-        aRv.Throw(NS_ERROR_FAILURE);
-        return;
-      }
-      DictReturn(aCx, aResult, false, value, aRv);
-      break;
+    JS::Rooted<JS::Value> dictValue(aCx);
+    if (!ToJSValue(aCx, dict, &dictValue)) {
+      aRv.Throw(NS_ERROR_FAILURE);
+      return;
     }
-    case IterableIteratorType::Entries:
-    {
-      JS::Rooted<JS::Value> key(aCx);
-      if (!ToJSValue(aCx, mIterableObj->GetKeyAtIndex(mIndex), &key)) {
-        aRv.Throw(NS_ERROR_FAILURE);
-        return;
-      }
-      if (!ToJSValue(aCx, mIterableObj->GetValueAtIndex(mIndex), &value)) {
-        aRv.Throw(NS_ERROR_FAILURE);
-        return;
-      }
-      RootedDictionary<IterableKeyAndValueResult> dict(aCx);
-      dict.mDone = false;
-      // Dictionary values are a Sequence, which is a FallibleTArray, so we need
-      // to check returns when appending.
-      if (!dict.mValue.AppendElement(key, mozilla::fallible)) {
-        aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
-        return;
-      }
-      if (!dict.mValue.AppendElement(value, mozilla::fallible)) {
-        aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
-        return;
-      }
-      JS::Rooted<JS::Value> dictValue(aCx);
-      if (!ToJSValue(aCx, dict, &dictValue)) {
-        aRv.Throw(NS_ERROR_FAILURE);
-        return;
-      }
-      aResult.set(&dictValue.toObject());
-      break;
-    }
-    default:
-      MOZ_CRASH("Invalid iterator type!");
-    }
-    ++mIndex;
-  }
-  virtual ~IterableIterator() {}
-
-  bool
-  WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto, JS::MutableHandle<JSObject*> aObj)
-  {
-    return (*mWrapFunc)(aCx, this, aGivenProto, aObj);
+    aResult.set(&dictValue.toObject());
   }
 
 protected:
-  // Tells whether this is a key, value, or entries iterator.
-  IterableIteratorType mIteratorType;
-  // Binding Implementation Object that we're iterating over.
-  nsRefPtr<T> mIterableObj;
-  // Current index of iteration.
-  uint32_t mIndex;
-  // Function pointer to binding-type-specific Wrap() call for this iterator.
-  WrapFunc mWrapFunc;
+  virtual ~IterableIterator() {}
 
   // Since we're templated on a binding, we need to possibly CC it, but can't do
   // that through macros. So it happens here.
@@ -184,6 +125,170 @@ protected:
     IterableIterator<T>* tmp = this;
     NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mIterableObj);
   }
+
+  // Binding Implementation object that we're iterating over.
+  nsRefPtr<T> mIterableObj;
+};
+
+template<typename T>
+class OneTypeIterableIterator final : public IterableIterator<T>
+{
+public:
+  using typename IterableIterator<T>::IterableIteratorType;
+  using IterableIterator<T>::DictReturn;
+  using IterableIterator<T>::KeyAndValueReturn;
+  typedef bool (*WrapFunc)(JSContext* aCx,
+                           OneTypeIterableIterator<T>* aObject,
+                           JS::Handle<JSObject*> aGivenProto,
+                           JS::MutableHandle<JSObject*> aReflector);
+
+  OneTypeIterableIterator(T* aIterableObj,
+                          IterableIteratorType aIteratorType,
+                          WrapFunc aWrapFunc)
+    : IterableIterator<T>(aIterableObj)
+    , mIteratorType(aIteratorType)
+    , mWrapFunc(aWrapFunc)
+    , mIndex(0)
+  {
+    MOZ_ASSERT(mWrapFunc);
+  }
+
+  void
+  Next(JSContext* aCx, JS::MutableHandle<JSObject*> aResult, ErrorResult& aRv)
+  {
+    JS::Rooted<JS::Value> value(aCx, JS::UndefinedValue());
+    if (mIndex >= this->mIterableObj->GetIterableLength()) {
+      DictReturn(aCx, aResult, true, value, aRv);
+      return;
+    }
+
+    switch (mIteratorType) {
+    case IterableIteratorType::Keys:
+    case IterableIteratorType::Values:
+    {
+      if (!ToJSValue(aCx, this->mIterableObj->GetValueAtIndex(mIndex), &value)) {
+        aRv.Throw(NS_ERROR_FAILURE);
+        return;
+      }
+      DictReturn(aCx, aResult, false, value, aRv);
+      break;
+    }
+    case IterableIteratorType::Entries:
+    {
+      if (!ToJSValue(aCx, this->mIterableObj->GetValueAtIndex(mIndex), &value)) {
+        aRv.Throw(NS_ERROR_FAILURE);
+        return;
+      }
+      KeyAndValueReturn(aCx, value, value, aResult, aRv);
+      break;
+    }
+    default:
+      MOZ_CRASH("Invalid iterator type!");
+    }
+    ++mIndex;
+  }
+
+  bool
+  WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto, JS::MutableHandle<JSObject*> aObj)
+  {
+    return (*mWrapFunc)(aCx, this, aGivenProto, aObj);
+  }
+
+protected:
+  virtual ~OneTypeIterableIterator() {}
+
+  // Tells whether this is a key, value, or entries iterator.
+  IterableIteratorType mIteratorType;
+  // Function pointer to binding-type-specific Wrap() call for this iterator.
+  WrapFunc mWrapFunc;
+  // Current index of iteration.
+  uint32_t mIndex;
+};
+
+template<typename T>
+class TwoTypeIterableIterator final : public IterableIterator<T>
+{
+public:
+  using typename IterableIterator<T>::IterableIteratorType;
+  using IterableIterator<T>::DictReturn;
+  using IterableIterator<T>::KeyAndValueReturn;
+  typedef bool (*WrapFunc)(JSContext* aCx,
+                           TwoTypeIterableIterator<T>* aObject,
+                           JS::Handle<JSObject*> aGivenProto,
+                           JS::MutableHandle<JSObject*> aReflector);
+
+  TwoTypeIterableIterator(T* aIterableObj, IterableIteratorType aIteratorType,
+                          WrapFunc aWrapFunc)
+    : IterableIterator<T>(aIterableObj)
+    , mIteratorType(aIteratorType)
+    , mWrapFunc(aWrapFunc)
+    , mIndex(0)
+  {
+    MOZ_ASSERT(mWrapFunc);
+  }
+
+  void
+  Next(JSContext* aCx, JS::MutableHandle<JSObject*> aResult, ErrorResult& aRv)
+  {
+    JS::Rooted<JS::Value> value(aCx, JS::UndefinedValue());
+    if (mIndex >= this->mIterableObj->GetIterableLength()) {
+      DictReturn(aCx, aResult, true, value, aRv);
+      return;
+    }
+    switch (mIteratorType) {
+    case IterableIteratorType::Keys:
+    {
+      if (!ToJSValue(aCx, this->mIterableObj->GetKeyAtIndex(mIndex), &value)) {
+        aRv.Throw(NS_ERROR_FAILURE);
+        return;
+      }
+      DictReturn(aCx, aResult, false, value, aRv);
+      break;
+    }
+    case IterableIteratorType::Values:
+    {
+      if (!ToJSValue(aCx, this->mIterableObj->GetValueAtIndex(mIndex), &value)) {
+        aRv.Throw(NS_ERROR_FAILURE);
+        return;
+      }
+      DictReturn(aCx, aResult, false, value, aRv);
+      break;
+    }
+    case IterableIteratorType::Entries:
+    {
+      JS::Rooted<JS::Value> key(aCx);
+      if (!ToJSValue(aCx, this->mIterableObj->GetKeyAtIndex(mIndex), &key)) {
+        aRv.Throw(NS_ERROR_FAILURE);
+        return;
+      }
+      if (!ToJSValue(aCx, this->mIterableObj->GetValueAtIndex(mIndex), &value)) {
+        aRv.Throw(NS_ERROR_FAILURE);
+        return;
+      }
+      KeyAndValueReturn(aCx, key, value, aResult, aRv);
+      break;
+    }
+    default:
+      MOZ_CRASH("Invalid iterator type!");
+    }
+    ++mIndex;
+  }
+
+  bool
+  WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto, JS::MutableHandle<JSObject*> aObj)
+  {
+    return (*mWrapFunc)(aCx, this, aGivenProto, aObj);
+  }
+
+protected:
+  virtual ~TwoTypeIterableIterator() {}
+
+  // Tells whether this is a key, value, or entries iterator.
+  IterableIteratorType mIteratorType;
+  // Function pointer to binding-type-specific Wrap() call for this iterator.
+  WrapFunc mWrapFunc;
+  // Current index of iteration.
+  uint32_t mIndex;
 };
 
 } // namespace dom

dom/bindings/test/TestInterfaceIterableSingle.cpp

@@ -62,17 +62,11 @@ TestInterfaceIterableSingle::GetIterableLength() const
 }
 
 uint32_t
-TestInterfaceIterableSingle::GetKeyAtIndex(uint32_t index) const
+TestInterfaceIterableSingle::GetValueAtIndex(uint32_t index) const
 {
   MOZ_ASSERT(index < mValues.Length());
   return mValues.ElementAt(index);
 }
 
-uint32_t
-TestInterfaceIterableSingle::GetValueAtIndex(uint32_t index) const
-{
-  return GetKeyAtIndex(index);
-}
-
 } // namespace dom
 } // namespace mozilla

dom/bindings/test/TestInterfaceIterableSingle.h

@@ -37,7 +37,6 @@ public:
     Constructor(const GlobalObject& aGlobal, ErrorResult& rv);
 
   size_t GetIterableLength() const;
-  uint32_t GetKeyAtIndex(uint32_t aIndex) const;
   uint32_t GetValueAtIndex(uint32_t aIndex) const;
 private:
   virtual ~TestInterfaceIterableSingle() {}