JobController 1: Adding a new class HttpStreamFactoryImpl::JobController

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 c7c69f0602705b9fab565011633829c4f8d6f3f3..0ed706b6200472bb549bbd0905138275a85bedf3 100644
--- a/net/http/http_stream_factory_impl_job.h
+++ b/net/http/http_stream_factory_impl_job.h
@@ -41,7 +41,10 @@ class QuicHttpStream;
 class HttpStreamFactoryImpl::Job {
  public:
   // Constructor for non-alternative Job.
-  Job(HttpStreamFactoryImpl* stream_factory,
+  // Job is owned by JobController, hence |job_controller| is valid for the
+  // lifetime of the Job.
+  Job(JobController* job_controller,
+      JobType job_type,
       HttpNetworkSession* session,
       const HttpRequestInfo& request_info,
       RequestPriority priority,
@@ -50,8 +53,12 @@ class HttpStreamFactoryImpl::Job {
       HostPortPair destination,
       GURL origin_url,
       NetLog* net_log);
+
   // Constructor for alternative Job.
-  Job(HttpStreamFactoryImpl* stream_factory,
+  // Job is owned by JobController, hence |job_controller| is valid for the
+  // lifetime of the Job.
+  Job(JobController* job_controller,
+      JobType job_type,
       HttpNetworkSession* session,
       const HttpRequestInfo& request_info,
       RequestPriority priority,
@@ -63,9 +70,9 @@ class HttpStreamFactoryImpl::Job {
       NetLog* net_log);
   ~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);
+  // Start initiates the process of creating a new HttpStream.
+  // |job_controller_| will be notified upon completion.
+  void Start(HttpStreamRequest::StreamType stream_type);
 
   // Preconnect will attempt to request |num_streams| sockets from the
   // appropriate ClientSocketPool.
@@ -83,8 +90,8 @@ class HttpStreamFactoryImpl::Job {
   // deleting it.
   void Resume(Job* job, const base::TimeDelta& delay);
 
-  // Used to detach the Job from |request|.
-  void Orphan(const Request* request);
+  // Used to detach the Job.

[Randy Smith (Not in Mondays), 2016/05/16 20:46:39]

It's still not clear to me what this means in the context of a caller of the
Job.  What does it change about the functionality the Job exports?  How will the
job behave differently after this function is called?  Maybe "Notify the Job
that it's result will not be used directly; job may cancel itself if there is no
value to completing the connection."  (Though that's based on my reading of the
code and hence may be wrong.)

+  void Orphan();
 
   void SetPriority(RequestPriority priority);
 
@@ -95,16 +102,16 @@ class HttpStreamFactoryImpl::Job {
   const BoundNetLog& net_log() const { return net_log_; }
   HttpStreamRequest::StreamType stream_type() const { return stream_type_; }
 
+  HttpStream* ReleaseStream() { return stream_.release(); }

[Randy Smith (Not in Mondays), 2016/05/16 20:46:39]

This is passing ownership; why doesn't it return a std::unique_ptr<>?

[Zhongyi Shi, 2016/05/16 23:08:53]

Done.

+
+  BidirectionalStreamImpl* ReleaseBidirectionalStream() {
+    return bidirectional_stream_impl_.release();
+  }
+
   const SSLConfig& server_ssl_config() const;
   const SSLConfig& proxy_ssl_config() const;
   const ProxyInfo& proxy_info() const;
 
-  // 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 ReportJobSucceededForRequest();
@@ -112,6 +119,8 @@ class HttpStreamFactoryImpl::Job {
   // Marks that the other |job| has completed.
   void MarkOtherJobComplete(const Job& job);
 
+  JobType job_type() const { return job_type_; }
+
  private:
   FRIEND_TEST_ALL_PREFIXES(HttpStreamFactoryImplRequestTest, DelayMainJob);
 
@@ -318,8 +327,6 @@ class HttpStreamFactoryImpl::Job {
                               const AddressList& addresses,
                               const BoundNetLog& net_log);
 
-  Request* request_;
-
   const HttpRequestInfo request_info_;
   RequestPriority priority_;
   ProxyInfo proxy_info_;
@@ -330,7 +337,6 @@ class HttpStreamFactoryImpl::Job {
   CompletionCallback io_callback_;
   std::unique_ptr<ClientSocketHandle> connection_;
   HttpNetworkSession* const session_;
-  HttpStreamFactoryImpl* const stream_factory_;
   State next_state_;
   ProxyService::PacRequest* pac_request_;
   SSLInfo ssl_info_;
@@ -349,6 +355,11 @@ class HttpStreamFactoryImpl::Job {
   // AlternativeService for the other Job if this is not an alternative Job.
   AlternativeService other_job_alternative_service_;
 
+  // Unowned. |this| job is owned by |job_controller_|.
+  JobController* job_controller_;
+
+  JobType job_type_;
+
   // This is the Job we're dependent on. It will notify us if/when it's OK to
   // proceed.
   Job* blocking_job_;