bug 378637 part 7 - new spdysession() no longer takes first transaction r=hurley
authorPatrick McManus <mcmanus@ducksong.com>
Fri, 16 May 2014 11:46:11 -0400
changeset 183602 a6a1c61cba7d
parent 183601 fcf7f73bd7bb
child 183603 3b293e8b4713
push id26799
push userphilringnalda@gmail.com
push date2014-05-18 00:55 +0000
treeherdermozilla-central@00ef3a7d7aa7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershurley
bugs378637
milestone32.0a1
bug 378637 part 7 - new spdysession() no longer takes first transaction r=hurley
netwerk/protocol/http/ASpdySession.cpp
netwerk/protocol/http/ASpdySession.h
netwerk/protocol/http/Http2Session.cpp
netwerk/protocol/http/Http2Session.h
netwerk/protocol/http/SpdySession3.cpp
netwerk/protocol/http/SpdySession3.h
netwerk/protocol/http/SpdySession31.cpp
netwerk/protocol/http/SpdySession31.h
netwerk/protocol/http/nsHttpConnection.cpp
--- a/netwerk/protocol/http/ASpdySession.cpp
+++ b/netwerk/protocol/http/ASpdySession.cpp
@@ -25,42 +25,39 @@
 
 #include "mozilla/Telemetry.h"
 
 namespace mozilla {
 namespace net {
 
 ASpdySession *
 ASpdySession::NewSpdySession(uint32_t version,
-                             nsAHttpTransaction *aTransaction,
-                             nsISocketTransport *aTransport,
-                             int32_t aPriority)
+                             nsISocketTransport *aTransport)
 {
   // This is a necko only interface, so we can enforce version
   // requests as a precondition
   MOZ_ASSERT(version == SPDY_VERSION_3 ||
              version == SPDY_VERSION_31 ||
              version == NS_HTTP2_DRAFT_VERSION,
              "Unsupported spdy version");
 
   // Don't do a runtime check of IsSpdyV?Enabled() here because pref value
   // may have changed since starting negotiation. The selected protocol comes
   // from a list provided in the SERVER HELLO filtered by our acceptable
   // versions, so there is no risk of the server ignoring our prefs.
 
   Telemetry::Accumulate(Telemetry::SPDY_VERSION2, version);
 
-  if (version == SPDY_VERSION_3)
-    return new SpdySession3(aTransaction, aTransport, aPriority);
-
-  if (version == SPDY_VERSION_31)
-    return new SpdySession31(aTransaction, aTransport, aPriority);
-
-  if (version == NS_HTTP2_DRAFT_VERSION)
-    return new Http2Session(aTransaction, aTransport, aPriority);
+  if (version == SPDY_VERSION_3) {
+    return new SpdySession3(aTransport);
+  } else  if (version == SPDY_VERSION_31) {
+    return new SpdySession31(aTransport);
+  } else  if (version == NS_HTTP2_DRAFT_VERSION) {
+    return new Http2Session(aTransport);
+  }
 
   return nullptr;
 }
 
 SpdyInformation::SpdyInformation()
 {
   Version[0] = SPDY_VERSION_3;
   VersionString[0] = NS_LITERAL_CSTRING("spdy/3");
--- a/netwerk/protocol/http/ASpdySession.h
+++ b/netwerk/protocol/http/ASpdySession.h
@@ -20,20 +20,17 @@ class ASpdySession : public nsAHttpTrans
 public:
   virtual bool AddStream(nsAHttpTransaction *, int32_t) = 0;
   virtual bool CanReuse() = 0;
   virtual bool RoomForMoreStreams() = 0;
   virtual PRIntervalTime IdleTime() = 0;
   virtual uint32_t ReadTimeoutTick(PRIntervalTime now) = 0;
   virtual void DontReuse() = 0;
 
-  static ASpdySession *NewSpdySession(uint32_t version,
-                                      nsAHttpTransaction *,
-                                      nsISocketTransport *,
-                                      int32_t);
+  static ASpdySession *NewSpdySession(uint32_t version, nsISocketTransport *);
 
   virtual void PrintDiagnostics (nsCString &log) = 0;
 
   bool ResponseTimeoutEnabled() const MOZ_OVERRIDE MOZ_FINAL {
     return true;
   }
 
   const static uint32_t kSendingChunkSize = 4095;
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -58,19 +58,17 @@ const uint8_t Http2Session::kMagicHello[
 };
 
 #define RETURN_SESSION_ERROR(o,x)  \
 do {                             \
   (o)->mGoAwayReason = (x);      \
   return NS_ERROR_ILLEGAL_VALUE; \
   } while (0)
 
-Http2Session::Http2Session(nsAHttpTransaction *aHttpTransaction,
-                           nsISocketTransport *aSocketTransport,
-                           int32_t firstPriority)
+Http2Session::Http2Session(nsISocketTransport *aSocketTransport)
   : mSocketTransport(aSocketTransport)
   , mSegmentReader(nullptr)
   , mSegmentWriter(nullptr)
   , mNextStreamID(3) // 1 is reserved for Updgrade handshakes
   , mConcurrentHighWater(0)
   , mDownstreamState(BUFFERING_OPENING_SETTINGS)
   , mInputFrameBufferSize(kDefaultBufferSize)
   , mInputFrameBufferUsed(0)
@@ -95,40 +93,36 @@ Http2Session::Http2Session(nsAHttpTransa
   , mLocalSessionWindow(kDefaultRwin)
   , mServerSessionWindow(kDefaultRwin)
   , mOutputQueueSize(kDefaultQueueSize)
   , mOutputQueueUsed(0)
   , mOutputQueueSent(0)
   , mLastReadEpoch(PR_IntervalNow())
   , mPingSentEpoch(0)
 {
-    MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
-
-    static uint64_t sSerial;
-    mSerial = ++sSerial;
-
-    LOG3(("Http2Session::Http2Session %p transaction 1 = %p serial=0x%X\n",
-          this, aHttpTransaction, mSerial));
-
-    mConnection = aHttpTransaction->Connection();
-    mInputFrameBuffer = new char[mInputFrameBufferSize];
-    mOutputQueueBuffer = new char[mOutputQueueSize];
-    mDecompressBuffer.SetCapacity(kDefaultBufferSize);
-    mDecompressor.SetCompressor(&mCompressor);
-
-    mPushAllowance = gHttpHandler->SpdyPushAllowance();
-
-    mSendingChunkSize = gHttpHandler->SpdySendingChunkSize();
-    SendHello();
-
-    if (!aHttpTransaction->IsNullTransaction())
-      AddStream(aHttpTransaction, firstPriority);
-    mLastDataReadEpoch = mLastReadEpoch;
-
-    mPingThreshold = gHttpHandler->SpdyPingThreshold();
+  MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
+
+  static uint64_t sSerial;
+  mSerial = ++sSerial;
+
+  LOG3(("Http2Session::Http2Session %p serial=0x%X\n", this, mSerial));
+
+  mInputFrameBuffer = new char[mInputFrameBufferSize];
+  mOutputQueueBuffer = new char[mOutputQueueSize];
+  mDecompressBuffer.SetCapacity(kDefaultBufferSize);
+  mDecompressor.SetCompressor(&mCompressor);
+
+  mPushAllowance = gHttpHandler->SpdyPushAllowance();
+
+  mSendingChunkSize = gHttpHandler->SpdySendingChunkSize();
+  SendHello();
+
+  mLastDataReadEpoch = mLastReadEpoch;
+
+  mPingThreshold = gHttpHandler->SpdyPingThreshold();
 }
 
 // Copy the 32 bit number into the destination, using network byte order
 // in the destination.
 template<typename charType> static void
 CopyAsNetwork32(charType dest,   // where to store it
                 uint32_t number) // the 32 bit number in native format
 {
@@ -385,16 +379,28 @@ Http2Session::AddStream(nsAHttpTransacti
 
   // integrity check
   if (mStreamTransactionHash.Get(aHttpTransaction)) {
     LOG3(("   New transaction already present\n"));
     MOZ_ASSERT(false, "AddStream duplicate transaction pointer");
     return false;
   }
 
+  // assert that
+  // a] in the case we have a connection, that the new transaction connection
+  //    is either undefined or on the same connection
+  // b] in the case we don't have a connection, that the new transaction
+  //    connection is defined so we can adopt it
+  MOZ_ASSERT((mConnection && (!aHttpTransaction->Connection() ||
+                              mConnection == aHttpTransaction->Connection())) ||
+             (!mConnection && aHttpTransaction->Connection()));
+
+  if (!mConnection) {
+    mConnection = aHttpTransaction->Connection();
+  }
   aHttpTransaction->SetConnection(this);
   Http2Stream *stream = new Http2Stream(aHttpTransaction, this, aPriority);
 
   LOG3(("Http2Session::AddStream session=%p stream=%p NextID=0x%X (tentative)",
         this, stream, mNextStreamID));
 
   mStreamTransactionHash.Put(aHttpTransaction, stream);
 
--- a/netwerk/protocol/http/Http2Session.h
+++ b/netwerk/protocol/http/Http2Session.h
@@ -33,17 +33,17 @@ class Http2Session MOZ_FINAL : public AS
 {
 public:
   NS_DECL_ISUPPORTS
     NS_DECL_NSAHTTPTRANSACTION
     NS_DECL_NSAHTTPCONNECTION(mConnection)
     NS_DECL_NSAHTTPSEGMENTREADER
     NS_DECL_NSAHTTPSEGMENTWRITER
 
-   Http2Session(nsAHttpTransaction *, nsISocketTransport *, int32_t);
+   Http2Session(nsISocketTransport *);
   ~Http2Session();
 
   bool AddStream(nsAHttpTransaction *, int32_t);
   bool CanReuse() { return !mShouldGoAway && !mClosed; }
   bool RoomForMoreStreams();
 
   // When the connection is active this is called up to once every 1 second
   // return the interval (in seconds) that the connection next wants to
--- a/netwerk/protocol/http/SpdySession3.cpp
+++ b/netwerk/protocol/http/SpdySession3.cpp
@@ -39,19 +39,17 @@ namespace net {
 // nsISupports, so this magic is taken from nsHttpPipeline that
 // implements some of the same abstract classes.
 NS_IMPL_ADDREF(SpdySession3)
 NS_IMPL_RELEASE(SpdySession3)
 NS_INTERFACE_MAP_BEGIN(SpdySession3)
     NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsAHttpConnection)
 NS_INTERFACE_MAP_END
 
-SpdySession3::SpdySession3(nsAHttpTransaction *aHttpTransaction,
-                           nsISocketTransport *aSocketTransport,
-                           int32_t firstPriority)
+SpdySession3::SpdySession3(nsISocketTransport *aSocketTransport)
   : mSocketTransport(aSocketTransport)
   , mSegmentReader(nullptr)
   , mSegmentWriter(nullptr)
   , mNextStreamID(1)
   , mConcurrentHighWater(0)
   , mDownstreamState(BUFFERING_FRAME_HEADER)
   , mInputFrameBufferSize(kDefaultBufferSize)
   , mInputFrameBufferUsed(0)
@@ -74,30 +72,26 @@ SpdySession3::SpdySession3(nsAHttpTransa
   , mPingSentEpoch(0)
   , mNextPingID(1)
 {
   MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
 
   static uint64_t sSerial;
   mSerial = ++sSerial;
 
-  LOG3(("SpdySession3::SpdySession3 %p transaction 1 = %p serial=0x%X\n",
-        this, aHttpTransaction, mSerial));
-
-  mConnection = aHttpTransaction->Connection();
+  LOG3(("SpdySession3::SpdySession3 %p serial=0x%X\n", this, mSerial));
+
   mInputFrameBuffer = new char[mInputFrameBufferSize];
   mOutputQueueBuffer = new char[mOutputQueueSize];
   zlibInit();
 
   mPushAllowance = gHttpHandler->SpdyPushAllowance();
   mSendingChunkSize = gHttpHandler->SpdySendingChunkSize();
   GenerateSettings();
 
-  if (!aHttpTransaction->IsNullTransaction())
-    AddStream(aHttpTransaction, firstPriority);
   mLastDataReadEpoch = mLastReadEpoch;
 
   mPingThreshold = gHttpHandler->SpdyPingThreshold();
 }
 
 PLDHashOperator
 SpdySession3::ShutdownEnumerator(nsAHttpTransaction *key,
                                  nsAutoPtr<SpdyStream3> &stream,
@@ -346,16 +340,28 @@ SpdySession3::AddStream(nsAHttpTransacti
 
   // integrity check
   if (mStreamTransactionHash.Get(aHttpTransaction)) {
     LOG3(("   New transaction already present\n"));
     MOZ_ASSERT(false, "AddStream duplicate transaction pointer");
     return false;
   }
 
+  // assert that
+  // a] in the case we have a connection, that the new transaction connection
+  //    is either undefined or on the same connection
+  // b] in the case we don't have a connection, that the new transaction
+  //    connection is defined so we can adopt it
+  MOZ_ASSERT((mConnection && (!aHttpTransaction->Connection() ||
+                              mConnection == aHttpTransaction->Connection())) ||
+             (!mConnection && aHttpTransaction->Connection()));
+
+  if (!mConnection) {
+    mConnection = aHttpTransaction->Connection();
+  }
   aHttpTransaction->SetConnection(this);
   SpdyStream3 *stream = new SpdyStream3(aHttpTransaction, this, aPriority);
 
   LOG3(("SpdySession3::AddStream session=%p stream=%p NextID=0x%X (tentative)",
         this, stream, mNextStreamID));
 
   mStreamTransactionHash.Put(aHttpTransaction, stream);
 
--- a/netwerk/protocol/http/SpdySession3.h
+++ b/netwerk/protocol/http/SpdySession3.h
@@ -32,17 +32,17 @@ class SpdySession3 MOZ_FINAL : public AS
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSAHTTPTRANSACTION
   NS_DECL_NSAHTTPCONNECTION(mConnection)
   NS_DECL_NSAHTTPSEGMENTREADER
   NS_DECL_NSAHTTPSEGMENTWRITER
 
-  SpdySession3(nsAHttpTransaction *, nsISocketTransport *, int32_t);
+  SpdySession3(nsISocketTransport *);
   ~SpdySession3();
 
   bool AddStream(nsAHttpTransaction *, int32_t);
   bool CanReuse() { return !mShouldGoAway && !mClosed; }
   bool RoomForMoreStreams();
 
   // When the connection is active this is called up to once every 1 second
   // return the interval (in seconds) that the connection next wants to
--- a/netwerk/protocol/http/SpdySession31.cpp
+++ b/netwerk/protocol/http/SpdySession31.cpp
@@ -40,19 +40,17 @@ namespace net {
 // nsISupports, so this magic is taken from nsHttpPipeline that
 // implements some of the same abstract classes.
 NS_IMPL_ADDREF(SpdySession31)
 NS_IMPL_RELEASE(SpdySession31)
 NS_INTERFACE_MAP_BEGIN(SpdySession31)
 NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsAHttpConnection)
 NS_INTERFACE_MAP_END
 
-SpdySession31::SpdySession31(nsAHttpTransaction *aHttpTransaction,
-                             nsISocketTransport *aSocketTransport,
-                             int32_t firstPriority)
+SpdySession31::SpdySession31(nsISocketTransport *aSocketTransport)
   : mSocketTransport(aSocketTransport)
   , mSegmentReader(nullptr)
   , mSegmentWriter(nullptr)
   , mNextStreamID(1)
   , mConcurrentHighWater(0)
   , mDownstreamState(BUFFERING_FRAME_HEADER)
   , mInputFrameBufferSize(kDefaultBufferSize)
   , mInputFrameBufferUsed(0)
@@ -77,31 +75,27 @@ SpdySession31::SpdySession31(nsAHttpTran
   , mPingSentEpoch(0)
   , mNextPingID(1)
 {
   MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
 
   static uint64_t sSerial;
   mSerial = ++sSerial;
 
-  LOG3(("SpdySession31::SpdySession31 %p transaction 1 = %p serial=0x%X\n",
-        this, aHttpTransaction, mSerial));
-
-  mConnection = aHttpTransaction->Connection();
+  LOG3(("SpdySession31::SpdySession31 %p serial=0x%X\n", this, mSerial));
+
   mInputFrameBuffer = new char[mInputFrameBufferSize];
   mOutputQueueBuffer = new char[mOutputQueueSize];
   zlibInit();
 
   mPushAllowance = gHttpHandler->SpdyPushAllowance();
 
   mSendingChunkSize = gHttpHandler->SpdySendingChunkSize();
   GenerateSettings();
 
-  if (!aHttpTransaction->IsNullTransaction())
-    AddStream(aHttpTransaction, firstPriority);
   mLastDataReadEpoch = mLastReadEpoch;
 
   mPingThreshold = gHttpHandler->SpdyPingThreshold();
 }
 
 PLDHashOperator
 SpdySession31::ShutdownEnumerator(nsAHttpTransaction *key,
                                   nsAutoPtr<SpdyStream31> &stream,
@@ -350,16 +344,28 @@ SpdySession31::AddStream(nsAHttpTransact
 
   // integrity check
   if (mStreamTransactionHash.Get(aHttpTransaction)) {
     LOG3(("   New transaction already present\n"));
     MOZ_ASSERT(false, "AddStream duplicate transaction pointer");
     return false;
   }
 
+  // assert that
+  // a] in the case we have a connection, that the new transaction connection
+  //    is either undefined or on the same connection
+  // b] in the case we don't have a connection, that the new transaction
+  //    connection is defined so we can adopt it
+  MOZ_ASSERT((mConnection && (!aHttpTransaction->Connection() ||
+                              mConnection == aHttpTransaction->Connection())) ||
+             (!mConnection && aHttpTransaction->Connection()));
+
+  if (!mConnection) {
+    mConnection = aHttpTransaction->Connection();
+  }
   aHttpTransaction->SetConnection(this);
   SpdyStream31 *stream = new SpdyStream31(aHttpTransaction, this, aPriority);
 
   LOG3(("SpdySession31::AddStream session=%p stream=%p NextID=0x%X (tentative)",
         this, stream, mNextStreamID));
 
   mStreamTransactionHash.Put(aHttpTransaction, stream);
 
--- a/netwerk/protocol/http/SpdySession31.h
+++ b/netwerk/protocol/http/SpdySession31.h
@@ -31,17 +31,17 @@ class SpdySession31 MOZ_FINAL : public A
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSAHTTPTRANSACTION
   NS_DECL_NSAHTTPCONNECTION(mConnection)
   NS_DECL_NSAHTTPSEGMENTREADER
   NS_DECL_NSAHTTPSEGMENTWRITER
 
-  SpdySession31(nsAHttpTransaction *, nsISocketTransport *, int32_t);
+  SpdySession31(nsISocketTransport *);
   ~SpdySession31();
 
   bool AddStream(nsAHttpTransaction *, int32_t);
   bool CanReuse() { return !mShouldGoAway && !mClosed; }
   bool RoomForMoreStreams();
 
   // When the connection is active this is called up to once every 1 second
   // return the interval (in seconds) that the connection next wants to
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -192,53 +192,49 @@ nsHttpConnection::StartSpdy(uint8_t spdy
         LOG(("unexpected rv from nnsAHttpTransaction::TakeSubTransactions()"));
         MOZ_ASSERT(false,
                    "unexpected result from "
                    "nsAHttpTransaction::TakeSubTransactions()");
         mTransaction->Close(NS_ERROR_ABORT);
         return;
     }
 
+    mSpdySession = ASpdySession::NewSpdySession(spdyVersion, mSocketTransport);
     if (NS_FAILED(rv)) { // includes NS_ERROR_NOT_IMPLEMENTED
         MOZ_ASSERT(list.IsEmpty(), "sub transaction list not empty");
 
         // This is ok - treat mTransaction as a single real request.
         // Wrap the old http transaction into the new spdy session
         // as the first stream.
-        mSpdySession = ASpdySession::NewSpdySession(spdyVersion,
-                                                    mTransaction, mSocketTransport,
-                                                    mPriority);
+        if (!mSpdySession->AddStream(mTransaction, mPriority)) {
+            MOZ_ASSERT(false); // this cannot happen!
+            mTransaction->Close(NS_ERROR_ABORT);
+            return;
+        }
         LOG(("nsHttpConnection::StartSpdy moves single transaction %p "
              "into SpdySession %p\n", mTransaction.get(), mSpdySession.get()));
     }
     else {
         int32_t count = list.Length();
 
         LOG(("nsHttpConnection::StartSpdy moving transaction list len=%d "
              "into SpdySession %p\n", count, mSpdySession.get()));
 
         if (!count) {
             mTransaction->Close(NS_ERROR_ABORT);
             return;
         }
 
         for (int32_t index = 0; index < count; ++index) {
-            if (!mSpdySession) {
-                mSpdySession = ASpdySession::NewSpdySession(spdyVersion,
-                                                            list[index], mSocketTransport,
-                                                            mPriority);
-            }
-            else {
-                // AddStream() cannot fail
-                if (!mSpdySession->AddStream(list[index], mPriority)) {
-                    MOZ_ASSERT(false, "SpdySession::AddStream failed");
-                    LOG(("SpdySession::AddStream failed\n"));
-                    mTransaction->Close(NS_ERROR_ABORT);
-                    return;
-                }
+            // AddStream() cannot fail
+            if (!mSpdySession->AddStream(list[index], mPriority)) {
+                MOZ_ASSERT(false, "SpdySession::AddStream failed");
+                LOG(("SpdySession::AddStream failed\n"));
+                mTransaction->Close(NS_ERROR_ABORT);
+                return;
             }
         }
     }
 
     // Disable TCP Keepalives - use SPDY ping instead.
     rv = DisableTCPKeepalives();
     if (NS_WARN_IF(NS_FAILED(rv))) {
         LOG(("nsHttpConnection::StartSpdy [%p] DisableTCPKeepalives failed "