Closed Bug 72383 Opened 24 years ago Closed 23 years ago

nsIHTTPChannel::IsPending() stopped working (was OK in 0.8).

Categories

(Core :: Networking: HTTP, defect)

Sun
Solaris
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla0.9.1

People

(Reporter: galfandary, Assigned: darin.moz)

References

Details

(Keywords: regression)

Attachments

(1 file)

Also, it would be helpful if the implementation of nsIInputStream::Available() for a stream from nsIChannel::Open() would return an error if there is no more data to read and the underlying nsIHTTPChannel is done (!IsPending()), thus a call to IsPending() won't be necessary if all I want is to know that the stream has ended.
I propose the following fix to the problem: nsresult nsHTTPPipelinedRequest::IsPending(PRBool *result) { nsresult rv = NS_OK; if (mCurrentReadRequest) rv = mCurrentReadRequest->IsPending(result); else *result = mTransport ? PR_TRUE : PR_FALSE; return rv; } NB: actually, isn't the existence of mTransport by itself evidence enough that the request is pending?
I believe that there were some pretty big changes in this area, Comments for changes to http://lxr.mozilla.org/mozilla/source/netwerk/protocol/http/src/nsHTTPChannel.cpp valeski@netscape.com Mar 12 18:00 sr=rpotts, r=gagan. 70743 . switching over to new extensible URI::SchemeIs() api. and changing existing implementations over to new api. also modified nsHTTP and nsHTTPS handlers a bit to make things cleaner. Setting bug status to New and dding Valeski to the bug. I'm not a develper but my queries for ansIHTTPChannel::IsPending() in LXR turned up nothing and looking over recent changes to nsHTTPChannel.cpp, that API change comment caught my eye. You might find information on this change discussed in netscape.public.mozilla.netlib
Status: UNCONFIRMED → NEW
Ever confirmed: true
I'd be really suprised if my schemeIs() changes caused this.
IsPending() comes from nsIRequest, which nsHTTPChannel implements. This sounds like a problem with nsSocketTransport's nsSocketReadRequest::IsPending method.
Attached patch Fixes the problem (deleted) — Splinter Review
Status: NEW → ASSIGNED
Keywords: nsbeta1
Target Milestone: --- → mozilla0.9
The problem is that mCurrentReadRequest == NULL up until nsHTTPPipelinedRequest::OnStopRequest() is called. Thus the wrong return value during this time is not caused by mCurrentReadRequest but is simply the default in its absence. This default is wrong and as I mentioned above replacing it with: *result = mTransport ? PR_TRUE : PR_FALSE; fixes the problem.
Guy: that's not even enough. IsPending() should return TRUE between the time that AsyncOpen and OnStopRequest are called. Otherwise, it should return FALSE. My patch to the socket transport is still needed, but you are right... it doesn't solve this problem. A pipelined request does not necessarily exist before AsyncOpen returns to its caller. So, IsPending() would return FALSE if tested right after calling AsyncOpen. I think we just need a flag on nsHTTPRequest.
kicking back to 0.9.1
Keywords: regression
Target Milestone: mozilla0.9 → mozilla0.9.1
my changes for bug 76866 will fix this bug as well.
Depends on: 76866
fixed with http branch landing
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Guy Alfandary, is this problem fixed for you now?
Verified. Reporter, reopen if there is still a problem.
Status: RESOLVED → VERIFIED
QA Contact: tever → junruh
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: