[webkit-dev] Making DOMWindow less epic (was Re: WebKit modularization)

[Maciej Stachowiak]

On Feb 24, 2012, at 11:58 AM, Adam Barth wrote:

> On Fri, Feb 24, 2012 at 11:50 AM, Maciej Stachowiak <mjs at apple.com> wrote:
>> On Feb 24, 2012, at 10:27 AM, Adam Barth wrote:
>>> 2012/2/24 Maciej Stachowiak <mjs at apple.com>:
>>>> I too am surprised that HTML-related APIs would be refactored as a result of modularization. This change may be justifiable on its own merits, but it doesn't seem like a logical part of a project to make self-contained features more modular. At the very least, to avoid confusion, changes like that should be kept clearly separate from the modularization effort, or else, someone could explain the relationship if there is one and its not obvious.
>>> 
>>> Fair enough.  I've detached those bugs from the larger meta bug.
>>> 
>>> These patches have a different goal than the other patches attached to
>>> that meta bug.  Much in the same way that we've moved code out of
>>> Frame.h over time, these patches are intended to make DOMWindow.idl
>>> more readable.  The net result will (hopefully!) be a file that's more
>>> focused on concerns that actually relate to DOMWindow (e.g.,
>>> name/closed/opener/parent/top) rather than a dumping ground for every
>>> random thing that needs to be in the global scope.
>>> 
>>> In retrospect, we should have presented this work separately so folks
>>> could have discussed its merits separately.  I think we got tied up in
>>> the implementation detail that the same mechanism makes both projects
>>> possible.
>> 
>> I think this case is a little different than Frame.h, because with Frame, we can actually move related methods to separate objects, thus actually splitting up the interface. This creates an actual separation of concerns in the code if done right, not just a file split. I'm not sure that having multiple files which actually all form a single interface is equally beneficial. It doesn't seem hugely worse, but it does not seem obviously better to me either.
> 
> It's quite analogous to Frame.h in the sense that this mechanism also
> lets us move related methods to a separate object.
> 
> Consider a case such as
> <http://trac.webkit.org/browser/trunk/Source/WebCore/fileapi/DOMWindowFileSystem.idl>.
> When we moved webkitRequestFileSystem from DOMWindow.idl to
> DOMWindowFileSystem.idl, the code for webkitRequestFileSystem moved
> from DOMWindow.cpp to
> <http://trac.webkit.org/browser/trunk/Source/WebCore/fileapi/DOMWindowFileSystem.cpp#L53>,
> separating these concerns from DOMWindow.

Splitting out the file-related stuff from DOMWindow seems justified based on modularity grounds, presuming we see File API as a self-contained feature. So no objection there.

> 
> In the case of <http://trac.webkit.org/browser/trunk/Source/WebCore/html/DOMWindowHTML.idl>,
> the C++ code that backs those interfaces is already in
> Source/WebCore/html.  The net result is 100 less lines of "noise" in
> DOMWindow.idl.

It seems to me like the practical difference of the IDL move in this case is changing the IDL file where you need to add HTML elements, and adding an extra place you need to look to see what's in the global scope. It's not obvious to me that this is an improvement, but like I said, it doesn't seem terrible.

I think there is a potentially even better approach here though. HTMLTagNames.in is already used to autogenerate most of the things that need to mention every HTML element. If it was used to avoid the need to explicitly mention all HTML element constructors in any IDL file at all, for instance by autogenerating one, then it would reduce the number of places that have to mention every HTML element by 1, and eliminate one of the possible mistakes when adding a new HTML element.

Regards,
Maciej