Bug 803519. Require indexed/named getters if we have creators/deleters/setters. r=peterv

dom/bindings/Codegen.py

@@ -5061,6 +5061,15 @@ class CGProxyIndexedGetter(CGProxySpecialOperation):
         self.templateValues = templateValues
         CGProxySpecialOperation.__init__(self, descriptor, 'IndexedGetter')
 
+class CGProxyIndexedPresenceChecker(CGProxyIndexedGetter):
+    """
+    Class to generate a call that checks whether an indexed property exists.
+
+    For now, we just delegate to CGProxyIndexedGetter
+    """
+    def __init__(self, descriptor):
+        CGProxyIndexedGetter.__init__(self, descriptor)
+
 class CGProxyIndexedSetter(CGProxySpecialOperation):
     """
     Class to generate a call to an indexed setter.
@@ -5077,6 +5086,15 @@ class CGProxyNamedGetter(CGProxySpecialOperation):
         self.templateValues = templateValues
         CGProxySpecialOperation.__init__(self, descriptor, 'NamedGetter')
 
+class CGProxyNamedPresenceChecker(CGProxyNamedGetter):
+    """
+    Class to generate a call that checks whether a named property exists.
+
+    For now, we just delegate to CGProxyNamedGetter
+    """
+    def __init__(self, descriptor):
+        CGProxyNamedGetter.__init__(self, descriptor)
+
 class CGProxyNamedSetter(CGProxySpecialOperation):
     """
     Class to generate a call to a named setter.
@@ -5150,11 +5168,9 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(ClassMethod):
         indexedSetter = self.descriptor.operations['IndexedSetter']
 
         setOrIndexedGet = ""
-        if indexedGetter or indexedSetter:
+        if self.descriptor.supportsIndexedProperties():
             setOrIndexedGet += "int32_t index = GetArrayIndexFromId(cx, id);\n"
-
+            readonly = toStringBool(indexedSetter is None)
-        if indexedGetter:
-            readonly = toStringBool(self.descriptor.operations['IndexedSetter'] is None)
             fillDescriptor = "FillPropertyDescriptor(desc, proxy, %s);\nreturn true;" % readonly
             templateValues = {'jsvalRef': 'desc->value', 'jsvalPtr': '&desc->value',
                               'obj': 'proxy', 'successCode': fillDescriptor}
@@ -5192,8 +5208,7 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(ClassMethod):
                                 CGIndenter(CGGeneric(get)).define() +
                                 "}\n\n")
 
-        namedGetter = self.descriptor.operations['NamedGetter']
+        if self.descriptor.supportsNamedProperties():
-        if namedGetter:
             readonly = toStringBool(self.descriptor.operations['NamedSetter'] is None)
             fillDescriptor = "FillPropertyDescriptor(desc, proxy, %s);\nreturn true;" % readonly
             templateValues = {'jsvalRef': 'desc->value', 'jsvalPtr': '&desc->value',
@@ -5252,7 +5267,7 @@ class CGDOMJSProxyHandler_defineProperty(ClassMethod):
                     CGIndenter(CGProxyIndexedSetter(self.descriptor)).define() +
                     "  return true;\n" +
                     "}\n") % (self.descriptor.nativeType)
-        elif self.descriptor.operations['IndexedGetter']:
+        elif self.descriptor.supportsIndexedProperties():
             set += ("if (GetArrayIndexFromId(cx, id) >= 0) {\n" +
                     "  return ThrowErrorMessage(cx, MSG_NO_PROPERTY_SETTER, \"%s\");\n" +
                     "}\n") % self.descriptor.name
@@ -5273,7 +5288,7 @@ class CGDOMJSProxyHandler_defineProperty(ClassMethod):
                     CGIndenter(CGProxyNamedSetter(self.descriptor)).define() + "\n" +
                     "  return true;\n" +
                     "}\n") % (self.descriptor.nativeType)
