Closed Bug 912549 Opened 7 years ago Closed 6 years ago

Add Spdy/3.1 support

Categories

(Core :: Networking: HTTP, defect)

20 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27
Tracking Status
relnote-firefox --- 27+

People

(Reporter: mcmanus, Assigned: mcmanus)

References

Details

Attachments

(1 file, 1 obsolete file)

spdy/3.1 is essentially a spec level bugfix change to sdpy/3

It adds the concept of a session flow control window in addition to the existing per stream flow control windows. Sending is limited by the smaller of these two windows.

We found that without this change some receivers felt the need to set very small stream windows in order to limit the aggregate amount of data that could technically receive across all of the parallel streams - or even worse limit the number of streams. Both of those approaches just slow stuff down in the common case even though they are protections against the worst case (when all the parallel streams are each sending a POST/PUT - not likely).

By adding an aggregate window the receiver can get the protection it needs without being overly cheap about individual windows or the number of streams.

This change has already been picked up by the draft HTTP/2 spec.

We can maintain roughly equivalent code size by removing support for spdy/2. Twitter has already done this and google is expected to. The trains are moving forward. (that will be a different patch)
Attached patch spdy/3.1 (obsolete) — Splinter Review
Attachment #799553 - Flags: review?(hurley)
Blocks: 912550
Comment on attachment 799553 [details] [diff] [review]
spdy/3.1

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

I probably spent way too much time on this given it's almost entirely the same as spdy/3 (which we've been living with for quite a while), but I do these things out of love.

Just a couple minor comments below, r=me with those addressed.

::: netwerk/base/public/nsILoadGroup.idl
@@ +88,5 @@
>  }
>  }
>  %}
>  
> +[ptr] native SpdyPushCache3Ptr(mozilla::net::SpdyPushCache);

Rename this SpdyPushCachePtr (similar below) to match other names, as well as the comment in nsLoadGroup.cpp

::: netwerk/protocol/http/SpdyPush31.cpp
@@ +30,5 @@
> +  , mStatus(NS_OK)
> +  , mPushCompleted(false)
> +  , mDeferCleanupOnSuccess(true)
> +{
> +  LOG3(("SpdyPushedStream31 ctor this=%p 0x%X\n", this, aID));

label aID output

::: netwerk/protocol/http/SpdyPush31.h
@@ +74,5 @@
> +public:
> +  NS_DECL_ISUPPORTS
> +    NS_DECL_NSAHTTPTRANSACTION
> +
> +    SpdyPush31TransactionBuffer();

nit: indent level

::: netwerk/protocol/http/SpdySession31.cpp
@@ +94,5 @@
> +      AddStream(aHttpTransaction, firstPriority);
> +    mLastDataReadEpoch = mLastReadEpoch;
> +
> +    mPingThreshold = gHttpHandler->SpdyPingThreshold();
> +  }

nit: indentation level of entire ctor body

@@ +248,5 @@
> +    return;
> +  }
> +
> +  if (mPingSentEpoch) {
> +    LOG(("SpdySession31::ReadTimeoutTick %p handle outstanding ping\n"));

Missing argument

::: netwerk/protocol/http/SpdySession31.h
@@ +35,5 @@
> +    NS_DECL_NSAHTTPSEGMENTREADER
> +    NS_DECL_NSAHTTPSEGMENTWRITER
> +
> +   SpdySession31(nsAHttpTransaction *, nsISocketTransport *, int32_t);
> +  ~SpdySession31();

nit: indentation level

::: netwerk/protocol/http/SpdyStream31.cpp
@@ +71,5 @@
> +    mLocalWindow = spdySession->PushAllowance();
> +
> +    mTxInlineFrame = new uint8_t[mTxInlineFrameSize];
> +    mDecompressBuffer = new char[mDecompressBufferSize];
> +  }

nit: indentation of ctor body
Attachment #799553 - Flags: review?(hurley) → review+
Attached patch spdy/3.1Splinter Review
review comments and bitrot. checkin blocked on external factors
Attachment #799553 - Attachment is obsolete: true
Attachment #803157 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/20e385cea505
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Depends on: 922174
Depends on: 1113065
Depends on: 1248197
You need to log in before you can comment on or make changes to this bug.