Bug 821438. Allow inheritance from interfaces with Unforgeable attributes. r=peterv

dom/bindings/Bindings.conf

@@ -605,6 +605,11 @@ DOMInterfaces = {
                          '__stringifier' : 'Stringify' }
         },
 
+'TestChildInterface' : {
+        'headerFile': 'TestBindingHeader.h',
+        'register': False,
+        },
+
 'TestNonCastableInterface' : {
         'headerFile': 'TestBindingHeader.h',
         'register': False,

dom/bindings/parser/WebIDL.py

@@ -524,22 +524,6 @@ class IDLInterface(IDLObjectWithScope):
                                    self.parent.identifier.name),
                                   [self.location, self.parent.location])
 
-            # Now make sure our parent doesn't have any [Unforgeable]
-            # attributes.  We don't need to check its ancestors, because it has
-            # already checked those.  We don't need to check its consequential
-            # interfaces, because it has already imported those into its
-            # .members.
-            unforgeableParentMembers = [
-                attr for attr in parent.members
-                if attr.isAttr() and attr.isUnforgeable() ]
-            if len(unforgeableParentMembers) != 0:
-                locs = [self.location, parent.location]
-                locs.extend(attr.location for attr in unforgeableParentMembers)
-                raise WebIDLError("Interface %s inherits from %s, which has "
-                                  "[Unforgeable] members" %
-                                  (self.identifier.name, parent.identifier.name),
-                                  locs)
-
         for iface in self.implementedInterfaces:
             iface.finish(scope)
 
@@ -571,7 +555,7 @@ class IDLInterface(IDLObjectWithScope):
         if ctor is not None:
             ctor.finish(scope)
 
-        # Make a copy of our member list, so things tht implement us
+        # Make a copy of our member list, so things that implement us
         # can get those without all the stuff we implement ourselves
         # admixed.
         self.originalMembers = list(self.members)
@@ -599,6 +583,37 @@ class IDLInterface(IDLObjectWithScope):
             for ancestorConsequential in ancestor.getConsequentialInterfaces():
                 ancestorConsequential.interfacesBasedOnSelf.add(self)
 
+        if self.parent:
+            # Make sure we don't shadow any of the [Unforgeable] attributes on
+            # our ancestor interfaces.  We don't have to worry about
+            # consequential interfaces here, because those have already been
+            # imported into the relevant .members lists.  And we don't have to
+            # worry about anything other than our parent, because it has already
+            # imported its ancestors unforgeable attributes into its member
+            # list.
+            for unforgeableAttr in (attr for attr in self.parent.members if
+                                    attr.isAttr() and not attr.isStatic() and
+                                    attr.isUnforgeable()):
+                shadows = [ m for m in self.members if
+                            (m.isAttr() or m.isMethod()) and
+                            not m.isStatic() and
+                            m.identifier.name == unforgeableAttr.identifier.name ]
+                if len(shadows) != 0:
+                    locs = [unforgeableAttr.location] + [ s.location for s
+                                                          in shadows ]
+                    raise WebIDLError("Interface %s shadows [Unforgeable] "
+                                      "members of %s" %
+                                      (self.identifier.name,
+                                       ancestor.identifier.name),
+                                      locs)
+                # And now just stick it in our members, since we won't be
+                # inheriting this down the proto chain.  If we really cared we
+                # could try to do something where we set up the unforgeable
+                # attributes of ancestor interfaces, with their corresponding
+                # getters, on our interface, but that gets pretty complicated
+                # and seems unnecessary.
+                self.members.append(unforgeableAttr)
+
         # Ensure that there's at most one of each {named,indexed}
         # {getter,setter,creator,deleter} and at most one stringifier.
         specialMembersSeen = {}

dom/bindings/parser/tests/test_unforgeable.py

@@ -1,8 +1,71 @@
 def WebIDLTest(parser, harness):
