Bug 784812: Implement real dependencies for WebIDL bindings. r=bz,ted

dom/bindings/BindingGen.py

@@ -5,46 +5,53 @@
 import os
 import cPickle
 from Configuration import Configuration
-from Codegen import CGBindingRoot, replaceFileIfChanged
+from Codegen import CGBindingRoot
 
-def generate_binding_header(config, outputprefix, webidlfile):
+def generate_binding_header(config, outputprefix, srcprefix, webidlfile):
     """
     |config| Is the configuration object.
     |outputprefix| is a prefix to use for the header guards and filename.
     """
 
     filename = outputprefix + ".h"
+    depsname = ".deps/" + filename + ".pp"
     root = CGBindingRoot(config, outputprefix, webidlfile)
-    if replaceFileIfChanged(filename, root.declare()):
+    with open(filename, 'wb') as f:
-        print "Generating binding header: %s" % (filename)
+        f.write(root.declare())
+    with open(depsname, 'wb') as f:
+        f.write("\n".join(filename + ": " + os.path.join(srcprefix, x) for x in root.deps()))
 
-def generate_binding_cpp(config, outputprefix, webidlfile):
+def generate_binding_cpp(config, outputprefix, srcprefix, webidlfile):
     """
     |config| Is the configuration object.
     |outputprefix| is a prefix to use for the header guards and filename.
     """
 
     filename = outputprefix + ".cpp"
+    depsname = ".deps/" + filename + ".pp"
     root = CGBindingRoot(config, outputprefix, webidlfile)
-    if replaceFileIfChanged(filename, root.define()):
+    with open(filename, 'wb') as f:
-        print "Generating binding implementation: %s" % (filename)
+        f.write(root.define())
+    with open(depsname, 'wb') as f:
+        f.write("\n".join(filename + ": " + os.path.join(srcprefix, x) for x in root.deps()))
 
 def main():
 
     # Parse arguments.
     from optparse import OptionParser
-    usagestring = "usage: %prog [header|cpp] configFile outputPrefix webIDLFile"
+    usagestring = "usage: %prog [header|cpp] configFile outputPrefix srcPrefix webIDLFile"
     o = OptionParser(usage=usagestring)
     o.add_option("--verbose-errors", action='store_true', default=False,
                  help="When an error happens, display the Python traceback.")
     (options, args) = o.parse_args()
 
-    if len(args) != 4 or (args[0] != "header" and args[0] != "cpp"):
+    if len(args) != 5 or (args[0] != "header" and args[0] != "cpp"):
         o.error(usagestring)
     buildTarget = args[0]
     configFile = os.path.normpath(args[1])
     outputPrefix = args[2]
-    webIDLFile = os.path.normpath(args[3])
+    srcPrefix = os.path.normpath(args[3])
+    webIDLFile = os.path.normpath(args[4])
 
     # Load the parsing results
     f = open('ParserResults.pkl', 'rb')
@@ -56,9 +63,9 @@ def main():
 
     # Generate the prototype classes.
     if buildTarget == "header":
-        generate_binding_header(config, outputPrefix, webIDLFile);
+        generate_binding_header(config, outputPrefix, srcPrefix, webIDLFile);
     elif buildTarget == "cpp":
-        generate_binding_cpp(config, outputPrefix, webIDLFile);
+        generate_binding_cpp(config, outputPrefix, srcPrefix, webIDLFile);
     else:
         assert False # not reached
 

dom/bindings/Codegen.py

@@ -59,6 +59,9 @@ class CGThing():
     def define(self):
         """Produce code for a cpp file."""
         assert(False) # Override me!
+    def deps(self):
+        """Produce the deps for a pp file"""
+        assert(False) # Override me!
 
 class CGNativePropertyHooks(CGThing):
     """
@@ -309,6 +312,13 @@ class CGList(CGThing):
         return self.join(child.declare() for child in self.children if child is not None)
     def define(self):
         return self.join(child.define() for child in self.children if child is not None)
+    def deps(self):
+        deps = set()
+        for child in self.children:
+            if child is None:
+                continue
+            deps = deps.union(child.deps())
+        return deps
 
 class CGGeneric(CGThing):
     """
@@ -322,6 +332,8 @@ class CGGeneric(CGThing):
         return self.declareText
     def define(self):
         return self.defineText
+    def deps(self):
+        return set()
 
 # We'll want to insert the indent at the beginnings of lines, but we
 # don't want to indent empty lines.  So only indent lines that have a
@@ -387,6 +399,9 @@ class CGWrapper(CGThing):
                 defn.replace("\n", "\n" + (" " * len(self.definePre))))
         return self.definePre + defn + self.definePost
 
