Break reference cycle from HttpProxyClientSocket=>HttpNetworkSession=>...

Note that this undoes the fix for http://crbug.com/49387 which is now unnecessary without the cycle.
Some other miscellaneous cleanup is thrown in here.

BUG=55175
TEST=none

Review URL: http://codereview.chromium.org/3418018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@59873 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 93 insertions, 50 deletions

diff --git a/net/http/http_auth_controller.cc b/net/http/http_auth_controller.cc
index 5226b02..7fe07f4 100644
--- a/net/http/http_auth_controller.cc
+++ b/net/http/http_auth_controller.cc
@@ -54,14 +54,16 @@ std::string AuthChallengeLogMessage(HttpResponseHeaders* headers) {
 HttpAuthController::HttpAuthController(
     HttpAuth::Target target,
     const GURL& auth_url,
-    scoped_refptr<HttpNetworkSession> session)
+    HttpAuthCache* http_auth_cache,
+    HttpAuthHandlerFactory* http_auth_handler_factory)
     : target_(target),
       auth_url_(auth_url),
       auth_origin_(auth_url.GetOrigin()),
       auth_path_(HttpAuth::AUTH_PROXY ? std::string() : auth_url.path()),
       embedded_identity_used_(false),
       default_credentials_used_(false),
-      session_(session),
+      http_auth_cache_(http_auth_cache),
+      http_auth_handler_factory_(http_auth_handler_factory),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           io_callback_(this, &HttpAuthController::OnIOComplete)),
       user_callback_(NULL) {
@@ -113,14 +115,14 @@ bool HttpAuthController::SelectPreemptiveAuth(const BoundNetLog& net_log) {
   // is expected to be fast. LookupByPath() is fast in the common case, since
   // the number of http auth cache entries is expected to be very small.
   // (For most users in fact, it will be 0.)
-  HttpAuthCache::Entry* entry = session_->auth_cache()->LookupByPath(
+  HttpAuthCache::Entry* entry = http_auth_cache_->LookupByPath(
       auth_origin_, auth_path_);
   if (!entry)
     return false;
 
   // Try to create a handler using the previous auth challenge.
   scoped_ptr<HttpAuthHandler> handler_preemptive;
-  int rv_create = session_->http_auth_handler_factory()->
+  int rv_create = http_auth_handler_factory_->
       CreatePreemptiveAuthHandlerFromString(entry->auth_challenge(), target_,
                                             auth_origin_,
                                             entry->IncrementNonceCount(),
@@ -172,10 +174,10 @@ int HttpAuthController::HandleAuthChallenge(
         InvalidateCurrentHandler();
         break;
       case HttpAuth::AUTHORIZATION_RESULT_STALE:
-        if (session_->auth_cache()->UpdateStaleChallenge(auth_origin_,
-                                                         handler_->realm(),
-                                                         handler_->scheme(),
-                                                         challenge_used)) {
+        if (http_auth_cache_->UpdateStaleChallenge(auth_origin_,
+                                                   handler_->realm(),
+                                                   handler_->scheme(),
+                                                   challenge_used)) {
           handler_.reset();
           identity_ = HttpAuth::Identity();
         } else {
@@ -197,7 +199,7 @@ int HttpAuthController::HandleAuthChallenge(
                         !do_not_send_server_auth);
   if (!handler_.get() && can_send_auth) {
     // Find the best authentication challenge that we support.
-    HttpAuth::ChooseBestChallenge(session_->http_auth_handler_factory(),
+    HttpAuth::ChooseBestChallenge(http_auth_handler_factory_,
                                   headers, target_, auth_origin_,
                                   disabled_schemes_, net_log,
                                   &handler_);
@@ -276,10 +278,10 @@ void HttpAuthController::ResetAuth(const string16& username,
     case HttpAuth::IDENT_SRC_DEFAULT_CREDENTIALS:
       break;
     default:
-      session_->auth_cache()->Add(auth_origin_, handler_->realm(),
-                                  handler_->scheme(), handler_->challenge(),
-                                  identity_.username, identity_.password,
-                                  auth_path_);
+      http_auth_cache_->Add(auth_origin_, handler_->realm(),
+                            handler_->scheme(), handler_->challenge(),
+                            identity_.username, identity_.password,
+                            auth_path_);
       break;
   }
 }
@@ -303,9 +305,9 @@ void HttpAuthController::InvalidateRejectedAuthFromCache() {
   // Clear the cache entry for the identity we just failed on.
   // Note: we require the username/password to match before invalidating
   // since the entry in the cache may be newer than what we used last time.
-  session_->auth_cache()->Remove(auth_origin_, handler_->realm(),
-                                 handler_->scheme(), identity_.username,
-                                 identity_.password);
+  http_auth_cache_->Remove(auth_origin_, handler_->realm(),
+                           handler_->scheme(), identity_.username,
+                           identity_.password);
 }
 
 bool HttpAuthController::SelectNextAuthIdentityToTry() {
@@ -329,8 +331,8 @@ bool HttpAuthController::SelectNextAuthIdentityToTry() {
 
   // Check the auth cache for a realm entry.
   HttpAuthCache::Entry* entry =
-      session_->auth_cache()->Lookup(auth_origin_, handler_->realm(),
-                                     handler_->scheme());
+      http_auth_cache_->Lookup(auth_origin_, handler_->realm(),
+                               handler_->scheme());
 
   if (entry) {
     identity_.source = HttpAuth::IDENT_SRC_REALM_LOOKUP;
diff --git a/net/http/http_auth_controller.h b/net/http/http_auth_controller.h
index 5ee156c..097c4bc 100644
--- a/net/http/http_auth_controller.h
+++ b/net/http/http_auth_controller.h
@@ -22,7 +22,8 @@ namespace net {
 
 class AuthChallengeInfo;
 class HttpAuthHandler;
-class HttpNetworkSession;
+class HttpAuthHandlerFactory;
+class HttpAuthCache;
 class HttpRequestHeaders;
 struct HttpRequestInfo;
 
@@ -30,8 +31,10 @@ class HttpAuthController : public base::RefCounted<HttpAuthController> {
  public:
   // The arguments are self explanatory except possibly for |auth_url|, which
   // should be both the auth target and auth path in a single url argument.
-  HttpAuthController(HttpAuth::Target target, const GURL& auth_url,
-                     scoped_refptr<HttpNetworkSession> session);
+  HttpAuthController(HttpAuth::Target target,
+                     const GURL& auth_url,
+                     HttpAuthCache* http_auth_cache,
+                     HttpAuthHandlerFactory* http_auth_handler_factory);
 
   // Generate an authentication token for |target| if necessary. The return
   // value is a net error code. |OK| will be returned both in the case that
@@ -71,11 +74,12 @@ class HttpAuthController : public base::RefCounted<HttpAuthController> {
   virtual bool IsAuthSchemeDisabled(const std::string& scheme) const;
   virtual void DisableAuthScheme(const std::string& scheme);
 
- protected:  // So that we can mock this object.
+ private:
+  // So that we can mock this object.
   friend class base::RefCounted<HttpAuthController>;
+
   virtual ~HttpAuthController();
 
- private:
   // Searches the auth cache for an entry that encompasses the request's path.
   // If such an entry is found, updates |identity_| and |handler_| with the
   // cache entry's data and returns true.
@@ -138,7 +142,11 @@ class HttpAuthController : public base::RefCounted<HttpAuthController> {
   // in response to an HTTP authentication challenge.
   bool default_credentials_used_;
 
-  scoped_refptr<HttpNetworkSession> session_;
+  // These two are owned by the HttpNetworkSession/IOThread, which own the
+  // objects which reference |this|.  Therefore, these raw pointers are valid
+  // for the lifetime of this object.
+  HttpAuthCache* const http_auth_cache_;
+  HttpAuthHandlerFactory* const http_auth_handler_factory_;
 
   std::set<std::string> disabled_schemes_;
 
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 391c511..e04ed1d 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -619,8 +619,11 @@ int HttpNetworkTransaction::DoGenerateProxyAuthToken() {
     return OK;
   HttpAuth::Target target = HttpAuth::AUTH_PROXY;
   if (!auth_controllers_[target].get())
-    auth_controllers_[target] = new HttpAuthController(target, AuthURL(target),
-                                                       session_);
+    auth_controllers_[target] =
+        new HttpAuthController(target,
+                               AuthURL(target),
+                               session_->auth_cache(),
+                               session_->http_auth_handler_factory());
   return auth_controllers_[target]->MaybeGenerateAuthToken(request_,
                                                            &io_callback_,
                                                            net_log_);
@@ -637,8 +640,11 @@ int HttpNetworkTransaction::DoGenerateServerAuthToken() {
   next_state_ = STATE_GENERATE_SERVER_AUTH_TOKEN_COMPLETE;
   HttpAuth::Target target = HttpAuth::AUTH_SERVER;
   if (!auth_controllers_[target].get())
-    auth_controllers_[target] = new HttpAuthController(target, AuthURL(target),
-                                                       session_);
+    auth_controllers_[target] =
+        new HttpAuthController(target,
+                               AuthURL(target),
+                               session_->auth_cache(),
+                               session_->http_auth_handler_factory());
   if (!ShouldApplyServerAuth())
     return OK;
   return auth_controllers_[target]->MaybeGenerateAuthToken(request_,
diff --git a/net/http/http_proxy_client_socket.cc b/net/http/http_proxy_client_socket.cc
index 7d768d2..700f8b1 100644
--- a/net/http/http_proxy_client_socket.cc
+++ b/net/http/http_proxy_client_socket.cc
@@ -50,10 +50,14 @@ void BuildTunnelRequest(const HttpRequestInfo* request_info,
 }  // namespace
 
 HttpProxyClientSocket::HttpProxyClientSocket(
-    ClientSocketHandle* transport_socket, const GURL& request_url,
-    const std::string& user_agent, const HostPortPair& endpoint,
+    ClientSocketHandle* transport_socket,
+    const GURL& request_url,
+    const std::string& user_agent,
+    const HostPortPair& endpoint,
     const HostPortPair& proxy_server,
-    const scoped_refptr<HttpNetworkSession>& session, bool tunnel,
+    HttpAuthCache* http_auth_cache,
+    HttpAuthHandlerFactory* http_auth_handler_factory,
+    bool tunnel,
     bool using_spdy)
     : ALLOW_THIS_IN_INITIALIZER_LIST(
           io_callback_(this, &HttpProxyClientSocket::OnIOComplete)),
@@ -64,7 +68,9 @@ HttpProxyClientSocket::HttpProxyClientSocket(
       auth_(tunnel ?
           new HttpAuthController(HttpAuth::AUTH_PROXY,
                                  GURL("http://" + proxy_server.ToString()),
-                                 session) : NULL),
+                                 http_auth_cache,
+                                 http_auth_handler_factory)
+          : NULL),
       tunnel_(tunnel),
       using_spdy_(using_spdy),
       net_log_(transport_socket->socket()->NetLog()) {
diff --git a/net/http/http_proxy_client_socket.h b/net/http/http_proxy_client_socket.h
index 4702118..e80f697 100644
--- a/net/http/http_proxy_client_socket.h
+++ b/net/http/http_proxy_client_socket.h
@@ -25,6 +25,8 @@ namespace net {
 
 class AddressList;
 class ClientSocketHandle;
+class HttpAuthCache;
+class HttpAuthHandleFactory;
 class HttpStream;
 class IOBuffer;
 
@@ -38,7 +40,8 @@ class HttpProxyClientSocket : public ClientSocket {
                         const std::string& user_agent,
                         const HostPortPair& endpoint,
                         const HostPortPair& proxy_server,
-                        const scoped_refptr<HttpNetworkSession>& session,
+                        HttpAuthCache* http_auth_cache,
+                        HttpAuthHandlerFactory* http_auth_handler_factory,
                         bool tunnel,
                         bool using_spdy);
 
diff --git a/net/http/http_proxy_client_socket_pool.cc b/net/http/http_proxy_client_socket_pool.cc
index 487656b..047ee00 100644
--- a/net/http/http_proxy_client_socket_pool.cc
+++ b/net/http/http_proxy_client_socket_pool.cc
@@ -27,14 +27,16 @@ HttpProxySocketParams::HttpProxySocketParams(
     const GURL& request_url,
     const std::string& user_agent,
     HostPortPair endpoint,
-    scoped_refptr<HttpNetworkSession> session,
+    HttpAuthCache* http_auth_cache,
+    HttpAuthHandlerFactory* http_auth_handler_factory,
     bool tunnel)
     : tcp_params_(tcp_params),
       ssl_params_(ssl_params),
       request_url_(request_url),
       user_agent_(user_agent),
       endpoint_(endpoint),
-      session_(tunnel ? session : NULL),
+      http_auth_cache_(tunnel ? http_auth_cache : NULL),
+      http_auth_handler_factory_(tunnel ? http_auth_handler_factory : NULL),
       tunnel_(tunnel) {
   DCHECK((tcp_params == NULL && ssl_params != NULL) ||
          (tcp_params != NULL && ssl_params == NULL));
@@ -211,7 +213,9 @@ int HttpProxyConnectJob::DoHttpProxyConnect() {
                                 params_->request_url(),
                                 params_->user_agent(),
                                 params_->endpoint(),
-                                proxy_server, params_->session(),
+                                proxy_server,
+                                params_->http_auth_cache(),
+                                params_->http_auth_handler_factory(),
                                 params_->tunnel(),
                                 using_spdy_));
   int result = transport_socket_->Connect(&callback_);
diff --git a/net/http/http_proxy_client_socket_pool.h b/net/http/http_proxy_client_socket_pool.h
index c041c31..7dc8bdb 100644
--- a/net/http/http_proxy_client_socket_pool.h
+++ b/net/http/http_proxy_client_socket_pool.h
@@ -21,7 +21,8 @@
 namespace net {
 
 class HostResolver;
-class HttpNetworkSession;
+class HttpAuthCache;
+class HttpAuthHandlerFactory;
 class SSLClientSocketPool;
 class SSLSocketParams;
 class TCPClientSocketPool;
@@ -38,7 +39,8 @@ class HttpProxySocketParams : public base::RefCounted<HttpProxySocketParams> {
                         const GURL& request_url,
                         const std::string& user_agent,
                         HostPortPair endpoint,
-                        scoped_refptr<HttpNetworkSession> session,
+                        HttpAuthCache* http_auth_cache,
+                        HttpAuthHandlerFactory* http_auth_handler_factory,
                         bool tunnel);
 
   const scoped_refptr<TCPSocketParams>& tcp_params() const {
@@ -50,8 +52,9 @@ class HttpProxySocketParams : public base::RefCounted<HttpProxySocketParams> {
   const GURL& request_url() const { return request_url_; }
   const std::string& user_agent() const { return user_agent_; }
   const HostPortPair& endpoint() const { return endpoint_; }
-  const scoped_refptr<HttpNetworkSession>& session() {
-    return session_;
+  HttpAuthCache* http_auth_cache() const { return http_auth_cache_; }
+  HttpAuthHandlerFactory* http_auth_handler_factory() const {
+    return http_auth_handler_factory_;
   }
   const HostResolver::RequestInfo& destination() const;
   bool tunnel() const { return tunnel_; }
@@ -65,7 +68,8 @@ class HttpProxySocketParams : public base::RefCounted<HttpProxySocketParams> {
   const GURL request_url_;
   const std::string user_agent_;
   const HostPortPair endpoint_;
-  const scoped_refptr<HttpNetworkSession> session_;
+  HttpAuthCache* const http_auth_cache_;
+  HttpAuthHandlerFactory* const http_auth_handler_factory_;
   const bool tunnel_;
 
   DISALLOW_COPY_AND_ASSIGN(HttpProxySocketParams);
diff --git a/net/http/http_proxy_client_socket_pool_unittest.cc b/net/http/http_proxy_client_socket_pool_unittest.cc
index b375c7e..0a27d62 100644
--- a/net/http/http_proxy_client_socket_pool_unittest.cc
+++ b/net/http/http_proxy_client_socket_pool_unittest.cc
@@ -88,9 +88,16 @@ class HttpProxyClientSocketPoolTest : public TestWithHttpParam {
   // Returns the a correctly constructed HttpProxyParms
   // for the HTTP or HTTPS proxy.
   scoped_refptr<HttpProxySocketParams> GetParams(bool tunnel) {
-    return scoped_refptr<HttpProxySocketParams>(new HttpProxySocketParams(
-        GetTcpParams(), GetSslParams(), GURL("http://host/"), "",
-        HostPortPair("host", 80), session_, tunnel));
+    return scoped_refptr<HttpProxySocketParams>(
+        new HttpProxySocketParams(
+            GetTcpParams(),
+            GetSslParams(),
+            GURL("http://host/"),
+            "",
+            HostPortPair("host", 80),
+            session_->auth_cache(),
+            session_->http_auth_handler_factory(),
+            tunnel));
   }
 
   scoped_refptr<HttpProxySocketParams> GetTunnelParams() {
diff --git a/net/http/http_stream_request.cc b/net/http/http_stream_request.cc
index 64110a7..cbc2188 100644
--- a/net/http/http_stream_request.cc
+++ b/net/http/http_stream_request.cc
@@ -498,12 +498,15 @@ int HttpStreamRequest::DoInitConnection() {
         proxy_tcp_params = NULL;
       }
 
-      http_proxy_params = new HttpProxySocketParams(proxy_tcp_params,
-                                                    ssl_params,
-                                                    authentication_url,
-                                                    user_agent,
-                                                    endpoint_,
-                                                    session_, using_ssl_);
+      http_proxy_params =
+          new HttpProxySocketParams(proxy_tcp_params,
+                                    ssl_params,
+                                    authentication_url,
+                                    user_agent,
+                                    endpoint_,
+                                    session_->auth_cache(),
+                                    session_->http_auth_handler_factory(),
+                                    using_ssl_);
     } else {
       DCHECK(proxy_info()->is_socks());
       char socks_version;