Now that the HttpStreamFactoryImpl::Job is not marking Alternate-Protocol

as broken unless the main job succeeds, do not mark Alternate-Protocol
broken in the QuicStreamFactory unless the session was a failed 0-RTT
connection (in which case the Job already "succeeded").

Review URL: https://codereview.chromium.org/293063013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272442 0039d316-1c4b-4281-b951-d872f2087c98

net/http/http_stream_factory_impl_job.cc

@@ -989,20 +989,23 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
     return result;
   }
 
+  if (!ssl_started && result < 0 && original_url_.get()) {
+    job_status_ = STATUS_BROKEN;
+    MaybeMarkAlternateProtocolBroken();
+    return result;
+  }
+
   if (using_quic_) {
-    if (result < 0)
+    if (result < 0) {
+      job_status_ = STATUS_BROKEN;
+      MaybeMarkAlternateProtocolBroken();
       return result;
+    }
     stream_ = quic_request_.ReleaseStream();
     next_state_ = STATE_NONE;
     return OK;
   }
 
-  if (!ssl_started && result < 0 && original_url_.get()) {
-    job_status_ = STATUS_BROKEN;
-    MaybeMarkAlternateProtocolBroken();
-    return result;
-  }
-
   if (result < 0 && !ssl_started)
     return ReconsiderProxyAfterError(result);
   establishing_tunnel_ = false;
@@ -1527,8 +1530,8 @@ void HttpStreamFactoryImpl::Job::MaybeMarkAlternateProtocolBroken() {
   }
 
   if (job_status_ == STATUS_SUCCEEDED && other_job_status_ == STATUS_BROKEN) {
-      HistogramBrokenAlternateProtocolLocation(
-          BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_MAIN);
+    HistogramBrokenAlternateProtocolLocation(
+        BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_MAIN);
     session_->http_server_properties()->SetBrokenAlternateProtocol(
         HostPortPair::FromURL(request_info_.url));
   }

net/quic/quic_network_transaction_unittest.cc

@@ -798,16 +798,15 @@ TEST_P(QuicNetworkTransactionTest, BrokenAlternateProtocolReadError) {
 TEST_P(QuicNetworkTransactionTest, NoBrokenAlternateProtocolIfTcpFails) {
   HttpStreamFactory::EnableNpnSpdy3();  // Enables QUIC too.
 
-  // Alternate-protocol job
+  // Alternate-protocol job will fail when the session attempts to read.
   MockRead quic_reads[] = {
     MockRead(ASYNC, ERR_SOCKET_NOT_CONNECTED),
   };
   StaticSocketDataProvider quic_data(quic_reads, arraysize(quic_reads),
                                      NULL, 0);
-  quic_data.set_connect_data(MockConnect(ASYNC, ERR_SOCKET_NOT_CONNECTED));
   socket_factory_.AddSocketDataProvider(&quic_data);
 
-  // Main job which will succeed even though the alternate job fails.
+  // Main job will also fail.
   MockRead http_reads[] = {
     MockRead(ASYNC, ERR_SOCKET_NOT_CONNECTED),
   };
@@ -866,7 +865,7 @@ TEST_P(QuicNetworkTransactionTest, FailedZeroRttBrokenAlternateProtocol) {
   EXPECT_TRUE(quic_data.at_write_eof());
 }
 
-TEST_P(QuicNetworkTransactionTest, NoBrokenAlternateProtocolOnConnectFailure) {
+TEST_P(QuicNetworkTransactionTest, BrokenAlternateProtocolOnConnectFailure) {
   HttpStreamFactory::EnableNpnSpdy3();  // Enables QUIC too.
 
   // Alternate-protocol job will fail before creating a QUIC session.
@@ -890,7 +889,8 @@ TEST_P(QuicNetworkTransactionTest, NoBrokenAlternateProtocolOnConnectFailure) {
   CreateSession();
   AddQuicAlternateProtocolMapping(MockCryptoClientStream::COLD_START);
   SendRequestAndExpectHttpResponse("hello from http");
-  ExpectQuicAlternateProtocolMapping();
+
+  ExpectBrokenAlternateProtocolMapping();
 }
 
 TEST_P(QuicNetworkTransactionTest, ConnectionCloseDuringConnect) {

net/quic/quic_stream_factory.cc

@@ -582,9 +582,9 @@ void QuicStreamFactory::OnSessionGoingAway(QuicClientSession* session) {
     }
 
     active_sessions_.erase(*it);
-    ProcessGoingAwaySession(session, *it);
+    ProcessGoingAwaySession(session, *it, true);
   }
-  ProcessGoingAwaySession(session, all_sessions_[session]);
+  ProcessGoingAwaySession(session, all_sessions_[session], false);
   if (!aliases.empty()) {
     const IpAliasKey ip_alias_key(session->connection()->peer_address(),
                                   aliases.begin()->is_https());
@@ -816,12 +816,25 @@ void QuicStreamFactory::InitializeCachedStateInCryptoConfig(
 
 void QuicStreamFactory::ProcessGoingAwaySession(
     QuicClientSession* session,
-    const QuicServerId& server_id) {
+    const QuicServerId& server_id,
+    bool session_was_active) {
   if (!http_server_properties_)
     return;
 
   const QuicConnectionStats& stats = session->connection()->GetStats();
-  if (!session->IsCryptoHandshakeConfirmed()) {
+  if (session->IsCryptoHandshakeConfirmed()) {
+    HttpServerProperties::NetworkStats network_stats;
+    network_stats.srtt = base::TimeDelta::FromMicroseconds(stats.srtt_us);
+    network_stats.bandwidth_estimate = stats.estimated_bandwidth;
+    http_server_properties_->SetServerNetworkStats(server_id.host_port_pair(),
+                                                   network_stats);
+    return;
+  }
+
+  UMA_HISTOGRAM_COUNTS("Net.QuicHandshakeNotConfirmedNumPacketsReceived",
+                       stats.packets_received);
+
+  if (session_was_active) {
     // TODO(rch):  In the special case where the session has received no
     // packets from the peer, we should consider blacklisting this
     // differently so that we still race TCP but we don't consider the
@@ -830,16 +843,7 @@ void QuicStreamFactory::ProcessGoingAwaySession(
         BROKEN_ALTERNATE_PROTOCOL_LOCATION_QUIC_STREAM_FACTORY);
     http_server_properties_->SetBrokenAlternateProtocol(
         server_id.host_port_pair());
-    UMA_HISTOGRAM_COUNTS("Net.QuicHandshakeNotConfirmedNumPacketsReceived",
-                         stats.packets_received);
-    return;
   }
-
-  HttpServerProperties::NetworkStats network_stats;
-  network_stats.srtt = base::TimeDelta::FromMicroseconds(stats.srtt_us);
-  network_stats.bandwidth_estimate = stats.estimated_bandwidth;
-  http_server_properties_->SetServerNetworkStats(server_id.host_port_pair(),
-                                                 network_stats);
 }
 
 }  // namespace net

net/quic/quic_stream_factory.h

@@ -222,7 +222,8 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
       const scoped_ptr<QuicServerInfo>& server_info);
 
   void ProcessGoingAwaySession(QuicClientSession* session,
-                               const QuicServerId& server_id);
+                               const QuicServerId& server_id,
+                               bool was_session_active);
 
   bool require_confirmation_;
   HostResolver* host_resolver_;