jkeiser's mozilla blog

Event System Madness

I really need to get out of events and fix my other topembed+ bugs, but things keep pulling me back.

Some perfect ass is creating a new nsIDOMEvent, or retargeting the old one, with the target as the textnode, after all that hard work I did targeting the event at the element's parent. This is why bug 103055 has regressed even though it shouldn't have. This leads to some of the points I want to make in this bug, specifically the way we store our target.

Our event system needs fixing, and HandleDOMEvent in particular (the lynchpin of the event system) needs some serious lovin'. Several bugs have been filed on that. Here are some specific things I think we need to do:

1. Store the event target in the nsDOMEvent when you create it. Currently when you ask for the target, nsDOMEvent::GetTarget asks the ESM for the current target. It isn't stored when it's created. Worse, the range asks the ESM for the current target frame all the time--doesn't even cache it. We must be breaking here. The next item is related to this.
2. Make HandleDOMEvent just take an nsIDOMEvent*. We probably originally tried to avoid creating nsIDOMEvent*'s in order to keep heap allocations from happening, but since we actually have an arena for that now, that point is moot. Doing this would make the previous point easier to implement.
3. Move the retargeting stuff out of nsGenericElement::HandleDOMEvent into a separate function that it calls. It's ugly as sin and distracts from the real purpose of the function, which is to handle the DOM event.
4. Factor event flow out of elements. Default handling (like anchors or buttons handling clicks) is currently done by overriding nsGenericElement::HandleDOMEvent and doing your dirty deed there, calling the parent HandleDOMEvent before you do so. We should move it into a HandleDefault() method or maybe even a separate actual listener (HandleEvent()) that is called in its own event pass. (You could make this a special event group that is always called so you avoid the overhead of all those listeners, if we determine that the number of listeners would justify that.) Otherwise an element is responsible not just for handling clicks but also dispatching them. And who wants to be responsible for that? (Certainly not me.) This is a huge blocker for any kind of clean event groups. It also would make it easier to handle fixing the flow of default handlers to go the right direction, which is currently impossible.
5. Stop targeting events at frames. This actually sounds harder than anything else here, and is not something I've been personally bitten by, but it sure sounds like a good idea :) Perhaps we could keep targeting events at frames, but do it through a listener so that the event system is not directly burdened with it and the frames receive the events in their proper time.
6. Remove nsIPresContext* from HandleDOMEvent parameters. We need to accept the fact the we cannot, and will never, handle events on any presentation other than the primary presentation. Just grab that presentation from the doc / window and you are good to go. bz brought this one up.

Events Suck

I begin to understand why joki stopped checking in to events. Anything you do breaks everything else. I wonder when people are going to notice that I re-broke the original bug that started all this with that most recent checkin. Either you break selection or you break tooltips. Wonderful choice.

Documentation?!

I read bernd's CellMap documentation today. A very nice piece of work. Certainly bootstrapped me into being able to understand CellMap and its purpose well enough to review his patch :) Another purpose of documentation: you get reviews quicker. The other benefit, of course, is that you can convince people that you are Right because people have an odd reverence for words scrawled in a permanent place that makes them less inclined to argue uselessly.

Not that I'm going to dust off Ye Olde Documentation Effort again, but it is food for thought--particularly enough to make me want to finish the "form control quirks in Mozilla spec."

Double Reflow

In reviewing the patch for bug 26650 it has become apparent that our reflow system makes us do too much work because of the fact that everything happens in the Reflow() method. In this case, for example, we figure out the size of the children of the button first and then we have to reflow them again just to position them--but we recalculate the width when we do that. Double work.

What would happen in a less monolithic system would be a "GetPreferredSize" or "CalculateWidth" which did not change the position, followed by "GetPosition" which did not change the width. dbaron has touched on this many a time. Perhaps this will be part of any Grand Scheme to make reflow more maintainable / readable.

No More Blogger

This blog has now moved to a Movable Type system on my internal server. Let's see how this plays out.

Unit Testing

