412164 - Crash of Minefield while non-resumable download is running corrupts Download Manager

Status: VERIFIED WORKSFORME

(Toolkit :: Downloads API, defect, P4)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: downloads.sqlite which makes Download Manager unusable

This issue can be reproduced with my patch on bug 401846. After its checkin we will be able to pause (the old way) non-resumable downloads. If you are running such a download and Firefox crashes or quits due to any reason the Download Manager gets corrupted and doesn't respond to any event. You have to manually delete the downloads.sqlite to get rid of it.

It would be nice to have this bug fixed before the patch on bug 401846 will be checked-in.

I attached a broken downloads.sqlite file which only has to be copied into the profile folder. Afterwards to Download Manager is not usable.

Just asking for blocking firefox3.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #0)
> This issue can be reproduced with my patch on bug 401846. After its checkin we
> will be able to pause (the old way) non-resumable downloads.

It seems like we're removing the old "pause" UI in bug 410289. Should that be reconsidered given your patch for bug 401846?

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 296815 [details]
downloads.sqlite which makes Download Manager unusable

(In reply to comment #1)
> It seems like we're removing the old "pause" UI in bug 410289. Should that be
> reconsidered given your patch for bug 401846?

I don't think so. My patch on bug 401846 fixes the accidentally created and broken EntityID which let the Download Manager think the server supports resuming.

Even with the UI deleted this bug will happen when you download a file from such a server.

Comment 3

[Shawn Wilsher :sdwilsh]

(In reply to comment #2)
> Even with the UI deleted this bug will happen when you download a file from
> such a server.
How?  The old behavior (calling suspend) won't work anymore.  That sounds exactly like what you described in comment 0.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #3)
> How?  The old behavior (calling suspend) won't work anymore.  That sounds
> exactly like what you described in comment 0.

You don't need to pause (suspend) the download. As the summary describes you only have to start to download and kill Firefox afterwards while the download is *running*. And these steps will not change with the patch on bug 410289, correct?

Comment 5

[Shawn Wilsher :sdwilsh]

Edward - I thought we had code that tried to rectify this situation...

Comment 6

[Ed Lee :Mardak]

RestoreDatabaseState will set downloads that are notstarted, queued, downloading to autoresume which will cause RestoreActiveDownloads to start them. Perhaps there's an issue with resuming this particular download?

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

There are two uncaught exceptions when clicking resume or cancel:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDownloadManager.resumeDownload]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mozapps/content/downloads/downloads.js :: resumeDownload :: line 196"  data: no]

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDownloadManager.cancelDownload]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mozapps/content/downloads/downloads.js :: cancelDownload :: line 174"  data: no]

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So we have a bad download id?

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp&rev=1.154&mark=1351-1353&#1345

Comment 9

[Shawn Wilsher :sdwilsh]

(In reply to comment #8)
> So we have a bad download id?
It just means we were passed a non-active download at that point.

Comment 10

[Ed Lee :Mardak]

This shouldn't be an issue anymore. This problem is caused by fake-pausing a download and crashing. This means we're in PAUSED with no entityID.

If we crash when downloading a non-resumable, we'll try to restart it next time.

Comment 11

[Ed Lee :Mardak]

I did think about doing this...

 nsDownloadManager::RestoreActiveDownloads()
 {
   nsCOMPtr<mozIStorageStatement> stmt;
   nsresult rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
     "SELECT id "
     "FROM moz_downloads "
-    "WHERE (state = ?1 AND LENGTH(entityID) > 0) "
-      "OR autoResume != ?2"), getter_AddRefs(stmt));
+    "WHERE state = ?1 OR autoResume != ?2"), getter_AddRefs(stmt));


But that'll just put a paused download in the UI with a disabled resume button ;)

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Meanwhile this issue was fixed by a patch in the last days or weeks. Using the attached downloads.sqlite file doesn't stop displaying the download manager. Tested with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre) Gecko/2008030904 Minefield/3.0b5pre ID:2008030904

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #12)
> Meanwhile this issue was fixed by a patch in the last days or weeks. Using the
> attached downloads.sqlite file doesn't stop displaying the download manager.
> Tested with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre)
> Gecko/2008030904 Minefield/3.0b5pre ID:2008030904

Probably bug 412360; also, this now works for me also using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008030904 Minefield/3.0b5pre with the downloads.sqlite file from comment 0.  I'm able to download again even though CoD4MWDemoSetup.exe download entry isn't resumable due to bug 401846.

Verified