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

net/quic/quic_stream_factory.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/quic/quic_stream_factory.h"
 
 #include <set>
 
 #include "base/cpu.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 123 matching lines @@
 // Responsible for creating a new QUIC session to the specified server, and
 // for notifying any associated requests when complete.
 class QuicStreamFactory::Job {
  public:
   Job(QuicStreamFactory* factory,
       HostResolver* host_resolver,
       const HostPortPair& host_port_pair,
       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);
 
   // Creates a new job to handle the resumption of for connecting an
   // existing session.
   Job(QuicStreamFactory* factory,
       HostResolver* host_resolver,
       QuicClientSession* session,
       QuicServerId server_id);
 
   ~Job();
 
   int Run(const CompletionCallback& callback);
 
   int DoLoop(int rv);
   int DoResolveHost();
   int DoResolveHostComplete(int rv);
   int DoLoadServerInfo();
   int DoLoadServerInfoComplete(int rv);
   int DoConnect();
   int DoResumeConnect();
   int DoConnectComplete(int rv);
 
   void OnIOComplete(int rv);
 
+  void RunAuxilaryJob();
+
+  void CancelWaitForDataReadyPendingCallback();

[ramant (doing other things), 2015/02/06 19:30:31]

Deleted this method. The name of the function was too confusing.

+
+  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 {
     STATE_NONE,
     STATE_RESOLVE_HOST,
     STATE_RESOLVE_HOST_COMPLETE,
     STATE_LOAD_SERVER_INFO,
     STATE_LOAD_SERVER_INFO_COMPLETE,
     STATE_CONNECT,
     STATE_RESUME_CONNECT,
     STATE_CONNECT_COMPLETE,
   };
   IoState io_state_;
 
   QuicStreamFactory* factory_;
   SingleRequestHostResolver host_resolver_;
   QuicServerId server_id_;
   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_;
   AddressList address_list_;
   base::TimeTicks disk_cache_load_start_time_;
   base::TimeTicks dns_resolution_start_time_;
   base::WeakPtrFactory<Job> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(Job);
 };
 
 QuicStreamFactory::Job::Job(QuicStreamFactory* factory,
                             HostResolver* host_resolver,
                             const HostPortPair& host_port_pair,
                             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,
                             QuicClientSession* session,
                             QuicServerId server_id)
     : io_state_(STATE_RESUME_CONNECT),
       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() {
+  CancelWaitForDataReadyPendingCallback();
 }
 
 int QuicStreamFactory::Job::Run(const CompletionCallback& callback) {
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
     callback_ = callback;
 
   return rv > 0 ? OK : rv;
 }
 
@@ skipping 45 matching lines @@
 
   tracked_objects::ScopedTracker tracking_profile2(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::Job::OnIOComplete2"));
 
   if (rv != ERR_IO_PENDING && !callback_.is_null()) {
     callback_.Run(rv);
   }
 }
 
+void QuicStreamFactory::Job::RunAuxilaryJob() {
+  int rv = Run(base::Bind(&QuicStreamFactory::OnJobComplete,
+                          base::Unretained(factory_), this));
+  if (rv == OK)
+    factory_->OnJobComplete(this, rv);
+}
+
+void QuicStreamFactory::Job::CancelWaitForDataReadyPendingCallback() {
+  // If we are waiting for WaitForDataReadyCallback, then cancel the pending
+  // callback.
+  if (server_info_ && io_state_ == STATE_LOAD_SERVER_INFO_COMPLETE)
+    server_info_->CancelWaitForDataReadyCallback();
+}
+
+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)
-    return;
-  server_info_->CancelWaitForDataReadyCallback();
+  CancelWaitForDataReadyPendingCallback();

[Ryan Hamilton, 2015/02/06 18:22:47]

Does this call itself?

[ramant (doing other things), 2015/02/06 19:30:31]

Undid this change. Restored the original code.

IMO, names were too confusing. Same method name on Job as well as server_info
(and one has the word "Pending" in it etc). 

