I noticed that it is possible for a syncGUID to get generated twice during some install scenarios. I /think/ it reproduces for non-restartless add-ons only: 1) Install add-on from AddonInstall. In the onInstallEnded, the addon.syncGUID is defined as one thing. 2) Restart the application 3) The add-on actually gets installed and syncGUID is regenerated. I would expect the syncGUID from the first run to get carried over. This isn't a deal-breaker: Sync will reconcile the different syncGUIDs as things are synchronized. But, it is annoying. I'll address this as a follow-up to add-on sync.

I think you just need to add syncGUID to here http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/XPIProvider.jsm#7277

Adding the syncGUID field to importMetadata seemed to do it! I verified the new test code in test_install.js failed without the change to XPIProvider.jsm applied. test_bootstrap.js worked both before and after (I think that makes sense). My biggest concern with this patch is that I don't have enough test coverage. Is there anywhere else I really need test coverage?

Comment on attachment 583334 [details] [diff] [review] Reuse syncGUID, v1 Review of attachment 583334 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/extensions/test/xpcshell/test_bootstrap.js @@ +148,5 @@ > do_check_eq(install.type, "extension"); > do_check_eq(install.version, "1.0"); > do_check_eq(install.name, "Test Bootstrap 1"); > do_check_eq(install.state, AddonManager.STATE_DOWNLOADED); > + do_check_neq(install.addon.syncGUID); do_check_neq expects two arguments, I guess this is just checking it isn't undefined but be explicit about it @@ +189,5 @@ > > AddonManager.getAddonByID("bootstrap1@tests.mozilla.org", function(b1) { > do_check_neq(b1, null); > do_check_eq(b1.version, "1.0"); > + do_check_neq(b1.syncGUID); ditto

Pushed to inbound with extra null argument to do_check_eq() added: https://hg.mozilla.org/integration/mozilla-inbound/rev/f8d2a543a0d1

This is the actual patch that was committed. Adding as attachment because I'm requesting approval for Aurora. I would like this merged to Aurora because it improves the add-on sync experience and reduces the potential for bugs due to bug 710448.

Comment on attachment 585054 [details] [diff] [review] Reuse syncGUID, v2 [Triage Comment] Because of where we are in the cycle, and the possibility for data loss as outlined in 710448, I'm approving for Aurora.

Pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/e03d758c3c08

gps: is there anything to verify here?

Tracy: This was really an internal optimization. I'm not sure there is an easy way to verify this beyond verifying the Add-on Manager xpcshell tests pass.