355912 - Camino hangs when doing "Clean Up" on a big list of downloads

Status: RESOLVED FIXED

(Camino Graveyard :: Downloading, defect)

Description

[Håkan Waara]

Trunk, 20061009

I had quite a big list of downloads since I almost never use "Clean Up" to remove them.

Camino hung up for a good 8-10 seconds... We should really do this on a thread (using a simple progress dialog, perhaps reusing the one we now have for importing of bookmarks?)

Comment 1

[Håkan Waara]

Also, profiling what makes removing the completed download so slow would help. Maybe there's some simple optimization to be done.

Why aren't we just removing downloads.plist all at once, anyway?

Comment 2

[Stuart Morgan]

We definitely shouldn't make a new thread and a progress indicator, because there's no reason for it to take anything like that long.

We can't remove the whole plist because it doesn't remove in-progress downloads.  I haven't profiled, but glancing at the code making this way faster should be very easy; the cleanup is a looped call to the single-item remove, which does a complete re-layout of all the remaining views.

Taking.

Comment 3

[Stuart Morgan]

Attachment: suppress duplicate re-layouts

This should help quite a bit.

Comment 4

[Nick Kreeger]

Comment on attachment 241713 [details] [diff] [review]
suppress duplicate re-layouts

 // remove all inactive instances
 -(IBAction)cleanUpDownloads:(id)sender
 {
-  for (unsigned int i = 0; i < [mProgressViewControllers count]; i++)
+  for (int i = [mProgressViewControllers count] - 1; i >= 0; i--)
   {
     ProgressViewController* curProgressViewController = [mProgressViewControllers objectAtIndex:i];
     if ((![curProgressViewController isActive]) || [curProgressViewController isCanceled])
-    {
-      [self removeDownload:curProgressViewController]; // remove the download
-      i--; // leave index at the same position because the dl there got removed
-    }
+      [self removeDownload:curProgressViewController suppressRedraw:YES];

Looks like you have a tab here.

Looks good to me.

Comment 5

[Stuart Morgan]

Comment on attachment 241713 [details] [diff] [review]
suppress duplicate re-layouts

No tab; that's the right level of indentation for the |if| that it follows.

Comment 6

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 241713 [details] [diff] [review]
suppress duplicate re-layouts

sr=pink

Comment 7

[froodian (Ian Leue)]

Checked in on 1.8branch and trunk.