bug 708415 - spdystream review comments r=honzab

This commit is contained in:
Patrick McManus 2012-01-26 00:15:26 -05:00
Родитель 7687477d87
Коммит 4d9e0c7e34
3 изменённых файлов: 97 добавлений и 64 удалений

Просмотреть файл

@ -853,7 +853,7 @@ SpdySession::HandleSynReply(SpdySession *self)
self->mFrameDataStream->UpdateTransportReadEvents(self->mFrameDataSize);
if (!self->mFrameDataStream->SetFullyOpen()) {
if (self->mFrameDataStream->GetFullyOpen()) {
// "If an endpoint receives multiple SYN_REPLY frames for the same active
// stream ID, it must drop the stream, and send a RST_STREAM for the
// stream with the error PROTOCOL_ERROR."
@ -864,6 +864,7 @@ SpdySession::HandleSynReply(SpdySession *self)
self->GenerateRstStream(RST_PROTOCOL_ERROR, streamID);
return NS_ERROR_ILLEGAL_VALUE;
}
self->mFrameDataStream->SetFullyOpen();
self->mFrameDataLast = self->mFrameBuffer[4] & kFlag_Data_FIN;

Просмотреть файл

@ -76,13 +76,13 @@ SpdyStream::SpdyStream(nsAHttpTransaction *httpTransaction,
mRecvdFin(0),
mFullyOpen(0),
mSentWaitingFor(0),
mTxInlineFrameAllocation(SpdySession::kDefaultBufferSize),
mTxInlineFrameSize(0),
mTxInlineFrameSize(SpdySession::kDefaultBufferSize),
mTxInlineFrameUsed(0),
mTxInlineFrameSent(0),
mTxStreamFrameSize(0),
mTxStreamFrameSent(0),
mZlib(compressionContext),
mRequestBodyLen(0),
mRequestBodyLenRemaining(0),
mPriority(priority),
mTotalSent(0),
mTotalRead(0)
@ -91,7 +91,7 @@ SpdyStream::SpdyStream(nsAHttpTransaction *httpTransaction,
LOG3(("SpdyStream::SpdyStream %p", this));
mTxInlineFrame = new char[mTxInlineFrameAllocation];
mTxInlineFrame = new char[mTxInlineFrameSize];
}
SpdyStream::~SpdyStream()
@ -127,18 +127,21 @@ SpdyStream::ReadSegments(nsAHttpSegmentReader *reader,
rv = mTransaction->ReadSegments(this, count, countRead);
mSegmentReader = nsnull;
// Check to see if the transaction's request could only be partially written
// out due to network buffers. If that is the case mark the stream for
// callback when writing can proceed.
if (NS_SUCCEEDED(rv) &&
mUpstreamState == GENERATING_SYN_STREAM &&
!mSynFrameComplete)
mBlockedOnWrite = 1;
// Mark that we are blocked on read if we the http transaction
// is going to get us going again.
// Mark that we are blocked on read if the http transaction needs to
// provide more of the request message body.
if (rv == NS_BASE_STREAM_WOULD_BLOCK && !mBlockedOnWrite)
mRequestBlockedOnRead = 1;
if (!mBlockedOnWrite && NS_SUCCEEDED(rv) && (!*countRead)) {
LOG3(("ReadSegments %p Send Request data complete from %x",
LOG3(("ReadSegments %p: Sending request data complete, mUpstreamState=%x",
this, mUpstreamState));
if (mSentFinOnData) {
ChangeState(UPSTREAM_COMPLETE);
@ -163,7 +166,7 @@ SpdyStream::ReadSegments(nsAHttpSegmentReader *reader,
if (NS_SUCCEEDED(rv))
rv = NS_BASE_STREAM_WOULD_BLOCK;
if (!mTxInlineFrameSize) {
if (!mTxInlineFrameUsed) {
if (mSentFinOnData) {
ChangeState(UPSTREAM_COMPLETE);
rv = NS_OK;
@ -182,12 +185,12 @@ SpdyStream::ReadSegments(nsAHttpSegmentReader *reader,
mSegmentReader = reader;
rv = TransmitFrame(nsnull, nsnull);
mSegmentReader = nsnull;
if (!mTxInlineFrameSize)
if (!mTxInlineFrameUsed)
ChangeState(UPSTREAM_COMPLETE);
}
else {
rv = NS_OK;
mTxInlineFrameSize = 0; // cancel fin data packet
mTxInlineFrameUsed = 0; // cancel fin data packet
ChangeState(UPSTREAM_COMPLETE);
}
@ -258,7 +261,7 @@ SpdyStream::ParseHttpRequestHeaders(const char *buf,
// only client we are parsing
PRInt32 endHeader = mFlatHttpRequestHeaders.Find("\r\n\r\n");
if (endHeader == -1) {
if (endHeader == kNotFound) {
// We don't have all the headers yet
LOG3(("SpdyStream::ParseHttpRequestHeaders %p "
"Need more header bytes. Len = %d",
@ -333,8 +336,6 @@ SpdyStream::ParseHttpRequestHeaders(const char *buf,
mTxInlineFrame[17] = 0; /* unused */
// nsCString methodHeader;
// mTransaction->RequestHead()->Method()->ToUTF8String(methodHeader);
const char *methodHeader = mTransaction->RequestHead()->Method().get();
nsCString hostHeader;
@ -406,14 +407,15 @@ SpdyStream::ParseHttpRequestHeaders(const char *buf,
if (name.Equals("content-length")) {
PRInt64 len;
if (nsHttp::ParseInt64(val->get(), nsnull, &len))
mRequestBodyLen = len;
mRequestBodyLenRemaining = len;
}
}
mTxInlineFrameSize = 18;
mTxInlineFrameUsed = 18;
LOG3(("http request headers to encode are: \n%s",
mFlatHttpRequestHeaders.get()));
// Do not naively log the request headers here beacuse they might
// contain auth. The http transaction already logs the sanitized request
// headers at this same level so it is not necessary to do so here.
// The header block length
PRUint16 count = hdrHash.Count() + 4; /* method, scheme, url, version */
@ -435,7 +437,7 @@ SpdyStream::ParseHttpRequestHeaders(const char *buf,
// 4 to 7 are length and flags, which we can now fill in
(reinterpret_cast<PRUint32 *>(mTxInlineFrame.get()))[1] =
PR_htonl(mTxInlineFrameSize - 8);
PR_htonl(mTxInlineFrameUsed - 8);
NS_ABORT_IF_FALSE(!mTxInlineFrame[4],
"Size greater than 24 bits");
@ -444,16 +446,17 @@ SpdyStream::ParseHttpRequestHeaders(const char *buf,
// right on the syn stream packet.
if (mTransaction->RequestHead()->Method() != nsHttp::Post &&
mTransaction->RequestHead()->Method() != nsHttp::Put) {
mTransaction->RequestHead()->Method() != nsHttp::Put &&
mTransaction->RequestHead()->Method() != nsHttp::Options) {
mSentFinOnData = 1;
mTxInlineFrame[4] = SpdySession::kFlag_Data_FIN;
}
Telemetry::Accumulate(Telemetry::SPDY_SYN_SIZE, mTxInlineFrameSize - 18);
Telemetry::Accumulate(Telemetry::SPDY_SYN_SIZE, mTxInlineFrameUsed - 18);
// The size of the input headers is approximate
PRUint32 ratio =
(mTxInlineFrameSize - 18) * 100 /
(mTxInlineFrameUsed - 18) * 100 /
(11 + mTransaction->RequestHead()->RequestURI().Length() +
mFlatHttpRequestHeaders.Length());
@ -481,8 +484,8 @@ SpdyStream::UpdateTransportSendEvents(PRUint32 count)
NS_NET_STATUS_SENDING_TO,
mTotalSent);
if (!mSentWaitingFor && !mRequestBodyLen &&
mTxInlineFrameSent == mTxInlineFrameSize &&
if (!mSentWaitingFor && !mRequestBodyLenRemaining &&
mTxInlineFrameSent == mTxInlineFrameUsed &&
mTxStreamFrameSent == mTxStreamFrameSize) {
mSentWaitingFor = 1;
mTransaction->OnTransportStatus(mSocketTransport,
@ -495,14 +498,20 @@ nsresult
SpdyStream::TransmitFrame(const char *buf,
PRUint32 *countUsed)
{
NS_ABORT_IF_FALSE(mTxInlineFrameSize, "empty stream frame in transmit");
// You can call this function with no data and no out parameter in order to
// flush internal buffers that were previously blocked on writing. You can
// of course feed new data to it as well.
NS_ABORT_IF_FALSE(mTxInlineFrameUsed, "empty stream frame in transmit");
NS_ABORT_IF_FALSE(mSegmentReader, "TransmitFrame with null mSegmentReader");
NS_ABORT_IF_FALSE((buf && countUsed) || (!buf && !countUsed),
"TransmitFrame arguments inconsistent");
PRUint32 transmittedCount;
nsresult rv;
LOG3(("SpdyStream::TransmitFrame %p inline=%d of %d stream=%d of %d",
this, mTxInlineFrameSent, mTxInlineFrameSize,
this, mTxInlineFrameSent, mTxInlineFrameUsed,
mTxStreamFrameSent, mTxStreamFrameSize));
if (countUsed)
*countUsed = 0;
@ -512,16 +521,16 @@ SpdyStream::TransmitFrame(const char *buf,
// split between the inlineframe and the streamframe, then move the stream
// data into the inlineframe via copy in order to coalesce into one write.
// Given the interaction with ssl this is worth the small copy cost.
if (mTxStreamFrameSize && mTxInlineFrameSize &&
if (mTxStreamFrameSize && mTxInlineFrameUsed &&
!mTxInlineFrameSent && !mTxStreamFrameSent &&
mTxStreamFrameSize < SpdySession::kDefaultBufferSize &&
mTxInlineFrameSize + mTxStreamFrameSize < mTxInlineFrameAllocation) {
mTxInlineFrameUsed + mTxStreamFrameSize < mTxInlineFrameSize) {
LOG3(("Coalesce Transmit"));
memcpy (mTxInlineFrame + mTxInlineFrameSize,
memcpy (mTxInlineFrame + mTxInlineFrameUsed,
buf, mTxStreamFrameSize);
if (countUsed)
*countUsed += mTxStreamFrameSize;
mTxInlineFrameSize += mTxStreamFrameSize;
mTxInlineFrameUsed += mTxStreamFrameSize;
mTxStreamFrameSent = 0;
mTxStreamFrameSize = 0;
}
@ -530,9 +539,9 @@ SpdyStream::TransmitFrame(const char *buf,
// bytes through to the SpdySession and then the HttpConnection which calls
// the socket write function.
while (mTxInlineFrameSent < mTxInlineFrameSize) {
while (mTxInlineFrameSent < mTxInlineFrameUsed) {
rv = mSegmentReader->OnReadSegment(mTxInlineFrame + mTxInlineFrameSent,
mTxInlineFrameSize - mTxInlineFrameSent,
mTxInlineFrameUsed - mTxInlineFrameSent,
&transmittedCount);
LOG3(("SpdyStream::TransmitFrame for inline session=%p "
"stream=%p result %x len=%d",
@ -556,7 +565,14 @@ SpdyStream::TransmitFrame(const char *buf,
PRUint32 avail = mTxStreamFrameSize - mTxStreamFrameSent;
while (avail) {
NS_ABORT_IF_FALSE(countUsed, "null countused pointer in a stream context");
if (!buf) {
// this cannot happen
NS_ABORT_IF_FALSE(false, "Stream transmit with null buf argument to "
"TransmitFrame()");
LOG(("Stream transmit with null buf argument to TransmitFrame()\n"));
return NS_ERROR_UNEXPECTED;
}
rv = mSegmentReader->OnReadSegment(buf + offset, avail, &transmittedCount);
LOG3(("SpdyStream::TransmitFrame for regular session=%p "
@ -580,7 +596,7 @@ SpdyStream::TransmitFrame(const char *buf,
if (!avail) {
mTxInlineFrameSent = 0;
mTxInlineFrameSize = 0;
mTxInlineFrameUsed = 0;
mTxStreamFrameSent = 0;
mTxStreamFrameSize = 0;
}
@ -604,7 +620,7 @@ SpdyStream::GenerateDataFrameHeader(PRUint32 dataLength, bool lastFrame)
this, dataLength, lastFrame));
NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
NS_ABORT_IF_FALSE(!mTxInlineFrameSize, "inline frame not empty");
NS_ABORT_IF_FALSE(!mTxInlineFrameUsed, "inline frame not empty");
NS_ABORT_IF_FALSE(!mTxInlineFrameSent, "inline partial send not 0");
NS_ABORT_IF_FALSE(!mTxStreamFrameSize, "stream frame not empty");
NS_ABORT_IF_FALSE(!mTxStreamFrameSent, "stream partial send not 0");
@ -618,7 +634,7 @@ SpdyStream::GenerateDataFrameHeader(PRUint32 dataLength, bool lastFrame)
"control bit set unexpectedly");
NS_ABORT_IF_FALSE(!mTxInlineFrame[4], "flag bits set unexpectedly");
mTxInlineFrameSize = 8;
mTxInlineFrameUsed = 8;
mTxStreamFrameSize = dataLength;
if (lastFrame) {
@ -684,20 +700,20 @@ SpdyStream::ExecuteCompress(PRUint32 flushMode)
do
{
PRUint32 avail = mTxInlineFrameAllocation - mTxInlineFrameSize;
PRUint32 avail = mTxInlineFrameSize - mTxInlineFrameUsed;
if (avail < 1) {
SpdySession::EnsureBuffer(mTxInlineFrame,
mTxInlineFrameAllocation + 2000,
mTxInlineFrameSize,
mTxInlineFrameAllocation);
avail = mTxInlineFrameAllocation - mTxInlineFrameSize;
mTxInlineFrameSize + 2000,
mTxInlineFrameUsed,
mTxInlineFrameSize);
avail = mTxInlineFrameSize - mTxInlineFrameUsed;
}
mZlib->next_out = reinterpret_cast<unsigned char *> (mTxInlineFrame.get()) +
mTxInlineFrameSize;
mTxInlineFrameUsed;
mZlib->avail_out = avail;
deflate(mZlib, flushMode);
mTxInlineFrameSize += avail - mZlib->avail_out;
mTxInlineFrameUsed += avail - mZlib->avail_out;
} while (mZlib->avail_in > 0 || !mZlib->avail_out);
}
@ -726,8 +742,7 @@ SpdyStream::CompressToFrame(const char *data, PRUint32 len)
if (len > 0xffff)
len = 0xffff;
PRUint16 networkLen = len;
networkLen = PR_htons(len);
PRUint16 networkLen = PR_htons(len);
// write out the length
mZlib->next_in = reinterpret_cast<unsigned char *> (&networkLen);
@ -787,12 +802,24 @@ SpdyStream::OnReadSegment(const char *buf,
LOG3(("ParseHttpRequestHeaders %p used %d of %d. complete = %d",
this, *countRead, count, mSynFrameComplete));
if (mSynFrameComplete) {
NS_ABORT_IF_FALSE(mTxInlineFrameSize,
NS_ABORT_IF_FALSE(mTxInlineFrameUsed,
"OnReadSegment SynFrameComplete 0b");
rv = TransmitFrame(nsnull, nsnull);
// normalize a partial write with a WOULD_BLOCK into just a partial write
// as some code will take WOULD_BLOCK to mean an error with nothing
// written (e.g. nsHttpTransaction::ReadRequestSegment()
if (rv == NS_BASE_STREAM_WOULD_BLOCK && *countRead)
rv = NS_OK;
if (mTxInlineFrameSize)
// mTxInlineFrameUsed > 0 means the current frame is in progress
// of sending. mTxInlineFrameUsed is dropped to 0 after both the frame
// and its payload (if any) are completely sent out. Here during
// GENERATING_SYN_STREAM state we are sending just the headers. So, only
// when the frame is completely sent out, we proceed to
// GENERATING_REQUEST_BODY state.
if (mTxInlineFrameUsed)
ChangeState(SENDING_SYN_STREAM);
else
ChangeState(GENERATING_REQUEST_BODY);
@ -810,28 +837,31 @@ SpdyStream::OnReadSegment(const char *buf,
dataLength = NS_MIN(count, mChunkSize);
LOG3(("SpdyStream %p id %x request len remaining %d, "
"count avail %d, chunk used %d",
this, mStreamID, mRequestBodyLen, count, dataLength));
if (dataLength > mRequestBodyLen)
this, mStreamID, mRequestBodyLenRemaining, count, dataLength));
if (dataLength > mRequestBodyLenRemaining)
return NS_ERROR_UNEXPECTED;
mRequestBodyLen -= dataLength;
GenerateDataFrameHeader(dataLength, !mRequestBodyLen);
mRequestBodyLenRemaining -= dataLength;
GenerateDataFrameHeader(dataLength, !mRequestBodyLenRemaining);
ChangeState(SENDING_REQUEST_BODY);
// NO BREAK
case SENDING_REQUEST_BODY:
NS_ABORT_IF_FALSE(mTxInlineFrameSize, "OnReadSegment Send Data Header 0b");
NS_ABORT_IF_FALSE(mTxInlineFrameUsed, "OnReadSegment Send Data Header 0b");
rv = TransmitFrame(buf, countRead);
LOG3(("TransmitFrame() rv=%x returning %d data bytes. "
"Header is %d/%d Body is %d/%d.",
rv, *countRead,
mTxInlineFrameSent, mTxInlineFrameSize,
mTxInlineFrameSent, mTxInlineFrameUsed,
mTxStreamFrameSent, mTxStreamFrameSize));
// normalize a partial write with a WOULD_BLOCK into just a partial write
// as some code will take WOULD_BLOCK to mean an error with nothing
// written (e.g. nsHttpTransaction::ReadRequestSegment()
if (rv == NS_BASE_STREAM_WOULD_BLOCK && *countRead)
rv = NS_OK;
// If that frame was all sent, look for another one
if (!mTxInlineFrameSize)
if (!mTxInlineFrameUsed)
ChangeState(GENERATING_REQUEST_BODY);
break;

Просмотреть файл

@ -72,11 +72,11 @@ public:
}
// returns false if called more than once
bool SetFullyOpen()
bool GetFullyOpen() {return mFullyOpen;}
void SetFullyOpen()
{
bool result = !mFullyOpen;
NS_ABORT_IF_FALSE(!mFullyOpen, "SetFullyOpen already open");
mFullyOpen = 1;
return result;
}
nsAHttpTransaction *Transaction()
@ -113,7 +113,7 @@ private:
nsAutoPtr<nsCString> &,
void *);
void ChangeState(enum stateType );
void ChangeState(enum stateType);
nsresult ParseHttpRequestHeaders(const char *, PRUint32, PRUint32 *);
nsresult TransmitFrame(const char *, PRUint32 *);
void GenerateDataFrameHeader(PRUint32, bool);
@ -154,9 +154,11 @@ private:
// Flag is set when all http request headers have been read
PRUint32 mSynFrameComplete : 1;
// Flag is set when there is more request data to send and the
// stream needs to be rescheduled for writing. Sometimes this
// is done as a matter of fairness, not really due to blocking
// The mBlockedOnWrite flags is set when the stream is waiting for
// a write callback because there is more request data to be sent.
// This can be because the network blocked previous write attempts or
// it may be because they were cut short as a matter of fairness to the
// other streams.
PRUint32 mBlockedOnWrite : 1;
// Flag is set when the HTTP processor has more data to send
@ -180,8 +182,8 @@ private:
// The InlineFrame and associated data is used for composing control
// frames and data frame headers.
nsAutoArrayPtr<char> mTxInlineFrame;
PRUint32 mTxInlineFrameAllocation;
PRUint32 mTxInlineFrameSize;
PRUint32 mTxInlineFrameUsed;
PRUint32 mTxInlineFrameSent;
// mTxStreamFrameSize and mTxStreamFrameSent track the progress of
@ -201,7 +203,7 @@ private:
// for a stream closed indication. Relying on stream close results
// in an extra 0-length runt packet and seems to have some interop
// problems with the google servers.
PRInt64 mRequestBodyLen;
PRInt64 mRequestBodyLenRemaining;
// based on nsISupportsPriority definitions
PRInt32 mPriority;