Mark QUIC broken when the network blackholes after the handshake

net/quic/chromium/quic_http_stream_test.cc

Index: net/quic/chromium/quic_http_stream_test.cc
diff --git a/net/quic/chromium/quic_http_stream_test.cc b/net/quic/chromium/quic_http_stream_test.cc
index da5f35b3a9e6bb906c3b392148403c3b7da63434..b1aae1a2a727cb3e498ee60b2a553bfa85a90771 100644
--- a/net/quic/chromium/quic_http_stream_test.cc
+++ b/net/quic/chromium/quic_http_stream_test.cc
@@ -103,8 +103,11 @@ class AutoClosingStream : public QuicHttpStream {
  public:
   explicit AutoClosingStream(
       const base::WeakPtr<QuicChromiumClientSession>& session,
-      HttpServerProperties* http_server_properties)
-      : QuicHttpStream(session, http_server_properties) {}
+      HttpServerProperties* http_server_properties,
+      bool mark_quic_broken_when_network_suspected)
+      : QuicHttpStream(session,
+                       http_server_properties,
+                       mark_quic_broken_when_network_suspected) {}
 
   void OnHeadersAvailable(const SpdyHeaderBlock& headers,
                           size_t frame_len) override {
@@ -207,6 +210,7 @@ class QuicHttpStreamTest : public ::testing::TestWithParam<QuicVersion> {
       : use_closing_stream_(false),
         crypto_config_(crypto_test_utils::ProofVerifierForTesting()),
         read_buffer_(new IOBufferWithSize(4096)),
+        mark_quic_broken_when_network_suspected_(false),
         promise_id_(kServerDataStreamId1),
         stream_id_(kClientDataStreamId1),
         connection_id_(2),
@@ -332,16 +336,21 @@ class QuicHttpStreamTest : public ::testing::TestWithParam<QuicVersion> {
     session_->CryptoConnect(callback.callback());
     EXPECT_TRUE(session_->IsCryptoHandshakeConfirmed());
     stream_.reset(use_closing_stream_
-                      ? new AutoClosingStream(session_->GetWeakPtr(),
-                                              &http_server_properties_)
-                      : new QuicHttpStream(session_->GetWeakPtr(),
-                                           &http_server_properties_));
-
-    promised_stream_.reset(use_closing_stream_
-                               ? new AutoClosingStream(session_->GetWeakPtr(),
-                                                       &http_server_properties_)
-                               : new QuicHttpStream(session_->GetWeakPtr(),
-                                                    &http_server_properties_));
+                      ? new AutoClosingStream(
+                            session_->GetWeakPtr(), &http_server_properties_,
+                            mark_quic_broken_when_network_suspected_)
+                      : new QuicHttpStream(
+                            session_->GetWeakPtr(), &http_server_properties_,
+                            mark_quic_broken_when_network_suspected_));
+
+    promised_stream_.reset(
+        use_closing_stream_
+            ? new AutoClosingStream(session_->GetWeakPtr(),
+                                    &http_server_properties_,
+                                    mark_quic_broken_when_network_suspected_)
+            : new QuicHttpStream(session_->GetWeakPtr(),
+                                 &http_server_properties_,
+                                 mark_quic_broken_when_network_suspected_));
 
     push_promise_[":path"] = "/bar";
     push_promise_[":authority"] = "www.example.org";
@@ -584,6 +593,7 @@ class QuicHttpStreamTest : public ::testing::TestWithParam<QuicVersion> {
   string request_data_;
   string response_data_;
   QuicClientPushPromiseIndex push_promise_index_;
+  bool mark_quic_broken_when_network_suspected_;

[Bence, 2017/04/03 19:02:59]

This member is always false, so consider a file-scope const bool
kMarkQuicBrokenWhenNetworkSuspected to highlight this fact.

[Ryan Hamilton, 2017/04/05 19:26:20]

Done.

[Bence, 2017/04/05 23:49:46]

Sorry, I was rather thinking of setting kMarkQuicBrokenWhenNetworkSuspected
false and using it instead of its inverse everywhere. 
kMarkQuicBrokenWhenNetworkSuspected would then mean "whether to mark QUIC
broken" as opposed to "the value that would need to be used if QUIC was to be
marked broken" (as if it was an enum entry).

[Ryan Hamilton, 2017/04/08 00:07:23]

I see. As it happens, I ended up reverting the changes to this file.

 
   // For server push testing
   std::unique_ptr<QuicHttpStream> promised_stream_;
@@ -725,7 +735,8 @@ TEST_P(QuicHttpStreamTest, LoadTimingTwoRequests) {
             stream_->SendRequest(headers_, &response_, callback_.callback()));
 
   // Start a second request.
-  QuicHttpStream stream2(session_->GetWeakPtr(), &http_server_properties_);
+  QuicHttpStream stream2(session_->GetWeakPtr(), &http_server_properties_,
+                         mark_quic_broken_when_network_suspected_);
   TestCompletionCallback callback2;
   EXPECT_EQ(OK,
             stream2.InitializeStream(&request_, DEFAULT_PRIORITY,