326722 - On upgrade cleanup cruft from bug 319022

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Robert Strong (they/them - no direct email)]

There are several bugs that leave garbage data in the extensions datasource... specifically bug 319022 left download entries. Instead of checking / cleaning on each load, unload, or some other event I think it would be best to cleanup on upgrade in a similar manner as we currently migrate a 1.0 extensions datasource to the 1.5 extensions datasource. This way we won't cover up bugs that should be fixed and provide a clean slate for finding these bugs.

Comment 1

[Robert Strong (they/them - no direct email)]

Mel, you brought up this bug to me once before and if you have the time I'd appreciate your help with verifying this is working as expected, etc. when it is patched.

Comment 2

[Robert Strong (they/them - no direct email)]

This should also handle the case where there is a download item that is no longer in :mozilla:item:root sequence. These items don't show up in the EM ui but they are part of the "cruft" left behind and aren't used for anything at this point so are safe to remove.

Comment 3

[Robert Strong (they/them - no direct email)]

Attachment: sample extensions.rdf w/ lots of cruft

Comment 4

[Robert Strong (they/them - no direct email)]

Attachment: patch

I'm curious if there are other cases that need to be handled than the ones in the sample extensions.rdf... if anyone has cases not already handled (see below) please let me know in this bug by attaching an extensions.rdf with it in it.

This patch cleans up the attached sample and removes items that
* have a value for state
* have a value for downloadURL
* don't have a value for installLocation and the value of appDisabled is "true"

I'm going to wait a couple of days to request review so I can go through all of the extensions.rdf files I have received and give people a chance to attach new samples.

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: patch

Benjamin, I'd like to get this in for the app version change that mscott will be doing in the next couple of days.  By cleaning up the extensions datasource on upgrade we will be able to lessen bug reports for pre-existing bugs that have been patched. I would also need the patch from bug 319022 to land on the branch as well and I would like to get the patch in bug 324314 landed as well since it only takes affect on app upgrade as well.

Comment 6

[Robert Strong (they/them - no direct email)]

Comment on attachment 212138 [details] [diff] [review]
patch

bah... forgot one other change.

Comment 7

[Robert Strong (they/them - no direct email)]

Attachment: patch

Benjamin, see comment #5

I moved the closing of the zip reader since we close it on failure in getZipReaderForFile and I also added a check for there being a directory in the chrome dir for flat file structures... I ran into this while debugging.

Comment 8

[Robert Strong (they/them - no direct email)]

Comment on attachment 212140 [details] [diff] [review]
patch

I need this on the MOZILLA_1_8_BRANCH for the upcoming app version change

Comment 9

[Robert Strong (they/them - no direct email)]

Fixed on trunk and MOZILLA_1_8_BRANCH

Comment 10

[Scott MacGregor]

Thanks for getting this landed Rob. I might wait a little bit on the version change until we fix software update for nightly users: Bug 327140. I believe I need to make some AUS changes to account for the new version string and I'd like to wait until I know software update is working again.

Comment 11

[Andreas Lange]

(In reply to comment #4)
> 
> I'm going to wait a couple of days to request review so I can go through all of
> the extensions.rdf files I have received and give people a chance to attach new
> samples.
> 
One such file is in bug 318169.

Comment 12

[Robert Strong (they/them - no direct email)]

This patch removes the cruft left behind in the extensions.rdf that is attached to bug 318169.

Comment 13

[Robert Strong (they/them - no direct email)]

Note: this patch will only cleanup the cruft during an application upgrade where the version changes and this will be happening soon after mscott changes the version.

Comment 14

[Mel Reyes]

I couldn't ask for better timing, yesterday i was testing the rebuilding of the extensions.rdf by backing it up and seeing the difference after reinstalling and cleaning things up.  Granted I probably install/uninstall more extensions the any normal user, but I'm sure there are folks out there that will benefit from this beaut.

With my extensions.rdf rebuild found some weird things but none that couldn't be resolved as they were probably caused by the "make compatible" features, etc. but I did notice a drop from anywheres from 5 to 15k in file size from the origional of 60-80k total, I'll backup my files and see if I notice any size differences when the app bump comes up.

Comment 15

[Robert Strong (they/them - no direct email)]

Mel, if you'd like to force this to run the code just modify the LastVersion line in the profiles compatibility.ini
Sample
LastVersion=1.5_2006021610/1.8_2006021610
change it to
LastVersion=1.4_2006021610/1.8_2006021610

and modify the following in prefs.js
user_pref("extensions.lastAppVersion", "1.5");
change it to
user_pref("extensions.lastAppVersion", "1.4");

btw: do this while the app isn't running and the run the app... the changes don't have to be exactly like the examples.