Bug 1409570 - Ensure a pushed stream knows we've reached EOS. r?mcmanus
Previously, if a pushed stream was ended with a padding-only DATA frame
with the FIN bit set, and that stream didn't have a Content-Length,
there would be no way of knowing that the stream was finished. Now we
force-mark the push as complete if we hit a padding-only DATA frame with
the FIN bit set.
MozReview-Commit-ID: 7tk8x2FNgSj
--- a/netwerk/protocol/http/Http2Push.h
+++ b/netwerk/protocol/http/Http2Push.h
@@ -63,16 +63,17 @@ public:
bool IsOrphaned(TimeStamp now);
void OnPushFailed() { mDeferCleanupOnPush = false; mOnPushFailed = true; }
MOZ_MUST_USE nsresult GetBufferedData(char *buf, uint32_t count,
uint32_t *countWritten);
// overload of Http2Stream
virtual bool HasSink() override { return !!mConsumerStream; }
+ virtual void SetPushComplete() override { mPushCompleted = true; }
nsCString &GetRequestString() { return mRequestString; }
private:
Http2Stream *mConsumerStream; // paired request stream that consumes from
// real http/2 one.. null until a match is made.
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -3279,18 +3279,19 @@ Http2Session::WriteSegmentsAgain(nsAHttp
return rv;
}
if (mDownstreamState == DISCARDING_DATA_FRAME ||
mDownstreamState == DISCARDING_DATA_FRAME_PADDING) {
char trash[4096];
uint32_t discardCount = std::min(mInputFrameDataSize - mInputFrameDataRead,
4096U);
- LOG3(("Http2Session::WriteSegments %p trying to discard %d bytes of data",
- this, discardCount));
+ LOG3(("Http2Session::WriteSegments %p trying to discard %d bytes of %s",
+ this, discardCount,
+ mDownstreamState == DISCARDING_DATA_FRAME ? "data" : "padding"));
if (!discardCount && mDownstreamState == DISCARDING_DATA_FRAME) {
// Only do this short-cirtuit if we're not discarding a pure padding
// frame, as we need to potentially handle the stream FIN in those cases.
// See bug 1381016 comment 36 for more details.
ResetDownstreamState();
Unused << ResumeRecv();
return NS_BASE_STREAM_WOULD_BLOCK;
@@ -3312,19 +3313,25 @@ Http2Session::WriteSegmentsAgain(nsAHttp
mInputFrameDataRead += *countWritten;
if (mInputFrameDataRead == mInputFrameDataSize) {
Http2Stream *streamToCleanup = nullptr;
if (mInputFrameFinal) {
streamToCleanup = mInputFrameDataStream;
}
+ bool discardedPadding = (mDownstreamState == DISCARDING_DATA_FRAME_PADDING);
ResetDownstreamState();
if (streamToCleanup) {
+ if (discardedPadding && !(streamToCleanup->StreamID() & 1)) {
+ // Pushed streams are special on padding-only final data frames.
+ // See bug 1409570 comments 6-8 for details.
+ streamToCleanup->SetPushComplete();
+ }
CleanupStream(streamToCleanup, NS_OK, CANCEL_ERROR);
}
}
return rv;
}
if (mDownstreamState != BUFFERING_CONTROL_FRAME) {
MOZ_ASSERT(false); // this cannot happen
--- a/netwerk/protocol/http/Http2Stream.h
+++ b/netwerk/protocol/http/Http2Stream.h
@@ -148,16 +148,19 @@ public:
void SetPriority(uint32_t);
void SetPriorityDependency(uint32_t, uint8_t, bool);
void UpdatePriorityDependency();
// A pull stream has an implicit sink, a pushed stream has a sink
// once it is matched to a pull stream.
virtual bool HasSink() { return true; }
+ // This is a no-op on pull streams. Pushed streams override this.
+ virtual void SetPushComplete() { };
+
virtual ~Http2Stream();
Http2Session *Session() { return mSession; }
static MOZ_MUST_USE nsresult MakeOriginURL(const nsACString &origin,
RefPtr<nsStandardURL> &url);
static MOZ_MUST_USE nsresult MakeOriginURL(const nsACString &scheme,