On the mountain bike ride the other day, Syd brought up a really good idea for keeping tests up to date that I should have remembered from my earlier corporate days: putting unit tests in the classes or files containing the code to be tested. The theory is sound: the same as API documentation, except it won't compile if you change the API. And it's a little easier to put the test there--a static method in the interface. Plus it encourages modularity--you have to think about your class a little more if the test has to be able to use it in isolation.

You could write a harness and have people just add their classnames and header files into it.

What do Statistics and Nixon have in common?

More numbers: running two simultaneous copies of Mozilla with MOZ_NO_REMOTE=1 on a Linux 2.14.19 gcc 3.2 opt build gives SIZE=26576/SHARE=15320, SIZSE=23868/SHARE=13100. According to bbaetz, the SHARE number is "pretty much" the codesize. Pretty much in line with the Win32 numbers (remember that this is Mozilla, not embedding). Codesize is paramount.

Bloat Musings

Peter and I started looking into why embedders won't touch us with a ten-foot pole. Here are the results of today's foray into the untouchable.

What we found

Bloat is almost entirely due to code size. 10M out of 13.1M was code. Further, fully 25% of our entire startup bloat--including data--is the content+layout dll. Things we can do here:

1. Split out xul, xbl, svg and mathml into separate dlls that are only loaded when needed. A first step to this is to make nsCSSFrameConstructor pluggable. Also important is taking the XUL box model reflow out of XUL or else taking the current HTML elements that use it and using the normal, horrid Reflow method instead.
2. Find dead code. Form control layout has a lot of this. A full code analysis of what code is reached and what code is not reached would help a lot here.
3. Load less libraries. Haven't investigated enough here, but we are responsible for a lot of the symbols loaded from mfc, for example. Are we using all that stuff?

Bloat is also due to fragmentation, as evidenced by an open-2-windows, close-2-windows, open-2-windows cycle where the last two steps do not change the heap size. I suppose some objects in maxheap just stick around when we create new windows, for example, and when the memory is reclaimed it cannot be given back to the OS because of that. One potentially effective way to find out what is going on would be to bring up a new window, close that window, and look at memory to see what objects are still around. We could also have per-docshell allocators for elements and frames that reclaim memory when the docshell goes away, but I don't think we can avoid memory fragmentation with that--the OS gives us a contiguous block of memory.

Non-embedding bloat is very high (7M out of 19.8), indicating problems with chrome and history, as shown by the (mozilla - mfcembed) number. Chrome overlays are one problem (we load more stuff then we need). I believe that XBL is a big problem myself (see the Eternal XBL Form Control Problem). XBL handlers are thought to be one of the culprits, one of the major problems being that key events create one event listener for each key; I also suspect not enough sharing of the bindings in general. History (the list of URLs, titles and dates you visited stuff, which is fully loaded on every startup) takes up a very large percentage of memory, too, according to jrgm--1M for 9 days of active data. Probably because of an excessively verbose storage system.

Data:

DLL size data

Timeline:
Mozilla after loading 2 pages: 19.8M
Same Mozilla after pageload test: 26.75M
After another pageload test: 26.85M
After opening a second window and loading 2 pages: 26.4M (-400K!)
After pageload test on second window: 28.9M
Open 2 more windows: 29.3M
Close both new windows: 29.3M
Open 2 more windows again: 29.3M
mfcembed after loading 2 pages: 13.1M

Bookmark Roaming!

Today I wrote a little bookmark roaming script and added it to mozilla_tools. Nothing fancy, and it requires you to have ssh / scp and an account where you can ssh to. If you don't know what that is, don't try it. If you do know what it is, it's pretty easy to use.

I was looking through nsCSSFrameConstructor some more today for the refactoring of geometricParent crap and found out that for XHTML, any content that tries to be the root content will not get framed--except tables. God knows why; the spec happily tells us that must be the root element, and with good reason--if anything else is the root, positioning and stuff totally bombs (though we don't crash anymore for whatever reason--bug 178885 was the fix, taking it out has no effect). The relevant code is if (docElemIsTable) in ConstructDocElementFrame. Perhaps someone will tell me why in Thor's name we want to allow this. xhtml isn't really out there yet, for goodness' sakes, we shouldn't let people do stupid stuff already.

Let's see if / how

Let's see if / how this blogger deal works.