805466 - Remove old MozUpdater-i folders in $TEMP in PostUpdate

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Brian R. Bondy [:bbondy]]

+++ This bug was initially created as a clone of Bug #765598 +++

In Bug 765598 we delete newly created $Temp/MozUpdater/* folders, but we have a lot of old folders left over in $Temp/MozUpdater-i/*.  This bug is to delete those old directories.

Comment 1

[Brian R. Bondy [:bbondy]]

Attachment: Patch v0

I finished cleaning up the MozUpdater folders in NSIS but Ehsan just reminded me that this bug is not Windows only. So just posting this in case I ever need similar code.

Comment 2

[Brian R. Bondy [:bbondy]]

This bug is to remove old MozUpdater folders that exist in the user's temp directory.  To do this bug you will have code very similar to this patch:
https://bug765598.bugzilla.mozilla.org/attachment.cgi?id=674685

The difference that you'll do is:
1. Only try to delete the folders if MozUpdater-1 exists
2. Try to delete MozUpdater-i where i > 1
3. Do step 2 up to 10 times.
4. If you enumerate the whole temp directory and the count of deleted items is less than 10, then delete MozUpdater-1.

The change is in toolkit/mozapps/update/nsUpdateService.js inside cleanUpMozUpdaterDirs.

Comment 3

[Brian R. Bondy [:bbondy]]

More information:
You can see these temp directories on Windows in explorer by typing %temp% in run.

You can test this bug by typing in test code into the following:
1. Go to about:config
2. Set devtools.chrome.enabled to true
3. In the Firefox menu go to Web Developer -> Scratch Pad
4. In the Environment menu select Browser

Comment 4

[Adri Hilviu]

Attachment: Remove old MozUpdater-i folders in $TEMP in PostUpdate

Comment 5

[Brian R. Bondy [:bbondy]]

Comment on attachment 676588 [details] [diff] [review]
Remove old MozUpdater-i folders in $TEMP in PostUpdate

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

Thanks for the patch, looks great, only minor changes.

nit: Remove trailing whitespace on a few of the new lines.

::: toolkit/mozapps/update/nsUpdateService.js
@@ +760,5 @@
>                              getService(Components.interfaces.nsIProperties).
>                              get("TmpD", Components.interfaces.nsIFile);
> +
> +    // We used to store MozUpdater-i folders directly inside the temp directory.
> +    // "i" is the first available positive integer.

nit: Remove this line:
// "i" is the first available positive integer.

@@ +783,5 @@
> +      // items is less than 10, then delete MozUpdate-1.
> +      if (i < 10) {
> +        mozUpdaterDir1.remove(true);
> +      }
> +      return;

Let's remove the return; here so we still remove the newly created folders. Some machines that are used to run tests may have thousands of these folders, so it would be good not to let the other folders in the new location build up.

Comment 6

[Adri Hilviu]

Attachment: Remove old MozUpdater-i folders in $TEMP in PostUpdate

Applied the review changes.

Comment 7

[Brian R. Bondy [:bbondy]]

Comment on attachment 676917 [details] [diff] [review]
Remove old MozUpdater-i folders in $TEMP in PostUpdate

I'll land this on inbound soon.

Comment 8

[Brian R. Bondy [:bbondy]]

Thanks for the patch!
Now that you have a couple of patches in and accepted could you:

1) File a bug to get level 1 commit access.  This will allow you to push to try
More info here: http://www.mozilla.org/hacking/commit-access-policy/
Example bug: https://bugzilla.mozilla.org/show_bug.cgi?id=671744

2) Get editbugs access, just send an eamil and quote the 2 bugs you completed:
http://www.gerv.net/hacking/before-you-mail-gerv.html

3) Sign up here:
https://mozillians.org/en-US/
It'll give you a user page and be more visible as a member of the community.

Comment 9

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/bba3651100a4

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/bba3651100a4