Bug 1409570 - Ensure that transactions matched with http/2 pushed streams are properly finished. r=bagder
authorNicholas Hurley <hurley@mozilla.com>
Mon, 01 Oct 2018 21:52:57 +0000
changeset 439067 2f9e7263731f258f1d75688ea1e9ee41a352da62
parent 439066 d3aa8681ce9dfedda6db70666ac8e59d09a5e4b1
child 439068 00eb79fc9f8ae1a9ad35de97d3eb5ffe9b1108cc
push id34752
push usercbrindusan@mozilla.com
push dateTue, 02 Oct 2018 03:59:45 +0000
treeherdermozilla-central@08125cd5d357 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbagder
bugs1409570
milestone64.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 1409570 - Ensure that transactions matched with http/2 pushed streams are properly finished. r=bagder There was an earlier fix to this, that fixed part of the issue, but that fix was racy. In the case where the transaction was matched with the pushed stream before the pushed stream received its END_STREAM, and the response headers did not include a content-length, the transaction would never notice that the data was done being sent. When that transaction was necessary for the load event to fire, the page would get stuck in the loading state until the user explicitly cancelled. This new patch ensures that the transaction will notice the EOS by making sure the pushed stream gets inserted into the list of push streams with data in the case described above. (The previous patch, which is still in the tree, is still necessary, but not sufficient, to fix the issue.) Differential Revision: https://phabricator.services.mozilla.com/D7298
netwerk/protocol/http/Http2Session.cpp
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -3372,16 +3372,33 @@ Http2Session::WriteSegmentsAgain(nsAHttp
       bool discardedPadding = (mDownstreamState == DISCARDING_DATA_FRAME_PADDING);
       ResetDownstreamState();
 
       if (streamToCleanup) {
         if (discardedPadding && !(streamToCleanup->StreamID() & 1)) {
           // Pushed streams are special on padding-only final data frames.
           // See bug 1409570 comments 6-8 for details.
           streamToCleanup->SetPushComplete();
+          Http2Stream *pushSink = streamToCleanup->GetConsumerStream();
+          if (pushSink) {
+            bool enqueueSink = true;
+            for (auto iter = mPushesReadyForRead.begin();
+                 iter != mPushesReadyForRead.end();
+                 ++iter) {
+              if (*iter == pushSink) {
+                enqueueSink = false;
+                break;
+              }
+            }
+            if (enqueueSink) {
+              mPushesReadyForRead.Push(pushSink);
+              // No use trying to clean up, it won't do anything, anyway
+              streamToCleanup = nullptr;
+            }
+          }
         }
         CleanupStream(streamToCleanup, NS_OK, CANCEL_ERROR);
       }
     }
     return rv;
   }
 
   if (mDownstreamState != BUFFERING_CONTROL_FRAME) {