711877 - nsHttpTransaction::Init defeats DEFER_OPEN optimization for file streams by calling Available on the main thread

Status: RESOLVED WONTFIX

(Core :: Networking: HTTP, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Potential Patch

It looks like it should be pretty easy to defer this call to the socket thread.

Comment 1

[Patrick McManus [:mcmanus]]

Comment on attachment 582709 [details] [diff] [review]
Potential Patch

Review of attachment 582709 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/nsHttpTransaction.cpp
@@ -290,5 @@
>          mRequestStream = headers;
>  
> -    rv = mRequestStream->Available(&mRequestSize);
> -    if (NS_FAILED(rv)) return rv;
> -

I would leave this code in place for the else branch of the above test (the one where there isn't a message body).. see below.

@@ +388,5 @@
> +        // If this call fails there's not much we can do.
> +        // XXXkhuey should we set mRequestSize to 0xFFFFFFFFFFFFFFFF like
> +        // nsITransportEventSink's IDL says we should?
> +        mRequestStream->Available(&mRequestSize);
> +    }

This isn't going to work in OnTransportStatus - there isn't a guaranteed event at the transaction level that happens  before data is consumed out of mrequeststream.

(resolving, connect, etc will not happen if this is a persistent connection).

but I think you can put this code in nsHttpTransaction::ReadSegments() which is called to pull data out of mRequestStream. That will capture the message body case, and you can leave the non-message body case in init because it doesn't do any I/O.

@@ +458,5 @@
>  PRUint32
>  nsHttpTransaction::Available()
>  {
> +    NS_ABORT_IF_FALSE(!NS_IsMainThread(), "Wrong thread");
> +

the necko convention I am trying to move towards:
    NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");

Comment 2

[Jason Duell]

> the necko convention I am trying to move towards:
>    NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");

Sounds good--perhaps we should add a utility function/macro for this (which can have a better error message, yet stay under 80 chars)

Comment 3

[(dormant account)]

Kyle, can you snappy:prioritize this? It's not clear how problematic this is.

Comment 4

[(dormant account)]

ping khuey ^

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This is limited to uploads only, I think so it's fairly low priority.