[webkit-dev] The Care and Feeding of WebCore Modules

[Adam Barth]

On Tue, Feb 28, 2012 at 4:43 PM, Kentaro Hara <haraken at chromium.org> wrote:
> First of all, I am sorry we have done things too aggressively without
> a community consensus. I'll stop committing patches until reaching a
> consensus, and am happy to revert some patches that have already
> committed.
>
> As Maciej pointed out, let us clarify
>
>> 1) What should be a Module?
>
> IMO, the feature that should be a Module is a "self-contained" feature
> that has very loose coupling with WebCore code. It would be a
> port-specific feature or an experimental feature that perhaps is not
> even of interest to other ports. One practical criteria would be
>
> - If we can remove _all_ #ifdef flags from WebCore/ and the feature
> does not access WebCore/ code in a intrusive way, the feature should
> be a Module.
>
> In that sense, I think the existing candidates for Modules are as follows:
>
> gamepad
> geolocation
> indexeddb
> intents
> mediastream
> vibration
> websockets
> webaudio
>
>
>> == Current Non-Modules Using Module-Related Techniques for Loose Coupling ==
>>
>> devicemotion
>> deviceoritentation
>> fileapi
>> html
>> notifications
>> protocolhandler
>> speechinput
>> svg
>> webgl
>> websql
>> workers
>> xml
>
> As ap pointed out, now I agree that just factoring HTML-related APIs
> out of DOMWindow.idl to DOMWindowHTML.idl would just increase the
> number of files and make it difficult to track the code base. At
> present, the disadvantages would be larger than the advantages.
>
> So my answer to Maciej's second question:
>
>> 2) To what extent should Module-like techniques be applied to non-Modules?
>
> is that "let us stop applying Module-like techniques to non-Modules,
> until the modularization techniques mature well and the advantages of
> applying it become obvious". If there is no objection to this opinion,
> I'll revert back patches that have already committed around this.

IMHO, the changes to Page have been valuable.  Does anyone really
think we should undo, for example, these changes:

http://trac.webkit.org/changeset/108446/trunk/Source/WebCore/page/Page.h
http://trac.webkit.org/changeset/108446/trunk/Source/WebCore/page/Page.cpp

Reverting the DOMWindowHTML.idl change does seem like the right thing
to do, however.  There's certainly less value being created there.

Adam


