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
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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 "