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

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect)

Description

[Guy Alfandary]

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.

Comment 1

[Guy Alfandary]

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?

Comment 2

[Asa Dotzler [:asa]]

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

[e-mail] 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 

Comment 3

[Judson Valeski]

I'd be really suprised if my schemeIs() changes caused this.

Comment 4

[Darin Fisher]

IsPending() comes from nsIRequest, which nsHTTPChannel implements.  This sounds
like a problem with nsSocketTransport's nsSocketReadRequest::IsPending method.

Comment 5

[Darin Fisher]

Attachment: Fixes the problem

Comment 6

[Guy Alfandary]

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.

Comment 7

[Darin Fisher]

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.

Comment 8

[Darin Fisher]

kicking back to 0.9.1

Comment 9

[Darin Fisher]

my changes for bug 76866 will fix this bug as well.

Comment 10

[Darin Fisher]

fixed with http branch landing

Comment 11

[Asa Dotzler [:asa]]

Guy Alfandary, is this problem fixed for you now?

Comment 12

[John Unruh]

Verified. Reporter, reopen if there is still a problem.