Last Comment Bug 495680 - Problems with import of downloads.rdf after switch to toolkit download manager
: Problems with import of downloads.rdf after switch to toolkit download manager
Status: RESOLVED FIXED
: verified1.9.1
Product: Toolkit
Classification: Components
Component: Download Manager (show other bugs)
: Trunk
: x86 Windows XP
: -- normal with 1 vote (vote)
: ---
Assigned To: Frank Wein [:mcsmurf]
:
Mentors:
: 495681 495682 495683 (view as bug list)
Depends on: 381157
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-31 04:24 PDT by Sven Grull
Modified: 2009-08-20 14:42 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.3-fixed


Attachments
downloads.rdf to trigger error message (535 bytes, text/plain)
2009-05-31 05:24 PDT, Sven Grull
no flags Details
Patch (757 bytes, patch)
2009-06-29 12:59 PDT, Frank Wein [:mcsmurf]
sdwilsh: review+
Details | Diff | Review
Patch (5.21 KB, patch)
2009-07-11 19:28 PDT, Frank Wein [:mcsmurf]
sdwilsh: review+
samuel.sidler+old: approval1.9.1.3+
Details | Diff | Review

Description Sven Grull 2009-05-31 04:24:03 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090531 SeaMonkey/2.0b1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090531 SeaMonkey/2.0b1pre

New bug as described in https://bugzilla.mozilla.org/show_bug.cgi?id=381157#c89

Reproducible: Always

Steps to Reproduce:
1. if already exists delete downloads.sqlite
2. start a download
3. in download manager dialog choose "save file"
Actual Results:  
After selecting "save file" it takes quite a long time until windows file selector opens (Probably in this time downloads.rdf is converted to downloads.sqlite - downloads.rdf file size is about 340K).
For every entry in downloads.rdf the error message "x:\...\file.zip could not be saved, because an unknown error occurred. Try saving to a different location." is displayed.
In error console there are several "Error: too much recursion" shown.
After clicking through all error messages the download manager window opens showing the file progress as finished/failed/canceled. 
But no file is really downloaded. 
The partition with the download directory would have enough space to save the files.

Expected Results:  
Import of downloads.rdf happens without error messages.

I tried to strip down downloads.rdf to reproduce the error with a smaller file. But unfortunately the error occurs only with the original file. Because a lot of private urls in the file I can't attach it to this bug. I will try to create a downloads.rdf that triggers this bug the next days.
Comment 1 Sven Grull 2009-05-31 04:29:54 PDT
*** Bug 495683 has been marked as a duplicate of this bug. ***
Comment 2 Sven Grull 2009-05-31 04:30:32 PDT
*** Bug 495682 has been marked as a duplicate of this bug. ***
Comment 3 Sven Grull 2009-05-31 04:31:05 PDT
*** Bug 495681 has been marked as a duplicate of this bug. ***
Comment 4 Sven Grull 2009-05-31 05:24:58 PDT
Created attachment 380704 [details]
downloads.rdf to trigger error message

