Disable 0RTT if server and origin have different hosts.

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 7c24ecdaef4439814ed3cb2a0b246e2774bcabc0..4f4c7f8d69b273063255a903e3774ad3e4dd1605 100644
--- a/net/quic/quic_stream_factory.cc
+++ b/net/quic/quic_stream_factory.cc
@@ -139,7 +139,7 @@ class QuicStreamFactory::Job {
       HostResolver* host_resolver,
       const HostPortPair& host_port_pair,
       bool is_https,
-      bool was_alternate_protocol_recently_broken,
+      bool allow_zero_rtt,
       PrivacyMode privacy_mode,
       bool is_post,
       QuicServerInfo* server_info,
@@ -194,7 +194,9 @@ class QuicStreamFactory::Job {
   SingleRequestHostResolver host_resolver_;
   QuicServerId server_id_;
   bool is_post_;
-  bool was_alternate_protocol_recently_broken_;
+  // True iff server and origin have the same hostname

[Ryan Hamilton, 2015/05/18 14:29:52]

ditto

[Bence, 2015/05/18 15:17:54]

ditto

+  // and alternative service was not recently broken.
+  bool allow_zero_rtt_;

[Ryan Hamilton, 2015/05/18 14:29:52]

I'm not in love with this boolean combining two different ideas. Currently there
are a few reasons that we require confirmation:
* recently broken
* the factory requires it
* the request is a post
And currently, the job receives each of these different conditions and ||s them
together to require confirmation. My inclination would be to continue this
pattern by passing in a new server_and_origin_have_same_host variable.

Alternative, you could do this logic down in QuicStreamRequest::Request and
simply pass in a single require_confirmation boolean.

[Bence, 2015/05/18 15:17:54]

Great idea, following the first suggestion.  Thanks.

   scoped_ptr<QuicServerInfo> server_info_;
   bool started_another_job_;
   const BoundNetLog net_log_;
@@ -211,7 +213,7 @@ QuicStreamFactory::Job::Job(QuicStreamFactory* factory,
                             HostResolver* host_resolver,
                             const HostPortPair& host_port_pair,
                             bool is_https,
-                            bool was_alternate_protocol_recently_broken,
+                            bool allow_zero_rtt,
                             PrivacyMode privacy_mode,
                             bool is_post,
                             QuicServerInfo* server_info,
@@ -221,8 +223,7 @@ QuicStreamFactory::Job::Job(QuicStreamFactory* factory,
       host_resolver_(host_resolver),
       server_id_(host_port_pair, is_https, privacy_mode),
       is_post_(is_post),
-      was_alternate_protocol_recently_broken_(
-          was_alternate_protocol_recently_broken),
+      allow_zero_rtt_(allow_zero_rtt),
       server_info_(server_info),
       started_another_job_(false),
       net_log_(net_log),
@@ -238,10 +239,10 @@ QuicStreamFactory::Job::Job(QuicStreamFactory* factory,
       factory_(factory),
       host_resolver_(host_resolver),  // unused
       server_id_(server_id),
-      is_post_(false),                                 // unused
-      was_alternate_protocol_recently_broken_(false),  // unused
-      started_another_job_(false),                     // unused
-      net_log_(session->net_log()),                    // unused
+      is_post_(false),               // unused
+      allow_zero_rtt_(true),         // unused
+      started_another_job_(false),   // unused
+      net_log_(session->net_log()),  // unused
       session_(session),
       weak_factory_(this) {
 }
@@ -437,8 +438,7 @@ int QuicStreamFactory::Job::DoConnect() {
     return ERR_QUIC_PROTOCOL_ERROR;
   }
   bool require_confirmation =
-      factory_->require_confirmation() || is_post_ ||
-      was_alternate_protocol_recently_broken_;
+      factory_->require_confirmation() || is_post_ || !allow_zero_rtt_;
 
   rv = session_->CryptoConnect(
       require_confirmation,
@@ -485,14 +485,18 @@ QuicStreamRequest::~QuicStreamRequest() {
 int QuicStreamRequest::Request(const HostPortPair& host_port_pair,
                                bool is_https,
                                PrivacyMode privacy_mode,
+                               base::StringPiece origin_host,
                                base::StringPiece method,
                                const BoundNetLog& net_log,
                                const CompletionCallback& callback) {
   DCHECK(!stream_);
   DCHECK(callback_.is_null());
   DCHECK(factory_);
-  int rv = factory_->Create(host_port_pair, is_https, privacy_mode, method,
-                            net_log, this);
+  origin_host_.assign(origin_host.data(), origin_host.size());

[Ryan Hamilton, 2015/05/18 14:29:52]

It looks like this is the only place that origin_host_ is used. If that's true,
I recommend eliminating the member variable, and removing this line.

[Bence, 2015/05/18 15:17:54]

Oops.  Done.

+  bool server_and_origin_have_same_host = host_port_pair.host() == origin_host;
+  int rv =
+      factory_->Create(host_port_pair, is_https, privacy_mode,
+                       server_and_origin_have_same_host, method, net_log, this);
   if (rv == ERR_IO_PENDING) {
     host_port_pair_ = host_port_pair;
     net_log_ = net_log;
@@ -615,9 +619,11 @@ void QuicStreamFactory::set_require_confirmation(bool require_confirmation) {
 int QuicStreamFactory::Create(const HostPortPair& host_port_pair,
                               bool is_https,
                               PrivacyMode privacy_mode,
+                              bool server_and_origin_have_same_host,
                               base::StringPiece method,
                               const BoundNetLog& net_log,
                               QuicStreamRequest* request) {
+  server_and_origin_have_same_host_ = server_and_origin_have_same_host;
   QuicServerId server_id(host_port_pair, is_https, privacy_mode);
   if (HasActiveSession(server_id)) {
     request->set_stream(CreateIfSessionExists(server_id, net_log));
@@ -655,10 +661,10 @@ int QuicStreamFactory::Create(const HostPortPair& host_port_pair,
     }
   }
 
-  scoped_ptr<Job> job(new Job(this, host_resolver_, host_port_pair, is_https,
-                              WasQuicRecentlyBroken(server_id), privacy_mode,
-                              method == "POST" /* is_post */, quic_server_info,
-                              net_log));
+  scoped_ptr<Job> job(new Job(
+      this, host_resolver_, host_port_pair, is_https,
+      server_and_origin_have_same_host && !WasQuicRecentlyBroken(server_id),
+      privacy_mode, method == "POST" /* is_post */, quic_server_info, net_log));
   int rv = job->Run(base::Bind(&QuicStreamFactory::OnJobComplete,
                                base::Unretained(this), job.get()));
   if (rv == ERR_IO_PENDING) {
@@ -677,9 +683,10 @@ int QuicStreamFactory::Create(const HostPortPair& host_port_pair,
 void QuicStreamFactory::CreateAuxilaryJob(const QuicServerId server_id,
                                           bool is_post,
                                           const BoundNetLog& net_log) {
-  Job* aux_job = new Job(this, host_resolver_, server_id.host_port_pair(),
-                         server_id.is_https(), WasQuicRecentlyBroken(server_id),
-                         server_id.privacy_mode(), is_post, nullptr, net_log);
+  Job* aux_job = new Job(
+      this, host_resolver_, server_id.host_port_pair(), server_id.is_https(),
+      server_and_origin_have_same_host_ && !WasQuicRecentlyBroken(server_id),

[Ryan Hamilton, 2015/05/18 14:29:52]

It looks like this method is only called from the job. instead of storing
server_and_origin_have_same_host_ in the factory (which seems like a bug) I
would store this member in the job, and pass it in to this method.

[Bence, 2015/05/18 15:17:55]

Great idea, thanks.  Done.

+      server_id.privacy_mode(), is_post, nullptr, net_log);
   active_jobs_[server_id].insert(aux_job);
   task_runner_->PostTask(FROM_HERE,
                          base::Bind(&QuicStreamFactory::Job::RunAuxilaryJob,