912549 - Add Spdy/3.1 support

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Patrick McManus [:mcmanus]]

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)

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: spdy/3.1

Comment 2

[u408661]

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

Comment 3

[Patrick McManus [:mcmanus]]

Attachment: spdy/3.1

review comments and bitrot. checkin blocked on external factors

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/20e385cea505