704973 - XPIProvider syncGUID setter should not always call to database

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Gregory Szorc [:gps]]

According to :Mossop, XPIProvider.syncGUID's setter isn't implemented properly: It should support setting syncGUID to instances not yet backed by the database.

In addition, we need to set syncGUID earlier in an object's lifetime, preferably as soon as possible, so it is available to all listeners.

Comment 1

[Dave Townsend [:mossop]]

Basically you want to check |if (aAddon instanceof DBAddonInternal)| before attempting to push the ID to the database.

You can probably just create the syncGUID in loadManifestFromRDF or something, then it'd always be set.

Comment 2

[Gregory Szorc [:gps]]

Attachment: syncGUID follow-up, v1

This implements what was talked about in the first few comments.

1) Moved syncGUID generation to earlier in the object creation process
2) Don't always hit the DB when setting the syncGUID

I've verified that xpcshell and mochitests all pass.

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 578930 [details] [diff] [review]
syncGUID follow-up, v1

Review of attachment 578930 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +7628,5 @@
>        return val;
>  
> +    if (aAddon instanceof DBAddonInternal) {
> +      XPIDatabase.setAddonSyncGUID(aAddon, val);
> +    }

No braces needed around a single statement here.

Comment 4

[Gregory Szorc [:gps]]

Pushed to s-c with minor formatting change.

https://hg.mozilla.org/services/services-central/rev/91881c0cf3f3

Comment 5

[Gregory Szorc [:gps]]

Pushed to m-c: https://hg.mozilla.org/mozilla-central/rev/91881c0cf3f3