How we’re breaking some extensions in the near future

You may have read some reports about how we’re re-implementing the bulk of the extension manager in Firefox. It’s been a long running project (something like a year since I first really started planning how to do it). Things are finally started to come together and all being well we are likely to look at landing the first pieces of this on the trunk nightlies in as little as a weeks time. I’ll be up front, this isn’t going to be a perfect landing. There may be some thing that are missing and other bits where the user experience isn’t as perfect as it will be finally. Of course there may also be bugs we have to rush to fix. Despite all this we feel that we’re about at the point where exposing it to the hands of thousands of nightly testers is the best way forward. Your eyes spot things that we miss, even things that may seem obvious to you and you’re vital to us getting these sorts of features polished and really just how they should be before they get released to the world at large in a Firefox release.

One thing I wanted to mention is how this will affect the extensions themselves. In many cases as you might hope there will be no change at all, the extensions will continue to work as they did on previous trunk builds. Those that do anything to the add-ons manager UI will see obvious problems since that UI has changed considerably. There is also a set of extensions that are likely to see issues because of API changes. This is pretty much normal for any feature but I thought I’d mention two common uses that extension developers are going to have to change in their code.

Accessing file and version information for extensions

The extension manager interface as it used to exist is gone with the new code. Some extensions would have used this to access their extension’s version and packaged files. There is a replacement so it mostly just means changing code to something similar to this:

Components.utils.import("resource://gre/modules/AddonManager.jsm");

AddonManager.getAddonByID("my-addon@foo.com", function(addon) {
  alert("My extension's version is " + addon.version);
  alert("Did I remember to include that file.txt file in my XPI? " +
        addon.hasResource("file.txt") ? "YES!" : "No :(");
  alert("Let's pretend I did, it's available from the URL " + addon.getResourceURL("file.txt"));
});

