Monthly Archives: October 2008

I’m pleased to announce that the first pieces of the Private Browsing feature have just landed on Firefox trunk! This might not be something to get too excited about, since all of the landed code remains disabled for now, but it’s a big breakthrough for me, considering the fact that I’ve been playing with this code since January! Some of you may even remember that this feature was cut off Firefox 3 because of the fact that it was too big to take at that stage of Firefox 3 development, it will be included in the final release of Firefox 3.1.

Based on my latest chat with Marcia on IRC, the current plan is to finish the work on the code for private browsing, and land all of the remaining pieces by the end of October 26. After that, during the week of October 27, we are going to have a test week for the community to start testing the Private Browsing mode to make sure that it will be rock solid in the final release. Of couse, every piece of this patch has automated unit tests (on which Aaron has been helping me) to make sure that the feature at least works according to the functional specification, but a feature of this size still needs lots of human testing as well. Stay tuned for more updates on the schedule.

A little bit more of technical details follows. The pieces landed at this stage include the nsIPrivateBrowsingService, and the private browsing implementation and unit tests for the Places, Cookies, Content Preferences, and Form History modules. These include all of the code necessary to implement Private Browsing handling in those modules, but because the implementation has been designed to ignore the absence of the Private Browsing service if it’s not available (like the case of these pieces of shared code using in other applications than Firefox), nothing will change in the functionality of these modules. Those who want to test the Private Browsing mode should still run try server builds that I post to the Private Browsing bug.

For the details of what was checked in, check out the links below (pun intended):

I was trying to write a unit test for my modifications to the cache service as part of the implementation of private browsing, and the experience wasn’t as smooth as I would have liked. So, I decided to document my experience here.

Firstly, there doesn’t seem to be any automated unit tests for the cache service. The only test which uses the cache service directly inside netwerk/test/unit is the test for bug 356133. There are a bunch of stand-alone js and C++ tests residing in netwerk/test, but looking through the makefile, none of them seem to be built or run by default. Oh, and I know that the cache service code is fairly old, tested, and stable, but still, no unit tests at all? We’ve got to do better than that!

Moving on to writing the unit test anyway, the first step was to check if all three cache devices (memory, disk, and offline) are available. So, I wrote the following code in order to test this:

But I was quite surprised to find out that the visit operation does not find any devices whatsoever! After some time spent reading the source, I figured out that the cache service initializes the devices lazily. In other words, when the cache service is initialized, none of the cache devices get created, and they would each get created on a per-use basis. So, what I did was write some code to add a cache entry for each type of device, and then try to check the availability of the devices. The code I wrote was based on the storeCache function, and looked like this:

So, all was well when testing this on a debug xpcshell, so I decided to run the code on an optimizied non-debug build of xpcshell as well (like I always do), and boom! An NS_ERROR_FAILURE was being thrown on the nsICacheSession.openCacheEntry call. I tested this again and again, but I kept getting the same error in the non-debug build. After spending some time debugging the error, I found out that the culprit was this code. What happened was the cache service tried to get the profile directory in order to find a place to store the disk-based cache files (which will of course fail in xpcshell environment where the profile is not accessible), and in debug builds, it reverted to using the current process directory since the profile directory was not accessible. In non-debug builds, the code simply detected that there’s nowhere to store the cache files, so it reported an error which was thrown as an obscure NS_ERROR_FAILURE to the caller. Providing the profile directory manually did the trick:

Another oddity I encountered was when trying to read from the cache. A read from the cache should either lead to success, or failure by throwing NS_ERROR_CACHE_KEY_NOT_FOUND from nsICacheSession.openCacheEntry. So, I happily wrote the following code, hoping that all would be well.

But things were not that easy. The problem was that inside the private browsing mode (where the disk and offline cache devices are turned off and disabled), trying to read an entry from the disk device lead to the NS_ERROR_CACHE_KEY_NOT_FOUND exception, while trying to read data from the offline device lead to NS_ERROR_FAILURE. The fix was easy: just handle this error as another possible result value on the exception.

The cache service really needs some unit test, and documentation love. Nice things to see documented would be the lazy initialization of devices, and the fact that the cache service picks up the process directory in debug mode as the cache directory. I guess all of these can be covered in some sort of tutorial to the cache service APIs. Anyway, until such a turotial gets written, I hope this post would make the life of people who decide to use the cache service a bit easier.

Today I finally made a try server build which includes patches to some bugs, each of which fixing a problem that the ACID3 test is checking for. The build gives us 97/100 points on the ACID3 test, which is impressive. Here is the list of the fixes that this build includes:

The ACID3 tracking bug has details on the failing tests, and is also tracking the bugs filed in Bugzilla for fixing the remaining tests.

Oh, and another cool thing about this build (in case it’s not obvious already) is it includes SMIL support! SMIL is a W3C recommendation for creating declarative animations, which is integrated into SVG. When you check out this build, make sure to check out some SMIL tests available here.

Oh, and just to mention the obvious, neither of these patches are written by me; I just applied them on my mozilla-central hg clone, and pushed to the try server, with a simple patch I copied from Daniel Holbert’s patch to enable SMIL to build by default (because it doesn’t seem to be possible to pass a customized .mozconfig file to the try server to use in its builds, and SMIL support by default is turned off, unless –enable-smil is used in .mozconfig).

My work on the Private Browsing patch is soon going to enter a new stage. Four of the modules that the patch is touching already have unit tests. The only part of the patch which is not correctly implemented yet according to the recent changes in the functional spec is the download manager module, which needed a back-end change to support in-memory databases. I’ve implemented that in another bug I filed to track it, and my patch there is waiting for review.

In a recent discussion with mconnor, we decided that it would be best to split up the patch according to the boundaries of the modules that it’s touching, and ask for review on each part separately. I’m going to do this today. There are four modules which already have unit tests and are ready for review:

Cookies

Content Preferences

Passwords Manager

Authenticated Sessions

The Places module is also nearly ready for review, thanks to Aaron Train, a Seneca college student who has volunteered to write some unit tests for Private Browsing.

The nifty thing is that once I get the necessary reviews for each module, it would be possible to land it, because the unit tests are designed such that they would pass if the Private Browsing service is not available. I would still continue to publish monolithic patches in the Private Browsing bug, to make the lives of those who want to try out the full patch easier, so to reduce the amount of confusion, I’m going to mark the "for review" patches by naming them with this pattern: "[for review] module vn", where module is the name of the module and n is the revision of the module specific patch, initially set to 1 and incremented if a reviewer requires changes to the implementation of that module.

Stay tuned for future updates on the Private Browsing progress. Like always, feedback in form of comments on this blog, bug 248970, or email/IRC notes are welcome.