349971 - Download doesn't restart (is hung) from session restore

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Tracy Walker [:tracy]]

Seen on Linux Fx 2.0b2rc1 build 2006082104

1. Start Firefox
2. Open new windows, Open tabs in each window using pages from "Latest Headlines", Resize the windows and position them on the screen, Start a download from ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/.
3. Kill Firefox
4. Check that sessionstore.js is not deleted
5. Start Firefox
6. In the Session Restore dialog, choose Restore Session
7. Confirm that the download restarts.

tested results:
The download is hung with Pause link unresponsive. Cancel is the only thing that works in the Download Manager.

expected results: upon opening the Download Manager the download should be restarted.  Maybe the download is supposed to restart when Firefox session is restored.  I don't what the exact timing of this behavior is. But it should not hang.

Comment 1

[Dave Townsend [:mossop]]

*** Bug 350085 has been marked as a duplicate of this bug. ***

Comment 2

[Dave Townsend [:mossop]]

Not totally sure that this isn't a straight dupe.

Comment 3

[juan becerra [:juanb]]

It showed up following the same test case. I modified the steps in 350085 to bring attention to the Downloads part only.

Comment 4

[Simon Bünzli]

WFM using the 2006082403 branch nightly. Do you get any errors in the console or the Error Console? Note that may take the downloads a few seconds to get "unjammed".

Comment 5

[Tracy Walker [:tracy]]

I've waited several minutes for it to unjam with no success.  I do this in the Error console:
Error: [Exception... "Component returned failure code: 0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH) [nsILocalFile.initWithPath]"  nsresult: "0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH)"  location: "JS frame :: file:///home/twalker/temp/firefox/components/nsSessionStore.js :: anonymous :: line 1956"  data: no]
Source File: file:///home/twalker/temp/firefox/components/nsSessionStore.js
Line: 1956

nothing in the terminal

Comment 6

[Adam Guthrie]

I can confirm this is happening on a Linux branch build from today -- I get the same error as Tracy. 

Maybe this is Linux-only and has something to do with how Linux stores temporarily-downloaded files differently than Windows?

Comment 7

[Jeff Walden [:Waldo]]

I'll take this and see what I can do with it.

Comment 8

[Adam Guthrie]

I was actually looking into this earlier, so sorry to steal your thunder, Jeff.

Basically, the file "name" that we pass into nsILocalFile::InitWithPath is a file URI. 
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/browser/components/sessionstore/src/nsSessionStore.js&rev=1.38&mark=1964#1950

The *nix version of nsILocalFile doesn't like this too much: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpcom/io/nsLocalFileUnix.cpp&rev=1.132&mark=304,314-316#300, but the Windows version doesn't seem to care.

I guess we need to somehow need to pretty up |this._filename| before we pass it in.

Comment 9

[Jeff Walden [:Waldo]]

(In reply to comment #8)
> Basically, the file "name" that we pass into nsILocalFile::InitWithPath is a
> file URI. 

Only on non-Windows platforms.  On Windows, the path passed in is of the form "c:\foo\bar.baz".  On Linux and Mac (I tested both), it's of the form "/foo/bar/baz.quux".  The Windows form will pass the .initWithPath test because it *is* a local path.  The other two won't (presumably because "/foo/bar/baz.quux" isn't a URL, and somewhere along the line RDF converts the string into a proper URL).

> I guess we need to somehow need to pretty up |this._filename| before we pass
> it in.

Yeah.  Looking into what's the right thing to do here...

Comment 10

[Jeff Walden [:Waldo]]

Attachment: Hack-ish fix

Mano, while meandering through downloads code I came upon something similar to this which you wrote.  Care to look over this new manifestation of hackery?  ;-)

Comment 11

[Jeff Walden [:Waldo]]

(By the way, I know that this duplicates the resumed download, with the original one hanging; I think that should be a followup bug, if it's possible to fix that.  I'm 99% sure that that problem already existed on Windows without this patch, and it'll continue to not work even with the patch.)

Comment 12

[Asaf Romano (gone)]

Comment on attachment 235497 [details] [diff] [review]
Hack-ish fix

r=mano for now.

We should really fix the underlying issue here (bug 239948).

Comment 13

[Jeff Walden [:Waldo]]

Fixed on trunk minutes ago.

I know this works on Linux and should work on Macs with a very high degree of confidence (since paths on the two are syntactically similar), but I haven't been able to test Windows.  Could someone test this on Windows to be sure?  (Just start a big download, Ctrl+Alt+Del to success, and restart restoring session -- rest of the steps aren't needed.)

Leaving open so I remember to request branch approval assuming Windows works...

Comment 14

[Adam Guthrie]

Tracy, can you please verify this on Windows trunk so we can request approval and get this on branch?

Comment 15

[Carsten Book [:Tomcat]]

verfied fixed after serveral tests with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1b2) Gecko/20060906 BonEcho/2.0b2

Comment 16

[Carsten Book [:Tomcat]]

Tested also with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060906 Minefield/3.0a1 works as expected - see no error or regression from this patch. 

Comment 17

[Jeff Walden [:Waldo]]

Comment on attachment 235497 [details] [diff] [review]
Hack-ish fix

Verified to work on Windows, so requesting branch approval.  True download resuming would be ideal, but in the absence of that resuming downloads (which this patch enables on Mac and Linux) is the right thing to do.  This patch doesn't actually change much code, and the changes it does make are limited and correct.

Comment 18

[Jeff Walden [:Waldo]]

(In reply to comment #17)

s/resuming downloads/restarting downloads/, of course.

Comment 19

[Mike Connor [:mconnor]]

Comment on attachment 235497 [details] [diff] [review]
Hack-ish fix

a=mconnor on behalf of drivers, please get on branch ASAP.

Comment 20

[Jeff Walden [:Waldo]]

In on branch.

Comment 21

[Adam Guthrie]

Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b2) Gecko/20060908 BonEcho/2.0b2 and the steps to reproduce in comment 0.

I guess I'll file a follow-up bug for the duplicate downloads appearing...