Closed Bug 702819 Opened 13 years ago Closed 13 years ago

Make available syncGUID of add-ons uninstalled during startup

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: gps, Unassigned)

References

Details

Problem: When an add-on is uninstalled during application startup, Sync can see that an add-on is uninstalled. However, the only data available is the add-on's ID. Sync needs the syncGUID value for the uninstalled add-on so it knows what record to update. Furthermore, when the add-on is uninstalling at application startup, this is before Sync is loaded, so Sync can't observe a notification.

Hacky Solution: Sync grabs *all* add-on records from the sync server once at application startup, likely during the first sync. It looks at the set of add-ons it thinks should be installed locally and compares that to AddonManager.getAllAddons() and/or the IDs uninstalled at startup. It sees where a delete was missed and processes it. This is hacky because this isn't how sync engines are supposed to work. Sync engines are supposed to react to events since the last sync time and create updated records from those events. Consolidating with the server's complete state is very much frowned upon. rnewman will slap me for even mentioning it as a possible solution.

Proper solution: AddonManager should retain syncGUIDs of add-ons uninstalled during application startup. When Sync loads, it asks AddonManager for syncGUIDs which were uninstalled.

The proper solution could be expanded to retrieve all syncGUIDs modified during application startup. I'm thinking AddonManager.getStartupChangesSyncGUIDs() or something.

Ideally, Sync should have access to not only the syncGUID of the uninstalled add-on, but the fields that determine whether an add-on is syncable as well (currently scope, foreignInstall, and sourceURI). If these aren't provided, Sync has to assume the deleted syncGUID corresponds to an add-on that was actually synced, which may not always be true (since not all add-ons are synced) and this could lead to Sync records that don't need to exist since the only state is deleted.
Here's my (somewhat uninformed) perspective on this.

There are a few ways we could approach this problem. One would be to fix Bug 584771, and ensure that Sync is adequately initialized (its observers, for example) before operations such as on-start installations and uninstallations take place. I don't think that's going to happen for a whole pile of nasty reasons.

Another would be to implement one of a set of hacks: have the addon manager buffer installation notifications until Sync is active, for example. This gets us into a rat's nest of edge cases (e.g., if Sync isn't configured). Having the addon manager track startup change IDs is just another take on this same hack, and comes with its own issues: what happens if the app quits inside some interval (e.g., before Sync wakes up); when does the list get cleared, and who does it; etc.

So I think we ought to do this right: have the addon manager provide a permanent and accurate paper trail, such that Sync can ask "what records have changed since time T?" — in this case, which addons have been installed or removed. (This aligns with our rewrite of Sync internals to be async and less stateful: <https://wiki.mozilla.org/Services/Sync/FxSync/StoreRedesign>.)

Whenever a provider transitions between "uninstalled" and "installed", some pertinent details (for our purposes, the GUID and the timestamp) are recorded in a table. Disk IO will be occurring at this point anyway, so I'm not concerned with that. The recorded event occurs in the same transaction, which neatly ties up some loose ends.

The provider interface is extended to include a way to inspect this table. The XPI provider is extended to implement this interface, and the Sync engine altered to use that API rather than, or as well as, watching for events.
(In reply to Richard Newman [:rnewman] from comment #1)
> Here's my (somewhat uninformed) perspective on this.
> 
> There are a few ways we could approach this problem. One would be to fix Bug
> 584771, and ensure that Sync is adequately initialized (its observers, for
> example) before operations such as on-start installations and
> uninstallations take place. I don't think that's going to happen for a whole
> pile of nasty reasons.

That's not actually going to help you anyway as we don't send out notifications for stuff that happens during startup. I'm not even sure it is possible or desirable for sync to get initialised that early either.

> So I think we ought to do this right: have the addon manager provide a
> permanent and accurate paper trail, such that Sync can ask "what records
> have changed since time T?" — in this case, which addons have been installed
> or removed. (This aligns with our rewrite of Sync internals to be async and
> less stateful:
> <https://wiki.mozilla.org/Services/Sync/FxSync/StoreRedesign>.)

I don't think this is necessary at all and I would expect it to be a large amount of work to get right. Seems like a hammer for the small problem we have.

All we really need is to be able to get the Addon that has been uninstalled. We can either leave it in the database until Firefox is shutdown, or we can just hold onto it in memory for a very small memory hit. The latter is simpler but maybe the former would be preferable. I would expect a new API getUninstalledAddonByID to get it based on the startup changes record. For the database case an "uninstalled" field can be used to hide it from all other queries and may have additional uses for bug 640775 in the future.

Blair, what do you think would be the right course here?
(In reply to Dave Townsend (:Mossop) from comment #2)

> That's not actually going to help you anyway as we don't send out
> notifications for stuff that happens during startup.

Ah, fair enough.

> I'm not even sure it is
> possible or desirable for sync to get initialised that early either.

Yeah, hence my "nasty reasons" :D

> All we really need is to be able to get the Addon that has been uninstalled.
> We can either leave it in the database until Firefox is shutdown, or we can
> just hold onto it in memory for a very small memory hit.

You're assuming that Sync will run, and will reach syncing addons, before Firefox next shuts down… that's not always the case (particularly on current mobile, for what that's worth).

What we really need is for us to be able to see this information until the next *sync*, not next *shutdown*.

(I'm also thinking of Q1/Q2 next year: in the future trackerless implementation of Sync, we'd really like to delegate everything to a timestamp-aware storage layer.)
Blocks: 702806
It sounds like this problem would occur with other sync engines too. How do they deal with it?
(In reply to Blair McBride (:Unfocused) from comment #4)
> It sounds like this problem would occur with other sync engines too. How do
> they deal with it?

Sync typically relies on something else being a persistent store. When Sync runs, we query that other store for changes since time X. Currently, we have a bunch of observers supplementing this mechanism. But, we want to move away from that so Sync isn't always performing work.
I worked around this in Sync land. Details will become apparent in bug 534956 when the patch lands.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.