1217853 - include fewer implementation headers in generated binding headers

Status: NEW

(Core :: DOM: Core & HTML, defect)

Description

[Nathan Froyd [:froydnj]]

I'm sure there's some generic terminology I could talk about here, but I'm going to keep this specific.

dom/crypto/CryptoKey.h includes a bunch of NSS headers, among other things.  This wouldn't be so bad, except that mozilla/dom/SubtleCryptoBinding.h both forward-declares CryptoKey *and* includes the CryptoKey.h header, thus spraying the NSS includes (among other things) all over the place (mostly all the binding files, which seems suboptimal).

In this specific case, I believe this CryptoKey.h inclusion is necessary for

* Explicitly-declared |inline| and defined-in-class-definition copy constructors for dictionaries;
* The implicitly-declared destructors of the same.

The destructors are easy to handle, so we'll ignore those.

Is there any reason that the copy constructors are defined this way?  I note that the default constructor and the copy assignment operator are already defined out-of-line, so it doesn't seem too much of a stretch to define the copy constructor out-of-line too.

In the case of CryptoKey, it doesn't matter, but I guess we have to include the implementation headers in *Binding.h in the case of a union typedef, unless we want to out-of-line all the union methods, which seems suboptimal...

ni? bz for the copy constructor question and if he has additional insight on the union problem(s).

Comment 1

[Boris Zbarsky [:bzbarsky]]

> * Explicitly-declared |inline| and defined-in-class-definition copy constructors for
> dictionaries;

And more importantly the dictionary constructors taking a FastDictionaryInitializer.

These are inline because they're on the actual "hot" binding path: dictionary argument conversion.

On the other hand, maybe we just shouldn't worry about that; the cost of calling an out-of-line dictionary constructor is probably not significant compared to the cost of the actual dictionary init (which involves JSAPI calls and whatnot).

> but I guess we have to include the implementation headers in *Binding.h in the case of
> a union typedef

Sure.  This is not a problem; we can do different things for different member types.  Right now we carefully treat them all identically, but it's a big if cascade, so changing one branch is not hard.

There's a relevant comment in the codegen (in CGHeaders.__init__.addHeadersForType):

                        # Dictionaries with interface members rely on the
                        # actual class definition of that interface member
                        # being visible in the binding header, because they
                        # store them in RefPtr and have inline
                        # constructors/destructors.
                        #
                        # XXXbz maybe dictionaries with interface members
                        # should just have out-of-line constructors and
                        # destructors?