Backed out changeset 6e2c105f5730 (bug 1402014) for marionette, talos, and dom/push/ crashes @ mozilla::net::Http2Session::ReadSegmentsAgain

MozReview-Commit-ID: 4mZXcBAma9d

netwerk/protocol/http/Http2Session.cpp

@@ -119,7 +119,6 @@ Http2Session::Http2Session(nsISocketTransport *aSocketTransport, uint32_t versio
   , mAttemptingEarlyData(attemptingEarlyData)
   , mOriginFrameActivated(false)
   , mTlsHandshakeFinished(false)
-  , mFlushOK(false)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
@@ -385,13 +384,6 @@ Http2Session::AddStream(nsAHttpTransaction *aHttpTransaction,
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
-  MOZ_DIAGNOSTIC_ASSERT(!mFlushOK);
-  mFlushOK = true;
-  auto cleanup = MakeScopeExit([&] () {
-    MOZ_DIAGNOSTIC_ASSERT(mFlushOK);
-    mFlushOK = false;
-  });
-
   // integrity check
   if (mStreamTransactionHash.Get(aHttpTransaction)) {
     LOG3(("   New transaction already present\n"));
@@ -550,22 +542,6 @@ Http2Session::RealignOutputQueue()
   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
-  LOG3(("Http2Session::MaybeFlushOutputQueue mFlushOK = %d", mFlushOK));
-  if (mFlushOK) {
-    FlushOutputQueue();
-  } else {
-    SetWriteCallbacks();
-  }
-}
-
 void
 Http2Session::FlushOutputQueue()
 {
@@ -818,7 +794,7 @@ Http2Session::GeneratePing(bool isAck)
   }
 
   LogIO(this, nullptr, "Generate Ping", packet, kFrameHeaderBytes + 8);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
@@ -832,7 +808,7 @@ Http2Session::GenerateSettingsAck()
   mOutputQueueUsed += kFrameHeaderBytes;
   CreateFrameHeader(packet, 0, FRAME_TYPE_SETTINGS, kFlag_ACK, 0);
   LogIO(this, nullptr, "Generate Settings ACK", packet, kFrameHeaderBytes);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
@@ -845,7 +821,7 @@ Http2Session::GeneratePriority(uint32_t aID, uint8_t aPriorityWeight)
   char *packet = CreatePriorityFrame(aID, 0, aPriorityWeight);
 
   LogIO(this, nullptr, "Generate Priority", packet, kFrameHeaderBytes + 5);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
@@ -872,7 +848,7 @@ Http2Session::GenerateRstStream(uint32_t aStatusCode, uint32_t aID)
   NetworkEndian::writeUint32(packet + kFrameHeaderBytes, aStatusCode);
 
   LogIO(this, nullptr, "Generate Reset", packet, frameSize);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
@@ -895,7 +871,7 @@ Http2Session::GenerateGoAway(uint32_t aStatusCode)
   NetworkEndian::writeUint32(packet + frameSize - 4, aStatusCode);
 
   LogIO(this, nullptr, "Generate GoAway", packet, frameSize);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 // The Hello is comprised of
@@ -1017,7 +993,7 @@ Http2Session::SendHello()
     // Http2Session::OnTransportStatus. Yeah, that's right. YOU!
   }
 
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
@@ -1032,7 +1008,7 @@ Http2Session::SendPriorityFrame(uint32_t streamID,
   char *packet = CreatePriorityFrame(streamID, dependsOn, weight);
 
   LogIO(this, nullptr, "SendPriorityFrame", packet, kFrameHeaderBytes + 5);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 char *
@@ -2769,13 +2745,6 @@ Http2Session::ReadSegmentsAgain(nsAHttpSegmentReader *reader,
   MOZ_ASSERT(!mSegmentReader || !reader || (mSegmentReader == reader),
              "Inconsistent Write Function Callback");
 
-  MOZ_DIAGNOSTIC_ASSERT(!mFlushOK);
-  mFlushOK = true;
-  auto cleanup = MakeScopeExit([&] () {
-    MOZ_DIAGNOSTIC_ASSERT(mFlushOK);
-    mFlushOK = false;
-  });
-
   nsresult rv = ConfirmTLSProfile();
   if (NS_FAILED(rv)) {
     if (mGoAwayReason == INADEQUATE_SECURITY) {
@@ -2786,9 +2755,8 @@ Http2Session::ReadSegmentsAgain(nsAHttpSegmentReader *reader,
     return rv;
   }
 
-  if (reader) {
-    SetSegmentReader(reader);
-  }
+  if (reader)
+    mSegmentReader = reader;
 
   *countRead = 0;
 
@@ -2799,7 +2767,7 @@ Http2Session::ReadSegmentsAgain(nsAHttpSegmentReader *reader,
     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));
@@ -2818,7 +2786,7 @@ Http2Session::ReadSegmentsAgain(nsAHttpSegmentReader *reader,
     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);
@@ -2860,7 +2828,7 @@ Http2Session::ReadSegmentsAgain(nsAHttpSegmentReader *reader,
   // 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
@@ -3664,7 +3632,7 @@ Http2Session::UpdateLocalRwin(Http2Stream *stream, uint32_t bytes)
 
   UpdateLocalStreamWindow(stream, bytes);
   UpdateLocalSessionWindow(bytes);
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 }
 
 void
@@ -3753,7 +3721,7 @@ Http2Session::OnReadSegment(const char *buf,
   // 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
@@ -3795,7 +3763,7 @@ Http2Session::OnReadSegment(const char *buf,
   mOutputQueueUsed += count;
   *countRead = count;
 
-  MaybeFlushOutputQueue();
+  FlushOutputQueue();
 
   return NS_OK;
 }
@@ -3804,7 +3772,7 @@ 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))
@@ -4581,18 +4549,5 @@ Http2Session::TopLevelOuterContentWindowIdChanged(uint64_t 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

netwerk/protocol/http/Http2Session.h

@@ -220,7 +220,7 @@ public:
   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; }
@@ -563,11 +563,6 @@ private:
   // We should propagate this events to the first nsHttpTransaction.
   RefPtr<nsHttpTransaction> mFirstHttpTransaction;
   bool mTlsHandshakeFinished;
-
-  void SetSegmentReader(nsAHttpSegmentReader *);
-  void FlushOutputQueue();
-  bool mFlushOK;
-
 private:
 /// connect tunnels
   void DispatchOnTunnel(nsAHttpTransaction *, nsIInterfaceRequestor *);

netwerk/protocol/http/Http2Stream.cpp

@@ -958,7 +958,7 @@ Http2Stream::TransmitFrame(const char *buf,
   }
 
   if (!mAttempting0RTT) {
-    mSession->MaybeFlushOutputQueue();
+    mSession->FlushOutputQueue();
   }
 
   // calling this will trigger waiting_for if mRequestBodyLenRemaining is 0
@@ -966,6 +966,7 @@ Http2Stream::TransmitFrame(const char *buf,
 
   mTxInlineFrameUsed = 0;
   mTxStreamFrameSize = 0;
+
   return NS_OK;
 }