This downloads.rdf is a stripped version of my own file that triggers the described error.
Comment 5 Robert Kaiser (not working on stability any more) 2009-05-31 05:42:44 PDT
Could you copy one of the "Error: too much recursion" errors in here (Using the "Copy context menu item in the console)? The location where this happens might give us some info what is failing here. I suspect it's toolkit code but I'm not sure.
Comment 6 Frank Wein [:mcsmurf] 2009-05-31 05:50:17 PDT
So if you first open the Download Manager once and then start a download the problem does not happen?
Comment 7 Sven Grull 2009-05-31 06:29:23 PDT
(In reply to comment #5)
> Could you copy one of the "Error: too much recursion" errors in here (Using the
> "Copy context menu item in the console)? The location where this happens might
> give us some info what is failing here. 

There is no more data available, "Error: too much recursion" is already the copied entry.

(In reply to comment #6)
> So if you first open the Download Manager once and then start a download the
> problem does not happen?

Opening the download manager without starting any download is enough to trigger the errors.
Comment 8 Robert Kaiser (not working on stability any more) 2009-05-31 06:44:26 PDT
Shawn, any idea what could be happening there? This sound like it would be toolkit code, but we have nothing to confirm it...
Comment 9 Shawn Wilsher :sdwilsh 2009-05-31 07:05:05 PDT
well, the download state in the downloads.rdf is set to -1, which means it's in the NOT_STARTED state.  So, we'll go ahead and try to start it.  We check for downloads in this state at startup, and try to start them (startup of the service).
Comment 10 kmike 2009-06-10 06:46:08 PDT
I got a similar error after switching from pre-toolkit-download-manager build to Mozilla/5.0 (X11; U; Linux i686; rv:1.9.1pre) Gecko/20090610 SeaMonkey/2.0b1pre

One of the files has started downloading again after the conversion. Looking in the downloads.rdf, the file entry seems to be corrupted (a couple of duplicate lines):

  <RDF:Description RDF:about="file:///home/mike/NEW/proofforchobos.wmv"
                   NC:Name="proofforchobos.wmv">
    <NC:ProgressMode>normal</NC:ProgressMode>
    <NC:ProgressMode>none</NC:ProgressMode>
    <NC:StatusText>Downloading</NC:StatusText>
    <NC:StatusText>Finished</NC:StatusText>
    <NC:Transferred>0KB of  0KB</NC:Transferred>
    <NC:Transferred>55872KB of 55872KB</NC:Transferred>
    <NC:URL RDF:resource="http://rs425l33.rapidshare.com/files/181009094/3193060/proofforchobos.wmv"/>
    <NC:URL RDF:resource="http://rs425tl2.rapidshare.com/files/181009094/3196738
/proofforchobos.wmv"/>
    <NC:File RDF:resource="file:///home/mike/NEW/proofforchobos.wmv"/>
    <NC:DownloadState NC:parseType="Integer">0</NC:DownloadState>
    <NC:DownloadState NC:parseType="Integer">1</NC:DownloadState>
    <NC:ProgressPercent NC:parseType="Integer">0</NC:ProgressPercent>
    <NC:ProgressPercent NC:parseType="Integer">100</NC:ProgressPercent>
  </RDF:Description>

Also, all converted entries in the new d/l manager contain "0 bytes" in the "transferred" column. Is it a known bug?
Comment 11 Robert Kaiser (not working on stability any more) 2009-06-12 11:31:25 PDT
kmike:
Yes, your file seems to have some kind of strange corruption, having two things in there. The "0 bytes" thing is probably from us setting a number of defaults when importing the old SeaMonkey downloadmanager lists, as there's a lot of things that are not completely compatible in inporting.

Shawn, should we do something at import time so that we don't import erroneaos NOT_STARTED cases?

All in all, I think we should probably ship SeaMonkey 2.0 Beta 1 as it is wrt import, maybe relnote this and gather more feedback from that.
Comment 12 Shawn Wilsher :sdwilsh 2009-06-12 11:38:54 PDT
I guess we could just not import NOT_STARTED downloads.  That's kinda odd that it's in that state in the rdf file though...
Comment 13 Ian Neal 2009-06-15 16:59:43 PDT
Confirming this is a problem, I get this as well, but the 12 entries I have with problems are those with the following in:
<NC:DownloadState NC:parseType="Integer">-1</NC:DownloadState>
No duplicate lines for me.
I am presuming -1 has a meaning that means that download manager tries to process it in some way which it shouldn't.
Comment 14 Ian Neal 2009-06-15 17:20:57 PDT
Just opening the download manager is sufficient, you do not need to start a download to trigger the error messages.
At the moment there are no checks on state in nsDownloadManager::ImportDownloadHistory()
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp#628
Should be fairly easy to fix, but is in toolkit, so cannot really block this beta release on the issue. Saying that we will take a reviewed fix.
Nominate as a blocker for another release if you want.
Comment 15 Robert Kaiser (not working on stability any more) 2009-06-16 06:21:10 PDT
I think we probably should have a one-liner there switching the state of NOT_STARTED download to CANCELED, then those that might have rightfully had that state can always be retried ;-)
Comment 16 Shawn Wilsher :sdwilsh 2009-06-16 08:59:43 PDT
(In reply to comment #15)
> I think we probably should have a one-liner there switching the state of
> NOT_STARTED download to CANCELED, then those that might have rightfully had
> that state can always be retried ;-)
I concur
Comment 17 Frank Wein [:mcsmurf] 2009-06-29 12:59:11 PDT
Created attachment 385834 [details] [diff] [review]
Patch
Comment 18 Shawn Wilsher :sdwilsh 2009-06-29 14:59:29 PDT
Comment on attachment 385834 [details] [diff] [review]
Patch

r=sdwilsh

You'll have to add a unit test (not hard) for this to land on branch though.
Comment 19 Frank Wein [:mcsmurf] 2009-07-11 19:28:47 PDT
Created attachment 388116 [details] [diff] [review]
Patch

now with unit test
Comment 20 Shawn Wilsher :sdwilsh 2009-07-15 15:27:52 PDT
Comment on attachment 388116 [details] [diff] [review]
Patch

>+    if (state == -1)
>+      state = 3;
Could we use the named constants here please so it's clearer in the code as to what is going on?

r=sdwilsh
Comment 21 Robert Kaiser (not working on stability any more) 2009-08-06 13:41:36 PDT
Frank, can we move forward on this please? This is definitely wanted for SeaMonkey 2.0b2, even if there's no way of marking that.
Comment 22 Frank Wein [:mcsmurf] 2009-08-06 13:51:30 PDT
Comment on attachment 388116 [details] [diff] [review]
Patch

Wanted for the next SeaMonkey Beta, this could (in theory) affect users switching from Firefox 2 to Firefox 3.5.x, but as far as I know the FF 2 download manager has never set that state in the downloads.rdf file.
Comment 23 Robert Kaiser (not working on stability any more) 2009-08-11 12:53:06 PDT
Important info for approval and any later readers of this bug (just told it on IRC but should state it here as well):

The relevant code this is patching is only on 1.9.1, the whole downloads.rdf import code has been removed from mozilla-central for the 1.9.2 cycle. Because of that, this patch is only for landing on 1.9.1 and not mozilla-central.
Comment 24 Samuel Sidler (old account; do not CC) 2009-08-11 13:11:51 PDT
Comment on attachment 388116 [details] [diff] [review]
Patch

Approved for 1.9.1.3. a=ss
Comment 25 Frank Wein [:mcsmurf] 2009-08-12 06:59:12 PDT
Pushed to 1.9.1 branch, http://hg.mozilla.org/releases/mozilla-1.9.1/rev/56b99735ac85
Comment 26 [On PTO until 6/29] 2009-08-20 14:42:02 PDT
Verified for 1.9.1.3 using unit test.

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