Bug 1216751 part 1. Restrict value iterators to interfaces that have indexed properties and pair iterators to interfaces that do not have indexed properties. r=qdot
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 17 Feb 2016 22:57:57 -0500
changeset 284618 5fc801a87404a990d15075b40a20cb8155564572
parent 284617 37f1e7428a70c408ed73b0d7c1ea92d43d1239a8
child 284619 efad6236ce3f53cc3aa96bef3438882248423d2c
push id72042
push userbzbarsky@mozilla.com
push dateThu, 18 Feb 2016 03:58:26 +0000
treeherdermozilla-inbound@410ef34da2e7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot
bugs1216751
milestone47.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 1216751 part 1. Restrict value iterators to interfaces that have indexed properties and pair iterators to interfaces that do not have indexed properties. r=qdot
dom/bindings/Codegen.py
dom/bindings/Configuration.py
dom/bindings/parser/WebIDL.py
dom/bindings/parser/tests/test_interface_maplikesetlikeiterable.py
dom/bindings/test/TestInterfaceIterableSingle.cpp
dom/bindings/test/TestInterfaceIterableSingle.h
dom/bindings/test/test_iterable.html
dom/webidl/TestInterfaceJSMaplikeSetlikeIterable.webidl
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -1131,21 +1131,23 @@ class CGHeaders(CGWrapper):
             # getExtendedAttribute() returns a list, extract the entry.
             funcList = desc.interface.getExtendedAttribute("Func")
             if funcList is not None:
                 addHeaderForFunc(funcList[0])
 
             if desc.interface.maplikeOrSetlikeOrIterable:
                 # We need ToJSValue.h for maplike/setlike type conversions
                 bindingHeaders.add("mozilla/dom/ToJSValue.h")
-                # Add headers for the key and value types of the maplike, since
-                # they'll be needed for convenience functions
-                addHeadersForType((desc.interface.maplikeOrSetlikeOrIterable.keyType,
-                                   desc, None))
-                if desc.interface.maplikeOrSetlikeOrIterable.valueType:
+                # Add headers for the key and value types of the
+                # maplike/setlike/iterable, since they'll be needed for
+                # convenience functions
+                if desc.interface.maplikeOrSetlikeOrIterable.hasKeyType():
+                    addHeadersForType((desc.interface.maplikeOrSetlikeOrIterable.keyType,
+                                       desc, None))
+                if desc.interface.maplikeOrSetlikeOrIterable.hasValueType():
                     addHeadersForType((desc.interface.maplikeOrSetlikeOrIterable.valueType,
                                        desc, None))
 
         for d in dictionaries:
             if d.parent:
                 declareIncludes.add(self.getDeclarationFilename(d.parent))
             bindingHeaders.add(self.getDeclarationFilename(d))
 
@@ -2276,35 +2278,39 @@ class MethodDefiner(PropertyDefiner):
                 "name": "@@iterator",
                 "methodInfo": False,
                 "selfHostedName": "ArrayValues",
                 "length": 0,
                 "flags": "JSPROP_ENUMERATE",
                 "condition": MemberCondition()
             })
 
-        # Generate the maplike/setlike iterator, if one wasn't already
+        # Generate the maplike/setlike/iterable iterator, if one wasn't already
         # generated by a method. If we already have an @@iterator symbol, fail.
-        if descriptor.interface.maplikeOrSetlikeOrIterable:
+        # Note that for value iterators we don't need to do that, since those
+        # are only supported on interfaces that support indexed properties,
+        # which get an @@iterator automatically.
+        maplikeOrSetlikeOrIterable = descriptor.interface.maplikeOrSetlikeOrIterable
+        if (maplikeOrSetlikeOrIterable and
+            (not maplikeOrSetlikeOrIterable.isIterable() or
+             not maplikeOrSetlikeOrIterable.isValueIterator())):
             if hasIterator(methods, self.regular):
                 raise TypeError("Cannot have maplike/setlike/iterable interface with "
                                 "other members that generate @@iterator "
                                 "on interface %s, such as indexed getters "
                                 "or aliased functions." %
                                 self.descriptor.interface.identifier.name)
             for m in methods:
                 if (m.isMaplikeOrSetlikeOrIterableMethod() and
                     (((m.maplikeOrSetlikeOrIterable.isMaplike() or
                        (m.maplikeOrSetlikeOrIterable.isIterable() and
-                        m.maplikeOrSetlikeOrIterable.hasValueType())) and
+                        m.maplikeOrSetlikeOrIterable.isPairIterator())) and
                        m.identifier.name == "entries") or