+    def deps(self):
+        return self.child.deps()
+
 class CGIfWrapper(CGWrapper):
     def __init__(self, child, condition):
         pre = CGWrapper(CGGeneric(condition), pre="if (", post=") {\n",
@@ -4853,6 +4868,9 @@ class CGEnum(CGThing):
 """ % (len(self.enum.values()) + 1,
        ",\n    ".join(['{"' + val + '", ' + str(len(val)) + '}' for val in self.enum.values()]))
 
+    def deps(self):
+        return self.enum.getDeps()
+
 def getUnionAccessorSignatureType(type, descriptorProvider):
     """
     Returns the types that are used in the getter and setter signatures for
@@ -5743,6 +5761,8 @@ class CGPrototypeTraitsClass(CGClass):
                          templateArgs=templateArgs,
                          templateSpecialization=templateSpecialization,
                          enums=enums, typedefs=typedefs, isStruct=True)
+    def deps(self):
+        return set()
 
 class CGPrototypeIDMapClass(CGClass):
     def __init__(self, descriptor, indent=''):
@@ -5754,6 +5774,8 @@ class CGPrototypeIDMapClass(CGClass):
                          templateArgs=templateArgs,
                          templateSpecialization=templateSpecialization,
                          enums=enums, isStruct=True)
+    def deps(self):
+        return set()
 
 class CGClassForwardDeclare(CGThing):
     def __init__(self, name, isStruct=False):
@@ -5766,6 +5788,8 @@ class CGClassForwardDeclare(CGThing):
     def define(self):
         # Header only
         return ''
