93582 - nsFileChannel::AsyncOpen synchronously stat's the file before returning

Status: VERIFIED FIXED

(Core :: Networking: File, defect, P3)

Description

[Darin Fisher]

nsFileChannel::AsyncOpen synchronously stat's the file before returning.  This
seems wrong since AsyncOpen should be doing this "asynchronously" to avoid
delaying the calling thread.  This seems like a definite performance hit.

Comment 1

[Darin Fisher]

Attachment: v1.0 removes file->Exists check

Comment 2

[Darin Fisher]

this code was added as part of the solution for bug 69134.

Comment 3

[Darin Fisher]

this bug may have a noticeable impact on startup time.

Comment 4

[Darin Fisher]

... as well as cached page load times.

Comment 5

[Doug Turner (:dougt)]

Please ensure that http://bugzilla.mozilla.org/show_bug.cgi?id=69134 is not
reproducible with your fix.

Comment 6

[Darin Fisher]

Attachment: v1.1 copies status from file transport to file channel's mStatus

Comment 7

[Darin Fisher]

i could not reproduce bug 69134 with this patch applied.

also, i removed some redundant code from nsFileChannel::OnDataAvailable.  there
was code to cancel out the file transport read request on failure, but
cancelation would occur anyways if OnDataAvailable returns failure, so the
explicit call to Cancel is completely unnecessary.

Comment 8

[Darin Fisher]

gordon, dougt: can i get r/sr= from you guys on this patch?

Comment 9

[Doug Turner (:dougt)]

why did you remove the deathgrip:
 nsCOMPtr<nsIStreamListener> tempListener = this;

Not sure exactly why it is there though.

Comment 10

[Darin Fisher]

it's not a deathgrip because we can assume that the caller holds an owning
reference to the file channel for the duration of AsyncOpen.

Comment 11

[gordon]

r=gordon

Comment 12

[Doug Turner (:dougt)]

after looking at it, this deathgrip is useless. sr=me

Comment 13

[Darin Fisher]

fixed-on-trunk

Comment 14

[benc]

+ verifyme: needs all plats.

Comment 15

[benc]

Am I looking for anything besides the lack of a crash (absence of regression of
bug 69134 from fixing this bug)?

Passed standard "file URL to non-existent file" tests. Some minor problems to be
filed that are cosmetic.

VERIFIED (on-trunk)
MacOSX 2001-09-05-05
Win32 2001-09-05-03
Linux 2001-09-05-08

Comment 16

[benc]

really verified now.