-                    (((m.maplikeOrSetlikeOrIterable.isSetlike() or
-                       (m.maplikeOrSetlikeOrIterable.isIterable() and
-                        not m.maplikeOrSetlikeOrIterable.hasValueType()))) and
-                       m.identifier.name == "values"))):
+                    (m.maplikeOrSetlikeOrIterable.isSetlike() and
+                     m.identifier.name == "values"))):
                     self.regular.append({
                         "name": "@@iterator",
                         "methodName": m.identifier.name,
                         "length": methodLength(m),
                         "flags": "0",
                         "condition": PropertyDefiner.getControllingCondition(m,
                                                                              descriptor),
                     })
@@ -13068,18 +13074,19 @@ class CGForwardDeclarations(CGWrapper):
             builder.add(d.nativeType)
             # If we're an interface and we have a maplike/setlike declaration,
             # we'll have helper functions exposed to the native side of our
             # bindings, which will need to show up in the header. If either of
             # our key/value types are interfaces, they'll be passed as
             # arguments to helper functions, and they'll need to be forward
             # declared in the header.
             if d.interface.maplikeOrSetlikeOrIterable:
-                builder.forwardDeclareForType(d.interface.maplikeOrSetlikeOrIterable.keyType,
-                                              config)
+                if d.interface.maplikeOrSetlikeOrIterable.hasKeyType():
+                    builder.forwardDeclareForType(d.interface.maplikeOrSetlikeOrIterable.keyType,
+                                                  config)
                 if d.interface.maplikeOrSetlikeOrIterable.hasValueType():
                     builder.forwardDeclareForType(d.interface.maplikeOrSetlikeOrIterable.valueType,
                                                   config)
 
         # We just about always need NativePropertyHooks
         builder.addInMozillaDom("NativePropertyHooks", isStruct=True)
         builder.addInMozillaDom("ProtoAndIfaceCache")
         # Add the atoms cache type, even if we don't need it.
