Revert 60791 - HttpAuthHandlerFactory::CreateDefault requires a HostResolver argument.

Previously the default registry factory would crash if it saw a Negotiate header when trying to do a CNAME lookup because of a NULL resolver. Since then I've changed HttpAuthHandlerNegotiate to skip the CNAME lookup step when the resolver_ is NULL. 

So, one option is to make CreateDefault accept an optional HostResolver argument. My concern is that since passing in NULL is the easier choice, people will do it and encounter user problems when going through a Negotiate proxy. Hence, I force the correct behavior.

BUG=56819
TEST=net_unittests, try fetch client through a Negotiate authenticating proxy.

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

TBR=cbentzel@chromium.org
Review URL: http://codereview.chromium.org/3413039

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

6 files changed, 11 insertions, 28 deletions

diff --git a/net/http/http_auth_handler_factory.cc b/net/http/http_auth_handler_factory.cc
index 4ad7fe8..c9e0fb9 100644
--- a/net/http/http_auth_handler_factory.cc
+++ b/net/http/http_auth_handler_factory.cc
@@ -39,19 +39,15 @@ int HttpAuthHandlerFactory::CreatePreemptiveAuthHandlerFromString(
 }
 
 // static
-HttpAuthHandlerRegistryFactory* HttpAuthHandlerFactory::CreateDefault(
-    HostResolver* host_resolver) {
-  DCHECK(host_resolver);
+HttpAuthHandlerRegistryFactory* HttpAuthHandlerFactory::CreateDefault() {
   HttpAuthHandlerRegistryFactory* registry_factory =
       new HttpAuthHandlerRegistryFactory();
   registry_factory->RegisterSchemeFactory(
       "basic", new HttpAuthHandlerBasic::Factory());
   registry_factory->RegisterSchemeFactory(
       "digest", new HttpAuthHandlerDigest::Factory());
-  HttpAuthHandlerNegotiate::Factory* negotiate_factory =
-      new HttpAuthHandlerNegotiate::Factory();
-  negotiate_factory->set_host_resolver(host_resolver);
-  registry_factory->RegisterSchemeFactory("negotiate", negotiate_factory);
+  registry_factory->RegisterSchemeFactory(
+      "negotiate", new HttpAuthHandlerNegotiate::Factory());
   registry_factory->RegisterSchemeFactory(
       "ntlm", new HttpAuthHandlerNTLM::Factory());
   return registry_factory;
@@ -93,7 +89,7 @@ HttpAuthHandlerRegistryFactory* HttpAuthHandlerRegistryFactory::Create(
     HttpAuthHandlerNegotiate::Factory* negotiate_factory =
         new HttpAuthHandlerNegotiate::Factory();
     negotiate_factory->set_url_security_manager(security_manager);
-    DCHECK(host_resolver || negotiate_disable_cname_lookup);
+    DCHECK(host_resolver != NULL || negotiate_disable_cname_lookup);
     negotiate_factory->set_host_resolver(host_resolver);
     negotiate_factory->set_disable_cname_lookup(negotiate_disable_cname_lookup);
     negotiate_factory->set_use_port(negotiate_enable_port);
diff --git a/net/http/http_auth_handler_factory.h b/net/http/http_auth_handler_factory.h
index 06d0f83..887c6cd 100644
--- a/net/http/http_auth_handler_factory.h
+++ b/net/http/http_auth_handler_factory.h
@@ -106,11 +106,7 @@ class HttpAuthHandlerFactory {
   // Creates a standard HttpAuthHandlerRegistryFactory. The caller is
   // responsible for deleting the factory.
   // The default factory supports Basic, Digest, NTLM, and Negotiate schemes.
-  //
-  // |host_resolver| is used by the Negotiate authentication handler to perform
-  // CNAME lookups to generate a Kerberos SPN for the server. It must be
-  // non-NULL.
-  static HttpAuthHandlerRegistryFactory* CreateDefault(HostResolver* resolver);
+  static HttpAuthHandlerRegistryFactory* CreateDefault();
 
  private:
   // The URL security manager
diff --git a/net/http/http_auth_handler_factory_unittest.cc b/net/http/http_auth_handler_factory_unittest.cc
index 173e951..f845c8a 100644
--- a/net/http/http_auth_handler_factory_unittest.cc
+++ b/net/http/http_auth_handler_factory_unittest.cc
@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "base/scoped_ptr.h"
-#include "net/base/mock_host_resolver.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_auth_handler.h"
 #include "net/http/http_auth_handler_factory.h"
@@ -96,10 +95,9 @@ TEST(HttpAuthHandlerFactoryTest, RegistryFactory) {
 }
 
 TEST(HttpAuthHandlerFactoryTest, DefaultFactory) {
-  scoped_refptr<HostResolver> host_resolver(new MockHostResolver());
   URLSecurityManagerAllow url_security_manager;
   scoped_ptr<HttpAuthHandlerRegistryFactory> http_auth_handler_factory(
-      HttpAuthHandlerFactory::CreateDefault(host_resolver));
+      HttpAuthHandlerFactory::CreateDefault());
   http_auth_handler_factory->SetURLSecurityManager(
       "negotiate", &url_security_manager);
   GURL server_origin("http://www.example.com");
diff --git a/net/http/http_auth_unittest.cc b/net/http/http_auth_unittest.cc
index cc62214..a3f2b28 100644
--- a/net/http/http_auth_unittest.cc
+++ b/net/http/http_auth_unittest.cc
@@ -8,7 +8,6 @@
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
-#include "net/base/mock_host_resolver.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_auth.h"
 #include "net/http/http_auth_filter.h"
@@ -98,9 +97,8 @@ TEST(HttpAuthTest, ChooseBestChallenge) {
   GURL origin("http://www.example.com");
   std::set<std::string> disabled_schemes;
   URLSecurityManagerAllow url_security_manager;
-  scoped_refptr<HostResolver> host_resolver(new MockHostResolver());
   scoped_ptr<HttpAuthHandlerRegistryFactory> http_auth_handler_factory(
-      HttpAuthHandlerFactory::CreateDefault(host_resolver));
+      HttpAuthHandlerFactory::CreateDefault());
   http_auth_handler_factory->SetURLSecurityManager(
       "negotiate", &url_security_manager);
 
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index b04a0a5..2bcd753 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -112,8 +112,7 @@ struct SessionDependencies {
       : host_resolver(new MockHostResolver),
         proxy_service(ProxyService::CreateDirect()),
         ssl_config_service(new SSLConfigServiceDefaults),
-        http_auth_handler_factory(
-            HttpAuthHandlerFactory::CreateDefault(host_resolver)),
+        http_auth_handler_factory(HttpAuthHandlerFactory::CreateDefault()),
         spdy_session_pool(new SpdySessionPool(NULL)),
         net_log(NULL) {}
 
@@ -122,8 +121,7 @@ struct SessionDependencies {
       : host_resolver(new MockHostResolver),
         proxy_service(proxy_service),
         ssl_config_service(new SSLConfigServiceDefaults),
-        http_auth_handler_factory(
-            HttpAuthHandlerFactory::CreateDefault(host_resolver)),
+        http_auth_handler_factory(HttpAuthHandlerFactory::CreateDefault()),
         spdy_session_pool(new SpdySessionPool(NULL)),
         net_log(NULL) {}
 
diff --git a/net/http/http_proxy_client_socket_pool_unittest.cc b/net/http/http_proxy_client_socket_pool_unittest.cc
index 6d03c19..0a27d62 100644
--- a/net/http/http_proxy_client_socket_pool_unittest.cc
+++ b/net/http/http_proxy_client_socket_pool_unittest.cc
@@ -50,10 +50,8 @@ class HttpProxyClientSocketPoolTest : public TestWithHttpParam {
             kMaxSocketsPerGroup, tcp_histograms_, &tcp_client_socket_factory_)),
         ssl_socket_pool_(new MockSSLClientSocketPool(kMaxSockets,
             kMaxSocketsPerGroup, tcp_histograms_, &tcp_client_socket_factory_)),
-        host_resolver_(new MockHostResolver),
-        http_auth_handler_factory_(
-            HttpAuthHandlerFactory::CreateDefault(host_resolver_)),
-        session_(new HttpNetworkSession(host_resolver_,
+        http_auth_handler_factory_(HttpAuthHandlerFactory::CreateDefault()),
+        session_(new HttpNetworkSession(new MockHostResolver,
                                         ProxyService::CreateDirect(),
                                         &socket_factory_,
                                         new SSLConfigServiceDefaults,
@@ -120,7 +118,6 @@ class HttpProxyClientSocketPoolTest : public TestWithHttpParam {
   scoped_refptr<MockSSLClientSocketPool> ssl_socket_pool_;
 
   MockClientSocketFactory socket_factory_;
-  scoped_refptr<HostResolver> host_resolver_;
   scoped_ptr<HttpAuthHandlerFactory> http_auth_handler_factory_;
   scoped_refptr<HttpNetworkSession> session_;
   scoped_refptr<ClientSocketPoolHistograms> http_proxy_histograms_;