Remove HttpStreamFactoryImpl::Job::MarkAsAlternate.

net/http/http_stream_factory_impl_job.h

Index: net/http/http_stream_factory_impl_job.h
diff --git a/net/http/http_stream_factory_impl_job.h b/net/http/http_stream_factory_impl_job.h
index f7285be3330102ffba05a57e048668c2b4271808..14b00224cf9d65b245402b6c72b1ad0e822cedd0 100644
--- a/net/http/http_stream_factory_impl_job.h
+++ b/net/http/http_stream_factory_impl_job.h
@@ -36,12 +36,15 @@ class QuicHttpStream;
 // created for the StreamFactory.
 class HttpStreamFactoryImpl::Job {
  public:
+  // A Job is considered alternative iff

[Ryan Hamilton, 2015/06/02 17:26:56]

eschew "iff.  This reads just as unambiguiously when written as:

// A Job is considered alternative if 
// alternative_service.protocol != UNINITIALIZED_ALTERNATE_PROTOCOL.

That being said, since this class doesn't really describe hat it means to be
"considered alternative" I would be more inclined to say something which simply
documents how this argument is used.

That being said, I think it might be clearer to have two different constructor
forms. One which takes an AlternateService and one which does not. You can then
use c++ 11's delegated constructor, I believe to make this simple.

[Bence, 2015/06/02 19:32:16]

Great idea, thanks.

Done.

+  // alternative_service.protocol != UNINITIALIZED_ALTERNATE_PROTOCOL.
   Job(HttpStreamFactoryImpl* stream_factory,
       HttpNetworkSession* session,
       const HttpRequestInfo& request_info,
       RequestPriority priority,
       const SSLConfig& server_ssl_config,
       const SSLConfig& proxy_ssl_config,
+      AlternativeService alternative_service,
       NetLog* net_log);
   ~Job();
 
@@ -56,10 +59,6 @@ class HttpStreamFactoryImpl::Job {
   int RestartTunnelWithProxyAuth(const AuthCredentials& credentials);
   LoadState GetLoadState() const;
 
-  // Marks this Job as the "alternate" job, from Alternate-Protocol or Alt-Svc
-  // using the specified alternate service.
-  void MarkAsAlternate(AlternativeService alternative_service);
-
   // Tells |this| to wait for |job| to resume it.
   void WaitFor(Job* job);
 
@@ -146,13 +145,14 @@ class HttpStreamFactoryImpl::Job {
    public:
     ValidSpdySessionPool(SpdySessionPool* spdy_session_pool,
                          GURL& origin_url,
-                         bool is_spdy_alternate);
+                         bool is_spdy_alternative);
 
     // Returns OK if a SpdySession was not found (in which case |spdy_session|
     // is set to nullptr), or if one was found (in which case |spdy_session| is
     // set to it) and it has an associated SSL certificate with is valid for
     // |origin_url_|, or if this requirement does not apply because the Job is
-    // not a SPDY alternate job.  Returns the appropriate error code otherwise,
+    // not a SPDY alternative job.  Returns the appropriate error code
+    // otherwise,
     // in which case |spdy_session| should not be used.
     int FindAvailableSession(const SpdySessionKey& key,
                              const BoundNetLog& net_log,
@@ -160,7 +160,7 @@ class HttpStreamFactoryImpl::Job {
 
     // Creates a SpdySession and sets |spdy_session| to point to it.  Returns OK
     // if the associated SSL certificate is valid for |origin_url_|, or if this
-    // requirement does not apply because the Job is not a SPDY alternate job.
+    // requirement does not apply because the Job is not a SPDY alternative job.
     // Returns the appropriate error code otherwise, in which case
     // |spdy_session| should not be used.
     int CreateAvailableSessionFromSocket(
@@ -174,14 +174,14 @@ class HttpStreamFactoryImpl::Job {
    private:
     // Returns OK if |spdy_session| has an associated SSL certificate with is
     // valid for |origin_url_|, or if this requirement does not apply because
-    // the Job is not a SPDY alternate job, or if |spdy_session| is null.
+    // the Job is not a SPDY alternative job, or if |spdy_session| is null.
     // Returns appropriate error code otherwise.
     int CheckAlternativeServiceValidityForOrigin(
         base::WeakPtr<SpdySession> spdy_session);
 
     SpdySessionPool* const spdy_session_pool_;
     const GURL origin_url_;
-    const bool is_spdy_alternate_;
+    const bool is_spdy_alternative_;
   };
 
   void OnStreamReadyCallback();
@@ -231,13 +231,6 @@ class HttpStreamFactoryImpl::Job {
 
   bool IsHttpsProxyAndHttpUrl() const;
 
-  // Returns true iff this Job is an alternate, that is, iff MarkAsAlternate has
-  // been called.
-  bool IsAlternate() const;
-
-  // Returns true if this Job is a SPDY alternate job.
-  bool IsSpdyAlternate() const;
-
   // Sets several fields of |ssl_config| for |server| based on the proxy info
   // and other factors.
   void InitSSLConfig(const HostPortPair& server,
@@ -319,10 +312,14 @@ class HttpStreamFactoryImpl::Job {
   // original request when host mapping rules are set-up.
   GURL origin_url_;
 
-  // AlternateProtocol for this job if this is an alternate job.
-  AlternativeService alternative_service_;
+  // AlternativeService for this Job if this is an alternative Job.
+  const AlternativeService alternative_service_;
+
+  // Is this a SPDY or QUIC alternative Job?
+  const bool is_spdy_alternative_;
+  const bool is_quic_alternative_;

[Ryan Hamilton, 2015/06/02 17:26:57]

It's not obvious to me that this is a win, but perhaps it is. So this adds to
the memory used by each job (albeit not very much) and denormalizes information
out of alternative_service_ into these two new members. There's nothing *wrong*
with this of course, but I don't know that it's better?

[Bence, 2015/06/02 19:32:16]

I agree: it doesn't seem to matter much.  I reverted to methods for lack of
compelling reason to use members.

Done.

 
-  // AlternateProtocol for the other job if this is not an alternate job.
+  // AlternativeService for the other Job if this is not an alternative Job.
   AlternativeService other_job_alternative_service_;
 
   // This is the Job we're dependent on. It will notify us if/when it's OK to