Standard8’s Blog

In the address book there has been, for quite a while, various problems whilst managing addition and deletion of address book cards to and from Mailing Lists - things like cards not being removed from the display, duplicated cards displayed (but click onto a different address book and back again and it sorts itself out).

With thanks to Siva (from Spicebird), all these problems have now been fixed (with the exception of one remaining problem).

This is an excellent fix and should hopefully reduce at least some of the confusion for users have experienced whilst updating mailing lists, and ensure that we’ve got the right notifications being fired for extensions to pick up on.

The remaining issue we’ll probably take another look at once we’ve got some other back end work done. If you want to try it out then get the latest trunk nightly (at your own risk!) or wait for Shredder Alpha 2/SeaMonkey 2.0 Alpha 1.

Now that we have some unit test structures in place for MailNews, we want to make it easier for new tests to be added, and clearer as to the required procedure.

Therefore, we have defined a Rules and Review Policy for unit tests that are to go into the source directories mailnews/ and mail/. This policy does not affect the existing review requirements for patches on the main code.

As I’ve said before, at the moment we still do not require unit tests with every patch as the full archecture is still being developed. We do however, strongly encourage unit tests where possible. If you need help, feel free to ping me (Standard8) on irc.

After the mailnews/ and mail/ source trees reopened yesterday, I checked in a whole bunch of patches that had been queued up during the freeze. Some of them had been in my development tree, some of them hadn’t.

A little while later the SeaMonkey unit test tinderbox was orange. This surprised me, as I was fairly sure I had run all the mailnews tests with the various patches.

It turns out one of the patches (a follow up to bug 424570) had caused a regression where deleting address book cards wouldn’t work properly. This was one of those cases where before we had unit tests I wouldn’t have been surprised if we hadn’t detected the regression for a while.

So as Dan said, score 1 for MailNews automated tests. Although we’ve already seen regressions caught many times for the core tests, its good to see that the MailNews ones are now starting to catch regressions as well.

Today is the first Thunderbird test day using the potential Shredder Alpha 1 builds. Find out more about the test day here.

Having been working on various bits of Thunderbird’s Core MailNews code (and some of the specific code as well) since the Mozilla 1.8 branch was originally cut, it is exciting to be getting close to releasing the first alpha since the last major release. Whilst there is lots more work to do to get us to the final, its the first big step in the right direction.

So, please do come along and help us test the alpha candidates. The more testing we get, the more confidence we’ll have in the candidates (assuming no failures!), and feedback will help us learn where we need to improve for the next alphas, betas and the final.

(I actually found the bugs a few days ago, but only just got round to blogging about it)

A couple of days ago, I was reviewing a patch and came across a function call to an interface of the type:

void foo(out unsigned long aCount, [array, size_is(count)] out bar aResult);

Where bar was an object derived from nsISupports. The aResult was being freed with NS_Free, instead of NS_FREE_XPCOM_ISUPPORTS_POINTER_ARRAY. This would have ended up with MailNews leaking most of the objects in the array.

Being curious about where this came from, I did a quick search for the function, and found one instance already in the main code base, where we called NS_Free instead of the NS_FREE_XPCOM_ISUPPORTS_POINTER_ARRAY. It wasn’t a major leak and would only happen on migration of some of the tag preferences, and for most users would probably end up being 300 bytes. However bug 431404 was raised, and that will soon be checked in.

Having realised that at least one instance in MailNews did this wrong, I did a quick audit of the other function calls in MailNews that obtain an array in the style above. A bit of searching later and I filed the patch on bug 431414, this fixes several leaks in migration, one potential leak in the LDAP autocomplete code, and two leaks related to getting/receiving mail.

The leaks I found when getting/receiving mail, really are small, effectively 4 or 8 bytes per message or newsgroup post, so you probably won’t notice too much difference. These also won’t show up on the Leak stats as we don’t currently receive messages in those tests (that is one of the things I want to extend those tests to do).

Improving MailNews performance, reducing leaks, bloat etc, is something I really want to see happen, this is one small step along the road, and I’ll certainly be looking out for more instances where we do our memory management wrong.

Now that Thunderbird is in 3.0 Alpha 1 Code Freeze, I want to point out some of the changes since Thunderbird 2 to the Address Book back-end code and where it is likely to be going post-Alpha 1.

First, though, for all of those wanting to see improvements to the UI - we are starting to discuss these in more detail (keep an eye on the newsgroups), and are planning for them to be included Thunderbird 3.0 - the back-end improvements have currently taken priority as they will give us a much better basis for us to improve the UI.

So what are the main changes so far in the address book between Thunderbird 2.0 and Thunderbird 3.0alpha1?

[Read more →]

