Add MarkAlternativeServiceRecentlyBroken.

net/quic/quic_stream_factory.cc

Index: net/quic/quic_stream_factory.cc
diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc
index b654ef07bad4b849d8468f58e2f32a7a1cbcea6f..9ef25c552e43efc028bb3e83e808aac7d1a0cff7 100644
--- a/net/quic/quic_stream_factory.cc
+++ b/net/quic/quic_stream_factory.cc
@@ -1336,19 +1336,16 @@ void QuicStreamFactory::ProcessGoingAwaySession(
   DCHECK_EQ(QUIC, alternate.protocol);
 
   // Since the session was active, there's no longer an
-  // HttpStreamFactoryImpl::Job running which can mark it broken, unless the
-  // TCP job also fails. So to avoid not using QUIC when we otherwise could,
-  // we mark it as broken, and then immediately re-enable it. This leaves
-  // QUIC as "recently broken" which means that 0-RTT will be disabled but
-  // we'll still race.
-  http_server_properties_->SetBrokenAlternateProtocol(server);
-  http_server_properties_->ClearAlternateProtocol(server);

[Ryan Hamilton, 2015/03/11 23:02:34]

I didn't mention this explicitly, but you can remove this method now.

[Bence, 2015/03/12 01:12:53]

You mean remove from HttpServerProperties?  We'll need it to reset alternative
services upon receiving a new AltSvc header or frame.  Besides, it's currently
called at
https://code.google.com/p/chromium/codesearch#chromium/src/net/http/http_serv....

[Ryan Hamilton, 2015/03/12 03:31:40]

Hm. That's possible. I'm not sure that I love requiring the caller to first
clear then call set N-times, but perhaps that'll be best.
Sure, but if that's the only caller it could simply be inlined. But we can worry
about it later, as you say.

-  http_server_properties_->SetAlternateProtocol(
-      server, alternate.port, alternate.protocol, 1);
+  // HttpStreamFactoryImpl::Job running which can mark it broken, unless the TCP
+  // job also fails. So to avoid not using QUIC when we otherwise could, we mark
+  // it as recently broken, which means that 0-RTT will be disabled but we'll
+  // still race.
+  const AlternativeService alternative_service(alternate.protocol,
+                                               server.host(), alternate.port);
+  http_server_properties_->MarkAlternativeServiceRecentlyBroken(
+      alternative_service);

[Ryan Hamilton, 2015/03/11 20:28:02]

How 'bout just:

  http_server_properties_->MarkAlternativeServiceRecentlyBroken(
      AlternativeService(alternate.protocol, server.host(), alternate.port));

Actually, might as well just use QUIC instead of alternate.protocol. 

In fact, even better would be:

  http_server_properties_->MarkAlternativeServiceRecentlyBroken(
      AlternativeService(QUIC, server.host(), server.port));

The server's port should match the alternate but still...

I think that would let you simplify the code on 1325 to be:

  if (http_server_properties_->GetAlternateProtocol(server).protocol ==
      UNINITIALIZED_ALTERNATE_PROTOCOL) {
  }

(Or possibly introduce a HasAlternateProtocol()) method.

Oh, and while you're here, that code is actually wrong because it's for an
origin not a server so it can probably be removed altogether.

[Bence, 2015/03/12 01:12:53]

Done.

   DCHECK_EQ(QUIC,
             http_server_properties_->GetAlternateProtocol(server).protocol);
-  DCHECK(http_server_properties_->WasAlternateProtocolRecentlyBroken(
-      server));
 }
 
 }  // namespace net