Do not mark alternate-protocol broken unless the main job succeeds.

net/http/http_stream_factory_impl_job.h

 // 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.
 
 #ifndef NET_HTTP_HTTP_STREAM_FACTORY_IMPL_JOB_H_
 #define NET_HTTP_HTTP_STREAM_FACTORY_IMPL_JOB_H_
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ skipping 78 matching lines @@
   // Indicates whether or not this job is performing a preconnect.
   bool IsPreconnecting() const;
 
   // Indicates whether or not this Job has been orphaned by a Request.
   bool IsOrphaned() const;
 
   // Called to indicate that this job succeeded, and some other jobs
   // will be orphaned.
   void ReportJobSuccededForRequest();
 
+  // Marks that the other job |job| has completed.

[willchan no longer on Chromium, 2014/04/22 21:46:11]

Reads a little weird, I think it'd be better to delete the job prior to |job|.

[Ryan Hamilton, 2014/04/22 21:54:44]

What do you mean by "delete the job prior to job"?

[Ryan Hamilton, 2014/04/22 21:57:45]

Oh, I see. Done.

+  void MarkOtherJobComplete(const Job& job);
+
  private:
   enum State {
     STATE_START,
     STATE_RESOLVE_PROXY,
     STATE_RESOLVE_PROXY_COMPLETE,
 
     // Note that when Alternate-Protocol says we can connect to an alternate
     // port using a different protocol, we have the choice of communicating over
     // the original protocol, or speaking the alternate protocol (currently,
     // only npn-spdy) over an alternate port. For a cold page load, the http
@@ skipping 17 matching lines @@
     STATE_RESTART_TUNNEL_AUTH,
     STATE_RESTART_TUNNEL_AUTH_COMPLETE,
     STATE_CREATE_STREAM,
     STATE_CREATE_STREAM_COMPLETE,
     STATE_DRAIN_BODY_FOR_AUTH_RESTART,
     STATE_DRAIN_BODY_FOR_AUTH_RESTART_COMPLETE,
     STATE_DONE,
     STATE_NONE
   };
 
+  enum JobStatus {
+    STATUS_RUNNING,
+    STATUS_FAILED,
+    STATUS_BROKEN,
+    STATUS_SUCCEEDED
+  };
+
   void OnStreamReadyCallback();
   void OnWebSocketHandshakeStreamReadyCallback();
   // This callback function is called when a new SPDY session is created.
   void OnNewSpdySessionReadyCallback();
   void OnStreamFailedCallback(int result);
   void OnCertificateErrorCallback(int result, const SSLInfo& ssl_info);
   void OnNeedsProxyAuthCallback(const HttpResponseInfo& response_info,
                                 HttpAuthController* auth_controller);
   void OnNeedsClientAuthCallback(SSLCertRequestInfo* cert_info);
   void OnHttpsProxyTunnelResponseCallback(const HttpResponseInfo& response_info,
@@ skipping 69 matching lines @@
   bool ShouldForceSpdySSL() const;
 
   // Should we force SPDY to run without SSL for this stream request.
   bool ShouldForceSpdyWithoutSSL() const;
 
   // Should we force QUIC for this stream request.
   bool ShouldForceQuic() const;
 
   bool IsRequestEligibleForPipelining();
 
+  void MaybeMarkAlternateProtocolBroken();
+
   // Record histograms of latency until Connect() completes.
   static void LogHttpConnectedMetrics(const ClientSocketHandle& handle);
 
   // Invoked by the transport socket pool after host resolution is complete
   // to allow the connection to be aborted, if a matching SPDY session can
   // be found.  Will return ERR_SPDY_SESSION_ALREADY_EXISTS if such a
   // session is found, and OK otherwise.
   static int OnHostResolution(SpdySessionPool* spdy_session_pool,
                               const SpdySessionKey& spdy_session_key,
                               const AddressList& addresses,
@@ skipping 89 matching lines @@
 
   // Only used if |new_spdy_session_| is non-NULL.
   bool spdy_session_direct_;
 
   // Key used to identify the HttpPipelinedHost for |request_|.
   scoped_ptr<HttpPipelinedHost::Key> http_pipelining_key_;
 
   // True if an existing pipeline can handle this job's request.
   bool existing_available_pipeline_;
 
+  JobStatus job_status_;

[willchan no longer on Chromium, 2014/04/22 22:23:54]

At some point I think we need to remove the Alternate-Protocol logic from the
job and move it over to the HttpStreamFactoryImpl or the Request. I kinda feel
like a Job should not have to know that it's the "alternate". It should just run
to completion, and then notify some other object that it is done.

I'm still kicking around ideas in my head, but here are some things that come to
mind:
* Move the knowledge of the "alternate" to the Request or Factory
* Switch the Job to being an interface with a JobImpl as the base
implementation. Compose this base implementation with some other object that
understands the Alternate-Protocol relationship.

+  JobStatus other_job_status_;
+
   base::WeakPtrFactory<Job> ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Job);
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_STREAM_FACTORY_IMPL_JOB_H_