We could call disk cache's CancelWaitForDataReadyCallback twice
(https://code.google.com/p/chromium/codesearch#chromium/src/net/http/disk_cach...)

   OnIOComplete(OK);
 }
 
 int QuicStreamFactory::Job::DoResolveHost() {
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::Job::DoResolveHost"));
 
   // Start loading the data now, and wait for it after we resolve the host.
   if (server_info_) {
     server_info_->Start();
   }
 
   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_);
 }
 
 int QuicStreamFactory::Job::DoResolveHostComplete(int rv) {
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::Job::DoResolveHostComplete"));
 
   UMA_HISTOGRAM_TIMES("Net.QuicSession.HostResolutionTime",
                       base::TimeTicks::Now() - dns_resolution_start_time_);
   if (rv != OK)
     return rv;
 
   DCHECK(!factory_->HasActiveSession(server_id_));
 
   // Inform the factory of this resolution, which will set up
   // a session alias, if possible.
   if (factory_->OnResolution(server_id_, address_list_)) {
     return OK;
   }
 
-  io_state_ = STATE_LOAD_SERVER_INFO;
+  if (server_info_)
+    io_state_ = STATE_LOAD_SERVER_INFO;
+  else
+    io_state_ = STATE_CONNECT;
   return OK;
 }
 
 int QuicStreamFactory::Job::DoLoadServerInfo() {
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 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.
   int64 load_server_info_timeout_ms = factory_->load_server_info_timeout_ms_;
   if (factory_->load_server_info_timeout_srtt_multiplier_ > 0) {
     DCHECK_EQ(0, load_server_info_timeout_ms);
     load_server_info_timeout_ms =
         (factory_->load_server_info_timeout_srtt_multiplier_ *
          factory_->GetServerNetworkStatsSmoothedRttInMicroseconds(server_id_)) /
         1000;
   }
   if (load_server_info_timeout_ms > 0) {
     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_);

[Ryan Hamilton, 2015/02/06 18:22:47]

Nice! This block of code is clear, and obvious. Love it.

[ramant (doing other things), 2015/02/06 19:30:31]

Thanks much. Both of us are making this code better.

+  }
+  return rv;
 }
 
 int QuicStreamFactory::Job::DoLoadServerInfoComplete(int rv) {
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       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;
   return OK;
 }
 
 int QuicStreamFactory::Job::DoConnect() {
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
@@ skipping 26 matching lines @@
       factory_->require_confirmation() || is_post_ ||
       was_alternate_protocol_recently_broken_;
 
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile2(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::Job::DoConnect2"));
 
   rv = session_->CryptoConnect(
       require_confirmation,
-      base::Bind(&QuicStreamFactory::Job::OnIOComplete,
-                 base::Unretained(this)));
+      base::Bind(&QuicStreamFactory::Job::OnIOComplete, GetWeakPtr()));
   return rv;
 }
 
 int QuicStreamFactory::Job::DoResumeConnect() {
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 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;
 }
 
 int QuicStreamFactory::Job::DoConnectComplete(int rv) {
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::Job::DoConnectComplete"));
 
@@ skipping 95 matching lines @@
       config_(InitializeQuicConfig(connection_options)),
       supported_versions_(supported_versions),
       enable_port_selection_(enable_port_selection),
       always_require_handshake_confirmation_(
           always_require_handshake_confirmation),
       disable_connection_pooling_(disable_connection_pooling),
       load_server_info_timeout_ms_(load_server_info_timeout),
       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),
       weak_factory_(this) {
   DCHECK(transport_security_state_);
   crypto_config_.set_user_agent_id(user_agent_id);
   crypto_config_.AddCanonicalSuffix(".c.youtube.com");
   crypto_config_.AddCanonicalSuffix(".googlevideo.com");
   crypto_config_.SetProofVerifier(
       new ProofVerifierChromium(cert_verifier, transport_security_state));
   crypto_config_.SetChannelIDSource(
       new ChannelIDSourceChromium(channel_id_service));
   base::CPU cpu;
   if (cpu.has_aesni() && cpu.has_avx())
     crypto_config_.PreferAesGcm();
   if (!IsEcdsaSupported())
     crypto_config_.DisableEcdsa();
 }
 
 QuicStreamFactory::~QuicStreamFactory() {
   CloseAllSessions(ERR_ABORTED);
   while (!all_sessions_.empty()) {
     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) {
   require_confirmation_ = require_confirmation;
   if (http_server_properties_ && (!(local_address_ == IPEndPoint()))) {
     http_server_properties_->SetSupportsQuic(!require_confirmation,
                                              local_address_.address());
   }
 }
 
 int QuicStreamFactory::Create(const HostPortPair& host_port_pair,
                               bool is_https,
                               PrivacyMode privacy_mode,
                               base::StringPiece method,
                               const BoundNetLog& net_log,
                               QuicStreamRequest* request) {
   QuicServerId server_id(host_port_pair, is_https, privacy_mode);
   if (HasActiveSession(server_id)) {
     request->set_stream(CreateIfSessionExists(server_id, net_log));
     return OK;
   }
 
   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;
     if (http_server_properties_) {
       const AlternateProtocolMap& alternate_protocol_map =
           http_server_properties_->alternate_protocol_map();
       AlternateProtocolMap::const_iterator it =
           alternate_protocol_map.Peek(server_id.host_port_pair());
       if (it == alternate_protocol_map.end() || it->second.protocol != QUIC) {
         // If there is no entry for QUIC, consider that as a new server and
         // don't wait for Cache thread to load the data for that server.
         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));
     request->set_stream(CreateIfSessionExists(server_id, net_log));
   }
   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) {
   DCHECK(!HasActiveSession(server_id));
   if (disable_connection_pooling_) {
     return false;
   }
   for (const IPEndPoint& address : address_list) {
     const IpAliasKey ip_alias_key(address, server_id.is_https());
     if (!ContainsKey(ip_aliases_, ip_alias_key))
       continue;
 
     const SessionSet& sessions = ip_aliases_[ip_alias_key];
     for (QuicClientSession* session : sessions) {
       if (!session->CanPool(server_id.host(), server_id.privacy_mode()))
         continue;
       active_sessions_[server_id] = session;
       session_aliases_[session].insert(server_id);
       return true;
     }
   }
   return false;
 }
 
 void QuicStreamFactory::OnJobComplete(Job* job, int rv) {
+  QuicServerId server_id = job->server_id();
   if (rv == OK) {
     // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
     tracked_objects::ScopedTracker tracking_profile1(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "422516 QuicStreamFactory::OnJobComplete1"));
 
     if (!always_require_handshake_confirmation_)
       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 (RequestSet::iterator it = job_requests_map_[server_id].begin();
+         it != job_requests_map_[server_id].end(); ++it) {

[Ryan Hamilton, 2015/02/06 18:22:48]

nit: As long as you're here I think you can probably clean this up with a
range-based for loop:

for (Request request : job_requests_map_[server_id]) {
  ...
}

[ramant (doing other things), 2015/02/06 19:30:31]

+1 to the above. Thanks much. 

Done.

+      DCHECK(HasActiveSession(server_id));
+      (*it)->set_stream(CreateIfSessionExists(server_id, (*it)->net_log()));
+    }
+  } else {
+    JobSet* jobs = &(active_jobs_[server_id]);

[Ryan Hamilton, 2015/02/06 18:22:47]

Can you add a comment here explaining what this code is doing?  

Totally your call, but I tend to prefer errors to be handled at the top of a
method so I'd probably write this as:

if (rv != OK) {
  ...
}

if (rv == OK) {
 ...
}

But either approach is fine with me.

[ramant (doing other things), 2015/02/06 19:30:31]

Done.

+    if (jobs->size() > 1) {
+      job->Cancel();
+      jobs->erase(job);
+      delete job;
+      return;
     }
   }
 
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile2(
       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
     // profile which can not be deleted via callbacks.
     request->OnRequestComplete(rv);
   }
 
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile3(
       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
 // matching session already exists.  Returns nullptr otherwise.
 scoped_ptr<QuicHttpStream> QuicStreamFactory::CreateIfSessionExists(
     const QuicServerId& server_id,
     const BoundNetLog& net_log) {
   if (!HasActiveSession(server_id)) {
     DVLOG(1) << "No active session";
     return scoped_ptr<QuicHttpStream>();
@@ skipping 58 matching lines @@
 
   const IpAliasKey ip_alias_key(session->connection()->peer_address(),
                                 aliases.begin()->is_https());
   ip_aliases_[ip_alias_key].erase(session);
   if (ip_aliases_[ip_alias_key].empty()) {
     ip_aliases_.erase(ip_alias_key);
   }
   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);
 }
 
 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);
 }
 
 void QuicStreamFactory::CloseAllSessions(int error) {
   while (!active_sessions_.empty()) {
     size_t initial_size = active_sessions_.size();
     active_sessions_.begin()->second->CloseSessionOnError(error);
     DCHECK_NE(initial_size, active_sessions_.size());
   }
   while (!all_sessions_.empty()) {
@@ skipping 49 matching lines @@
   // kind of change it is, we have to flush the socket
   // pools to be safe.
   CloseAllSessions(ERR_CERT_DATABASE_CHANGED);
 }
 
 bool QuicStreamFactory::HasActiveSession(
     const QuicServerId& server_id) const {
   return ContainsKey(active_sessions_, server_id);
 }
 
+bool QuicStreamFactory::HasActiveJob(const QuicServerId& key) const {
+  return ContainsKey(active_jobs_, key);
+}
+

[Ryan Hamilton, 2015/02/06 18:22:47]

Did this move to match the order in the .h file?

[ramant (doing other things), 2015/02/06 19:30:31]

Acknowledged.

 int QuicStreamFactory::CreateSession(
     const QuicServerId& server_id,
     scoped_ptr<QuicServerInfo> server_info,
     const AddressList& address_list,
     const BoundNetLog& net_log,
     QuicClientSession** session) {
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile1(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::CreateSession1"));
@@ skipping 194 matching lines @@
   UMA_HISTOGRAM_BOOLEAN("Net.QuicSession.ClosedDuringInitializeSession",
                         closed_during_initialize);
   if (closed_during_initialize) {
     DLOG(DFATAL) << "Session closed during initialize";
     *session = nullptr;
     return ERR_CONNECTION_CLOSED;
   }
   return OK;
 }
 
-bool QuicStreamFactory::HasActiveJob(const QuicServerId& key) const {
-  return ContainsKey(active_jobs_, key);
-}
-
 void QuicStreamFactory::ActivateSession(
     const QuicServerId& server_id,
     QuicClientSession* session) {
   DCHECK(!HasActiveSession(server_id));
   UMA_HISTOGRAM_COUNTS("Net.QuicActiveSessions", active_sessions_.size());
   active_sessions_[server_id] = session;
   session_aliases_[session].insert(server_id);
   const IpAliasKey ip_alias_key(session->connection()->peer_address(),
                                 server_id.is_https());
   DCHECK(!ContainsKey(ip_aliases_[ip_alias_key], session));
   ip_aliases_[ip_alias_key].insert(session);
 }
 
 int64 QuicStreamFactory::GetServerNetworkStatsSmoothedRttInMicroseconds(
     const QuicServerId& server_id) const {
   if (!http_server_properties_)
     return 0;
   const ServerNetworkStats* stats =
       http_server_properties_->GetServerNetworkStats(
           server_id.host_port_pair());
   if (stats == nullptr)
     return 0;
   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) {
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.
   tracked_objects::ScopedTracker tracking_profile1(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::InitializeCachedStateInCryptoConfig1"));
 
   // |server_info| will be NULL, if a non-empty server config already exists in
   // the memory cache. This is a minor optimization to avoid LookupOrCreate.
@@ skipping 93 matching lines @@
   http_server_properties_->ClearAlternateProtocol(server);
   http_server_properties_->SetAlternateProtocol(
       server, alternate.port, alternate.protocol, 1);
   DCHECK_EQ(QUIC,
             http_server_properties_->GetAlternateProtocol(server).protocol);
   DCHECK(http_server_properties_->WasAlternateProtocolRecentlyBroken(
       server));
 }
 
 }  // namespace net