+    parser.parse("""
+            interface Child : Parent {
+            };
+            interface Parent {
+              [Unforgeable] readonly attribute long foo;
+            };
+        """)
+
+    results = parser.finish()
+    harness.check(len(results), 2,
+                  "Should be able to inherit from an interface with "
+                  "[Unforgeable] properties.")
+
+    parser = parser.reset();
+    parser.parse("""
+            interface Child : Parent {
+              const short foo = 10;
+            };
+            interface Parent {
+              [Unforgeable] readonly attribute long foo;
+            };
+        """)
+
+    results = parser.finish()
+    harness.check(len(results), 2,
+                  "Should be able to inherit from an interface with "
+                  "[Unforgeable] properties even if we have a constant with "
+                  "the same name.")
+
+    parser = parser.reset();
+    parser.parse("""
+            interface Child : Parent {
+              static attribute short foo;
+            };
+            interface Parent {
+              [Unforgeable] readonly attribute long foo;
+            };
+        """)
+
+    results = parser.finish()
+    harness.check(len(results), 2,
+                  "Should be able to inherit from an interface with "
+                  "[Unforgeable] properties even if we have a static attribute "
+                  "with the same name.")
+
+    parser = parser.reset();
+    parser.parse("""
+            interface Child : Parent {
+              static void foo();
+            };
+            interface Parent {
+              [Unforgeable] readonly attribute long foo;
+            };
+        """)
+
+    results = parser.finish()
+    harness.check(len(results), 2,
+                  "Should be able to inherit from an interface with "
+                  "[Unforgeable] properties even if we have a static operation "
+                  "with the same name.")
+
+    parser = parser.reset();
     threw = False
     try:
         parser.parse("""
             interface Child : Parent {
+              void foo();
             };
             interface Parent {
               [Unforgeable] readonly attribute long foo;
@@ -12,14 +75,51 @@ def WebIDLTest(parser, harness):
         results = parser.finish()
     except:
         threw = True
-
-    harness.ok(threw, "Should have thrown.")
+    harness.ok(threw,
+               "Should have thrown when shadowing unforgeable attribute on "
+               "parent with operation.")
 
     parser = parser.reset();
     threw = False
     try:
         parser.parse("""
             interface Child : Parent {
+              attribute short foo;
+            };
+            interface Parent {
+              [Unforgeable] readonly attribute long foo;
+            };
+        """)
+
+        results = parser.finish()
+    except Exception,x:
+        threw = True
+    harness.ok(threw,
+               "Should have thrown when shadowing unforgeable attribute on "
+               "parent with attribute.")
+
+    parser = parser.reset();
+    parser.parse("""
+            interface Child : Parent {
+            };
+            interface Parent {};
+            interface Consequential {
+              [Unforgeable] readonly attribute long foo;
+            };
+            Parent implements Consequential;
+        """)
+
+    results = parser.finish()
+    harness.check(len(results), 4,
+                  "Should be able to inherit from an interface with a "
+                  "consequential interface with [Unforgeable] properties.")
+
+    parser = parser.reset();
+    threw = False
+    try:
+        parser.parse("""
+            interface Child : Parent {
+              void foo();
             };
             interface Parent {};
             interface Consequential {
@@ -32,7 +132,35 @@ def WebIDLTest(parser, harness):
     except:
         threw = True
 
-    harness.ok(threw, "Should have thrown.")
+    harness.ok(threw,
+               "Should have thrown when shadowing unforgeable attribute "
+               "of parent's consequential interface.")
+
+    parser = parser.reset();
+    threw = False
+    try:
+        parser.parse("""
+            interface Child : Parent {
+            };
+            interface Parent : GrandParent {};
+            interface GrandParent {};
+            interface Consequential {
+              [Unforgeable] readonly attribute long foo;
+            };
+            GrandParent implements Consequential;
+            interface ChildConsequential {
+              void foo();
+            };
+            Child implements ChildConsequential;
+        """)
+
+        results = parser.finish()
+    except:
+        threw = True
+
+    harness.ok(threw,
+               "Should have thrown when our consequential interface shadows unforgeable attribute "
+               "of ancestor's consequential interface.")
 
     parser = parser.reset();
     threw = False
@@ -47,4 +175,4 @@ def WebIDLTest(parser, harness):
     except:
         threw = True
 
-    harness.ok(threw, "Should have thrown.")
+    harness.ok(threw, "Should have thrown for writable [Unforgeable] attribute.")

dom/bindings/test/TestBindingHeader.h

@@ -868,6 +868,12 @@ public:
   void GetSupportedNames(nsTArray<nsString>&);
 };
 
+class TestChildInterface : public TestInterface
+{
+public:
+  NS_DECL_ISUPPORTS
+};
+
 } // namespace dom
 } // namespace mozilla
 

dom/bindings/test/TestCodeGen.webidl

@@ -451,6 +451,9 @@ interface TestInterface {
   // If you add things here, add them to TestExampleGen as well
 };
 
+interface TestChildInterface : TestInterface {
+};
+
 interface TestNonWrapperCacheInterface {
 };