Correctly handle SPDY GOAWAY frames.

net/spdy/spdy_session.cc

Index: net/spdy/spdy_session.cc
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index ba84ea12acbbe9e41dfea287860e52eda6fdbbef..f0e0053d5e2075050e6c3ad7ed2ec1c6d7646e30 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -1132,19 +1132,8 @@ void SpdySession::WriteSocket() {
   }
 }
 
-void SpdySession::CloseAllStreams(net::Error status) {
-  base::StatsCounter abandoned_streams("spdy.abandoned_streams");
-  base::StatsCounter abandoned_push_streams(
-      "spdy.abandoned_push_streams");
-
-  if (!active_streams_.empty())
-    abandoned_streams.Add(active_streams_.size());
-  if (!unclaimed_pushed_streams_.empty()) {
-    streams_abandoned_count_ += unclaimed_pushed_streams_.size();
-    abandoned_push_streams.Add(unclaimed_pushed_streams_.size());
-    unclaimed_pushed_streams_.clear();
-  }
-
+void SpdySession::CloseAllStreamsAfter(SpdyStreamId last_good_stream_id,
+                                       net::Error status) {
   for (int i = 0; i < NUM_PRIORITIES; ++i) {
     PendingStreamRequestQueue queue;
     queue.swap(pending_create_stream_queues_[i]);
@@ -1154,11 +1143,17 @@ void SpdySession::CloseAllStreams(net::Error status) {
     }
   }
 
-  while (!active_streams_.empty()) {
-    ActiveStreamMap::iterator it = active_streams_.begin();
+  ActiveStreamMap::iterator it =
+      active_streams_.lower_bound(last_good_stream_id + 1);
+  while (it != active_streams_.end()) {
     const scoped_refptr<SpdyStream>& stream = it->second;
+    ++it;
+    if (stream->stream_id() <= last_good_stream_id) {

[akalin, 2013/04/18 01:29:59]

you don't need this check, right? Since ActiveStreamMap is ordered.

[Ryan Hamilton, 2013/04/18 17:18:29]

Hah!  Yes :>  Done.

+      continue;
+    }
     LogAbandonedStream(stream, status);
     DeleteStream(stream->stream_id(), status);
+    streams_abandoned_count_++;

[akalin, 2013/04/18 01:29:59]

hunh. we weren't incrementing this correctly? i see.

[Ryan Hamilton, 2013/04/18 17:18:29]

Before I got started?  I think we were doing something like:

abandoned_streams.Add(active_streams_.size());

At least that was the state when I started hacking.  Looks like the name of that
var changed in a recent CL.

   }
 
   while (!created_streams_.empty()) {
@@ -1169,7 +1164,21 @@ void SpdySession::CloseAllStreams(net::Error status) {
     stream->OnClose(status);
   }
 
-  write_queue_.Clear();
+  write_queue_.RemovePendingWritesForStreamsAfter(last_good_stream_id);
+}
+
+void SpdySession::CloseAllStreams(net::Error status) {

[akalin, 2013/04/18 01:29:59]

please put this above to match order in header (or change the header order)

[Ryan Hamilton, 2013/04/18 17:18:29]

Done.

+  base::StatsCounter abandoned_streams("spdy.abandoned_streams");
+  base::StatsCounter abandoned_push_streams(
+      "spdy.abandoned_push_streams");
+
+  if (!unclaimed_pushed_streams_.empty()) {
+    streams_abandoned_count_ += unclaimed_pushed_streams_.size();
+    abandoned_push_streams.Add(unclaimed_pushed_streams_.size());
+    unclaimed_pushed_streams_.clear();
+  }
+
+  CloseAllStreamsAfter(0, status);
 }
 
 void SpdySession::LogAbandonedStream(const scoped_refptr<SpdyStream>& stream,
@@ -1743,14 +1752,7 @@ void SpdySession::OnGoAway(SpdyStreamId last_accepted_stream_id,
                  unclaimed_pushed_streams_.size(),
                  status));
   RemoveFromPool();
-  CloseAllStreams(net::ERR_ABORTED);
-
-  // TODO(willchan): Cancel any streams that are past the GoAway frame's
-  // |last_accepted_stream_id|.
-
-  // Don't bother killing any streams that are still reading.  They'll either
-  // complete successfully or get an ERR_CONNECTION_CLOSED when the socket is
-  // closed.
+  CloseAllStreamsAfter(last_accepted_stream_id, net::ERR_ABORTED);
 }
 
 void SpdySession::OnPing(uint32 unique_id) {