On Aug 17, 2010, at 9:35 AM, Eric Seidel wrote: > My theory on re-organizing document is we do the same thing we've been > doing to FrameLoader. Just start lopping off chunks. > > My understanding is Adam was attacking FrameLoader by just grabbing a > set of seemingly related member variables, throwing them in a separate > class (in the same file) and hitting compile. :) And then letting the > compiler explain to you what functions you should be moving off of the > big class and onto your new smaller class. > > Possible classes which could split of from Document: > > - A DOM API object (to handle all the actual api)
This one should just *be* Document. Our DOM objects are the DOM API. Some of the other pieces you mention might be reasonable chunks to break off. Regards, Maciej > - An event handling object (see all the > DEFINE_ATTRIBUTE_EVENT_LISTENER? Maybe this is part of the DOM API > object) > - All the style computation junk "uses*Rules, as well as the ownership > of the styleselector, etc. > - Form element tracking > - Management of the Rendering Tree? (RenderArena, RenderView should > be owned by some renderingTree() object instead it seems) > - Style and "color" management > - Marker management (wow, that's a huge section of needlessly Document code!) > - The JSC Wrapper cache > - Dashboard support > - URL management (base, cookies, etc.) > - Stylesheet management (maybe part of style/color management above) > > That list was just from scanning Document.h > > There is clearly a huge amount of low-hanging fruit. When Adam was > cleaning up FrameLoader, and when we more recently re-wrote the > DocumentParser infrastructure, we found and fixed *tons* of bugs when > the code was split down into bite-sized chunks. I suspect we'd find a > bunch of dead code and bugs if we started ripping Document.cpp apart. > > -eric > > On Tue, Aug 17, 2010 at 11:20 AM, Dean Jackson <d...@apple.com> wrote: >> >> On 17/08/2010, at 7:21 AM, Eric Seidel wrote: >> >>> My apologies for derailing your earlier discussion. I just was in >>> Document.cpp again yesterday and my mind was blown by the madness that >>> is that god-class. Then you posted about adding to Document (which >>> sounds like the right corse of action here!) and I took advantage of >>> your post for my stop-pooping-on-Document PSA. >>> >>> I too have 100% confidence in Dean here. :) As Maciej says, it's just >>> a controller on Document. >> >> Congratulations on being the first person to ever have confidence in me :) >> >> I too would like to know how to reorganise Document better. It is HUGE. >> Seeing as you are discussing similar topics on IRC with EricC, maybe now is >> the time to bring it up. >> >> Dean >> >>> >>> Carry on. >>> >>> -eric >>> >>> On Tue, Aug 17, 2010 at 8:50 AM, Dean Jackson <d...@apple.com> wrote: >>>> >>>> On 17/08/2010, at 12:22 AM, Maciej Stachowiak wrote: >>>> >>>>> >>>>> On Aug 16, 2010, at 10:52 PM, Eric Seidel wrote: >>>>> >>>>>> Where-ever it goes, please don't put it on Document directly. :) >>>>>> (Feel free to tie it to Document's lifetime, just don't add to >>>>>> Document's 300+ methods.) >>>>>> >>>>>> The madness must stop... Document is long overdue for some weightloss... >>>>> >>>>> I assume Dean is suggesting that Document would gain a new data member >>>>> (DeviceMotionController?) which strikes me as a reasonable approach. >>>> >>>> Right - either one or two (+DeviceOrientationController). I do think the >>>> controllers of both events could be a single object - but that is another >>>> discussion. >>>> >>>> If it wasn't on Document, what do you suggest otherwise? DOMWindow? >>>> Putting it on Frame but tying it to Document seems less than perfect. >>>> >>>> Dean >>>> >>>>> >>>>>> >>>>>> -eric >>>>>> >>>>>> On Mon, Aug 16, 2010 at 11:43 PM, Dean Jackson <d...@apple.com> wrote: >>>>>>> Hi, >>>>>>> >>>>>>> I've been looking into implementing the clients for >>>>>>> DeviceOrientation/Motion Events. Currently, the controllers for these >>>>>>> events are members of Page. I think they are better suited on Document. >>>>>>> >>>>>>> Here are a few reasons: >>>>>>> >>>>>>> - Page isn't tied to any actual web page or document. Would we want to >>>>>>> share the same controller and client across multiple web pages? I don't >>>>>>> think so. >>>>>>> - Document is already the place that is listening for these events >>>>>>> - It's easy to suspend and resume the client from the Document-level >>>>>>> when the user navigates to another page, or the document enters the >>>>>>> cache, or a platform needs to temporarily suspend events for some >>>>>>> reason. >>>>>>> - When the API is on Page, it is hidden in the WebView, from where it >>>>>>> is difficult to access. >>>>>>> - it would allow the client to live in WebCore. This avoids tweaking >>>>>>> the WebView implementations for all platforms to pass messages back and >>>>>>> forth >>>>>>> https://bugs.webkit.org/show_bug.cgi?id=41616 >>>>>>> >>>>>>> I assume one of the advantages of having them on Page is that it allows >>>>>>> a Mock Controller to be easily created for testing from Dump Render >>>>>>> Tree. Am I right? Is this that important? >>>>>>> >>>>>>> FWIW, Geolocation seems to take both approaches. One implementation is >>>>>>> down in Navigator/Document/DOMWindow, but the mock controller is on >>>>>>> Page. I've found the low-level approach much easier to implement. >>>>>>> >>>>>>> Thoughts? >>>>> >>>>> For this sort of thing, it seems reasonable to me that there is a layer >>>>> of the implementation that is a per-document controller, and then below >>>>> that a singleton object in case we don't need things to happen multiple >>>>> times per document. It doesn't seem especially helpful to have something >>>>> happen at the Page level, in normal operation. >>>>> >>>>> The reason it seems a singleton would be useful is that you only want to >>>>> register with the OS service once, and then multicast the relevant >>>>> notifications to all clients that are listening. >>>>> >>>>> For purposes of substituting a mock controller: >>>>> >>>>> (1) If there is a singleton beneath the per-document controllers, perhaps >>>>> the mock object can insert itself at that level. >>>>> (2) Even if the mock controller is at the per-document level, it is >>>>> likely still sufficient for most tests; it might mean a little extra work >>>>> if we need to specifically test geolocation or device motion/orientation >>>>> from a subframe. >>>>> >>>>> Regards, >>>>> Maciej >>>>> >>>> >>>> >> >> _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
