HttpAuthHandler's are no longer refcounted.

Since HttpAuthHandler objects are no longer contained inside of the
HttpAuthCache, the lifetime of the handlers is more clearly defined.

TEST=net_unittests (including some changes)
BUG=42222

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

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

net/http/http_auth.cc

@@ -25,12 +25,12 @@ void HttpAuth::ChooseBestChallenge(
     Target target,
     const GURL& origin,
     const BoundNetLog& net_log,
-    scoped_refptr<HttpAuthHandler>* handler) {
+    scoped_ptr<HttpAuthHandler>* handler) {
   DCHECK(http_auth_handler_factory);
 
   // A connection-based authentication scheme must continue to use the
   // existing handler object in |*handler|.
-  if (*handler && (*handler)->is_connection_based()) {
+  if (handler->get() && (*handler)->is_connection_based()) {
     const std::string header_name = GetChallengeHeaderName(target);
     std::string challenge;
     void* iter = NULL;
@@ -43,20 +43,20 @@ void HttpAuth::ChooseBestChallenge(
   }
 
   // Choose the challenge whose authentication handler gives the maximum score.
-  scoped_refptr<HttpAuthHandler> best;
+  scoped_ptr<HttpAuthHandler> best;
   const std::string header_name = GetChallengeHeaderName(target);
   std::string cur_challenge;
   void* iter = NULL;
   while (headers->EnumerateHeader(&iter, header_name, &cur_challenge)) {
-    scoped_refptr<HttpAuthHandler> cur;
+    scoped_ptr<HttpAuthHandler> cur;
     int rv = http_auth_handler_factory->CreateAuthHandlerFromString(
         cur_challenge, target, origin, net_log, &cur);
     if (rv != OK) {
       LOG(WARNING) << "Unable to create AuthHandler. Status: "
                    << ErrorToString(rv) << " Challenge: " << cur_challenge;
       continue;
     }
-    if (cur && (!best || best->score() < cur->score()))
+    if (cur.get() && (!best.get() || best->score() < cur->score()))
       best.swap(cur);
   }
   handler->swap(best);

net/http/http_auth.h

@@ -5,6 +5,7 @@
 #ifndef NET_HTTP_HTTP_AUTH_H_
 #define NET_HTTP_HTTP_AUTH_H_
 
+#include "base/scoped_ptr.h"
 #include "net/http/http_util.h"
 
 template <class T> class scoped_refptr;
@@ -95,7 +96,7 @@ class HttpAuth {
       Target target,
       const GURL& origin,
       const BoundNetLog& net_log,
-      scoped_refptr<HttpAuthHandler>* handler);
+      scoped_ptr<HttpAuthHandler>* handler);
 
   // ChallengeTokenizer breaks up a challenge string into the the auth scheme
   // and parameter list, according to RFC 2617 Sec 1.2:

net/http/http_auth_cache_unittest.cc

@@ -60,26 +60,26 @@ TEST(HttpAuthCacheTest, Basic) {
 
   // Add cache entries for 3 realms: "Realm1", "Realm2", "Realm3"
 
-  scoped_refptr<HttpAuthHandler> realm1_handler =
-      new MockAuthHandler("basic", "Realm1", HttpAuth::AUTH_SERVER);
+  scoped_ptr<HttpAuthHandler> realm1_handler(
+      new MockAuthHandler("basic", "Realm1", HttpAuth::AUTH_SERVER));
   cache.Add(origin, realm1_handler->realm(), realm1_handler->scheme(),
             "Basic realm=Realm1", L"realm1-user", L"realm1-password",
             "/foo/bar/index.html");
 
-  scoped_refptr<HttpAuthHandler> realm2_handler =
-      new MockAuthHandler("basic", "Realm2", HttpAuth::AUTH_SERVER);
+  scoped_ptr<HttpAuthHandler> realm2_handler(
+      new MockAuthHandler("basic", "Realm2", HttpAuth::AUTH_SERVER));
   cache.Add(origin, realm2_handler->realm(), realm2_handler->scheme(),
             "Basic realm=Realm2", L"realm2-user", L"realm2-password",
             "/foo2/index.html");
 
-  scoped_refptr<HttpAuthHandler> realm3_basic_handler =
-      new MockAuthHandler("basic", "Realm3", HttpAuth::AUTH_PROXY);
+  scoped_ptr<HttpAuthHandler> realm3_basic_handler(
+      new MockAuthHandler("basic", "Realm3", HttpAuth::AUTH_PROXY));
   cache.Add(origin, realm3_basic_handler->realm(),
             realm3_basic_handler->scheme(), "Basic realm=Realm3",
             L"realm3-basic-user", L"realm3-basic-password", "");
 
-  scoped_refptr<HttpAuthHandler> realm3_digest_handler =
-      new MockAuthHandler("digest", "Realm3", HttpAuth::AUTH_PROXY);
+  scoped_ptr<HttpAuthHandler> realm3_digest_handler(
+      new MockAuthHandler("digest", "Realm3", HttpAuth::AUTH_PROXY));
   cache.Add(origin, realm3_digest_handler->realm(),
             realm3_digest_handler->scheme(), "Digest realm=Realm3",
             L"realm3-digest-user", L"realm3-digest-password",
@@ -217,8 +217,8 @@ TEST(HttpAuthCacheTest, AddToExistingEntry) {
   GURL origin("http://www.foobar.com:70");
   const std::string auth_challenge = "Basic realm=MyRealm";
 
-  scoped_refptr<HttpAuthHandler> handler =
-      new MockAuthHandler("basic", "MyRealm", HttpAuth::AUTH_SERVER);
+  scoped_ptr<HttpAuthHandler> handler(
+      new MockAuthHandler("basic", "MyRealm", HttpAuth::AUTH_SERVER));
 
   HttpAuthCache::Entry* orig_entry = cache.Add(
       origin, handler->realm(), handler->scheme(), auth_challenge,
@@ -242,17 +242,17 @@ TEST(HttpAuthCacheTest, AddToExistingEntry) {
 TEST(HttpAuthCacheTest, Remove) {
   GURL origin("http://foobar2.com");
 
-  scoped_refptr<HttpAuthHandler> realm1_handler =
-      new MockAuthHandler("basic", "Realm1", HttpAuth::AUTH_SERVER);
+  scoped_ptr<HttpAuthHandler> realm1_handler(
+      new MockAuthHandler("basic", "Realm1", HttpAuth::AUTH_SERVER));
 
-  scoped_refptr<HttpAuthHandler> realm2_handler =
-      new MockAuthHandler("basic", "Realm2", HttpAuth::AUTH_SERVER);
+  scoped_ptr<HttpAuthHandler> realm2_handler(
+      new MockAuthHandler("basic", "Realm2", HttpAuth::AUTH_SERVER));
 
-  scoped_refptr<HttpAuthHandler> realm3_basic_handler =
-      new MockAuthHandler("basic", "Realm3", HttpAuth::AUTH_SERVER);
+  scoped_ptr<HttpAuthHandler> realm3_basic_handler(
+      new MockAuthHandler("basic", "Realm3", HttpAuth::AUTH_SERVER));
 
-  scoped_refptr<HttpAuthHandler> realm3_digest_handler =
-      new MockAuthHandler("digest", "Realm3", HttpAuth::AUTH_SERVER);
+  scoped_ptr<HttpAuthHandler> realm3_digest_handler(
+      new MockAuthHandler("digest", "Realm3", HttpAuth::AUTH_SERVER));
 
   HttpAuthCache cache;
   cache.Add(origin, realm1_handler->realm(), realm1_handler->scheme(),
@@ -322,12 +322,8 @@ class HttpAuthCacheEvictionTest : public testing::Test {
   }
 
   void AddPathToRealm(int realm_i, int path_i) {
-    scoped_refptr<HttpAuthHandler> handler = new MockAuthHandler(
-        "basic",
-        GenerateRealm(realm_i), HttpAuth::AUTH_SERVER);
-    std::string path = GeneratePath(realm_i, path_i);
-    cache_.Add(origin_, handler->realm(), handler->scheme(),
-               handler->challenge(), L"username", L"password", path);
+    cache_.Add(origin_, GenerateRealm(realm_i), "basic", "",
+               L"username", L"password", GeneratePath(realm_i, path_i));
   }
 
   void CheckRealmExistence(int realm_i, bool exists) {

net/http/http_auth_handler.h

@@ -7,7 +7,6 @@
 
 #include <string>
 
-#include "base/ref_counted.h"
 #include "net/base/completion_callback.h"
 #include "net/base/net_log.h"
 #include "net/http/http_auth.h"
@@ -21,8 +20,10 @@ struct HttpRequestInfo;
 // HttpAuthHandler is the interface for the authentication schemes
 // (basic, digest, NTLM, Negotiate).
 // HttpAuthHandler objects are typically created by an HttpAuthHandlerFactory.
-class HttpAuthHandler : public base::RefCounted<HttpAuthHandler> {
+class HttpAuthHandler {
  public:
+  virtual ~HttpAuthHandler() {}
+
   // Initializes the handler using a challenge issued by a server.
   // |challenge| must be non-NULL and have already tokenized the
   // authentication scheme, but none of the tokens occuring after the
@@ -130,10 +131,6 @@ class HttpAuthHandler : public base::RefCounted<HttpAuthHandler> {
     IS_CONNECTION_BASED = 1 << 1,
   };
 
-  friend class base::RefCounted<HttpAuthHandler>;
-
-  virtual ~HttpAuthHandler() { }
-
   // Initializes the handler using a challenge issued by a server.
   // |challenge| must be non-NULL and have already tokenized the
   // authentication scheme, but none of the tokens occuring after the

net/http/http_auth_handler_basic.cc

@@ -80,10 +80,10 @@ int HttpAuthHandlerBasic::Factory::CreateAuthHandler(
     CreateReason reason,
     int digest_nonce_count,
     const BoundNetLog& net_log,
-    scoped_refptr<HttpAuthHandler>* handler) {
+    scoped_ptr<HttpAuthHandler>* handler) {
   // TODO(cbentzel): Move towards model of parsing in the factory
   //                 method and only constructing when valid.
-  scoped_refptr<HttpAuthHandler> tmp_handler(new HttpAuthHandlerBasic());
+  scoped_ptr<HttpAuthHandler> tmp_handler(new HttpAuthHandlerBasic());
   if (!tmp_handler->InitFromChallenge(challenge, target, origin, net_log))
     return ERR_INVALID_RESPONSE;
   handler->swap(tmp_handler);

net/http/http_auth_handler_basic.h

@@ -24,7 +24,7 @@ class HttpAuthHandlerBasic : public HttpAuthHandler {
                                   CreateReason reason,
                                   int digest_nonce_count,
                                   const BoundNetLog& net_log,
-                                  scoped_refptr<HttpAuthHandler>* handler);
+                                  scoped_ptr<HttpAuthHandler>* handler);
   };
 
   virtual int GenerateAuthToken(const std::wstring& username,

net/http/http_auth_handler_basic_unittest.cc

@@ -28,7 +28,7 @@ TEST(HttpAuthHandlerBasicTest, GenerateAuthToken) {
   HttpAuthHandlerBasic::Factory factory;
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
     std::string challenge = "Basic realm=\"Atlantis\"";
-    scoped_refptr<HttpAuthHandler> basic = new HttpAuthHandlerBasic;
+    scoped_ptr<HttpAuthHandler> basic;
     EXPECT_EQ(OK, factory.CreateAuthHandlerFromString(
         challenge, HttpAuth::AUTH_SERVER, origin, BoundNetLog(), &basic));
     std::string credentials;
@@ -67,7 +67,7 @@ TEST(HttpAuthHandlerBasicTest, InitFromChallenge) {
   GURL origin("http://www.example.com");
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
     std::string challenge = tests[i].challenge;
-    scoped_refptr<HttpAuthHandler> basic = new HttpAuthHandlerBasic;
+    scoped_ptr<HttpAuthHandler> basic;
     int rv = factory.CreateAuthHandlerFromString(
         challenge, HttpAuth::AUTH_SERVER, origin, BoundNetLog(), &basic);
     EXPECT_EQ(tests[i].expected_rv, rv);

net/http/http_auth_handler_digest.cc

@@ -307,10 +307,10 @@ int HttpAuthHandlerDigest::Factory::CreateAuthHandler(
     CreateReason reason,
     int digest_nonce_count,
     const BoundNetLog& net_log,
-    scoped_refptr<HttpAuthHandler>* handler) {
+    scoped_ptr<HttpAuthHandler>* handler) {
   // TODO(cbentzel): Move towards model of parsing in the factory
   //                 method and only constructing when valid.
-  scoped_refptr<HttpAuthHandler> tmp_handler(
+  scoped_ptr<HttpAuthHandler> tmp_handler(
       new HttpAuthHandlerDigest(digest_nonce_count));
   if (!tmp_handler->InitFromChallenge(challenge, target, origin, net_log))
     return ERR_INVALID_RESPONSE;

net/http/http_auth_handler_digest.h

@@ -27,7 +27,7 @@ class HttpAuthHandlerDigest : public HttpAuthHandler {
                                   CreateReason reason,
                                   int digest_nonce_count,
                                   const BoundNetLog& net_log,
-                                  scoped_refptr<HttpAuthHandler>* handler);
+                                  scoped_ptr<HttpAuthHandler>* handler);
   };
 
   virtual int GenerateAuthToken(const std::wstring& username,

net/http/http_auth_handler_digest_unittest.cc

@@ -5,6 +5,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 #include "base/basictypes.h"
+#include "net/base/net_errors.h"
 #include "net/http/http_auth_handler_digest.h"
 
 namespace net {
@@ -100,14 +101,25 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) {
     }
   };
 
+  GURL origin("http://www.example.com");
+  scoped_ptr<HttpAuthHandlerDigest::Factory> factory(
+      new HttpAuthHandlerDigest::Factory());
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
-    std::string challenge(tests[i].challenge);
-
-    scoped_refptr<HttpAuthHandlerDigest> digest = new HttpAuthHandlerDigest(1);
-    HttpAuth::ChallengeTokenizer tok(challenge.begin(), challenge.end());
-    bool ok = digest->ParseChallenge(&tok);
-
-    EXPECT_EQ(tests[i].parsed_success, ok);
+    scoped_ptr<HttpAuthHandler> handler;
+    int rv = factory->CreateAuthHandlerFromString(tests[i].challenge,
+                                                  HttpAuth::AUTH_SERVER,
+                                                  origin,
+                                                  BoundNetLog(),
+                                                  &handler);
+    if (tests[i].parsed_success) {
+      EXPECT_EQ(OK, rv);
+    } else {
+      EXPECT_NE(OK, rv);
+      continue;
+    }
+    ASSERT_TRUE(handler != NULL);
+    HttpAuthHandlerDigest* digest =
+        static_cast<HttpAuthHandlerDigest*>(handler.get());
     EXPECT_STREQ(tests[i].parsed_realm, digest->realm_.c_str());
     EXPECT_STREQ(tests[i].parsed_nonce, digest->nonce_.c_str());
     EXPECT_STREQ(tests[i].parsed_domain, digest->domain_.c_str());
@@ -250,13 +262,20 @@ TEST(HttpAuthHandlerDigestTest, AssembleCredentials) {
     }
   };
   GURL origin("http://www.example.com");
+  scoped_ptr<HttpAuthHandlerDigest::Factory> factory(
+      new HttpAuthHandlerDigest::Factory());
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
-    scoped_refptr<HttpAuthHandlerDigest> digest = new HttpAuthHandlerDigest(1);
-    std::string challenge = tests[i].challenge;
-    HttpAuth::ChallengeTokenizer tok(challenge.begin(), challenge.end());
-    EXPECT_TRUE(digest->InitFromChallenge(&tok, HttpAuth::AUTH_SERVER, origin,
-                                          BoundNetLog()));
-
+    scoped_ptr<HttpAuthHandler> handler;
+    int rv = factory->CreateAuthHandlerFromString(tests[i].challenge,
+                                                  HttpAuth::AUTH_SERVER,
+                                                  origin,
+                                                  BoundNetLog(),
+                                                  &handler);
+    EXPECT_EQ(OK, rv);
+    ASSERT_TRUE(handler != NULL);
+
+    HttpAuthHandlerDigest* digest =
+        static_cast<HttpAuthHandlerDigest*>(handler.get());
     std::string creds = digest->AssembleCredentials(tests[i].req_method,
                                                     tests[i].req_path,
                                                     tests[i].username,

net/http/http_auth_handler_factory.cc

@@ -20,7 +20,7 @@ int HttpAuthHandlerFactory::CreateAuthHandlerFromString(
     HttpAuth::Target target,
     const GURL& origin,
     const BoundNetLog& net_log,
-    scoped_refptr<HttpAuthHandler>* handler) {
+    scoped_ptr<HttpAuthHandler>* handler) {
   HttpAuth::ChallengeTokenizer props(challenge.begin(), challenge.end());
   return CreateAuthHandler(&props, target, origin, CREATE_CHALLENGE, 1,
                            net_log, handler);
@@ -32,7 +32,7 @@ int HttpAuthHandlerFactory::CreatePreemptiveAuthHandlerFromString(
     const GURL& origin,
     int digest_nonce_count,
     const BoundNetLog& net_log,
-    scoped_refptr<HttpAuthHandler>* handler) {
+    scoped_ptr<HttpAuthHandler>* handler) {
   HttpAuth::ChallengeTokenizer props(challenge.begin(), challenge.end());
   return CreateAuthHandler(&props, target, origin, CREATE_PREEMPTIVE,
                            digest_nonce_count, net_log, handler);
@@ -90,15 +90,15 @@ int HttpAuthHandlerRegistryFactory::CreateAuthHandler(
     CreateReason reason,
     int digest_nonce_count,
     const BoundNetLog& net_log,
-    scoped_refptr<HttpAuthHandler>* handler) {
+    scoped_ptr<HttpAuthHandler>* handler) {
   if (!challenge->valid()) {
-    *handler = NULL;
+    handler->reset();
     return ERR_INVALID_RESPONSE;
   }
   std::string lower_scheme = StringToLowerASCII(challenge->scheme());
   FactoryMap::iterator it = factory_map_.find(lower_scheme);
   if (it == factory_map_.end()) {
-    *handler = NULL;
+    handler->reset();
     return ERR_UNSUPPORTED_AUTH_SCHEME;
   }
   DCHECK(it->second);

net/http/http_auth_handler_factory.h

@@ -74,7 +74,7 @@ class HttpAuthHandlerFactory {
                                 CreateReason create_reason,
                                 int digest_nonce_count,
                                 const BoundNetLog& net_log,
-                                scoped_refptr<HttpAuthHandler>* handler) = 0;
+                                scoped_ptr<HttpAuthHandler>* handler) = 0;
 
   // Creates an HTTP authentication handler based on the authentication
   // challenge string |challenge|.
@@ -85,7 +85,7 @@ class HttpAuthHandlerFactory {
                                   HttpAuth::Target target,
                                   const GURL& origin,
                                   const BoundNetLog& net_log,
-                                  scoped_refptr<HttpAuthHandler>* handler);
+                                  scoped_ptr<HttpAuthHandler>* handler);
 
   // Creates an HTTP authentication handler based on the authentication
   // challenge string |challenge|.
@@ -98,7 +98,7 @@ class HttpAuthHandlerFactory {
       const GURL& origin,
       int digest_nonce_count,
       const BoundNetLog& net_log,
-      scoped_refptr<HttpAuthHandler>* handler);
+      scoped_ptr<HttpAuthHandler>* handler);
 
   // Creates a standard HttpAuthHandlerRegistryFactory. The caller is
   // responsible for deleting the factory.
@@ -149,7 +149,7 @@ class HttpAuthHandlerRegistryFactory : public HttpAuthHandlerFactory {
                                 CreateReason reason,
                                 int digest_nonce_count,
                                 const BoundNetLog& net_log,
-                                scoped_refptr<HttpAuthHandler>* handler);
+                                scoped_ptr<HttpAuthHandler>* handler);
 
  private:
   typedef std::map<std::string, HttpAuthHandlerFactory*> FactoryMap;