Comment 39 for bug 861664

Created attachment 562976
patch rev 1

This is the patch that seems like the best solution. We still cache the entire manifest when preparing to install (for backwards compatibility) however on the startup it completely loads the install manifest from install.rdf (thus parsing it and getting all the fields that the new Firefox supports) and then also loads the cached manifest and copies across only certain appropriate properties. If any of the properties expected seem to be missing then we just ignore and carry on. This should guarantee a safe and working add-on install.

There is a small performance penalty associated here because we have to read install.rdf on startup for every new add-on. I haven't measured the cost of that yet but I expect it to be low and as it is only for the case when a new add-on is installed I'd expect it to be worth it for correcting this.

I think the only risk here is that if I have missed any properties to copy across. That would appear as the add-on changing state in some way on upgrade (maybe getting disabled etc.) but I am growing confident that I have them all. I'll be double checking that in the morning but figured I might as well put the patch up now as adding properties to that list won't really change the review. We can also be over-zealous about listing properties there (maybe even just try to copy all properties) because the new code will simply ignore any it doesn't find and the cached properties will always be the same or newer than those loaded from install.rdf.

The testcase from bug 664833 already checks installing an update when the DB schema and app version changes. The only thing missing was to make the cached manifest have a necessary property missing so I updated the test to do that. Without this fix the test fails in the same way that I've been seeing in this bug.

I've pushed this to the try server, if people want to test builds they will be showing up at http://<email address hidden>