Add AlternateProtocolInfo to HttpStreamFactoryImpl::Job.

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 23 matching lines @@
 // An HttpStreamRequestImpl exists for each stream which is in progress of being
 // created for the StreamFactory.
 class HttpStreamFactoryImpl::Job {
  public:
   Job(HttpStreamFactoryImpl* stream_factory,
       HttpNetworkSession* session,
       const HttpRequestInfo& request_info,
       RequestPriority priority,
       const SSLConfig& server_ssl_config,
       const SSLConfig& proxy_ssl_config,
-      NetLog* net_log);
+      NetLog* net_log,
+      AlternateProtocolInfo alternate_protocol);

[Ryan Hamilton, 2015/01/30 23:12:51]

I'm not sure that I'm a fan of moving this to the constructor. Since only
alternate jobs (those which have had MarkAsAlternate called) should have
alternatives it seems odd to pass in here, instead of when marking it alternate.

Or, I guess, you could add both of these arguments to the constructor, but that
doesn't feel super awesome?

[Bence, 2015/01/31 00:32:01]

Well, if you look at
https://codereview.chromium.org/665083009/diff/400001/net/http/http_stream_fa...,
you can see that both the alternate job and the non-alternate one must keep a
copy of alternate_protocol, because they will need to pass it to
SetBrokenAlternateProtocol().  This is because each server will be allowed to
have multiple AlternateProtocols, and SetBrokenAlternateProtocol() will need to
know which one to mark as broken.

[Ryan Hamilton, 2015/01/31 01:17:48]

I'm not sure that's the best way to accomplish this. It seems like that change
makes alternate_protocol_ mean two different things. it's either the protocol
being used by this jobs (if it's an alternate job) or else is that alternate
protocol being used but the other job (if this is a main job). Instead, it might
make more sense to change MarkOtherJobComplete() so that it takes the
AlternateProtocol (if any) from the other job and then save it in a member like
other_job_alternate_protocol_.

[Bence, 2015/02/02 15:08:17]

Excellent idea!  Done.

   ~Job();
 
   // Start initiates the process of creating a new HttpStream. |request| will be
   // notified upon completion if the Job has not been Orphan()'d.
   void Start(Request* request);
 
   // Preconnect will attempt to request |num_streams| sockets from the
   // appropriate ClientSocketPool.
   int Preconnect(int num_streams);
 
   int RestartTunnelWithProxyAuth(const AuthCredentials& credentials);
   LoadState GetLoadState() const;
 
   // Marks this Job as the "alternate" job, from Alternate-Protocol. Tracks the
   // original url so we can mark the Alternate-Protocol as broken if
-  // we fail to connect.  |alternate| specifies the alternate protocol to use
-  // and alternate port to connect to.
-  void MarkAsAlternate(const GURL& original_url,
-                       AlternateProtocolInfo alternate);
+  // we fail to connect.
+  void MarkAsAlternate(const GURL& original_url);
 
   // Tells |this| to wait for |job| to resume it.
   void WaitFor(Job* job);
 
   // Tells |this| that |job| has determined it still needs to continue
   // connecting, so allow |this| to continue. If this is not called, then
   // |request_| is expected to cancel |this| by deleting it.
   void Resume(Job* job);
 
   // Used to detach the Job from |request|.
@@ skipping 194 matching lines @@
   HostPortPair origin_;
 
   // The origin url we're trying to reach. This url may be different from the
   // original request when host mapping rules are set-up.
   GURL origin_url_;
 
   // If this is a Job for an "Alternate-Protocol", then this will be non-NULL
   // and will specify the original URL.
   scoped_ptr<GURL> original_url_;
 
+  // AlternateProtocol for the alternate job (regardless of whether it is this
+  // job or not).
+  AlternateProtocolInfo alternate_protocol_;
+
   // This is the Job we're dependent on. It will notify us if/when it's OK to
   // proceed.
   Job* blocking_job_;
 
   // |waiting_job_| is a Job waiting to see if |this| can reuse a connection.
   // If |this| is unable to do so, we'll notify |waiting_job_| that it's ok to
   // proceed and then race the two Jobs.
   Job* waiting_job_;
 
   // True if handling a HTTPS request, or using SPDY with SSL
@@ skipping 48 matching lines @@
   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_