Last Comment Bug 805466 - Remove old MozUpdater-i folders in $TEMP in PostUpdate
: Remove old MozUpdater-i folders in $TEMP in PostUpdate
Status: RESOLVED FIXED
[mentor=bbondy][lang=js]
:
Product: Toolkit
Classification: Components
Component: Application Update (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla19
Assigned To: Adri Hilviu
:
Mentors:
Depends on:
Blocks: 765598
  Show dependency treegraph
 
Reported: 2012-10-25 09:22 PDT by Brian R. Bondy [:bbondy]
Modified: 2012-11-01 06:50 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v0 (1.23 KB, patch)
2012-10-25 09:57 PDT, Brian R. Bondy [:bbondy]
no flags Details | Diff | Splinter Review
Remove old MozUpdater-i folders in $TEMP in PostUpdate (2.42 KB, patch)
2012-10-30 07:15 PDT, Adri Hilviu
netzen: feedback+
Details | Diff | Splinter Review
Remove old MozUpdater-i folders in $TEMP in PostUpdate (2.34 KB, patch)
2012-10-30 22:39 PDT, Adri Hilviu
netzen: review+
Details | Diff | Splinter Review

Description Brian R. Bondy [:bbondy] 2012-10-25 09:22:42 PDT
+++ 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] 2012-10-25 09:57:03 PDT
Created attachment 675180 [details] [diff] [review]
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] 2012-10-25 10:02:01 PDT
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] 2012-10-25 10:25:15 PDT
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 2012-10-30 07:15:37 PDT
Created attachment 676588 [details] [diff] [review]
Remove old MozUpdater-i folders in $TEMP in PostUpdate
Comment 5 Brian R. Bondy [:bbondy] 2012-10-30 07:43:49 PDT
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 2012-10-30 22:39:32 PDT
Created attachment 676917 [details] [diff] [review]
Remove old MozUpdater-i folders in $TEMP in PostUpdate

Applied the review changes.
Comment 7 Brian R. Bondy [:bbondy] 2012-10-31 11:29:14 PDT
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] 2012-10-31 11:34:26 PDT
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] 2012-10-31 17:47:25 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/bba3651100a4
Comment 10 Ed Morley [:emorley] 2012-11-01 06:50:36 PDT
https://hg.mozilla.org/mozilla-central/rev/bba3651100a4

Note You need to log in before you can comment on or make changes to this bug.