Bug 216336 – [WebIDL] Split supplemental interfaces out of Document

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

[WebIDL] Split supplemental interfaces out of Document

[Sam Weinig]

Created attachment 408393 [details]
Patch

[Darin Adler]

Comment on attachment 408393 [details]
Patch

Do these really all need to be separate source files?

[Sam Weinig]

(In reply to Darin Adler from comment #2)
> Comment on attachment 408393 [details]
> Patch
> 
> Do these really all need to be separate source files?

That’s a great point. I’m not sure what the best thing to do here is. Currently we only support supplements that are in their own files, but that is easy to fix and shouldn’t affect the decision. It would be good to have an easy to follow rule for how we divide things up.

[Konstantin Tokarev]

Speaking as one who often refers to idl files to determine if certain API is available, it would be more convenient to have all parts of Document in the same file. OTOH, if respective C++ implementations were located in separate files, it would make sense to have separate IDLs, though AFAICS it's not the case here.

[Sam Weinig]

I think it might help me think about this more clearly if I break down the different types of supplemental interfaces we have:

1) 'partial interface' which uses Supplemental model

An example of this is DocumentPictureInPicture.idl, which uses the class DocumentPictureInPicture (`class DocumentPictureInPicture : public Supplement<Document>...`) from DocumentPictureInPicture.h/cpp. 

2) `partial interface` which is completely implemented in the main class.

An example of this is DocumentAnimations.idl. It adds the timeline attribute to Document, which is completely implemented in Document.h/cpp.

3) `interface mixin`

These, currently, all must be implemented by the interface they are included into. An example of this is FontFaceSource.idl which adds the fonts attribute to Document, which is completely implemented in Document.h/cpp. However, as this is a mixin that can also used by other interfaces, putting it in Document.idl would not make sense.

4) `partial interface mixin`

These also, currently, all must be implemented by the interface they are included into. An example of this is DocumentOrShadowRootStyleSheets.idl, which adds the stylesheets attribute to Document and is completely implemented in Document.h/cpp. However, as this is an addition to a mixin that can also used by other interfaces, putting it in Document.idl would not make sense. It might make sense to put it in DocumentOrShadowRoot.idl though.

[Sam Weinig]

(In reply to Konstantin Tokarev from comment #4)
> Speaking as one who often refers to idl files to determine if certain API is
> available, it would be more convenient to have all parts of Document in the
> same file. OTOH, if respective C++ implementations were located in separate
> files, it would make sense to have separate IDLs, though AFAICS it's not the
> case here.

One idea to make this easier would be to have a consistent naming of partial interface IDL files, perhaps one that is distinct from our normal naming conventions. For instance, perhaps we could take inspiration from some cocoa naming conventions for categories and do something like Document+WebAnimationAdditions.idl instead of DocumentAnimations.idl. Then, when looking for things in Document, one would search for "Document+".

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 408393 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Sam Weinig]

Created attachment 408533 [details]
Patch

[EWS]

Committed r266926: <https://trac.webkit.org/changeset/266926>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408533 [details].

[Radar WebKit Bug Importer]

<rdar://problem/68713612>