For those of you that have passed by the SeaMonkey tinderbox tree recently, you’ll probably have noticed some new tests taking place: Mail RLk, Mail Lk, Mail MH, Mail A. These are the first part of providing performance tests for MailNews.

The new tests take similar measurements to the Browser leak and bloat tests. Rather than loading lots of pages, we (currently) simply start up and close the Main Mail, Address Book and Compose Windows with a basic profile in place (for those of you who may have noticed the big jump in numbers that’s when we added the Compose Window in).

I’d like to say thank you to Andrew Schultz (one of my co-members of the SeaMonkey Council) for allowing me to try out the patches on his tinderbox.

If you want to know more about how do the tests and what the numbers mean, I’ve already documented that on the wiki page.

These tests will hopefully be appearing on the Thunderbird tinderbox tree soon as well

At the moment, this is just the early version of these tests - we wanted to get something going so that we had a basic set of numbers. As mentioned on the wiki page we have ideas for running more extensive tests, so don’t be surprised if the numbers vary a bit over the next few weeks.

I’ll also be taking a look at starting to get other performance tests together soon - based on how we have implemented the Leak and Bloat tests has already given me some ideas.

Another area I’ll be looking at is getting xpcshell tests running for Thunderbird - currently some of the tests won’t pass, and then there is how to do mochitest-like-tests. Again, I’ve got a few ideas, just need some time to look into them.

On a slightly different note, I’ll be attending the Calendar Face-2-Face meeting next weekend in Hamburg.

Today is my first day working for Mozilla Messaging. This will be very different to my old job, for starters, I will be working from home (in the UK). However I’m looking forward to it, and I already have lots of things to do. Currently I have on my list:

• Address Book Reorganisation - Based on some ideas I put together, Joshua has already done a lot of work on actually implementing the first major parts of this. There’s more work on some of the interfaces to do, especially to support writeable LDAP directories. Once I’ve done that, I’ll blog a bit more about what we are doing and why.
• Password Manager in MailNews - currently, MailNews requires the old “wallet” password manager, this is largely unmaintained code, and we want to move to using toolkit’s password manager. I have already implemented the necessary interfaces in toolkit’s password manager, I now need to work on the required changes to MailNews to complete this.
• Automatic xpcshell unit testing - in the last few months, we have started adding some basic xpcshell unit tests to MailNews, however the SeaMonkey trunk tinderbox is the only one that runs these tests. We want to get the Thunderbird trunk tinderboxes running them as well, however that requires some issues fixing to do with the way Thunderbird is configured (because it is not a browser).
• Automatic UI and Performance testing - Additionally we want to expand the capabilities of MailNews to include Mochitests (or something similar) and performance testing. I think these will take some time to set up properly, but I wanted to let you all know we are thinking about them.

Alongside those items, I’ll also be handling most of the “normal” background tasks, e.g. reviews, bug fixes, backend-improvements, helping the community, helping on bug days etc.

There’s lots of work for me to do, so I’d better stop writing the blog and get on with it

A few days ago, I wondered if any of the current “XPCOM objects created/destroyed from static ctor/dtor” warnings were caused by MailNews. A bit of digging later and I filed bug 424677 - nsImapProtocol.cpp had the only warning that came from MailNews; I checked in the fix a few hours ago.

It just so happens that the fix I checked in for the static object was additionally the fix for the last 4 bytes that the Rlk test on the SeaMonkey tinderboxes had been showing for quite a while. This is no where near a major leak fix, and although will still do leak (as per the newer Lk test), it is really satisfying to have at least one of the numbers at zero.

I’d like to note that the main part of reducing Rlk was done by Firefox, who last August, got its Rlk figure down to 0 bytes. Their work on reducing leaks in the core code has helped benefit everyone - I just happened to find the last bit for MailNews/SeaMonkey.

As I start this blog, I have a week left at my current employer, Thales UK (Aerospace). In two week’s time, I will be starting my new job at Mozilla Messaging (yes I’ve got a week’s holiday in between).

For those who don’t know so much about me, here’s a quick resume:

• I live in Leicester in the United Kingdom.
• I am a Christian.
• I left University in 2001 with a First Class degree in Electronic and Software Engineering.
• Thales has been my employer since then (albeit with different company names).
• I started hacking on MailNews around the second half of 2004 in my spare-time. Since then I’ve mainly worked on the Address Book, though I’m starting to branch out more now.
• I am a member of the SeaMonkey council.
• I am a MailNews peer.

The Mozilla Messaging job mainly came around because of my spare-time work on MailNews. Its an exciting prospect (though not without some nerves as it is a big change for me), and I’m hoping that with the support of the community, the Mozilla Messaging team can bring Thunderbird forward and produce the most useful, enjoyable communications tool possible.

More on what I’m actually doing in a few days…