--- a/dom/bindings/Configuration.py
+++ b/dom/bindings/Configuration.py
@@ -903,13 +903,13 @@ def getAllTypes(descriptors, dictionarie
             yield (t, None, dictionary)
     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:
+    if iterableDecl.isValueIterator():
         iterClass = "OneTypeIterableIterator"
     else:
         iterClass = "TwoTypeIterableIterator"
     return "mozilla::dom::%s<%s>" % (iterClass, descriptor.nativeType)
--- a/dom/bindings/parser/WebIDL.py
+++ b/dom/bindings/parser/WebIDL.py
@@ -1093,17 +1093,17 @@ class IDLInterface(IDLObjectWithScope, I
                     raise WebIDLError("Interface with [Global] inherits from "
                                       "interface with [OverrideBuiltins]",
                                       [self.location, parent.location])
                 parent._isOnGlobalProtoChain = True
                 parent = parent.parent
 
     def validate(self):
         # We don't support consequential unforgeable interfaces.  Need to check
-        # this here, becaue in finish() an interface might not know yet that
+        # this here, because in finish() an interface might not know yet that
         # it's consequential.
         if self.getExtendedAttribute("Unforgeable") and self.isConsequential():
             raise WebIDLError(
                 "%s is an unforgeable consequential interface" %
                 self.identifier.name,
                 [self.location] +
                 list(i.location for i in
                      (self.interfacesBasedOnSelf - {self})))
@@ -1112,28 +1112,35 @@ class IDLInterface(IDLObjectWithScope, I
         if self.getExtendedAttribute("Unforgeable") and self.hasChildInterfaces():
             locations = ([self.location] +
                          list(i.location for i in
                               self.interfacesBasedOnSelf if i.parent == self))
             raise WebIDLError("%s is an unforgeable ancestor interface" %
                               self.identifier.name,
                               locations)
 
+        indexedGetter = None
+        hasLengthAttribute = False
         for member in self.members:
             member.validate()
 
             if self.isCallback() and member.getExtendedAttribute("Replaceable"):
                 raise WebIDLError("[Replaceable] used on an attribute on "
                                   "interface %s which is a callback interface" %
                                   self.identifier.name,
                                   [self.location, member.location])
 
             # Check that PutForwards refers to another attribute and that no
-            # cycles exist in forwarded assignments.
+            # cycles exist in forwarded assignments.  Also check for a
+            # integer-typed "length" attribute.
             if member.isAttr():
+                if (member.identifier.name == "length" and
+                    member.type.isInteger()):
+                    hasLengthAttribute = True
+
                 iface = self
                 attr = member
                 putForwards = attr.getExtendedAttribute("PutForwards")
                 if putForwards and self.isCallback():
                     raise WebIDLError("[PutForwards] used on an attribute "
                                       "on interface %s which is a callback "
                                       "interface" % self.identifier.name,
                                       [self.location, member.location])
@@ -1161,18 +1168,21 @@ class IDLInterface(IDLObjectWithScope, I
                                           (attr.identifier.name, iface, putForwards),
                                           [attr.location])
 
                     iface = forwardIface
                     attr = fowardAttr
                     putForwards = attr.getExtendedAttribute("PutForwards")
 
             # Check that the name of an [Alias] doesn't conflict with an
-            # interface member.
+            # interface member and whether we support indexed properties.
             if member.isMethod():
+                if member.isGetter() and member.isIndexed():
+                    indexedGetter = member
+
                 for alias in member.aliases:
                     if self.isOnGlobalProtoChain():
                         raise WebIDLError("[Alias] must not be used on a "
                                           "[Global] interface operation",
                                           [member.location])
                     if (member.getExtendedAttribute("Exposed") or
                         member.getExtendedAttribute("ChromeOnly") or
                         member.getExtendedAttribute("Pref") or
@@ -1223,16 +1233,45 @@ class IDLInterface(IDLObjectWithScope, I
         # interface object, unless they're navigator properties.
         if (self.isExposedConditionally() and
             not self.hasInterfaceObject() and
             not self.getNavigatorProperty()):
             raise WebIDLError("Interface with no interface object is "
                               "exposed conditionally",
                               [self.location])
 
+        # Value iterators are only allowed on interfaces with indexed getters,
+        # and pair iterators are only allowed on interfaces without indexed
+        # getters.
+        if self.isIterable():
+            iterableDecl = self.maplikeOrSetlikeOrIterable
+            if iterableDecl.isValueIterator():
+                if not indexedGetter:
+                    raise WebIDLError("Interface with value iterator does not "
+                                      "support indexed properties",
+                                      [self.location])
+
+                if iterableDecl.valueType != indexedGetter.signatures()[0][0]:
+                    raise WebIDLError("Iterable type does not match indexed "
+                                      "getter type",
+                                      [iterableDecl.location,
+                                       indexedGetter.location])
+
+                if not hasLengthAttribute:
+                    raise WebIDLError('Interface with value iterator does not '
+                                      'have an integer-typed "length" attribute',
+                                      [self.location])
+            else:
+                assert iterableDecl.isPairIterator()
+                if indexedGetter:
+                    raise WebIDLError("Interface with pair iterator supports "
+                                      "indexed properties",
+                                      [self.location, iterableDecl.location,
+                                       indexedGetter.location])
+
     def isInterface(self):
         return True
 
     def isExternal(self):
         return False
 
     def setIsConsequentialInterfaceOf(self, other):
         self._consequential = True
@@ -3421,17 +3460,20 @@ class IDLInterfaceMember(IDLObjectWithId
                               [self.location])
         self.aliases.append(alias)
 
 
 class IDLMaplikeOrSetlikeOrIterableBase(IDLInterfaceMember):
 
     def __init__(self, location, identifier, ifaceType, keyType, valueType, ifaceKind):
         IDLInterfaceMember.__init__(self, location, identifier, ifaceKind)
-        assert isinstance(keyType, IDLType)
+        if keyType is not None:
+            assert isinstance(keyType, IDLType)
+        else:
+            assert valueType is not None
         assert ifaceType in ['maplike', 'setlike', 'iterable']
         if valueType is not None:
             assert isinstance(valueType, IDLType)
         self.keyType = keyType
         self.valueType = valueType
         self.maplikeOrSetlikeOrIterableType = ifaceType
         self.disallowedMemberNames = []
         self.disallowedNonMethodNames = []
@@ -3440,16 +3482,19 @@ class IDLMaplikeOrSetlikeOrIterableBase(
         return self.maplikeOrSetlikeOrIterableType == "maplike"
 
     def isSetlike(self):
         return self.maplikeOrSetlikeOrIterableType == "setlike"
 
     def isIterable(self):
         return self.maplikeOrSetlikeOrIterableType == "iterable"
 
+    def hasKeyType(self):
+        return self.keyType is not None
+
     def hasValueType(self):
         return self.valueType is not None
 
     def checkCollisions(self, members, isAncestor):
         for member in members:
             # Check that there are no disallowed members
             if (member.identifier.name in self.disallowedMemberNames and
                 not ((member.isMethod() and member.isMaplikeOrSetlikeOrIterableMethod()) or
@@ -3527,23 +3572,24 @@ class IDLMaplikeOrSetlikeOrIterableBase(
                 [IDLExtendedAttribute(self.location, ("DependsOn", "Everything")),
                  IDLExtendedAttribute(self.location, ("Affects", "Nothing"))])
         if newObject:
             method.addExtendedAttributes(
                 [IDLExtendedAttribute(self.location, ("NewObject",))])
         members.append(method)
 
     def resolve(self, parentScope):
-        self.keyType.resolveType(parentScope)
+        if self.keyType:
+            self.keyType.resolveType(parentScope)
         if self.valueType:
             self.valueType.resolveType(parentScope)
 
     def finish(self, scope):
         IDLInterfaceMember.finish(self, scope)
-        if not self.keyType.isComplete():
+        if self.keyType and not self.keyType.isComplete():
             t = self.keyType.complete(scope)
 
             assert not isinstance(t, IDLUnresolvedType)
             assert not isinstance(t, IDLTypedefType)
             assert not isinstance(t.name, IDLUnresolvedIdentifier)
             self.keyType = t
         if self.valueType and not self.valueType.isComplete():
             t = self.valueType.complete(scope)
@@ -3555,19 +3601,22 @@ class IDLMaplikeOrSetlikeOrIterableBase(
 
     def validate(self):
         IDLInterfaceMember.validate(self)
 
     def handleExtendedAttribute(self, attr):
         IDLInterfaceMember.handleExtendedAttribute(self, attr)
 
     def _getDependentObjects(self):
+        deps = set()
+        if self.keyType:
+            deps.add(self.keyType)
         if self.valueType:
-            return set([self.keyType, self.valueType])
-        return set([self.keyType])
+            deps.add(self.valueType)
+        return deps
 
 # Iterable adds ES6 iterator style functions and traits
 # (keys/values/entries/@@iterator) to an interface.
 class IDLIterable(IDLMaplikeOrSetlikeOrIterableBase):
 
     def __init__(self, location, identifier, keyType, valueType=None, scope=None):
         IDLMaplikeOrSetlikeOrIterableBase.__init__(self, location, identifier,
                                                    "iterable", keyType, valueType,
@@ -3588,16 +3637,22 @@ class IDLIterable(IDLMaplikeOrSetlikeOrI
                        affectsNothing=True, newObject=True)
         # object keys()
         self.addMethod("keys", members, False, self.iteratorType,
                        affectsNothing=True, newObject=True)
         # object values()
         self.addMethod("values", members, False, self.iteratorType,
                        affectsNothing=True, newObject=True)
 
+    def isValueIterator(self):
+        return not self.isPairIterator()
+
+    def isPairIterator(self):
+        return self.hasKeyType()
+
 # MaplikeOrSetlike adds ES6 map-or-set-like traits to an interface.
 class IDLMaplikeOrSetlike(IDLMaplikeOrSetlikeOrIterableBase):
 
     def __init__(self, location, identifier, maplikeOrSetlikeType,
                  readonly, keyType, valueType):
         IDLMaplikeOrSetlikeOrIterableBase.__init__(self, location, identifier, maplikeOrSetlikeType,
                                                    keyType, valueType, IDLInterfaceMember.Tags.MaplikeOrSetlike)
         self.readonly = readonly
@@ -5433,20 +5488,23 @@ class Parser(Tokenizer):
     def p_Iterable(self, p):
         """
             Iterable : ITERABLE LT Type GT SEMICOLON
                      | ITERABLE LT Type COMMA Type GT SEMICOLON
         """
         location = self.getLocation(p, 2)
         identifier = IDLUnresolvedIdentifier(location, "__iterable",
                                              allowDoubleUnderscore=True)
-        keyType = p[3]
-        valueType = None
         if (len(p) > 6):
+            keyType = p[3]
             valueType = p[5]
+        else:
+            keyType = None
+            valueType = p[3]
+
         p[0] = IDLIterable(location, identifier, keyType, valueType, self.globalScope())
 
     def p_Setlike(self, p):
         """
             Setlike : ReadOnly SETLIKE LT Type GT SEMICOLON
         """
         readonly = p[1]
         maplikeOrSetlikeType = p[2]
--- a/dom/bindings/parser/tests/test_interface_maplikesetlikeiterable.py
+++ b/dom/bindings/parser/tests/test_interface_maplikesetlikeiterable.py
@@ -73,33 +73,39 @@ def WebIDLTest(parser, harness):
                                                            "__clear",
                                                            "__delete"]] +
                           mapRWMembers)
 
     # OK, now that we've used iterableMembers to set up the above, append
     # __iterable to it for the iterable<> case.
     iterableMembers.append(("__iterable", WebIDL.IDLIterable))
 
+    valueIterableMembers = list(iterableMembers)
+    valueIterableMembers.append(("__indexedgetter", WebIDL.IDLMethod))
+    valueIterableMembers.append(("length", WebIDL.IDLAttribute))
+
     disallowedIterableNames = ["keys", "entries", "values"]
     disallowedMemberNames = ["forEach", "has", "size"] + disallowedIterableNames
     mapDisallowedMemberNames = ["get"] + disallowedMemberNames
     disallowedNonMethodNames = ["clear", "delete"]
     mapDisallowedNonMethodNames = ["set"] + disallowedNonMethodNames
     setDisallowedNonMethodNames = ["add"] + disallowedNonMethodNames
 
     #
     # Simple Usage Tests
     #
 
     shouldPass("Iterable (key only)",
                """
                interface Foo1 {
                iterable<long>;
+               readonly attribute unsigned long length;
+               getter long(unsigned long index);
                };
-               """, iterableMembers,
+               """, valueIterableMembers,
                # numProductions == 2 because of the generated iterator iface,
                numProductions=2)
 
     shouldPass("Iterable (key and value)",
                """
                interface Foo1 {
                iterable<long, long>;
                };
--- a/dom/bindings/test/TestInterfaceIterableSingle.cpp
+++ b/dom/bindings/test/TestInterfaceIterableSingle.cpp
@@ -18,17 +18,17 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(TestInt
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(TestInterfaceIterableSingle)
 NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
 NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
 
 TestInterfaceIterableSingle::TestInterfaceIterableSingle(nsPIDOMWindowInner* aParent)
   : mParent(aParent)
 {
-  for(int i = 0; i < 3; ++i) {
+  for (int i = 0; i < 3; ++i) {
     mValues.AppendElement(i);
   }
 }
 
 //static
 already_AddRefed<TestInterfaceIterableSingle>
 TestInterfaceIterableSingle::Constructor(const GlobalObject& aGlobal,
                                          ErrorResult& aRv)
@@ -56,17 +56,35 @@ TestInterfaceIterableSingle::GetParentOb
 }
 
 size_t
 TestInterfaceIterableSingle::GetIterableLength() const
 {
   return mValues.Length();
 }
 
-uint32_t
+int32_t
 TestInterfaceIterableSingle::GetValueAtIndex(uint32_t index) const
 {
   MOZ_ASSERT(index < mValues.Length());
   return mValues.ElementAt(index);
 }
 
+uint32_t
+TestInterfaceIterableSingle::Length() const
+{
+  return mValues.Length();
+}
+
+int32_t
+TestInterfaceIterableSingle::IndexedGetter(uint32_t aIndex, bool& aFound) const
+{
+  if (aIndex >= mValues.Length()) {
+    aFound = false;
+    return 0;
+  }
+
+  aFound = true;
+  return mValues[aIndex];
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/bindings/test/TestInterfaceIterableSingle.h
+++ b/dom/bindings/test/TestInterfaceIterableSingle.h
@@ -32,19 +32,22 @@ public:
   explicit TestInterfaceIterableSingle(nsPIDOMWindowInner* aParent);
   nsPIDOMWindowInner* GetParentObject() const;
   virtual JSObject* WrapObject(JSContext* aCx,
                                JS::Handle<JSObject*> aGivenProto) override;
   static already_AddRefed<TestInterfaceIterableSingle>
     Constructor(const GlobalObject& aGlobal, ErrorResult& rv);
 
   size_t GetIterableLength() const;
-  uint32_t GetValueAtIndex(uint32_t aIndex) const;
+  int32_t GetValueAtIndex(uint32_t aIndex) const;
+  uint32_t Length() const;
+  int32_t IndexedGetter(uint32_t aIndex, bool& aFound) const;
+
 private:
   virtual ~TestInterfaceIterableSingle() {}
   nsCOMPtr<nsPIDOMWindowInner> mParent;
-  nsTArray<uint32_t> mValues;
+  nsTArray<int32_t> mValues;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_TestInterfaceIterableSingle_h
--- a/dom/bindings/test/test_iterable.html
+++ b/dom/bindings/test/test_iterable.html
@@ -45,16 +45,18 @@
          }
        }
 
        var itr;
        // Simple single type iterable creation and functionality test
        info("IterableSingle: Testing simple iterable creation and functionality");
        itr = new TestInterfaceIterableSingle();
        testExistence("IterableSingle: ", itr, base_properties);
+       is(itr[Symbol.iterator], Array.prototype[Symbol.iterator],
+          "IterableSingle: Should be using %ArrayIterator% for @@iterator");
        var keys = [...itr.keys()];
        var values = [...itr.values()];
        var entries = [...itr.entries()];
        var key_itr = itr.keys();
        var value_itr = itr.values();
        var entries_itr = itr.entries();
        for (var i = 0; i < 3; ++i) {
          var key = key_itr.next();
--- a/dom/webidl/TestInterfaceJSMaplikeSetlikeIterable.webidl
+++ b/dom/webidl/TestInterfaceJSMaplikeSetlikeIterable.webidl
@@ -45,16 +45,18 @@ interface TestInterfaceSetlike {
 interface TestInterfaceSetlikeNode {
   setlike<Node>;
 };
 
 [Constructor(),
  Pref="dom.expose_test_interfaces"]
 interface TestInterfaceIterableSingle {
   iterable<long>;
+  getter long(unsigned long index);
+  readonly attribute unsigned long length;
 };
 
 [Constructor(),
  Pref="dom.expose_test_interfaces"]
 interface TestInterfaceIterableDouble {
   iterable<DOMString, DOMString>;
 };