Hopefully to developers who used the old APIs this will look a lot nicer than it used to, I certainly prefer it. There are a couple of things to note. The old API would give you an nsIFile directly to play with. The new API gives you a URL. It is currently possible to get an nsIFile from this (as it is a file:// url), however in the future we may be no longer extracting the files out of an XPI for an extension so it is worth trying to just use the URL where you can. We might add a simple method for getting say an input stream to the files as well.

The second thing to note is that this API is asynchronous. As performance becomes more and more of an issue for the platform we have to start moving APIs to be asynchronous to allow file accesses to happen on background threads leaving the UI responsive while we wait for data. In most cases this probably won’t cause developers much of a problem, unfortunately it is difficult to make this appear to operate synchronously safely. People may know the trick of spinning an event loop while you wait for data, that is kind of unsafe since it can allow unexpected reeentrancy into code that isn’t anticipating it.

This change to an asynchronous API brings up the other main change that I am sad to have to make but cannot see a way around.

FUEL has to change

FUEL was designed to be a simple stable wrapper around the nitty gritty world of parts of XPCOM and XUL. It has I believed served fairly well in that purpose though in retrospect there are some things it might have been nice to have different about it. The problem right now though is that FUEL has wrappers around the old extension manager and they are synchronous. They simply cannot stay and work with the new extension manage, which means we have to change a part of the FUEL API slightly. This is unfortunate but I’ve tried to make the change as low impact as possible, there is in fact just a single change. Previously to get an add-ons version you might have done:

alert(Application.extensions.get("my-addon@foo.com").version);

Now you will have to do:

Application.getExtensions(function(extensions) {
  alert(extensions.get("my-addon@foo.com").version);
});

Again it switches from synchronous to asynchronous, but that is really the only change here. The extensions object passed into the callback function is basically the same as the current Application.extensions object, only a slight difference in that it is a snapshot of the list of extensions at the time of retrieval. That doesn’t make a great deal of difference for normal extensions as they cannot install or uninstall without a restart but it may have an effect if you are looking at the new restartless add-ons, in which case you can just get a new copy of the extensions object.

18 thoughts on “How we’re breaking some extensions in the near future”

  1. Will there be a simple synchronous API to get basic metadata as there currently is with nsIUpdateItem? It seems excessive to need to go asynchronous for just the basics in there. Addon metadata shouldn’t need to take any notable amount of time to fetch.

    1. tbh, i can’t see how that code sample in the post could be simpler. async + simple = win.

    2. No, there will be no synchronous API. Add-on metadata can take time to fetch, we don’t want to load the data on startup so the first time someone uses the service we have to load and verify the database. We also don’t want to keep the information in memory so every lookup requires going back to the database. The real time may be small in reality, but everything counts particularly when you look at the mobile platforms.

  2. Could you explain what kind of problems spinning the event loop while waiting could cause in the case of ‘getExtensions’? Doesn’t the asynchronous version spin the event loop anyway? (The only difference I can see is that the syntax is more explicit.)

    1. I’m afraid to say that the specifics are a little beyond me, but I know there are security problems involved with it. I know we have had to fix bugs involving code that has done this in the past. The asynchronous version essentially exits the currently running JS and falls back to the main window event loop that is already running.

  3. I think you underestimate the issues caused by having just an asynchronous API.
    There are currently 990 files in AMO extensions referencing getItemForID, lots of which being after the .version property (see addons mxr).

    A lot of people need to obtain information in a synchronous matter which you say yourself is hard (or even impossible) to get right.
    So a lot of havoc to unfold.
    I glanced through the addonsmgr code and it seems actually damn easy to implement a synchronous API to the meta data.
    |var addon = AddonManager.getAddon(“my-addon@foo.com”);| seems even more simple than your proposed change, doesn’t it?

    Besides: I actually fail to see the perf win here. The only thing that is different is that it is asynchronous. Do you have data that suggests that replacing a single synchronous SQL statement will actually perform worse beyond margin of error than pushing an asynchronous statement with all the inter-thread synchro, events, js closures, array cloning and gc?
    Considering that a major use case are first-run/update pages, so the information would be queried during startup no matter what API.

    Hate to say it but it seems to me that you’re on the edge of over-engineering things for a non-existent or at least negligible performance win in this regard. There is simply no cost-effectiveness. Any gain would be reversed by people refactoring their stuff and using more closures and code, adding more bugs in the process.

    I know that this change alone would mean a lot of work for me to do the necessary refactoring.
    Actually I don’t think that refactoring to use asynchronous APIs is even possible in some instances, e.g. we will ship an nsIAboutModule implementation using the extension manager data to construct the real URI.

    I have two options now, both being bad (using the async API is not an option):
    * hard-code the information (AMO reviewers will thank me for causing more diffs by that, and I might forget to update this information; error prone)
    * I might access the sqlite db myself and perform the query myself. Obviously pretty damn bad for forward-compatiblity and who knows what nasty side-effects this might cause.
    Actually, even if it was possible to do it all asynchronously I’m not rather sure how to proceed. Spend lots of time getting the newly required closures right, or just hard-code/query myself. Frankly I think I’d just do one of the latter, and lots of other people too.

    My conclusion:
    Pretty, pretty please add an synchronous API to grab addon information.

    As to the “no extraction for XPIs” thing:
    Pretty bad idea to start with IMO. Read the bug some days ago (so maybe missing the latest comments), but all the edge-cases (platform libraries, extensions doing crude things to be able to access and even execute files after all, etc.) combined with the expected negligible win (fastload, double-jarred extensions) and the vast amount of work this would require is a no go to me.
    I’d just focus on implementing low risk/few manhours stuff like a fastload cache for extension preferences, as preferences prominently showed up in the straces.

    1. Besides: I actually fail to see the perf win here. The only thing that is different is that it is asynchronous. Do you have data that suggests that replacing a single synchronous SQL statement will actually perform worse beyond margin of error than pushing an asynchronous statement with all the inter-thread synchro, events, js closures, array cloning and gc?

      If you assume nothing else is happening on the system, then yes, it won’t be faster. However, if you have another application accessing the disk, or even your application accessing the disk, something is going to lose, plain and simple. Synchronous I/O is the primary cause of all the little lock-ups you may see in Firefox (if you don’t see them, consider yourself lucky).

      Considering that a major use case are first-run/update pages, so the information would be queried during startup no matter what API.

      And that information can easily be generated asynchronously.

      I might access the sqlite db myself and perform the query myself. Obviously pretty damn bad for forward-compatiblity and who knows what nasty side-effects this might cause.

      This is very easy to prevent by opening the database with an exclusive lock and not using a shared connection. Cookies already does this, as does Places.

      Pretty bad idea to start with IMO. Read the bug some days ago (so maybe missing the latest comments), but all the edge-cases (platform libraries, extensions doing crude things to be able to access and even execute files after all, etc.) combined with the expected negligible win (fastload, double-jarred extensions) and the vast amount of work this would require is a no go to me.

      You seem to be making the assumption that because Firefox is fast enough for you, it is fast enough for everyone else. There are much slower computers out there than yours, and we have users on them.

  4. I probably will switch to an experimental SeaMonkey trunk for my daily work soon, I’m looking forward to testing this work there and I’m exited to see it as a part of SeaMonkey 2.1 which will be based on the Mozilla 1.9.3 platform!

    1. I’m not sure I completely agree. Certainly changing underlying concepts would force a change in a higher level API, but otherwise provided the API has been fully thought out it can work. Had we had the forethought to make the FUEL APIs asynchronous this would have been a non-issue. The asynchronous API can handle the underlying platform being synchronous or asynchronous.

Comments are closed.