> On Wed, Feb 29, 2012 at 8:47 AM, Adam Barth <abarth at webkit.org> wrote:
>> On Tue, Feb 28, 2012 at 1:59 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>>> On Feb 28, 2012, at 12:20 PM, Adam Barth wrote:
>>>> On Tue, Feb 28, 2012 at 11:52 AM, Maciej Stachowiak <mjs at apple.com> wrote:
>>>>> But it seems like what is actually happening is wholesale rapid application
>>>>> of this pattern to all of WebCore, including even things that aren't in the
>>>>> Modules directory. So it's starting to seem more like a major restructuring
>>>>> of WebCore than like a lower-impact way of doing new peripheral features. It
>>>>> seems to me that starting on a more limited scale would have a better
>>>>> opportunity to measure WebKit community buy-in. If the main motivation for
>>>>> this project is hackability, then I think it needs to make people feel like
>>>>> hackability is actually improving. Ideally we'd measure that with an
>>>>> incremental step of some kind before applying it everywhere.
>>>>
>>>> I think this is a mischaracterization of the work we've been doing in
>>>> this area.  Are the particular patches you think would benefit from
>>>> more discussion?  You've mentioned the DOMWindowHTML patch, which we
>>>> can certainly revert and re-consider later.
>>>
>>> I'm sorry if you think I mischaracterized your work, and I see in retrospect that my wording was overly judgmental. Here's the point I was trying to get at. The discussion threads about the Module have been superficially about the mechanism, but I think at the root of these discussions are the following two and a half questions:
>>>
>>> 1) What should be a Module (now that we understand what that implies in addition to a directory move)?
>>>    1)a) In addition to what specific existing code should be a Module, what standards should we apply in the future?
>>> 2) To what extent should Module-like techniques be applied to non-Modules?
>>
>> Those sound like good questions.  Our approach hasn't been to try to
>> answer all these questions upfront but rather to work on the low
>> hanging fruit, learn, and iterate.
>>
>>> I think if we all have a shared understanding of the answers to these questions, it will greatly reduce the confusion/controversy around Module-related changes, and therefore make it easier for you and the others working on this project to get your work done
>>>
>>> Below is my understanding of the current state of the code and potential future plans, which anyone more knowledgable should feel free to correct.
>>>
>>> Once we have the right list to look at, I'm sure I and others will have comments about specifics. For example, I think File API (as opposed to Filesystem API more specifically) would be a poor candidate for a module since it is used by core code.
>>
>> That's consistent with our current thinking (which is different from
>> our previous thinking, for exactly the reasons you state above).
>>
>>> However, I'd like to make sure I'm looking at the right data before I comment, since I'm not sure modularizing File API is even part of the plan any more.
>>>
>>> == Existing Modules ==
>>>
>>> gamepad
>>> geolocation
>>> indexeddb
>>
>> ^^^ indexeddb is more of a work-in-progress than the rest of these.
>> We're still sorting out the relationship with SQLDatabase.
>>
>>> intents
>>> mediastream
>>> vibration
>>> websockets
>>>
>>> == Possibly Planned Modules, Based on Previous Emails and the Spreadsheet ==
>>>
>>> fileapi
>>
>> ^^^ Currently, we're thinking this wouldn't be a good choice because
>> concepts like Blob are used throughout WebCore.
>>
>>> filesystem
>>> notifications
>>> microdata
>>
>> ^^^ This one probably isn't a great choice either because it's
>> relatively tightly coupled to HTML (and is part of the HTML
>> specification itself).
>>
>>> pagevisibility
>>> pointerlock
>>
>> ^^^ After discussing this feature in
>> <https://bugs.webkit.org/show_bug.cgi?id=79625>, we marked the bug
>> WONTFIX.  There some discussion in the bug about why.
>>
>>> protocolhandler
>>> storage
>>
>> ^^^ There's a bunch of different features mixed together in this
>> directory.  IndexedDB and SQLDatabase look like good candidates for
>> becoming modules.  I haven't studied the rest of the code to have much
>> of an opinion on the rest.
>>
>>> webaudio
>>> workers
>>
>> ^^^ I view workers more as a "hub" rather than a "spoke", much like
>> DOMWindow is a hub.  We don't have any plans to modularize workers.
>>
>>> == Current Non-Modules Using Module-Related Techniques for Loose Coupling ==
>>>
>>> devicemotion
>>> deviceoritentation
>>> fileapi
>>> html
>>> notifications
>>> protocolhandler
>>> speechinput
>>> svg
>>> webgl
>>> websql
>>> workers
>>> xml
>>
>> In some of these cases, such as notifications and websql, our hope is
>> to fully factor them out of WebCore proper.  We're taking an
>> incremental approach to loosen the coupling first and (if we succeed).
>>
>> In other case, such as speechinput, we're using this same machinery to
>> loosen the coupling.  For example, we've removed a bunch of "junk"
>> code from Page:
>>
>> http://trac.webkit.org/changeset/108446/trunk/Source/WebCore/page/Page.h
>> http://trac.webkit.org/changeset/108446/trunk/Source/WebCore/page/Page.cpp
>>
>>> == Possibly Planned Future Uses of Module Techniques for non-Modules ==
>>>
>>> events
>>
>> Similar to speechinput, there are a bunch of "controller" objects that
>> hang off of Page to share its lifetime but don't otherwise interact
>> with each other or with Page itself.  Using
>> <https://trac.webkit.org/wiki/Modules#Associatingstatewithcoreobjects>
>> we can remove this bloat/complexity/ifdefs.
>>
>> Adam
>
>
>
> --
> Kentaro Hara, Tokyo, Japan (http://haraken.info)