Retry request upon GOAWAY frame with NO_ERROR.

net/spdy/spdy_session.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/spdy/spdy_session.h"
 
 #include <algorithm>
 #include <limits>
 #include <map>
 #include <utility>
@@ skipping 2677 matching lines @@
 
   net_log_.AddEvent(
       NetLogEventType::HTTP2_SESSION_RECV_GOAWAY,
       base::Bind(&NetLogSpdyRecvGoAwayCallback, last_accepted_stream_id,
                  active_streams_.size(), unclaimed_pushed_streams_.size(),
                  error_code, debug_data));
   MakeUnavailable();
   if (error_code == ERROR_CODE_HTTP_1_1_REQUIRED) {
     // TODO(bnc): Record histogram with number of open streams capped at 50.
     DoDrainSession(ERR_HTTP_1_1_REQUIRED, "HTTP_1_1_REQUIRED for stream.");
+  } else if (error_code == ERROR_CODE_NO_ERROR) {

[Zhongyi Shi, 2017/04/19 00:40:28]

I am not familiar with SPDY code, but according to your CL description: should
we evaluate the Last-Stream-ID vs stream id corresponding to the request before
populating the ERR_SPDY_SERVER_REFUSED_STREAM? Or is that already handled
somewhere else, just not present in this CL?

That also suggests a unit test in spdy_network_transaction_unittest.cc where the
GO_AWAY_FRAME is received with ERROR_CODE_NO_ERROR but inappropriate stream id,
test that won't cause us to retry on a new connection. 

[Bence, 2017/04/19 12:12:15]

Excellent questions.

In StartGoingAway() there is a loop [1] that calls LogAbandonedActiveStream only
for active streams with stream_id exceeding Last-Stream-ID.  Active streams with
lower stream_ids are kept as active, because the server is possibly still
sending data on them.

As for the unittest, this is what SpdyNetworkTransactionTest.GracefulGoaway [2]
does: |goaway| is the GOAWAY frame with a very high Last-Stream-ID value, and
the second request is accepted on the same connection after receiving this
frame.

[1]
https://cs.chromium.org/chromium/src/net/spdy/spdy_session.cc?q=startgoingawa...
[2]
https://cs.chromium.org/chromium/src/net/spdy/spdy_network_transaction_unitte...

[Zhongyi Shi, 2017/04/19 17:19:18]

Ah, I see, thanks for the clarification and code walkthrough!  

+    StartGoingAway(last_accepted_stream_id, ERR_SPDY_SERVER_REFUSED_STREAM);
   } else {
     StartGoingAway(last_accepted_stream_id, ERR_ABORTED);
   }
   // This is to handle the case when we already don't have any active
   // streams (i.e., StartGoingAway() did nothing). Otherwise, we have
   // active streams and so the last one being closed will finish the
   // going away process (see DeleteStream()).
   MaybeFinishGoingAway();
 }
 
@@ skipping 483 matching lines @@
     if (!queue->empty()) {
       SpdyStreamId stream_id = queue->front();
       queue->pop_front();
       return stream_id;
     }
   }
   return 0;
 }
 
 }  // namespace net