Closed
Bug 711877
Opened 13 years ago
Closed 8 years ago
nsHttpTransaction::Init defeats DEFER_OPEN optimization for file streams by calling Available on the main thread
Categories
(Core :: Networking: HTTP, defect)
Core
Networking: HTTP
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: khuey, Assigned: khuey)
References
Details
(Whiteboard: [Snappy:p2][necko-backlog])
Attachments
(1 file)
2.36 KB,
patch
|
Details | Diff | Splinter Review |
It looks like it should be pretty easy to defer this call to the socket thread.
Attachment #582709 -
Flags: review?(mcmanus)
Assignee | ||
Updated•13 years ago
|
Whiteboard: [Snappy]
Comment 1•12 years ago
|
||
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");
Attachment #582709 -
Flags: review?(mcmanus)
Comment 2•12 years ago
|
||
> 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•12 years ago
|
||
Kyle, can you snappy:prioritize this? It's not clear how problematic this is.
Comment 4•12 years ago
|
||
ping khuey ^
Assignee | ||
Comment 5•12 years ago
|
||
This is limited to uploads only, I think so it's fairly low priority.
Updated•12 years ago
|
Whiteboard: [Snappy] → [Snappy:p2]
Updated•8 years ago
|
Whiteboard: [Snappy:p2] → [Snappy:p2][necko-backlog]
Assignee | ||
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•