Backout e15196e25f9e for causing bug 1415387
authorNicholas Hurley <hurley@mozilla.com>
Wed, 08 Nov 2017 10:30:01 -0800
changeset 444091 cc25abc4eab682a2c9881cf0d81343bc28ef34f0
parent 444090 24468798c7a6bd80e63351cf0c8842cda4ace7e3
child 444092 a06f5967bcb6d135c7283e0cc9adab5ddb3442a3
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1415387
milestone58.0a1
backs oute15196e25f9e783b2c04689936074990908170b0
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
Backout e15196e25f9e for causing bug 1415387
netwerk/protocol/http/Http2Session.cpp
netwerk/protocol/http/Http2Session.h
netwerk/protocol/http/Http2Stream.cpp
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -114,18 +114,16 @@ Http2Session::Http2Session(nsISocketTran
   , mPingSentEpoch(0)
   , mPreviousUsed(false)
   , mWaitingForSettingsAck(false)
   , mGoAwayOnPush(false)
   , mUseH2Deps(false)
   , mAttemptingEarlyData(attemptingEarlyData)
   , mOriginFrameActivated(false)
   , mTlsHandshakeFinished(false)
-  , mFlushOKAddStream(false)
-  , mFlushOKReadSegments(false)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
   static uint64_t sSerial;
   mSerial = ++sSerial;
 
   LOG3(("Http2Session::Http2Session %p serial=0x%" PRIX64 "\n", this, mSerial));
 
@@ -381,23 +379,16 @@ Http2Session::RegisterStreamID(Http2Stre
 bool
 Http2Session::AddStream(nsAHttpTransaction *aHttpTransaction,
                         int32_t aPriority,
                         bool aUseTunnel,
                         nsIInterfaceRequestor *aCallbacks)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
-  MOZ_DIAGNOSTIC_ASSERT(!mFlushOKAddStream);
-  mFlushOKAddStream = true;
-  auto cleanup = MakeScopeExit([&] () {
-    MOZ_DIAGNOSTIC_ASSERT(mFlushOKAddStream);
-    mFlushOKAddStream = false;
-  });
-
   // integrity check
   if (mStreamTransactionHash.Get(aHttpTransaction)) {
     LOG3(("   New transaction already present\n"));
     MOZ_ASSERT(false, "AddStream duplicate transaction pointer");
     return false;
   }
 
   if (!mConnection) {
@@ -547,34 +538,16 @@ Http2Session::RealignOutputQueue()
   mOutputQueueUsed -= mOutputQueueSent;
   memmove(mOutputQueueBuffer.get(),
           mOutputQueueBuffer.get() + mOutputQueueSent,
           mOutputQueueUsed);
   mOutputQueueSent = 0;
 }
 
 void
-Http2Session::MaybeFlushOutputQueue()
-{
-  // Only try to flush the output queue if we know any mSegmentReader that's set
-  // is properly set through the right channels. Otherwise, just set our write
-  // callbacks so the connection can call in with a proper segment reader that
-  // we'll be sure we can write to.
-  // See bug 1402014 comment 6
-  MOZ_ASSERT(OnSocketThread(), "not on socket thread");
-  LOG3(("Http2Session::MaybeFlushOutputQueue mFlushOKAddStream=%d, "
-        "mFlushOKReadSegments=%d", mFlushOKAddStream, mFlushOKReadSegments));
-  if (mFlushOKAddStream || mFlushOKReadSegments) {
-    FlushOutputQueue();
-  } else {
-    SetWriteCallbacks();
-  }
-}
-
-void
 Http2Session::FlushOutputQueue()
 {
   if (!mSegmentReader || !mOutputQueueUsed)
     return;
 
   nsresult rv;
   uint32_t countRead;
   uint32_t avail = mOutputQueueUsed - mOutputQueueSent;
@@ -816,44 +789,44 @@ Http2Session::GeneratePing(bool isAck)
     memcpy(packet + kFrameHeaderBytes,
            mInputFrameBuffer.get() + kFrameHeaderBytes, 8);
   } else {
     CreateFrameHeader(packet, 8, FRAME_TYPE_PING, 0, 0);
     memset(packet + kFrameHeaderBytes, 0, 8);
   }
 
   LogIO(this, nullptr, "Generate Ping", packet, kFrameHeaderBytes + 8);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
 Http2Session::GenerateSettingsAck()
 {
   // need to generate ack of this settings frame
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
   LOG3(("Http2Session::GenerateSettingsAck %p\n", this));
 
   char *packet = EnsureOutputBuffer(kFrameHeaderBytes);
   mOutputQueueUsed += kFrameHeaderBytes;
   CreateFrameHeader(packet, 0, FRAME_TYPE_SETTINGS, kFlag_ACK, 0);
   LogIO(this, nullptr, "Generate Settings ACK", packet, kFrameHeaderBytes);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
 Http2Session::GeneratePriority(uint32_t aID, uint8_t aPriorityWeight)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
   LOG3(("Http2Session::GeneratePriority %p %X %X\n",
         this, aID, aPriorityWeight));
 
   char *packet = CreatePriorityFrame(aID, 0, aPriorityWeight);
 
   LogIO(this, nullptr, "Generate Priority", packet, kFrameHeaderBytes + 5);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
 Http2Session::GenerateRstStream(uint32_t aStatusCode, uint32_t aID)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
   // make sure we don't do this twice for the same stream (at least if we
@@ -870,17 +843,17 @@ Http2Session::GenerateRstStream(uint32_t
   uint32_t frameSize = kFrameHeaderBytes + 4;
   char *packet = EnsureOutputBuffer(frameSize);
   mOutputQueueUsed += frameSize;
   CreateFrameHeader(packet, 4, FRAME_TYPE_RST_STREAM, 0, aID);
 
   NetworkEndian::writeUint32(packet + kFrameHeaderBytes, aStatusCode);
 
   LogIO(this, nullptr, "Generate Reset", packet, frameSize);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
 Http2Session::GenerateGoAway(uint32_t aStatusCode)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
   LOG3(("Http2Session::GenerateGoAway %p code=%X\n", this, aStatusCode));
 
@@ -893,17 +866,17 @@ Http2Session::GenerateGoAway(uint32_t aS
 
   // last-good-stream-id are bytes 9-12 reflecting pushes
   NetworkEndian::writeUint32(packet + kFrameHeaderBytes, mOutgoingGoAwayID);
 
   // bytes 13-16 are the status code.
   NetworkEndian::writeUint32(packet + frameSize - 4, aStatusCode);
 
   LogIO(this, nullptr, "Generate GoAway", packet, frameSize);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 // The Hello is comprised of
 // 1] 24 octets of magic, which are designed to
 // flush out silent but broken intermediaries
 // 2] a settings frame which sets a small flow control window for pushes
 // 3] a window update frame which creates a large session flow control window
 // 4] 6 priority frames for streams which will never be opened with headers
@@ -1015,32 +988,32 @@ Http2Session::SendHello()
     MOZ_ASSERT(mNextStreamID == kUrgentStartGroupID);
     CreatePriorityNode(kUrgentStartGroupID, 0, 240, "urgentStart");
     mNextStreamID += 2;
     // Hey, you! YES YOU! If you add/remove any groups here, you almost
     // certainly need to change the lookup of the stream/ID hash in
     // Http2Session::OnTransportStatus. Yeah, that's right. YOU!
   }
 
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
 Http2Session::SendPriorityFrame(uint32_t streamID,
                                 uint32_t dependsOn,
                                 uint8_t weight)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
   LOG3(("Http2Session::SendPriorityFrame %p Frame 0x%X depends on 0x%X "
         "weight %d\n", this, streamID, dependsOn, weight));
 
   char *packet = CreatePriorityFrame(streamID, dependsOn, weight);
 
   LogIO(this, nullptr, "SendPriorityFrame", packet, kFrameHeaderBytes + 5);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 char *
 Http2Session::CreatePriorityFrame(uint32_t streamID,
                                   uint32_t dependsOn,
                                   uint8_t weight)
 {
   char *packet = EnsureOutputBuffer(kFrameHeaderBytes + 5);
@@ -2767,47 +2740,39 @@ nsresult
 Http2Session::ReadSegmentsAgain(nsAHttpSegmentReader *reader,
                                 uint32_t count, uint32_t *countRead, bool *again)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
   MOZ_ASSERT(!mSegmentReader || !reader || (mSegmentReader == reader),
              "Inconsistent Write Function Callback");
 
-  MOZ_DIAGNOSTIC_ASSERT(!mFlushOKReadSegments);
-  mFlushOKReadSegments = true;
-  auto cleanup = MakeScopeExit([&] () {
-    MOZ_DIAGNOSTIC_ASSERT(mFlushOKReadSegments);
-    mFlushOKReadSegments = false;
-  });
-
   nsresult rv = ConfirmTLSProfile();
   if (NS_FAILED(rv)) {
     if (mGoAwayReason == INADEQUATE_SECURITY) {
       LOG3(("Http2Session::ReadSegments %p returning INADEQUATE_SECURITY %" PRIx32,
             this, static_cast<uint32_t>(NS_ERROR_NET_INADEQUATE_SECURITY)));
       rv = NS_ERROR_NET_INADEQUATE_SECURITY;
     }
     return rv;
   }
 
-  if (reader) {
-    SetSegmentReader(reader);
-  }
+  if (reader)
+    mSegmentReader = reader;
 
   *countRead = 0;
 
   LOG3(("Http2Session::ReadSegments %p", this));
 
   Http2Stream *stream = static_cast<Http2Stream *>(mReadyForWrite.PopFront());
   if (!stream) {
     LOG3(("Http2Session %p could not identify a stream to write; suspending.",
           this));
     uint32_t availBeforeFlush = mOutputQueueUsed - mOutputQueueSent;
-    MaybeFlushOutputQueue();
+    FlushOutputQueue();
     uint32_t availAfterFlush = mOutputQueueUsed - mOutputQueueSent;
     if (availBeforeFlush != availAfterFlush) {
       LOG3(("Http2Session %p ResumeRecv After early flush in ReadSegments", this));
       Unused << ResumeRecv();
     }
     SetWriteCallbacks();
     if (mAttemptingEarlyData) {
       // We can still try to send our preamble as early-data
@@ -2816,17 +2781,17 @@ Http2Session::ReadSegmentsAgain(nsAHttpS
     return *countRead ? NS_OK : NS_BASE_STREAM_WOULD_BLOCK;
   }
 
   uint32_t earlyDataUsed = 0;
   if (mAttemptingEarlyData) {
     if (!stream->Do0RTT()) {
       LOG3(("Http2Session %p will not get early data from Http2Stream %p 0x%X",
             this, stream, stream->StreamID()));
-      MaybeFlushOutputQueue();
+      FlushOutputQueue();
       SetWriteCallbacks();
       if (!mCannotDo0RTTStreams.Contains(stream)) {
         mCannotDo0RTTStreams.AppendElement(stream);
       }
       // We can still send our preamble
       *countRead = mOutputQueueUsed - mOutputQueueSent;
       return *countRead ? NS_OK : NS_BASE_STREAM_WOULD_BLOCK;
     }
@@ -2858,17 +2823,17 @@ Http2Session::ReadSegmentsAgain(nsAHttpS
           stream->StreamID()));
     m0RTTStreams.AppendElement(stream);
   }
 
   // Not every permutation of stream->ReadSegents produces data (and therefore
   // tries to flush the output queue) - SENDING_FIN_STREAM can be an example
   // of that. But we might still have old data buffered that would be good
   // to flush.
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 
   // Allow new server reads - that might be data or control information
   // (e.g. window updates or http replies) that are responses to these writes
   Unused << ResumeRecv();
 
   if (stream->RequestBlockedOnRead()) {
 
     // We are blocked waiting for input - either more http headers or
@@ -3669,17 +3634,17 @@ void
 Http2Session::UpdateLocalRwin(Http2Stream *stream, uint32_t bytes)
 {
   // make sure there is room for 2 window updates even though
   // we may not generate any.
   EnsureOutputBuffer(2 * (kFrameHeaderBytes + 4));
 
   UpdateLocalStreamWindow(stream, bytes);
   UpdateLocalSessionWindow(bytes);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
 Http2Session::Close(nsresult aReason)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
   if (mClosed)
@@ -3758,17 +3723,17 @@ Http2Session::OnReadSegment(const char *
                             uint32_t count, uint32_t *countRead)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
   nsresult rv;
 
   // If we can release old queued data then we can try and write the new
   // data directly to the network without using the output queue at all
   if (mOutputQueueUsed)
-    MaybeFlushOutputQueue();
+    FlushOutputQueue();
 
   if (!mOutputQueueUsed && mSegmentReader) {
     // try and write directly without output queue
     rv = mSegmentReader->OnReadSegment(buf, count, countRead);
 
     if (rv == NS_BASE_STREAM_WOULD_BLOCK) {
       *countRead = 0;
     } else if (NS_FAILED(rv)) {
@@ -3800,26 +3765,26 @@ Http2Session::OnReadSegment(const char *
 
   if ((mOutputQueueUsed + count) > (mOutputQueueSize - kQueueReserved))
     return NS_BASE_STREAM_WOULD_BLOCK;
 
   memcpy(mOutputQueueBuffer.get() + mOutputQueueUsed, buf, count);
   mOutputQueueUsed += count;
   *countRead = count;
 
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 
   return NS_OK;
 }
 
 nsresult
 Http2Session::CommitToSegmentSize(uint32_t count, bool forceCommitment)
 {
   if (mOutputQueueUsed && !mAttemptingEarlyData)
-    MaybeFlushOutputQueue();
+    FlushOutputQueue();
 
   // would there be enough room to buffer this if needed?
   if ((mOutputQueueUsed + count) <= (mOutputQueueSize - kQueueReserved))
     return NS_OK;
 
   // if we are using part of our buffers already, try again later unless
   // forceCommitment is set.
   if (mOutputQueueUsed && !forceCommitment)
@@ -4586,23 +4551,10 @@ Http2Session::TopLevelOuterContentWindow
 
   mCurrentForegroundTabOuterContentWindowId = windowId;
 
   for (auto iter = mStreamTransactionHash.Iter(); !iter.Done(); iter.Next()) {
     iter.Data()->TopLevelOuterContentWindowIdChanged(windowId);
   }
 }
 
-void
-Http2Session::SetSegmentReader(nsAHttpSegmentReader *reader)
-{
-  LOG3(("Http2Session::SetSegmentReader this=%p mClosed=%d mSegmentReader=%p reader=%p",
-        this, mClosed, mSegmentReader, reader));
-  MOZ_DIAGNOSTIC_ASSERT(!mSegmentReader || reader == mSegmentReader);
-  if (mClosed) {
-    mSegmentReader = nullptr;
-  } else {
-    mSegmentReader = reader;
-  }
-}
-
 } // namespace net
 } // namespace mozilla
--- a/netwerk/protocol/http/Http2Session.h
+++ b/netwerk/protocol/http/Http2Session.h
@@ -215,17 +215,17 @@ public:
 
   // a similar version for Http2Stream
   void TransactionHasDataToWrite(Http2Stream *);
 
   // an overload of nsAHttpSegementReader
   virtual MOZ_MUST_USE nsresult CommitToSegmentSize(uint32_t size,
                                                     bool forceCommitment) override;
   MOZ_MUST_USE nsresult BufferOutput(const char *, uint32_t, uint32_t *);
-  void     MaybeFlushOutputQueue();
+  void     FlushOutputQueue();
   uint32_t AmountOfOutputBuffered() { return mOutputQueueUsed - mOutputQueueSent; }
 
   uint32_t GetServerInitialStreamWindow() { return mServerInitialStreamWindow; }
 
   MOZ_MUST_USE bool TryToActivate(Http2Stream *stream);
   void ConnectPushedStream(Http2Stream *stream);
   void ConnectSlowConsumer(Http2Stream *stream);
 
@@ -558,22 +558,16 @@ private:
     nsHttpRequestHead mRequestHead;
   };
 
   // A h2 session will be created before all socket events are trigered,
   // e.g. NS_NET_STATUS_TLS_HANDSHAKE_ENDED and for TFO many others.
   // We should propagate this events to the first nsHttpTransaction.
   RefPtr<nsHttpTransaction> mFirstHttpTransaction;
   bool mTlsHandshakeFinished;
-
-  void SetSegmentReader(nsAHttpSegmentReader *);
-  void FlushOutputQueue();
-  bool mFlushOKAddStream;
-  bool mFlushOKReadSegments;
-
 private:
 /// connect tunnels
   void DispatchOnTunnel(nsAHttpTransaction *, nsIInterfaceRequestor *);
   void CreateTunnel(nsHttpTransaction *, nsHttpConnectionInfo *, nsIInterfaceRequestor *);
   void RegisterTunnel(Http2Stream *);
   void UnRegisterTunnel(Http2Stream *);
   uint32_t FindTunnelCount(nsHttpConnectionInfo *);
   nsDataHashtable<nsCStringHashKey, uint32_t> mTunnelHash;
--- a/netwerk/protocol/http/Http2Stream.cpp
+++ b/netwerk/protocol/http/Http2Stream.cpp
@@ -953,17 +953,17 @@ Http2Stream::TransmitFrame(const char *b
 
     Http2Session::LogIO(mSession, this, "Writing from Transaction Buffer",
                          buf, transmittedCount);
 
     *countUsed += mTxStreamFrameSize;
   }
 
   if (!mAttempting0RTT) {
-    mSession->MaybeFlushOutputQueue();
+    mSession->FlushOutputQueue();
   }
 
   // calling this will trigger waiting_for if mRequestBodyLenRemaining is 0
   UpdateTransportSendEvents(mTxInlineFrameUsed + mTxStreamFrameSize);
 
   mTxInlineFrameUsed = 0;
   mTxStreamFrameSize = 0;