+    def deps(self):
+        return set()
 
 class CGProxySpecialOperation(CGPerSignatureCall):
     """
@@ -6442,6 +6466,8 @@ class CGDescriptor(CGThing):
 
         assert not descriptor.concrete or descriptor.interface.hasInterfacePrototypeObject()
 
+        self._deps = descriptor.interface.getDeps()
+
         cgThings = []
         # These are set to true if at least one non-static
         # method/getter/setter exist on the interface.
@@ -6578,6 +6604,8 @@ class CGDescriptor(CGThing):
         return self.cgRoot.declare()
     def define(self):
         return self.cgRoot.define()
+    def deps(self):
+        return self._deps
 
 class CGNamespacedEnum(CGThing):
     def __init__(self, namespace, enumName, names, values, comment=""):
@@ -6782,6 +6810,9 @@ class CGDictionary(CGThing):
                 "defineMembers": "\n\n".join(memberDefines)
                 })
 
+    def deps(self):
+        return self.dictionary.getDeps()
+
     @staticmethod
     def makeDictionaryName(dictionary, workers):
         suffix = "Workers" if workers else ""
@@ -7203,6 +7234,9 @@ class CGBindingRoot(CGThing):
     def define(self):
         return stripTrailingWhitespace(self.root.define())
 
+    def deps(self):
+        return self.root.deps()
+
 class CGNativeMember(ClassMethod):
     def __init__(self, descriptor, member, name, signature, extendedAttrs,
                  breakAfter=True, passCxAsNeeded=True, visibility="public",
@@ -7778,6 +7812,7 @@ class CGCallback(CGClass):
     def __init__(self, idlObject, descriptorProvider, baseName, methods,
                  getters=[], setters=[]):
         self.baseName = baseName
+        self._deps = idlObject.getDeps()
         name = idlObject.identifier.name
         if descriptorProvider.workers:
             name += "Workers"
@@ -7867,6 +7902,9 @@ class CGCallback(CGClass):
                             body=bodyWithoutThis),
                 method]
 
+    def deps(self):
+        return self._deps
+
 class CGCallbackFunction(CGCallback):
     def __init__(self, callback, descriptorProvider):
         CGCallback.__init__(self, callback, descriptorProvider,

dom/bindings/Makefile.in

@@ -75,7 +75,6 @@ EXPORTS_$(binding_include_path) = \
   TypedArray.h \
   UnionConversions.h \
   UnionTypes.h \
-  $(exported_binding_headers) \
   $(NULL)
 
 LOCAL_INCLUDES += -I$(topsrcdir)/js/xpconnect/src \
@@ -97,8 +96,34 @@ LOCAL_INCLUDES += \
   $(NULL)
 endif
 
+# XXXkhuey this is a terrible hack to avoid blowing out the command line
+ifneq (,$(filter-out all chrome default export realchrome tools clean clobber clobber_all distclean realclean,$(MAKECMDGOALS)))
+$(shell echo "$(addsuffix .pp,$(binding_header_files))" > pp.list)
+$(shell echo "$(addsuffix .pp,$(binding_cpp_files))" >> pp.list)
+
+# The script mddepend.pl checks the dependencies and writes to stdout
+# one rule to force out-of-date objects. For example,
+#   foo.o boo.o: FORCE
+# The script has an advantage over including the *.pp files directly
+# because it handles the case when header files are removed from the build.
+# 'make' would complain that there is no way to build missing headers.
+ALL_PP_RESULTS = $(shell cat pp.list | $(PERL) $(BUILD_TOOLS)/mddepend.pl)
+$(eval $(ALL_PP_RESULTS))
+
+endif
+
+EXPORTS_GENERATED_FILES := $(exported_binding_headers)
+EXPORTS_GENERATED_DEST := $(DIST)/include/$(binding_include_path)
+EXPORTS_GENERATED_TARGET := webidl-export
+INSTALL_TARGETS += EXPORTS_GENERATED
+
 include $(topsrcdir)/config/rules.mk
 
+# We need to create a separate target so we can ensure that the pickle is
+# done before generating headers.
+export:: ParserResults.pkl
+	$(MAKE) webidl-export
+
 # If you change bindinggen_dependencies here, change it in
 # dom/bindings/test/Makefile.in too.
 bindinggen_dependencies := \
@@ -107,7 +132,6 @@ bindinggen_dependencies := \
   Configuration.py \
   Codegen.py \
   parser/WebIDL.py \
-  ParserResults.pkl \
   $(GLOBAL_DEPS) \
   $(NULL)
 
@@ -129,20 +153,26 @@ $(test_webidl_files): %: $(srcdir)/test/%
 
 $(binding_header_files): %Binding.h: $(bindinggen_dependencies) \
                                      %.webidl \
+                                     $(call mkdir_deps,$(MDDEPDIR)) \
                                      $(NULL)
 	PYTHONDONTWRITEBYTECODE=1 $(PYTHON) $(topsrcdir)/config/pythonpath.py \
 	  $(PLY_INCLUDE) -I$(srcdir)/parser \
 	  $(srcdir)/BindingGen.py header \
-	  $(srcdir)/Bindings.conf $*Binding \
+	  $(srcdir)/Bindings.conf \
+	  $*Binding \
+	  $(topsrcdir)/dom/webidl/ \
 	  $*.webidl
 
 $(binding_cpp_files): %Binding.cpp: $(bindinggen_dependencies) \
                                     %.webidl \
+                                    $(call mkdir_deps,$(MDDEPDIR)) \
                                     $(NULL)
 	PYTHONDONTWRITEBYTECODE=1 $(PYTHON) $(topsrcdir)/config/pythonpath.py \
 	  $(PLY_INCLUDE) -I$(srcdir)/parser \
 	  $(srcdir)/BindingGen.py cpp \
-	  $(srcdir)/Bindings.conf $*Binding \
+	  $(srcdir)/Bindings.conf \
+	  $*Binding \
+	  $(topsrcdir)/dom/webidl/ \
 	  $*.webidl
 
 $(globalgen_targets): ParserResults.pkl
@@ -195,4 +225,6 @@ GARBAGE += \
 # don't have issues with .cpp files being compiled before we've generated the
 # headers they depend on.  This is really only needed for the test files, since
 # the non-test headers are all exported above anyway.
-export:: $(binding_header_files)
+webidl-export:: $(binding_header_files)
+
+.PHONY: webidl-export

dom/bindings/parser/WebIDL.py

@@ -174,6 +174,38 @@ class IDLObject(object):
     def handleExtendedAttribute(self, attr):
         assert False # Override me!
 
+    def _getDependentObjects(self):
+        assert False # Override me!
+
+    def getDeps(self, visited=None):
+        """ Return a set of files that this object depends on.  If any of
+            these files are changed the parser needs to be rerun to regenerate
+            a new IDLObject.
+
+            The visited argument is a set of all the objects already visited.
+            We must test to see if we are in it, and if so, do nothing.  This
+            prevents infinite recursion."""
+
+        # NB: We can't use visited=set() above because the default value is
+        # evaluated when the def statement is evaluated, not when the function
+        # is executed, so there would be one set for all invocations.
+        if visited == None:
+            visited = set()
+
+        if self in visited:
+            return set()
+
+        visited.add(self)
+
+        deps = set()
+        if self.filename() != "<builtin>":
+            deps.add(self.filename())
+
+        for d in self._getDependentObjects():
+            deps = deps.union(d.getDeps(visited))
+
+        return deps
+
 class IDLScope(IDLObject):
     def __init__(self, location, parentScope, identifier):
         IDLObject.__init__(self, location)
@@ -443,6 +475,9 @@ class IDLExternalInterface(IDLObjectWithIdentifier):
     def resolve(self, parentScope):
         pass
 
+    def _getDependentObjects(self):
+        return set()
+
 class IDLInterface(IDLObjectWithScope):
     def __init__(self, location, parentScope, name, parent, members,
                  isPartial):
@@ -916,6 +951,13 @@ class IDLInterface(IDLObjectWithScope):
         # Put the new members at the beginning
         self.members = members + self.members
 
+    def _getDependentObjects(self):
+        deps = set(self.members)
+        deps.union(self.implementedInterfaces)
+        if self.parent:
+            deps.add(self.parent)
+        return deps
+
 class IDLDictionary(IDLObjectWithScope):
     def __init__(self, location, parentScope, name, parent, members):
         assert isinstance(parentScope, IDLScope)
@@ -986,6 +1028,11 @@ class IDLDictionary(IDLObjectWithScope):
     def addExtendedAttributes(self, attrs):
         assert len(attrs) == 0
 
+    def _getDependentObjects(self):
+        deps = set(self.members)
+        if (self.parent):
+            deps.add(self.parent)
+        return deps
 
 class IDLEnum(IDLObjectWithIdentifier):
     def __init__(self, location, parentScope, name, values):
@@ -1014,6 +1061,9 @@ class IDLEnum(IDLObjectWithIdentifier):
     def addExtendedAttributes(self, attrs):
         assert len(attrs) == 0
 
+    def _getDependentObjects(self):
+        return set()
+
 class IDLType(IDLObject):
     Tags = enum(
         # The integer types
@@ -1303,6 +1353,9 @@ class IDLNullableType(IDLType):
             return False
         return self.inner.isDistinguishableFrom(other)
 
+    def _getDependentObjects(self):
+        return self.inner._getDependentObjects()
+
 class IDLSequenceType(IDLType):
     def __init__(self, location, parameterType):
         assert not parameterType.isVoid()
@@ -1373,6 +1426,9 @@ class IDLSequenceType(IDLType):
         return (other.isPrimitive() or other.isString() or other.isEnum() or
                 other.isDate() or other.isNonCallbackInterface())
 
+    def _getDependentObjects(self):
+        return self.inner._getDependentObjects()
+
 class IDLUnionType(IDLType):
     def __init__(self, location, memberTypes):
         IDLType.__init__(self, location, "")
@@ -1476,6 +1532,9 @@ class IDLUnionType(IDLType):
                 return False
         return True
 
+    def _getDependentObjects(self):
+        return set(self.memberTypes)
+
 class IDLArrayType(IDLType):
     def __init__(self, location, parameterType):
         assert not parameterType.isVoid()
@@ -1551,6 +1610,9 @@ class IDLArrayType(IDLType):
         return (other.isPrimitive() or other.isString() or other.isEnum() or
                 other.isDate() or other.isNonCallbackInterface())
 
+    def _getDependentObjects(self):
+        return self.inner._getDependentObjects()
+
 class IDLTypedefType(IDLType, IDLObjectWithIdentifier):
     def __init__(self, location, innerType, name):
         IDLType.__init__(self, location, innerType.name)
@@ -1638,6 +1700,9 @@ class IDLTypedefType(IDLType, IDLObjectWithIdentifier):
     def isDistinguishableFrom(self, other):
         return self.inner.isDistinguishableFrom(other)
 
+    def _getDependentObjects(self):
+        return self.inner._getDependentObjects()
+
 class IDLWrapperType(IDLType):
     def __init__(self, location, inner):
         IDLType.__init__(self, location, inner.identifier.name)
@@ -1741,6 +1806,23 @@ class IDLWrapperType(IDLType):
         assert other.isObject()
         return False
 
+    def _getDependentObjects(self):
+        # NB: The codegen for an interface type depends on
+        #  a) That the identifier is in fact an interface (as opposed to
+        #     a dictionary or something else).
+        #  b) The native type of the interface.
+        #  If we depend on the interface object we will also depend on
+        #  anything the interface depends on which is undesirable.  We
+        #  considered implementing a dependency just on the interface type
+        #  file, but then every modification to an interface would cause this
+        #  to be regenerated which is still undesirable.  We decided not to
+        #  depend on anything, reasoning that:
+        #  1) Changing the concrete type of the interface requires modifying
+        #     Bindings.conf, which is still a global dependency.
+        #  2) Changing an interface to a dictionary (or vice versa) with the
+        #     same identifier should be incredibly rare.
+        return set()
+
 class IDLBuiltinType(IDLType):
 
     Types = enum(
@@ -1906,6 +1988,9 @@ class IDLBuiltinType(IDLType):
                  (self.isTypedArray() and not other.isArrayBufferView() and not
                   (other.isTypedArray() and other.name == self.name)))))
 
+    def _getDependentObjects(self):
+        return set()
+
 BuiltinTypes = {
       IDLBuiltinType.Types.byte:
           IDLBuiltinType(BuiltinLocation("<builtin type>"), "Byte",
@@ -2064,6 +2149,9 @@ class IDLValue(IDLObject):
             raise WebIDLError("Cannot coerce type %s to type %s." %
                               (self.type, type), [location])
 
+    def _getDependentObjects(self):
+        return set()
+
 class IDLNullValue(IDLObject):
     def __init__(self, location):
         IDLObject.__init__(self, location)
@@ -2081,7 +2169,10 @@ class IDLNullValue(IDLObject):
         nullValue = IDLNullValue(self.location)
         nullValue.type = type
         return nullValue
-        
+
+    def _getDependentObjects(self):
+        return set()
+  
 
 class IDLInterfaceMember(IDLObjectWithIdentifier):
 
@@ -2162,6 +2253,9 @@ class IDLConst(IDLInterfaceMember):
     def validate(self):
         pass
 
+    def _getDependentObjects(self):
+        return set([self.type, self.value])
+
 class IDLAttribute(IDLInterfaceMember):
     def __init__(self, location, identifier, type, readonly, inherit=False,
                  static=False, stringifier=False):
@@ -2307,6 +2401,9 @@ class IDLAttribute(IDLInterfaceMember):
     def isUnforgeable(self):
         return self._unforgeable
 
+    def _getDependentObjects(self):
+        return set([self.type])
+
 class IDLArgument(IDLObjectWithIdentifier):
     def __init__(self, location, identifier, type, optional=False, defaultValue=None, variadic=False, dictionaryMember=False):
         IDLObjectWithIdentifier.__init__(self, location, None, identifier)
@@ -2379,6 +2476,12 @@ class IDLArgument(IDLObjectWithIdentifier):
                                                                self.location)
             assert self.defaultValue
 
+    def _getDependentObjects(self):
+        deps = set([self.type])
+        if self.defaultValue:
+            deps.add(self.defaultValue)
+        return deps
+
 class IDLCallbackType(IDLType, IDLObjectWithScope):
     def __init__(self, location, parentScope, identifier, returnType, arguments):
         assert isinstance(returnType, IDLType)
@@ -2446,6 +2549,9 @@ class IDLCallbackType(IDLType, IDLObjectWithScope):
         if len(unhandledAttrs) != 0:
             IDLType.addExtendedAttributes(self, unhandledAttrs)
 
+    def _getDependentObjects(self):
+        return set([self._returnType] + self._arguments)
+
 class IDLMethodOverload:
     """
     A class that represents a single overload of a WebIDL method.  This is not
