QUIC - Race two connections. One connection which loads data from disk

cache sends a CHLO and another connection that doesn't wait to load
server config from disk cache and sends INCHOATE_HELLO.

This is not enabled. QuicStreamFactory tests with this flag enabled
and disabled. Tested chrome and all unit tests with this flag enabled.

R=rch@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#315117}

net/quic/quic_crypto_client_stream.cc

@@ -196,7 +196,7 @@ void QuicCryptoClientStream::DoHandshakeLoop(
         break;
       case STATE_SEND_CHLO:
         DoSendCHLO(in, cached);
-        return;
+        return;  // return waiting to hear from server.
       case STATE_RECV_REJ:
         DoReceiveREJ(in, cached);
         break;
@@ -638,7 +638,7 @@ bool QuicCryptoClientStream::RequiresChannelID(
     return false;
   }
   const CryptoHandshakeMessage* scfg = cached->GetServerConfig();
-  if (!scfg) {  // scfg may be null when we send an inchoate CHLO.
+  if (!scfg) {  // scfg may be null then we send an inchoate CHLO.
     return false;
   }
   const QuicTag* their_proof_demands;

net/quic/quic_protocol.h

@@ -513,6 +513,8 @@ enum QuicErrorCode {
   QUIC_TOO_MANY_OUTSTANDING_SENT_PACKETS = 68,
   // The connection has too many outstanding received packets.
   QUIC_TOO_MANY_OUTSTANDING_RECEIVED_PACKETS = 69,
+  // The quic connection job to load server config is cancelled.
+  QUIC_CONNECTION_CANCELLED = 70,
 
   // Crypto errors.
 
@@ -570,7 +572,7 @@ enum QuicErrorCode {
   QUIC_VERSION_NEGOTIATION_MISMATCH = 55,
 
   // No error. Used as bound while iterating.
-  QUIC_LAST_ERROR = 70,
+  QUIC_LAST_ERROR = 71,
 };
 
 struct NET_EXPORT_PRIVATE QuicPacketPublicHeader {

net/quic/quic_stream_factory.cc

@@ -141,7 +141,7 @@ class QuicStreamFactory::Job {
       bool is_https,
       bool was_alternate_protocol_recently_broken,
       PrivacyMode privacy_mode,
-      base::StringPiece method,
+      bool is_post,
       QuicServerInfo* server_info,
       const BoundNetLog& net_log);
 
@@ -167,15 +167,15 @@ class QuicStreamFactory::Job {
 
   void OnIOComplete(int rv);
 
+  void RunAuxilaryJob();
+
+  void Cancel();
+
   void CancelWaitForDataReadyCallback();
 
-  CompletionCallback callback() {
-    return callback_;
-  }
+  const QuicServerId server_id() const { return server_id_; }
 
-  const QuicServerId server_id() const {
-    return server_id_;
-  }
+  base::WeakPtr<Job> GetWeakPtr() { return weak_factory_.GetWeakPtr(); }
 
  private:
   enum IoState {
@@ -196,6 +196,7 @@ class QuicStreamFactory::Job {
   bool is_post_;
   bool was_alternate_protocol_recently_broken_;
   scoped_ptr<QuicServerInfo> server_info_;
+  bool started_another_job_;
   const BoundNetLog net_log_;
   QuicClientSession* session_;
   CompletionCallback callback_;
@@ -212,20 +213,22 @@ QuicStreamFactory::Job::Job(QuicStreamFactory* factory,
                             bool is_https,
                             bool was_alternate_protocol_recently_broken,
                             PrivacyMode privacy_mode,
-                            base::StringPiece method,
+                            bool is_post,
                             QuicServerInfo* server_info,
                             const BoundNetLog& net_log)
     : io_state_(STATE_RESOLVE_HOST),
       factory_(factory),
       host_resolver_(host_resolver),
       server_id_(host_port_pair, is_https, privacy_mode),
-      is_post_(method == "POST"),
+      is_post_(is_post),
       was_alternate_protocol_recently_broken_(
           was_alternate_protocol_recently_broken),
       server_info_(server_info),
+      started_another_job_(false),
       net_log_(net_log),
       session_(nullptr),
-      weak_factory_(this) {}
+      weak_factory_(this) {
+}
 
 QuicStreamFactory::Job::Job(QuicStreamFactory* factory,
                             HostResolver* host_resolver,
@@ -235,13 +238,18 @@ QuicStreamFactory::Job::Job(QuicStreamFactory* factory,
       factory_(factory),
       host_resolver_(host_resolver),  // unused
       server_id_(server_id),
-      is_post_(false),  // unused
+      is_post_(false),                                 // unused
       was_alternate_protocol_recently_broken_(false),  // unused
-      net_log_(session->net_log()),  // unused
+      started_another_job_(false),                     // unused
+      net_log_(session->net_log()),                    // unused
       session_(session),
-      weak_factory_(this) {}
+      weak_factory_(this) {
+}
 
 QuicStreamFactory::Job::~Job() {
+  // If disk cache has a pending WaitForDataReadyCallback, cancel that callback.
+  if (server_info_)
+    server_info_->CancelWaitForDataReadyCallback();
 }
 
 int QuicStreamFactory::Job::Run(const CompletionCallback& callback) {
@@ -307,6 +315,19 @@ void QuicStreamFactory::Job::OnIOComplete(int rv) {
   }
 }
 
+void QuicStreamFactory::Job::RunAuxilaryJob() {
+  int rv = Run(base::Bind(&QuicStreamFactory::OnJobComplete,
+                          base::Unretained(factory_), this));
+  if (rv != ERR_IO_PENDING)
+    factory_->OnJobComplete(this, rv);
+}
+
+void QuicStreamFactory::Job::Cancel() {
+  callback_.Reset();
+  if (session_)
+    session_->connection()->SendConnectionClose(QUIC_CONNECTION_CANCELLED);
+}
+
 void QuicStreamFactory::Job::CancelWaitForDataReadyCallback() {
   // If we are waiting for WaitForDataReadyCallback, then cancel the callback.
   if (io_state_ != STATE_LOAD_SERVER_INFO_COMPLETE)
@@ -329,11 +350,9 @@ int QuicStreamFactory::Job::DoResolveHost() {
   io_state_ = STATE_RESOLVE_HOST_COMPLETE;
   dns_resolution_start_time_ = base::TimeTicks::Now();
   return host_resolver_.Resolve(
-      HostResolver::RequestInfo(server_id_.host_port_pair()),
-      DEFAULT_PRIORITY,
+      HostResolver::RequestInfo(server_id_.host_port_pair()), DEFAULT_PRIORITY,
       &address_list_,
-      base::Bind(&QuicStreamFactory::Job::OnIOComplete,
-                 weak_factory_.GetWeakPtr()),
+      base::Bind(&QuicStreamFactory::Job::OnIOComplete, GetWeakPtr()),
       net_log_);
 }
 
@@ -356,7 +375,10 @@ int QuicStreamFactory::Job::DoResolveHostComplete(int rv) {
     return OK;
   }
 
-  io_state_ = STATE_LOAD_SERVER_INFO;
+  if (server_info_)
+    io_state_ = STATE_LOAD_SERVER_INFO;
+  else
+    io_state_ = STATE_CONNECT;
   return OK;
 }
 
@@ -368,8 +390,7 @@ int QuicStreamFactory::Job::DoLoadServerInfo() {
 
   io_state_ = STATE_LOAD_SERVER_INFO_COMPLETE;
 
-  if (!server_info_)
-    return OK;
+  DCHECK(server_info_);
 
   // To mitigate the effects of disk cache taking too long to load QUIC server
   // information, set up a timer to cancel WaitForDataReady's callback.
@@ -385,14 +406,20 @@ int QuicStreamFactory::Job::DoLoadServerInfo() {
     factory_->task_runner_->PostDelayedTask(
         FROM_HERE,
         base::Bind(&QuicStreamFactory::Job::CancelWaitForDataReadyCallback,
-                   weak_factory_.GetWeakPtr()),
+                   GetWeakPtr()),
         base::TimeDelta::FromMilliseconds(load_server_info_timeout_ms));
   }
 
   disk_cache_load_start_time_ = base::TimeTicks::Now();
-  return server_info_->WaitForDataReady(
-      base::Bind(&QuicStreamFactory::Job::OnIOComplete,
-                 weak_factory_.GetWeakPtr()));
+  int rv = server_info_->WaitForDataReady(
+      base::Bind(&QuicStreamFactory::Job::OnIOComplete, GetWeakPtr()));
+  if (rv == ERR_IO_PENDING && factory_->enable_connection_racing()) {
+    // If we are waiting to load server config from the disk cache, then start
+    // another job.
+    started_another_job_ = true;
+    factory_->CreateAuxilaryJob(server_id_, is_post_, net_log_);
+  }
+  return rv;
 }
 
 int QuicStreamFactory::Job::DoLoadServerInfoComplete(int rv) {
@@ -401,13 +428,20 @@ int QuicStreamFactory::Job::DoLoadServerInfoComplete(int rv) {
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::Job::DoLoadServerInfoComplete"));
 
-  if (server_info_) {
-    UMA_HISTOGRAM_TIMES("Net.QuicServerInfo.DiskCacheWaitForDataReadyTime",
-                        base::TimeTicks::Now() - disk_cache_load_start_time_);
-  }
+  UMA_HISTOGRAM_TIMES("Net.QuicServerInfo.DiskCacheWaitForDataReadyTime",
+                      base::TimeTicks::Now() - disk_cache_load_start_time_);
 
-  if (rv != OK) {
+  if (rv != OK)
     server_info_.reset();
+
+  if (started_another_job_ &&
+      (!server_info_ || server_info_->state().server_config.empty() ||
+       !factory_->CryptoConfigCacheIsEmpty(server_id_))) {
+    // If we have started another job and if we didn't load the server config
+    // from the disk cache or if we have received a new server config from the
+    // server, then cancel the current job.
+    io_state_ = STATE_NONE;
+    return ERR_CONNECTION_CLOSED;
   }
 
   io_state_ = STATE_CONNECT;
@@ -454,8 +488,7 @@ int QuicStreamFactory::Job::DoConnect() {
 
   rv = session_->CryptoConnect(
       require_confirmation,
-      base::Bind(&QuicStreamFactory::Job::OnIOComplete,
-                 base::Unretained(this)));
+      base::Bind(&QuicStreamFactory::Job::OnIOComplete, GetWeakPtr()));
   return rv;
 }
 
@@ -468,8 +501,7 @@ int QuicStreamFactory::Job::DoResumeConnect() {
   io_state_ = STATE_CONNECT_COMPLETE;
 
   int rv = session_->ResumeCryptoConnect(
-      base::Bind(&QuicStreamFactory::Job::OnIOComplete,
-                 base::Unretained(this)));
+      base::Bind(&QuicStreamFactory::Job::OnIOComplete, GetWeakPtr()));
 
   return rv;
 }
@@ -585,6 +617,7 @@ QuicStreamFactory::QuicStreamFactory(
       load_server_info_timeout_srtt_multiplier_(
           load_server_info_timeout_srtt_multiplier),
       enable_truncated_connection_ids_(enable_truncated_connection_ids),
+      enable_connection_racing_(false),
       port_seed_(random_generator_->RandUint64()),
       check_persisted_supports_quic_(true),
       task_runner_(nullptr),
@@ -610,7 +643,11 @@ QuicStreamFactory::~QuicStreamFactory() {
     delete all_sessions_.begin()->first;
     all_sessions_.erase(all_sessions_.begin());
   }
-  STLDeleteValues(&active_jobs_);
+  while (!active_jobs_.empty()) {
+    const QuicServerId server_id = active_jobs_.begin()->first;
+    STLDeleteElements(&(active_jobs_[server_id]));
+    active_jobs_.erase(server_id);
+  }
 }
 
 void QuicStreamFactory::set_require_confirmation(bool require_confirmation) {
@@ -634,12 +671,16 @@ int QuicStreamFactory::Create(const HostPortPair& host_port_pair,
   }
 
   if (HasActiveJob(server_id)) {
-    Job* job = active_jobs_[server_id];
-    active_requests_[request] = job;
-    job_requests_map_[job].insert(request);
+    active_requests_[request] = server_id;
+    job_requests_map_[server_id].insert(request);
     return ERR_IO_PENDING;
   }
 
+  // TODO(rtenneti): |task_runner_| is used by the Job. Initialize task_runner_
+  // in the constructor after WebRequestActionWithThreadsTest.* tests are fixed.
+  if (!task_runner_)
+    task_runner_ = base::MessageLoop::current()->message_loop_proxy().get();
+
   QuicServerInfo* quic_server_info = nullptr;
   if (quic_server_info_factory_) {
     bool load_from_disk_cache = true;
@@ -654,34 +695,22 @@ int QuicStreamFactory::Create(const HostPortPair& host_port_pair,
         load_from_disk_cache = false;
       }
     }
-    if (load_from_disk_cache) {
-      QuicCryptoClientConfig::CachedState* cached =
-          crypto_config_.LookupOrCreate(server_id);
-      DCHECK(cached);
-      if (cached->IsEmpty()) {
-        quic_server_info = quic_server_info_factory_->GetForServer(server_id);
-      }
+    if (load_from_disk_cache && CryptoConfigCacheIsEmpty(server_id)) {
+      quic_server_info = quic_server_info_factory_->GetForServer(server_id);
     }
   }
-  // TODO(rtenneti): Initialize task_runner_ in the constructor after
-  // WebRequestActionWithThreadsTest.* tests are fixed.
-  if (!task_runner_)
-    task_runner_ = base::MessageLoop::current()->message_loop_proxy().get();
 
-  bool was_alternate_protocol_recently_broken =
-      http_server_properties_ &&
-      http_server_properties_->WasAlternateProtocolRecentlyBroken(
-          server_id.host_port_pair());
   scoped_ptr<Job> job(new Job(this, host_resolver_, host_port_pair, is_https,
-                              was_alternate_protocol_recently_broken,
-                              privacy_mode, method, quic_server_info, net_log));
+                              WasAlternateProtocolRecentlyBroken(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) {
-    active_requests_[request] = job.get();
-    job_requests_map_[job.get()].insert(request);
-    active_jobs_[server_id] = job.release();
+    active_requests_[request] = server_id;
+    job_requests_map_[server_id].insert(request);
+    active_jobs_[server_id].insert(job.release());
+    return rv;
   }
   if (rv == OK) {
     DCHECK(HasActiveSession(server_id));
@@ -690,6 +719,19 @@ int QuicStreamFactory::Create(const HostPortPair& host_port_pair,
   return rv;
 }
 
+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(),
+                         WasAlternateProtocolRecentlyBroken(server_id),
+                         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,
+                                    aux_job->GetWeakPtr()));
+}
+
 bool QuicStreamFactory::OnResolution(
     const QuicServerId& server_id,
     const AddressList& address_list) {
@@ -715,6 +757,19 @@ bool QuicStreamFactory::OnResolution(
 }
 
 void QuicStreamFactory::OnJobComplete(Job* job, int rv) {
+  QuicServerId server_id = job->server_id();
+  if (rv != OK) {
+    JobSet* jobs = &(active_jobs_[server_id]);
+    if (jobs->size() > 1) {
+      // If there is another pending job, then we can delete this job and let
+      // the other job handle the request.
+      job->Cancel();
+      jobs->erase(job);
+      delete job;
+      return;
+    }
+  }
+
   if (rv == OK) {
     // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
     tracked_objects::ScopedTracker tracking_profile1(
@@ -725,11 +780,9 @@ void QuicStreamFactory::OnJobComplete(Job* job, int rv) {
       set_require_confirmation(false);
 
     // Create all the streams, but do not notify them yet.
-    for (RequestSet::iterator it = job_requests_map_[job].begin();
-         it != job_requests_map_[job].end() ; ++it) {
-      DCHECK(HasActiveSession(job->server_id()));
-      (*it)->set_stream(CreateIfSessionExists(job->server_id(),
-                                              (*it)->net_log()));
+    for (QuicStreamRequest* request : job_requests_map_[server_id]) {
+      DCHECK(HasActiveSession(server_id));
+      request->set_stream(CreateIfSessionExists(server_id, request->net_log()));
     }
   }
 
@@ -738,10 +791,10 @@ void QuicStreamFactory::OnJobComplete(Job* job, int rv) {
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::OnJobComplete2"));
 
-  while (!job_requests_map_[job].empty()) {
-    RequestSet::iterator it = job_requests_map_[job].begin();
+  while (!job_requests_map_[server_id].empty()) {
+    RequestSet::iterator it = job_requests_map_[server_id].begin();
     QuicStreamRequest* request = *it;
-    job_requests_map_[job].erase(it);
+    job_requests_map_[server_id].erase(it);
     active_requests_.erase(request);
     // Even though we're invoking callbacks here, we don't need to worry
     // about |this| being deleted, because the factory is owned by the
@@ -754,10 +807,14 @@ void QuicStreamFactory::OnJobComplete(Job* job, int rv) {
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::OnJobComplete3"));
 
-  active_jobs_.erase(job->server_id());
-  job_requests_map_.erase(job);
-  delete job;
-  return;
+  for (Job* other_job : active_jobs_[server_id]) {
+    if (other_job != job)
+      other_job->Cancel();
+  }
+
+  STLDeleteElements(&(active_jobs_[server_id]));
+  active_jobs_.erase(server_id);
+  job_requests_map_.erase(server_id);
 }
 
 // Returns a newly created QuicHttpStream owned by the caller, if a
@@ -836,7 +893,7 @@ void QuicStreamFactory::OnSessionConnectTimeout(
   QuicServerId server_id = *aliases.begin();
   session_aliases_.erase(session);
   Job* job = new Job(this, host_resolver_, session, server_id);
-  active_jobs_[server_id] = job;
+  active_jobs_[server_id].insert(job);
   int rv = job->Run(base::Bind(&QuicStreamFactory::OnJobComplete,
                                base::Unretained(this), job));
   DCHECK_EQ(ERR_IO_PENDING, rv);
@@ -844,8 +901,8 @@ void QuicStreamFactory::OnSessionConnectTimeout(
 
 void QuicStreamFactory::CancelRequest(QuicStreamRequest* request) {
   DCHECK(ContainsKey(active_requests_, request));
-  Job* job = active_requests_[request];
-  job_requests_map_[job].erase(request);
+  QuicServerId server_id = active_requests_[request];
+  job_requests_map_[server_id].erase(request);
   active_requests_.erase(request);
 }
 
@@ -915,6 +972,10 @@ bool QuicStreamFactory::HasActiveSession(
   return ContainsKey(active_sessions_, server_id);
 }
 
+bool QuicStreamFactory::HasActiveJob(const QuicServerId& key) const {
+  return ContainsKey(active_jobs_, key);
+}
+
 int QuicStreamFactory::CreateSession(
     const QuicServerId& server_id,
     scoped_ptr<QuicServerInfo> server_info,
@@ -1129,10 +1190,6 @@ int QuicStreamFactory::CreateSession(
   return OK;
 }
 
-bool QuicStreamFactory::HasActiveJob(const QuicServerId& key) const {
-  return ContainsKey(active_jobs_, key);
-}
-
 void QuicStreamFactory::ActivateSession(
     const QuicServerId& server_id,
     QuicClientSession* session) {
@@ -1158,6 +1215,20 @@ int64 QuicStreamFactory::GetServerNetworkStatsSmoothedRttInMicroseconds(
   return stats->srtt.InMicroseconds();
 }
 
+bool QuicStreamFactory::WasAlternateProtocolRecentlyBroken(
+    const QuicServerId& server_id) const {
+  return http_server_properties_ &&
+         http_server_properties_->WasAlternateProtocolRecentlyBroken(
+             server_id.host_port_pair());
+}
+
+bool QuicStreamFactory::CryptoConfigCacheIsEmpty(
+    const QuicServerId& server_id) {
+  QuicCryptoClientConfig::CachedState* cached =
+      crypto_config_.LookupOrCreate(server_id);
+  return cached->IsEmpty();
+}
+
 void QuicStreamFactory::InitializeCachedStateInCryptoConfig(
     const QuicServerId& server_id,
     const scoped_ptr<QuicServerInfo>& server_info) {

net/quic/quic_stream_factory.h

@@ -178,6 +178,11 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
     quic_server_info_factory_ = quic_server_info_factory;
   }
 
+  bool enable_connection_racing() const { return enable_connection_racing_; }
+  void set_enable_connection_racing(bool enable_connection_racing) {
+    enable_connection_racing_ = enable_connection_racing;
+  }
+
  private:
   class Job;
   friend class test::QuicStreamFactoryPeer;
@@ -204,10 +209,17 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   typedef std::set<QuicClientSession*> SessionSet;
   typedef std::map<IpAliasKey, SessionSet> IPAliasMap;
   typedef std::map<QuicServerId, QuicCryptoClientConfig*> CryptoConfigMap;
-  typedef std::map<QuicServerId, Job*> JobMap;
-  typedef std::map<QuicStreamRequest*, Job*> RequestMap;
+  typedef std::set<Job*> JobSet;
+  typedef std::map<QuicServerId, JobSet> JobMap;
+  typedef std::map<QuicStreamRequest*, QuicServerId> RequestMap;
   typedef std::set<QuicStreamRequest*> RequestSet;
-  typedef std::map<Job*, RequestSet> JobRequestsMap;
+  typedef std::map<QuicServerId, RequestSet> ServerIDRequestsMap;
+
+  // Creates a job which doesn't wait for server config to be loaded from the
+  // disk cache. This job is started via a PostTask.
+  void CreateAuxilaryJob(const QuicServerId server_id,
+                         bool is_post,
+                         const BoundNetLog& net_log);
 
   // Returns a newly created QuicHttpStream owned by the caller, if a
   // matching session already exists.  Returns NULL otherwise.
@@ -233,6 +245,10 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   int64 GetServerNetworkStatsSmoothedRttInMicroseconds(
       const QuicServerId& server_id) const;
 
+  // Helped methods.
+  bool WasAlternateProtocolRecentlyBroken(const QuicServerId& server_id) const;
+  bool CryptoConfigCacheIsEmpty(const QuicServerId& server_id);
+
   // Initializes the cached state associated with |server_id| in
   // |crypto_config_| with the information in |server_info|.
   void InitializeCachedStateInCryptoConfig(
@@ -274,7 +290,7 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   QuicCryptoClientConfig crypto_config_;
 
   JobMap active_jobs_;
-  JobRequestsMap job_requests_map_;
+  ServerIDRequestsMap job_requests_map_;
   RequestMap active_requests_;
 
   QuicVersionVector supported_versions_;
@@ -305,6 +321,11 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   // Set this for setting config's BytesForConnectionIdToSend (TCID param) to 0.
   bool enable_truncated_connection_ids_;
 
+  // Set if we want to race connections - one connection that sends
+  // INCHOATE_HELLO and another connection that sends CHLO after loading server
+  // config from the disk cache.
+  bool enable_connection_racing_;
+
   // Each profile will (probably) have a unique port_seed_ value.  This value is
   // used to help seed a pseudo-random number generator (PortSuggester) so that
   // we consistently (within this profile) suggest the same ephemeral port when

net/quic/quic_stream_factory_test.cc

@@ -34,6 +34,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::StringPiece;
+using std::ostream;
 using std::string;
 using std::vector;
 
@@ -43,6 +44,34 @@ namespace test {
 namespace {
 const char kDefaultServerHostName[] = "www.google.com";
 const int kDefaultServerPort = 443;
+
+// Run all tests with all the combinations of versions and
+// enable_connection_racing.
+struct TestParams {
+  TestParams(const QuicVersion version, bool enable_connection_racing)
+      : version(version), enable_connection_racing(enable_connection_racing) {}
+
+  friend ostream& operator<<(ostream& os, const TestParams& p) {
+    os << "{ version: " << QuicVersionToString(p.version);
+    os << " enable_connection_racing: " << p.enable_connection_racing << " }";
+    return os;
+  }
+
+  QuicVersion version;
+  bool enable_connection_racing;
+};
+
+// Constructs various test permutations.
+vector<TestParams> GetTestParams() {
+  vector<TestParams> params;
+  QuicVersionVector all_supported_versions = QuicSupportedVersions();
+  for (const QuicVersion version : all_supported_versions) {
+    params.push_back(TestParams(version, false));
+    params.push_back(TestParams(version, true));
+  }
+  return params;
+}
+
 }  // namespace anonymous
 
 class QuicStreamFactoryPeer {
@@ -100,6 +129,16 @@ class QuicStreamFactoryPeer {
                                        size_t load_server_info_timeout) {
     factory->load_server_info_timeout_ms_ = load_server_info_timeout;
   }
+
+  static void SetEnableConnectionRacing(QuicStreamFactory* factory,
+                                        bool enable_connection_racing) {
+    factory->enable_connection_racing_ = enable_connection_racing;
+  }
+
+  static size_t GetNumberOfActiveJobs(QuicStreamFactory* factory,
+                                      const QuicServerId& server_id) {
+    return (factory->active_jobs_[server_id]).size();
+  }
 };
 
 class MockQuicServerInfo : public QuicServerInfo {
@@ -135,14 +174,13 @@ class MockQuicServerInfoFactory : public QuicServerInfoFactory {
   }
 };
 
-
-class QuicStreamFactoryTest : public ::testing::TestWithParam<QuicVersion> {
+class QuicStreamFactoryTest : public ::testing::TestWithParam<TestParams> {
  protected:
   QuicStreamFactoryTest()
       : random_generator_(0),
         clock_(new MockClock()),
         runner_(new TestTaskRunner(clock_)),
-        maker_(GetParam(), 0, clock_),
+        maker_(GetParam().version, 0, clock_),
         cert_verifier_(CertVerifier::CreateDefault()),
         channel_id_service_(
             new ChannelIDService(new DefaultChannelIDStore(nullptr),
@@ -158,7 +196,7 @@ class QuicStreamFactoryTest : public ::testing::TestWithParam<QuicVersion> {
                  clock_,
                  kDefaultMaxPacketSize,
                  std::string(),
-                 SupportedVersions(GetParam()),
+                 SupportedVersions(GetParam().version),
                  /*enable_port_selection=*/true,
                  /*always_require_handshake_confirmation=*/false,
                  /*disable_connection_pooling=*/false,
@@ -171,6 +209,8 @@ class QuicStreamFactoryTest : public ::testing::TestWithParam<QuicVersion> {
         privacy_mode_(PRIVACY_MODE_DISABLED) {
     factory_.set_require_confirmation(false);
     clock_->AdvanceTime(QuicTime::Delta::FromSeconds(1));
+    QuicStreamFactoryPeer::SetEnableConnectionRacing(
+        &factory_, GetParam().enable_connection_racing);
   }
 
   scoped_ptr<QuicHttpStream> CreateIfSessionExists(
@@ -244,7 +284,7 @@ class QuicStreamFactoryTest : public ::testing::TestWithParam<QuicVersion> {
     QuicStreamId stream_id = kClientDataStreamId1;
     return maker_.MakeRstPacket(
         1, true, stream_id,
-        AdjustErrorForVersion(QUIC_RST_ACKNOWLEDGEMENT, GetParam()));
+        AdjustErrorForVersion(QUIC_RST_ACKNOWLEDGEMENT, GetParam().version));
   }
 
   MockQuicServerInfoFactory quic_server_info_factory_;
@@ -266,8 +306,9 @@ class QuicStreamFactoryTest : public ::testing::TestWithParam<QuicVersion> {
   TestCompletionCallback callback_;
 };
 
-INSTANTIATE_TEST_CASE_P(Version, QuicStreamFactoryTest,
-                        ::testing::ValuesIn(QuicSupportedVersions()));
+INSTANTIATE_TEST_CASE_P(Version,
+                        QuicStreamFactoryTest,
+                        ::testing::ValuesIn(GetTestParams()));
 
 TEST_P(QuicStreamFactoryTest, CreateIfSessionExists) {
   EXPECT_EQ(nullptr, CreateIfSessionExists(host_port_pair_, net_log_).get());
@@ -1562,6 +1603,10 @@ TEST_P(QuicStreamFactoryTest, CryptoConfigWhenProofIsInvalid) {
 }
 
 TEST_P(QuicStreamFactoryTest, CancelWaitForDataReady) {
+  // Don't race quic connections when testing cancel reading of server config
+  // from disk cache.
+  if (GetParam().enable_connection_racing)
+    return;
   factory_.set_quic_server_info_factory(&quic_server_info_factory_);
   QuicStreamFactoryPeer::SetTaskRunner(&factory_, runner_.get());
   const size_t kLoadServerInfoTimeoutMs = 50;
@@ -1604,5 +1649,52 @@ TEST_P(QuicStreamFactoryTest, CancelWaitForDataReady) {
   EXPECT_TRUE(socket_data.at_write_eof());
 }
 
+TEST_P(QuicStreamFactoryTest, RacingConnections) {
+  if (!GetParam().enable_connection_racing)
+    return;
+  factory_.set_quic_server_info_factory(&quic_server_info_factory_);
+  QuicStreamFactoryPeer::SetTaskRunner(&factory_, runner_.get());
+  const size_t kLoadServerInfoTimeoutMs = 50;
+  QuicStreamFactoryPeer::SetLoadServerInfoTimeout(&factory_,
+                                                  kLoadServerInfoTimeoutMs);
+
+  MockRead reads[] = {
+      MockRead(ASYNC, OK, 0)  // EOF
+  };
+  DeterministicSocketData socket_data(reads, arraysize(reads), nullptr, 0);
+  socket_factory_.AddSocketDataProvider(&socket_data);
+  socket_data.StopAfter(1);
+
+  MockRead reads2[] = {
+      MockRead(ASYNC, 0, 0)  // EOF
+  };
+  DeterministicSocketData socket_data2(reads2, arraysize(reads2), nullptr, 0);
+  socket_factory_.AddSocketDataProvider(&socket_data2);
+  socket_data2.StopAfter(1);
+
+  crypto_client_stream_factory_.set_handshake_mode(
+      MockCryptoClientStream::ZERO_RTT);
+  host_resolver_.set_synchronous_mode(true);
+  host_resolver_.rules()->AddIPLiteralRule(host_port_pair_.host(),
+                                           "192.168.0.1", "");
+
+  QuicStreamRequest request(&factory_);
+  QuicServerId server_id(host_port_pair_, is_https_, privacy_mode_);
+  EXPECT_EQ(ERR_IO_PENDING,
+            request.Request(host_port_pair_, is_https_, privacy_mode_, "GET",
+                            net_log_, callback_.callback()));
+  EXPECT_EQ(2u,
+            QuicStreamFactoryPeer::GetNumberOfActiveJobs(&factory_, server_id));
+
+  runner_->RunNextTask();
+
+  scoped_ptr<QuicHttpStream> stream = request.ReleaseStream();
+  EXPECT_TRUE(stream.get());
+  EXPECT_TRUE(socket_data.at_read_eof());
+  EXPECT_TRUE(socket_data.at_write_eof());
+  EXPECT_EQ(0u,
+            QuicStreamFactoryPeer::GetNumberOfActiveJobs(&factory_, server_id));
+}
+
 }  // namespace test
 }  // namespace net

net/quic/quic_utils.cc

@@ -221,6 +221,7 @@ const char* QuicUtils::ErrorToString(QuicErrorCode error) {
     RETURN_STRING_LITERAL(QUIC_VERSION_NEGOTIATION_MISMATCH);
     RETURN_STRING_LITERAL(QUIC_TOO_MANY_OUTSTANDING_SENT_PACKETS);
     RETURN_STRING_LITERAL(QUIC_TOO_MANY_OUTSTANDING_RECEIVED_PACKETS);
+    RETURN_STRING_LITERAL(QUIC_CONNECTION_CANCELLED);
     RETURN_STRING_LITERAL(QUIC_LAST_ERROR);
     // Intentionally have no default case, so we'll break the build
     // if we add errors and don't put them here.