Eyes Above The Waves

Speaking of architectural misfeatures, one that's been glaring for quite some time now is the Gecko "view system" --- nsIView, nsIViewManager and friends. Originally views were intended to be used as "heavyweight" rendering objects for elements that used "advanced" features such as translucency. Over time it became clear that it was simpler to just support those features directly in the "frame tree" of rendering objects that most elements have, and now we see that everything in the view system would be simpler and more performant if it was handled by the frame system.

However, there are large chunks of functionality still handled in the view system. One big one is scrolling. Scrollable elements have an nsHTMLScrollFrame (or nsXULScrollFrame) which handles layout, scrollbars, and related stuff, except for the actual scrolling --- the visual movement. That is handled by nsScrollPortView. Well, until today, when I landed a series of patches to move all scrolling functionality in the view system out to the frame system. The hard part of these patches is that there were two interfaces exposed to let other parts of Gecko manipulate scrolling --- nsIScrollableFrame and nsIScrollableView. A lot of code was using the latter view-system interface; APIs present in nsIScrollableView but not nsIScrollableFrame had to be added to nsIScrollableFrame, and then all the callers of nsIScrollableView APIs had to be modified to use the frame API instead. This was a lot of work --- not really very hard, but difficult to get right without accidentally causing regressions.

Anyway, it's done. A lot of code that used to have to mess around with views no longer has to know about them. Scrolling code is simplified since it's all in one place. Page load is little more efficient because we don't have to construct scrollable view objects. Better still, this just happened to fix the long hang at the end of Firefox loading the HTML5 spec, since that was all about updating a big pile of scrollable view objects! (There are still some issues with slow script execution ... they're being worked on.)

When I did this work I decided to do a little experiment in development style by breaking up the work into a very large number of small patches, managed with Mercurial Queues. The final commit has 37 separate changesets. I thought this might be overkill since our culture, and Bugzilla, isn't really designed to handle that many patches for a single bug. While Bugzilla (especially the way we use it for code review) could certainly be much better at handling a large set of patches, overall I declare the experiment a resounding success. I think this approach made it much easier for me to keep track of things, made code review much easier and less daunting, and made it much easier for me to keep the patches up to date over several months as the tree changed underneath it. I'll do it again. In fact, I'm already doing it again with the layers work --- more about that later :-).