Closed Bug 355912 Opened 14 years ago Closed 14 years ago

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

Categories

(Camino Graveyard :: Downloading, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hwaara, Assigned: stuart.morgan+bugzilla)

Details

(Keywords: fixed1.8.1, hang)

Attachments

(1 file)

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?)
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?
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.
Assignee: nobody → stuart.morgan
This should help quite a bit.
Attachment #241713 - Flags: review?(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.
Attachment #241713 - Flags: review?(nick.kreeger) → review+
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.
Attachment #241713 - Flags: superreview?(mikepinkerton)
Comment on attachment 241713 [details] [diff] [review]
suppress duplicate re-layouts

sr=pink
Attachment #241713 - Flags: superreview?(mikepinkerton) → superreview+
Checked in on 1.8branch and trunk.
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.