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



13 years ago
13 years ago


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


({fixed1.8.1, hang})

fixed1.8.1, hang



(1 attachment)



13 years ago
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

13 years ago
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

13 years ago
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.

Assignee: nobody → stuart.morgan

Comment 3

13 years ago
This should help quite a bit.
Attachment #241713 - Flags: review?(nick.kreeger)

Comment 4

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

 // remove all inactive instances
-  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 5

13 years ago
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

Attachment #241713 - Flags: superreview?(mikepinkerton) → superreview+
Checked in on 1.8branch and trunk.
Last Resolved: 13 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.