Closed Bug 495680 Opened 11 years ago Closed 11 years ago

Problems with import of downloads.rdf after switch to toolkit download manager

Categories

(Toolkit :: Downloads API, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.1 --- .3-fixed

People

(Reporter: bugzilla.spam2, Assigned: mcsmurf)

References

Details

(Keywords: verified1.9.1)

Attachments

(2 files, 1 obsolete file)

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.
Duplicate of this bug: 495683
Duplicate of this bug: 495682
Duplicate of this bug: 495681
Depends on: 381157
This downloads.rdf is a stripped version of my own file that triggers the described error.
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.
So if you first open the Download Manager once and then start a download the problem does not happen?
(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.
Shawn, any idea what could be happening there? This sound like it would be toolkit code, but we have nothing to confirm it...
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).
Flags: blocking-seamonkey2.0b1?
Version: unspecified → Trunk
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?
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.
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...
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Flags: blocking-seamonkey2.0b1? → blocking-seamonkey2.0b1-
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 ;-)
(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
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Attachment #385834 - Flags: review?(sdwilsh)
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.
Attachment #385834 - Flags: review?(sdwilsh) → review+
Component: Download & File Handling → Download Manager
Flags: blocking-seamonkey2.0b1-
Product: SeaMonkey → Toolkit
QA Contact: download → download.manager
Attached patch PatchSplinter Review
now with unit test
Attachment #385834 - Attachment is obsolete: true
Attachment #388116 - Flags: review?(sdwilsh)
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
Attachment #388116 - Flags: review?(sdwilsh) → review+
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 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.
Attachment #388116 - Flags: approval1.9.1.3?
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.
Attachment #388116 - Flags: approval1.9.1.3? → approval1.9.1.3+
Comment on attachment 388116 [details] [diff] [review]
Patch

Approved for 1.9.1.3. a=ss
Pushed to 1.9.1 branch, http://hg.mozilla.org/releases/mozilla-1.9.1/rev/56b99735ac85
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Verified for 1.9.1.3 using unit test.
Keywords: verified1.9.1
You need to log in before you can comment on or make changes to this bug.