-        elif self.descriptor.operations['NamedGetter']:
+        elif self.descriptor.supportsNamedProperties():
             set += ("if (JSID_IS_STRING(id)) {\n" +
                     "  JS::Value nameVal = STRING_TO_JSVAL(JSID_TO_STRING(id));\n" +
                     "  FakeDependentString name;\n"
@@ -5282,7 +5297,7 @@ class CGDOMJSProxyHandler_defineProperty(ClassMethod):
                     "    return false;\n" +
                     "  }\n" +
                     "  %s* self = UnwrapProxy(proxy);\n" +
-                    CGIndenter(CGProxyNamedGetter(self.descriptor)).define() +
+                    CGIndenter(CGProxyNamedPresenceChecker(self.descriptor)).define() +
                     "  if (found) {\n"
                     "    return ThrowErrorMessage(cx, MSG_NO_PROPERTY_SETTER, \"%s\");\n" +
                     "  }\n" +
@@ -5318,11 +5333,8 @@ class CGDOMJSProxyHandler_delete(ClassMethod):
                              "}")
                 body = (eval("CGProxy%sDeleter" % type)(self.descriptor).define() +
                         setBp)
-            elif self.descriptor.operations[type + 'Getter']:
+            elif eval("self.descriptor.supports%sProperties()" % type):
-                # XXXbz: We should be doing this for cases when we have a
+                body = (eval("CGProxy%sPresenceChecker" % type)(self.descriptor).define() +
-                # creator or setter too, but we have no way to find out
-                # whether the property name is supported in those cases!
-                body = (eval("CGProxy%sGetter" % type)(self.descriptor).define() +
                         "if (found) {\n"
                         "  // XXXbz we should throw if Throw is true!\n"
                         "  *bp = false;\n"
@@ -5373,8 +5385,7 @@ class CGDOMJSProxyHandler_getOwnPropertyNames(ClassMethod):
         self.descriptor = descriptor
     def getBody(self):
         # Per spec, we do indices, then named props, then everything else
-        indexedGetter = self.descriptor.operations['IndexedGetter']
+        if self.descriptor.supportsIndexedProperties():
-        if indexedGetter:
             addIndices = """uint32_t length = UnwrapProxy(proxy)->Length();
 MOZ_ASSERT(int32_t(length) >= 0);
 for (int32_t i = 0; i < int32_t(length); ++i) {
@@ -5387,11 +5398,7 @@ for (int32_t i = 0; i < int32_t(length); ++i) {
         else:
             addIndices = ""
 
-        supportsNames = (self.descriptor.operations['NamedGetter'] or
+        if self.descriptor.supportsNamedProperties():
-                         self.descriptor.operations['NamedSetter'] or
-                         self.descriptor.operations['NamedCreator'] or
-                         self.descriptor.operations['NamedDeleter'])
-        if supportsNames:
             addNames = """nsTArray<nsString> names;
 UnwrapProxy(proxy)->GetSupportedNames(names);
 if (!AppendNamedPropertyIds(cx, proxy, names, props)) {
@@ -5417,20 +5424,18 @@ class CGDOMJSProxyHandler_hasOwn(ClassMethod):
         ClassMethod.__init__(self, "hasOwn", "bool", args)
         self.descriptor = descriptor
     def getBody(self):
-        indexedGetter = self.descriptor.operations['IndexedGetter']
+        if self.descriptor.supportsIndexedProperties():
-        if indexedGetter:
             indexed = ("int32_t index = GetArrayIndexFromId(cx, id);\n" + 
                        "if (index >= 0) {\n" +
                        "  %s* self = UnwrapProxy(proxy);\n" +
-                       CGIndenter(CGProxyIndexedGetter(self.descriptor)).define() + "\n" +
+                       CGIndenter(CGProxyIndexedPresenceChecker(self.descriptor)).define() + "\n" +
                        "  *bp = found;\n" +
                        "  return true;\n" +
                        "}\n\n") % (self.descriptor.nativeType)
         else:
             indexed = ""
 
-        namedGetter = self.descriptor.operations['NamedGetter']
+        if self.descriptor.supportsNamedProperties():
-        if namedGetter:
             named = ("if (JSID_IS_STRING(id) && !HasPropertyOnPrototype(cx, proxy, this, id)) {\n" +
                      "  jsval nameVal = STRING_TO_JSVAL(JSID_TO_STRING(id));\n" +
                      "  FakeDependentString name;\n"
@@ -5440,7 +5445,7 @@ class CGDOMJSProxyHandler_hasOwn(ClassMethod):
                      "  }\n" +
                      "\n" +
                      "  %s* self = UnwrapProxy(proxy);\n" +
-                     CGIndenter(CGProxyNamedGetter(self.descriptor)).define() + "\n" +
+                     CGIndenter(CGProxyNamedPresenceChecker(self.descriptor)).define() + "\n" +
                      "  *bp = found;\n"
                      "  return true;\n"
                      "}\n" +
@@ -5483,8 +5488,7 @@ if (expando) {
 
         templateValues = {'jsvalRef': '*vp', 'jsvalPtr': 'vp', 'obj': 'proxy'}
 
-        indexedGetter = self.descriptor.operations['IndexedGetter']
+        if self.descriptor.supportsIndexedProperties():
-        if indexedGetter:
             getIndexedOrExpando = ("int32_t index = GetArrayIndexFromId(cx, id);\n" +
                                    "if (index >= 0) {\n" +
                                    "  %s* self = UnwrapProxy(proxy);\n" +
@@ -5499,8 +5503,7 @@ if (expando) {
         else:
             getIndexedOrExpando = getFromExpando + "\n"
 
-        namedGetter = self.descriptor.operations['NamedGetter']
+        if self.descriptor.supportsNamedProperties():
-        if namedGetter:
             getNamed = ("if (JSID_IS_STRING(id)) {\n" +
                         "  JS::Value nameVal = STRING_TO_JSVAL(JSID_TO_STRING(id));\n" +
                         "  FakeDependentString name;\n"
@@ -5557,8 +5560,7 @@ class CGDOMJSProxyHandler_getElementIfPresent(ClassMethod):
         ClassMethod.__init__(self, "getElementIfPresent", "bool", args)
         self.descriptor = descriptor
     def getBody(self):
-        indexedGetter = self.descriptor.operations['IndexedGetter']
+        if self.descriptor.supportsIndexedProperties():
-        if indexedGetter:
             successCode = """*present = found;
 return true;"""
             templateValues = {'jsvalRef': '*vp', 'jsvalPtr': 'vp',

dom/bindings/Configuration.py

@@ -239,6 +239,20 @@ class Descriptor(DescriptorProvider):
                 iface = iface.parent
 
             if self.proxy:
+                if (not operations['IndexedGetter'] and
+                    (operations['IndexedSetter'] or
+                     operations['IndexedDeleter'] or
+                     operations['IndexedCreator'])):
+                    raise SyntaxError("%s supports indexed properties but does "
+                                      "not have an indexed getter.\n%s" %
+                                      (self.interface, self.interface.location))
+                if (not operations['NamedGetter'] and
+                    (operations['NamedSetter'] or
+                     operations['NamedDeleter'] or
+                     operations['NamedCreator'])):
+                    raise SyntaxError("%s supports named properties but does "
+                                      "not have a named getter.\n%s" %
+                                      (self.interface, self.interface.location))
                 iface = self.interface
                 while iface:
                     iface.setUserData('hasProxyDescendant', True)
@@ -358,3 +372,9 @@ class Descriptor(DescriptorProvider):
             throws = member.getExtendedAttribute(throwsAttr)
         maybeAppendInfallibleToAttrs(attrs, throws)
         return attrs
+
+    def supportsIndexedProperties(self):
+        return self.operations['IndexedGetter'] is not None
+
+    def supportsNamedProperties(self):
+        return self.operations['NamedGetter'] is not None

dom/bindings/test/TestBindingHeader.h

@@ -641,6 +641,8 @@ public:
   virtual nsISupports* GetParentObject();
 
   void IndexedSetter(uint32_t, const nsAString&);
+  void IndexedGetter(uint32_t, bool&, nsString&);
+  uint32_t Length();
   void SetItem(uint32_t, const nsAString&);
 };
 
@@ -654,6 +656,7 @@ public:
   virtual nsISupports* GetParentObject();
 
   void NamedSetter(const nsAString&, TestIndexedSetterInterface&);
+  TestIndexedSetterInterface* NamedGetter(const nsAString&, bool&);
   void GetSupportedNames(nsTArray<nsString>&);
 };
 
@@ -667,7 +670,10 @@ public:
   virtual nsISupports* GetParentObject();
 
   void IndexedSetter(uint32_t, TestIndexedSetterInterface&);
+  TestIndexedSetterInterface* IndexedGetter(uint32_t, bool&);
+  uint32_t Length();
   void NamedSetter(const nsAString&, TestIndexedSetterInterface&);
+  TestIndexedSetterInterface* NamedGetter(const nsAString&, bool&);
   void SetNamedItem(const nsAString&, TestIndexedSetterInterface&);
   void GetSupportedNames(nsTArray<nsString>&);
 };
@@ -712,6 +718,8 @@ public:
 
   void IndexedDeleter(uint32_t, bool&);
   void IndexedDeleter(uint32_t) MOZ_DELETE;
+  long IndexedGetter(uint32_t, bool&);
+  uint32_t Length();
   void DelItem(uint32_t);
   void DelItem(uint32_t, bool&) MOZ_DELETE;
 };
@@ -727,6 +735,8 @@ public:
 
   bool IndexedDeleter(uint32_t, bool&);
   bool IndexedDeleter(uint32_t) MOZ_DELETE;
+  long IndexedGetter(uint32_t, bool&);
+  uint32_t Length();
   bool DelItem(uint32_t);
   bool DelItem(uint32_t, bool&) MOZ_DELETE;
 };
@@ -741,6 +751,7 @@ public:
   virtual nsISupports* GetParentObject();
 
   void NamedDeleter(const nsAString&, bool&);
+  long NamedGetter(const nsAString&, bool&);
   void GetSupportedNames(nsTArray<nsString>&);
 };
 
@@ -755,6 +766,7 @@ public:
 
   bool NamedDeleter(const nsAString&, bool&);
   bool NamedDeleter(const nsAString&) MOZ_DELETE;
+  long NamedGetter(const nsAString&, bool&);
   bool DelNamedItem(const nsAString&);
   bool DelNamedItem(const nsAString&, bool&) MOZ_DELETE;
   void GetSupportedNames(nsTArray<nsString>&);
@@ -770,9 +782,12 @@ public:
   virtual nsISupports* GetParentObject();
 
   void IndexedDeleter(uint32_t, bool&);
+  long IndexedGetter(uint32_t, bool&);
+  uint32_t Length();
 
   void NamedDeleter(const nsAString&, bool&);
   void NamedDeleter(const nsAString&) MOZ_DELETE;
+  long NamedGetter(const nsAString&, bool&);
   void DelNamedItem(const nsAString&);
   void DelNamedItem(const nsAString&, bool&) MOZ_DELETE;
   void GetSupportedNames(nsTArray<nsString>&);

dom/bindings/test/TestCodeGen.webidl

@@ -441,15 +441,19 @@ interface TestIndexedAndNamedGetterInterface {
 
 interface TestIndexedSetterInterface {
   setter creator void setItem(unsigned long index, DOMString item);
+  getter DOMString (unsigned long index);
 };
 
 interface TestNamedSetterInterface {
   setter creator void (DOMString name, TestIndexedSetterInterface item);
+  getter TestIndexedSetterInterface (DOMString name);
 };
 
 interface TestIndexedAndNamedSetterInterface {
   setter creator void (unsigned long index, TestIndexedSetterInterface item);
+  getter TestIndexedSetterInterface (unsigned long index);
   setter creator void setNamedItem(DOMString name, TestIndexedSetterInterface item);
+  getter TestIndexedSetterInterface (DOMString name);
 };
 
 interface TestIndexedAndNamedGetterAndSetterInterface : TestIndexedSetterInterface {
@@ -465,23 +469,29 @@ interface TestIndexedAndNamedGetterAndSetterInterface : TestIndexedSetterInterfa
 
 interface TestIndexedDeleterInterface {
   deleter void delItem(unsigned long index);
+  getter long (unsigned long index);
 };
 
 interface TestIndexedDeleterWithRetvalInterface {
   deleter boolean delItem(unsigned long index);
+  getter long (unsigned long index);
 };
 
 interface TestNamedDeleterInterface {
   deleter void (DOMString name);
+  getter long (DOMString name);
 };
 
 interface TestNamedDeleterWithRetvalInterface {
   deleter boolean delNamedItem(DOMString name);
+  getter long (DOMString name);
 };
 
 interface TestIndexedAndNamedDeleterInterface {
   deleter void (unsigned long index);
+  getter long (unsigned long index);
   deleter void delNamedItem(DOMString name);
+  getter long (DOMString name);
 };
 
 interface TestCppKeywordNamedMethodsInterface {