JobController 1: Adding a new class HttpStreamFactoryImpl::JobController

net/http/http_stream_factory_impl_request.h

Index: net/http/http_stream_factory_impl_request.h
diff --git a/net/http/http_stream_factory_impl_request.h b/net/http/http_stream_factory_impl_request.h
index d63715631611ee8c7ca750e13cf5a16d446f8b02..62bc87f7413a2862f163d6c47395ce57e4f6885b 100644
--- a/net/http/http_stream_factory_impl_request.h
+++ b/net/http/http_stream_factory_impl_request.h
@@ -27,7 +27,7 @@ class SpdySession;
 class HttpStreamFactoryImpl::Request : public HttpStreamRequest {
  public:
   Request(const GURL& url,
-          HttpStreamFactoryImpl* factory,
+          JobController* job_controller,

[Randy Smith (Not in Mondays), 2016/05/09 20:42:18]

Same reflex for documentation: Why can this class assume the JobController will
outlive it?

[Zhongyi Shi, 2016/05/12 07:26:24]

Done.

           HttpStreamRequest::Delegate* delegate,
           WebSocketHandshakeStreamBase::CreateHelper*
               websocket_handshake_stream_create_helper,
@@ -41,17 +41,13 @@ class HttpStreamFactoryImpl::Request : public HttpStreamRequest {
 
   const BoundNetLog& net_log() const { return net_log_; }
 
-  // Called when the Job determines the appropriate |spdy_session_key| for the
-  // Request. Note that this does not mean that SPDY is necessarily supported
-  // for this SpdySessionKey, since we may need to wait for NPN to complete
-  // before knowing if SPDY is available.
+  // Called when the JobController determines the appropriate |spdy_session_key|
+  // for the Request. Note that this does not mean that SPDY is necessarily
+  // supported for this SpdySessionKey, since we may need to wait for NPN to
+  // complete before knowing if SPDY is available.
   void SetSpdySessionKey(const SpdySessionKey& spdy_session_key);
   bool HasSpdySessionKey() const;
 
-  // Attaches |job| to this request. Does not mean that Request will use |job|,
-  // but Request will own |job|.
-  void AttachJob(HttpStreamFactoryImpl::Job* job);
-
   // Marks completion of the request. Must be called before OnStreamReady().
   void Complete(bool was_npn_negotiated,
                 NextProto protocol_negotiated,
@@ -61,19 +57,24 @@ class HttpStreamFactoryImpl::Request : public HttpStreamRequest {
   // SpdySessionRequestMap.
   void RemoveRequestFromSpdySessionRequestMap();
 
-  // Called by an attached Job if it sets up a SpdySession.
+  // Called by JobController if it sets up a SpdySession.
   // |stream| is null when |stream_type| is HttpStreamRequest::HTTP_STREAM.
   // |bidirectional_stream_spdy_impl| is null when |stream_type| is
   // HttpStreamRequest::BIDIRECTIONAL_STREAM.
   void OnNewSpdySessionReady(
-      Job* job,
       std::unique_ptr<HttpStream> stream,
       std::unique_ptr<BidirectionalStreamImpl> bidirectional_stream_spdy_impl,
       const base::WeakPtr<SpdySession>& spdy_session,
+      const SSLConfig& used_ssl_config,
+      const ProxyInfo& used_proxy_info,
+      bool was_npn_negotiated,
+      const NextProto& protocol_negotiated,
+      bool using_spdy,
+      const BoundNetLog& net_log,
       bool direct);
 
-  // Called by an attached Job to record connection attempts made by the socket
-  // layer for this stream request.
+  // Called by JobController to record connection attempts made by the socket
+  // layer in an attached Job for this stream request.
   void AddConnectionAttempts(const ConnectionAttempts& attempts);
 
   WebSocketHandshakeStreamBase::CreateHelper*
@@ -84,37 +85,29 @@ class HttpStreamFactoryImpl::Request : public HttpStreamRequest {
   // HttpStreamRequest::Delegate methods which we implement. Note we don't
   // actually subclass HttpStreamRequest::Delegate.
 
-  void OnStreamReady(Job* job,
-                     const SSLConfig& used_ssl_config,
+  void OnStreamReady(const SSLConfig& used_ssl_config,
                      const ProxyInfo& used_proxy_info,
                      HttpStream* stream);
-  void OnBidirectionalStreamImplReady(Job* job,
-                                      const SSLConfig& used_ssl_config,
+  void OnBidirectionalStreamImplReady(const SSLConfig& used_ssl_config,
                                       const ProxyInfo& used_proxy_info,
                                       BidirectionalStreamImpl* stream);
 
-  void OnWebSocketHandshakeStreamReady(Job* job,
-                                       const SSLConfig& used_ssl_config,
+  void OnWebSocketHandshakeStreamReady(const SSLConfig& used_ssl_config,
                                        const ProxyInfo& used_proxy_info,
                                        WebSocketHandshakeStreamBase* stream);
-  void OnStreamFailed(Job* job,
-                      int status,
+  void OnStreamFailed(int status,
                       const SSLConfig& used_ssl_config,
                       SSLFailureState ssl_failure_state);
-  void OnCertificateError(Job* job,
-                          int status,
+  void OnCertificateError(int status,
                           const SSLConfig& used_ssl_config,
                           const SSLInfo& ssl_info);
-  void OnNeedsProxyAuth(Job* job,
-                        const HttpResponseInfo& proxy_response,
+  void OnNeedsProxyAuth(const HttpResponseInfo& proxy_response,
                         const SSLConfig& used_ssl_config,
                         const ProxyInfo& used_proxy_info,
                         HttpAuthController* auth_controller);
-  void OnNeedsClientAuth(Job* job,
-                         const SSLConfig& used_ssl_config,
+  void OnNeedsClientAuth(const SSLConfig& used_ssl_config,
                          SSLCertRequestInfo* cert_info);
   void OnHttpsProxyTunnelResponse(
-      Job *job,
       const HttpResponseInfo& response_info,
       const SSLConfig& used_ssl_config,
       const ProxyInfo& used_proxy_info,
@@ -132,28 +125,15 @@ class HttpStreamFactoryImpl::Request : public HttpStreamRequest {
   HttpStreamRequest::StreamType stream_type() const { return stream_type_; }
 
  private:
-  // Used to bind |job| to the request and orphan all other jobs in |jobs_|.
-  void BindJob(Job* job);
-
-  // Used to orphan all jobs in |jobs_|.
-  void OrphanJobs();
-
-  // Used to cancel all jobs in |jobs_|.
-  void CancelJobs();
-
-  // Called when a Job succeeds.
-  void OnJobSucceeded(Job* job);
 
   const GURL url_;
-  HttpStreamFactoryImpl* const factory_;
+  JobController* const job_controller_;

[Ryan Hamilton, 2016/05/06 20:49:02]

nit: comment on ownership. 

// Unowned.

[Randy Smith (Not in Mondays), 2016/05/09 21:42:10]

And why it's safe; i.e. that the Request doesn't outlive the JobController
(which I don't think is true) or that it's nulled when the JobController is
destroyed.

[Zhongyi Shi, 2016/05/12 07:26:24]

Done.

+
   WebSocketHandshakeStreamBase::CreateHelper* const
       websocket_handshake_stream_create_helper_;
   HttpStreamRequest::Delegate* const delegate_;
   const BoundNetLog net_log_;
 
-  // At the point where Job is irrevocably tied to the Request, we set this.
-  std::unique_ptr<Job> bound_job_;
-  std::set<HttpStreamFactoryImpl::Job*> jobs_;
   std::unique_ptr<const SpdySessionKey> spdy_session_key_;
 
   bool completed_;