We should find a way to recover when the EM ends up in the 'needs restart' state as in bug 396695

I'm reasonably sure that attachment 285374 [details] [diff] [review] (the original patch in bug 396695) handles the recovery. We backed down from that though in favour of something less intrusive for 2.0.0.9.

Sounds good. Could you attach it to this bug and we'll try to get that in to 2.0.0.11. I've been trying to come up with a way to force the EM into this state without any success... have you had any success with being able to come up with a test so it can be verified?

Steps to put the EM into the faulty state: 1. Quit Firefox 2. Open extensions.cache 3. For the the first extension in the profile folder add "needs-upgrade" to the end of the line. 4. For some below that add "needs-install" to the end of the lines 5. Open extensions.rdf 6. Find the metadata for the extension marked as needs-upgrade. 7. Remove almost all of the data, leaving just name and version (the key missing part is the installLocation). Now when you start Firefox on every startup it will attempt to process the upgrade of the extension, failing because it has a null installLocation. The other extensions show up as being installed after a restart. I'll check over the patch and reattach this afternoon.

This is the patch against current branch. The startup cache knows the proper location that the upgraded add-on is in, reading the location from the datasource is redundant. The changes in this patch are already in the full trunk patch awaiting review in bug 396695

Comment on attachment 289501 [details] [diff] [review] patch rev 1 This recovers from an invalid case, shouldn't be any issues but want this on trunk before seeking approval for branch.

Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in; /cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v <-- nsExtensionManager.js.in new revision: 1.263; previous revision: 1.262 done

Somewhere along the development of this patch I dropped this case. Unless this is fixed we will fail to upgrade a theme not currently in use.

Comment on attachment 291391 [details] [diff] [review] missed case Doh!

Comment on attachment 291391 [details] [diff] [review] missed case Want approval to land this for b2, simple enough fix but without it we're liable to break for some users.

Checked in. Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in; /cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v <-- nsExtensionManager.js.in new revision: 1.264; previous revision: 1.263 done

Comment on attachment 289501 [details] [diff] [review] patch rev 1 We are still seeing some cases of this on branch it seems. I think we should land this there where hopefully it should resolve the problem for people.

Comment on attachment 291391 [details] [diff] [review] missed case Needs this bit too

Comment on attachment 289501 [details] [diff] [review] patch rev 1 approved for 1.8.1.12, a=dveditz for release-drivers

Comment on attachment 291391 [details] [diff] [review] missed case approved for 1.8.1.12, a=dveditz for release-drivers

Checked both into branch: Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in; /cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v <-- nsExtensionManager.js.in new revision: 1.144.2.64; previous revision: 1.144.2.63 done

I'm having trouble seeing the bad behavior in Firefox 2.0.0.11 and, thus, seeing the fixed behavior in 2.0.0.12RC. I've been following the steps in comment 3 but am unable to see an problem when restarting that gets fixed in Firefox 2.0.0.12.