@@ -2461,6 +2567,11 @@ class IDLMethodOverload:
         self.arguments = list(arguments)
         self.location = location
 
+    def _getDependentObjects(self):
+        deps = set(self.arguments)
+        deps.add(self.returnType)
+        return deps
+
 class IDLMethod(IDLInterfaceMember, IDLScope):
 
     Special = enum(
@@ -2808,6 +2919,12 @@ class IDLMethod(IDLInterfaceMember, IDLScope):
                                   [attr.location, self.location])
         IDLInterfaceMember.handleExtendedAttribute(self, attr)
 
+    def _getDependentObjects(self):
+        deps = set()
+        for overload in self._overloads:
+            deps.union(overload._getDependentObjects())
+        return deps
+
 class IDLImplementsStatement(IDLObject):
     def __init__(self, location, implementor, implementee):
         IDLObject.__init__(self, location)

dom/bindings/parser/tests/test_nullable_equivalency.py

@@ -90,7 +90,7 @@ def checkEquivalent(iface, harness):
     for attr in dir(type1):
         if attr.startswith('_') or \
            attr in ['nullable', 'builtin', 'filename', 'location',
-                    'inner', 'QName'] or \
+                    'inner', 'QName', 'getDeps'] or \
            (hasattr(type(type1), attr) and not callable(getattr(type1, attr))):
             continue
 

dom/bindings/test/Makefile.in

@@ -44,7 +44,6 @@ bindinggen_dependencies := \
   ../Configuration.py \
   ../Codegen.py \
   ../parser/WebIDL.py \
-  ../ParserResults.pkl \
   ../Makefile \
   $(GLOBAL_DEPS) \
   $(NULL)