Reverting 26419.

Looks like this CL caused failure in Modules Linux (valgrind)

http://chrome-buildbot.corp.google.com:8010/builders/Modules%20Linux%20(valgrind)/builds/1104/steps/valgrind%20test:%20base/logs/stdio

http://chrome-buildbot.corp.google.com:8010/builders/Modules%20Linux%20(valgrind)/builds/1103/steps/valgrind%20test:%20base/logs/stdio

[----------] 6 tests from ClipboardTest
[ RUN      ] ClipboardTest.ClearTest

command timed out: 1200 seconds without output, killing pid 7479
process killed by signal 9
program finished with exit code -1

TBR=phajdan.jr


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

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

3 files changed, 86 insertions, 102 deletions

diff --git a/net/ftp/ftp_auth_cache.cc b/net/ftp/ftp_auth_cache.cc
index d3bff90..c29146a 100644
--- a/net/ftp/ftp_auth_cache.cc
+++ b/net/ftp/ftp_auth_cache.cc
@@ -12,25 +12,20 @@ namespace net {
 // static
 const size_t FtpAuthCache::kMaxEntries = 10;
 
-FtpAuthCache::Entry* FtpAuthCache::Lookup(const GURL& origin) {
-  for (EntryList::iterator it = entries_.begin(); it != entries_.end(); ++it) {
-    if (it->origin == origin)
-      return &(*it);
-  }
-  return NULL;
+AuthData* FtpAuthCache::Lookup(const GURL& origin) {
+  Entry* entry = LookupEntry(origin);
+  return (entry ? entry->auth_data : NULL);
 }
 
-void FtpAuthCache::Add(const GURL& origin, const std::wstring& username,
-                       const std::wstring& password) {
+void FtpAuthCache::Add(const GURL& origin, AuthData* auth_data) {
   DCHECK(origin.SchemeIs("ftp"));
   DCHECK_EQ(origin.GetOrigin(), origin);
 
-  Entry* entry = Lookup(origin);
+  Entry* entry = LookupEntry(origin);
   if (entry) {
-    entry->username = username;
-    entry->password = password;
+    entry->auth_data = auth_data;
   } else {
-    entries_.push_front(Entry(origin, username, password));
+    entries_.push_front(Entry(origin, auth_data));
 
     // Prevent unbound memory growth of the cache.
     if (entries_.size() > kMaxEntries)
@@ -38,16 +33,22 @@ void FtpAuthCache::Add(const GURL& origin, const std::wstring& username,
   }
 }
 
-void FtpAuthCache::Remove(const GURL& origin, const std::wstring& username,
-                          const std::wstring& password) {
+void FtpAuthCache::Remove(const GURL& origin) {
   for (EntryList::iterator it = entries_.begin(); it != entries_.end(); ++it) {
-    if (it->origin == origin && it->username == username &&
-        it->password == password) {
+    if (it->origin == origin) {
       entries_.erase(it);
-      DCHECK(!Lookup(origin));
+      DCHECK(!LookupEntry(origin));
       return;
     }
   }
 }
 
+FtpAuthCache::Entry* FtpAuthCache::LookupEntry(const GURL& origin) {
+  for (EntryList::iterator it = entries_.begin(); it != entries_.end(); ++it) {
+    if (it->origin == origin)
+      return &(*it);
+  }
+  return NULL;
+}
+
 }  // namespace net
diff --git a/net/ftp/ftp_auth_cache.h b/net/ftp/ftp_auth_cache.h
index fc68d8a..1a5b088 100644
--- a/net/ftp/ftp_auth_cache.h
+++ b/net/ftp/ftp_auth_cache.h
@@ -6,9 +6,9 @@
 #define NET_FTP_FTP_AUTH_CACHE_H_
 
 #include <list>
-#include <string>
 
 #include "googleurl/src/gurl.h"
+#include "net/base/auth.h"
 
 namespace net {
 
@@ -25,40 +25,36 @@ class FtpAuthCache {
   // Maximum number of entries we allow in the cache.
   static const size_t kMaxEntries;
 
-  struct Entry {
-    Entry(const GURL& origin,
-          const std::wstring& username,
-          const std::wstring& password)
-        : origin(origin),
-          username(username),
-          password(password) {
-    }
-
-    const GURL origin;
-    std::wstring username;
-    std::wstring password;
-  };
-
   FtpAuthCache() {}
   ~FtpAuthCache() {}
 
-  // Return Entry corresponding to given |origin| or NULL if not found.
-  Entry* Lookup(const GURL& origin);
+  // Check if we have authentication data for ftp server at |origin|.
+  // Returns the address of corresponding AuthData object (if found) or NULL
+  // (if not found).
+  AuthData* Lookup(const GURL& origin);
 
-  // Add an entry for |origin| to the cache (consisting of |username| and
-  // |password|). If there is already an entry for |origin|, it will be
-  // overwritten.
-  void Add(const GURL& origin, const std::wstring& username,
-           const std::wstring& password);
+  // Add an entry for |origin| to the cache. If there is already an
+  // entry for |origin|, it will be overwritten. Both parameters are IN only.
+  void Add(const GURL& origin, AuthData* auth_data);
 
-  // Remove the entry for |origin| from the cache, if one exists and matches
-  // |username| and |password|.
-  void Remove(const GURL& origin, const std::wstring& username,
-              const std::wstring& password);
+  // Remove the entry for |origin| from the cache, if one exists.
+  void Remove(const GURL& origin);
 
  private:
+  struct Entry {
+    Entry(const GURL& origin, AuthData* auth_data)
+        : origin(origin),
+          auth_data(auth_data) {
+    }
+
+    const GURL origin;
+    scoped_refptr<AuthData> auth_data;
+  };
   typedef std::list<Entry> EntryList;
 
+  // Return Entry corresponding to given |origin| or NULL if not found.
+  Entry* LookupEntry(const GURL& origin);
+
   // Internal representation of cache, an STL list. This makes lookups O(n),
   // but we expect n to be very low.
   EntryList entries_;
diff --git a/net/ftp/ftp_auth_cache_unittest.cc b/net/ftp/ftp_auth_cache_unittest.cc
index 3f04d96..f74762b 100644
--- a/net/ftp/ftp_auth_cache_unittest.cc
+++ b/net/ftp/ftp_auth_cache_unittest.cc
@@ -8,51 +8,47 @@
 #include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using net::AuthData;
 using net::FtpAuthCache;
 
 TEST(FtpAuthCacheTest, LookupAddRemove) {
   FtpAuthCache cache;
 
   GURL origin1("ftp://foo1");
+  scoped_refptr<AuthData> data1(new AuthData());
+
   GURL origin2("ftp://foo2");
+  scoped_refptr<AuthData> data2(new AuthData());
+
+  GURL origin3("ftp://foo3");
+  scoped_refptr<AuthData> data3(new AuthData());
 
   // Lookup non-existent entry.
   EXPECT_EQ(NULL, cache.Lookup(origin1));
 
   // Add entry for origin1.
-  cache.Add(origin1, L"username1", L"password1");
-  FtpAuthCache::Entry* entry1 = cache.Lookup(origin1);
-  ASSERT_TRUE(entry1);
-  EXPECT_EQ(origin1, entry1->origin);
-  EXPECT_EQ(L"username1", entry1->username);
-  EXPECT_EQ(L"password1", entry1->password);
+  cache.Add(origin1, data1.get());
+  EXPECT_EQ(data1.get(), cache.Lookup(origin1));
 
   // Add an entry for origin2.
-  cache.Add(origin2, L"username2", L"password2");
-  FtpAuthCache::Entry* entry2 = cache.Lookup(origin2);
-  ASSERT_TRUE(entry2);
-  EXPECT_EQ(origin2, entry2->origin);
-  EXPECT_EQ(L"username2", entry2->username);
-  EXPECT_EQ(L"password2", entry2->password);
-
-  // The original entry1 should still be there.
-  EXPECT_EQ(entry1, cache.Lookup(origin1));
+  cache.Add(origin2, data2.get());
+  EXPECT_EQ(data1.get(), cache.Lookup(origin1));
+  EXPECT_EQ(data2.get(), cache.Lookup(origin2));
 
   // Overwrite the entry for origin1.
-  cache.Add(origin1, L"username3", L"password3");
-  FtpAuthCache::Entry* entry3 = cache.Lookup(origin1);
-  ASSERT_TRUE(entry3);
-  EXPECT_EQ(origin1, entry3->origin);
-  EXPECT_EQ(L"username3", entry3->username);
-  EXPECT_EQ(L"password3", entry3->password);
+  cache.Add(origin1, data3.get());
+  EXPECT_EQ(data3.get(), cache.Lookup(origin1));
+  EXPECT_EQ(data2.get(), cache.Lookup(origin2));
 
   // Remove entry of origin1.
-  cache.Remove(origin1, L"username3", L"password3");
+  cache.Remove(origin1);
   EXPECT_EQ(NULL, cache.Lookup(origin1));
+  EXPECT_EQ(data2.get(), cache.Lookup(origin2));
 
-  // Remove non-existent entry.
-  cache.Remove(origin1, L"username3", L"password3");
+  // Remove non-existent entry
+  cache.Remove(origin1);
   EXPECT_EQ(NULL, cache.Lookup(origin1));
+  EXPECT_EQ(data2.get(), cache.Lookup(origin2));
 }
 
 // Check that if the origin differs only by port number, it is considered
@@ -61,12 +57,16 @@ TEST(FtpAuthCacheTest, LookupWithPort) {
   FtpAuthCache cache;
 
   GURL origin1("ftp://foo:80");
+  scoped_refptr<AuthData> data1(new AuthData());
+
   GURL origin2("ftp://foo:21");
+  scoped_refptr<AuthData> data2(new AuthData());
 
-  cache.Add(origin1, L"username", L"password");
-  cache.Add(origin2, L"username", L"password");
+  cache.Add(origin1, data1.get());
+  cache.Add(origin2, data2.get());
 
-  EXPECT_NE(cache.Lookup(origin1), cache.Lookup(origin2));
+  EXPECT_EQ(data1.get(), cache.Lookup(origin1));
+  EXPECT_EQ(data2.get(), cache.Lookup(origin2));
 }
 
 TEST(FtpAuthCacheTest, NormalizedKey) {
@@ -76,61 +76,48 @@ TEST(FtpAuthCacheTest, NormalizedKey) {
 
   FtpAuthCache cache;
 
+  scoped_refptr<AuthData> data1(new AuthData());
+  scoped_refptr<AuthData> data2(new AuthData());
+
   // Add.
-  cache.Add(GURL("ftp://HoSt:21"), L"username", L"password");
+  cache.Add(GURL("ftp://HoSt:21"), data1.get());
 
   // Lookup.
-  FtpAuthCache::Entry* entry1 = cache.Lookup(GURL("ftp://HoSt:21"));
-  ASSERT_TRUE(entry1);
-  EXPECT_EQ(entry1, cache.Lookup(GURL("ftp://host:21")));
-  EXPECT_EQ(entry1, cache.Lookup(GURL("ftp://host")));
+  EXPECT_EQ(data1.get(), cache.Lookup(GURL("ftp://HoSt:21")));
+  EXPECT_EQ(data1.get(), cache.Lookup(GURL("ftp://host:21")));
+  EXPECT_EQ(data1.get(), cache.Lookup(GURL("ftp://host")));
 
   // Overwrite.
-  cache.Add(GURL("ftp://host"), L"othername", L"otherword");
-  FtpAuthCache::Entry* entry2 = cache.Lookup(GURL("ftp://HoSt:21"));
-  ASSERT_TRUE(entry2);
-  EXPECT_EQ(GURL("ftp://host"), entry2->origin);
-  EXPECT_EQ(L"othername", entry2->username);
-  EXPECT_EQ(L"otherword", entry2->password);
+  cache.Add(GURL("ftp://host"), data2.get());
+  EXPECT_EQ(data2.get(), cache.Lookup(GURL("ftp://HoSt:21")));
 
   // Remove
-  cache.Remove(GURL("ftp://HOsT"), L"othername", L"otherword");
-  EXPECT_EQ(NULL, cache.Lookup(GURL("ftp://host")));
-}
-
-TEST(FtpAuthCacheTest, OnlyRemoveMatching) {
-  FtpAuthCache cache;
-
-  cache.Add(GURL("ftp://host"), L"username", L"password");
-  EXPECT_TRUE(cache.Lookup(GURL("ftp://host")));
-
-  // Auth data doesn't match, shouldn't remove.
-  cache.Remove(GURL("ftp://host"), L"bogus", L"bogus");
-  EXPECT_TRUE(cache.Lookup(GURL("ftp://host")));
-
-  // Auth data matches, should remove.
-  cache.Remove(GURL("ftp://host"), L"username", L"password");
+  cache.Remove(GURL("ftp://HOsT"));
   EXPECT_EQ(NULL, cache.Lookup(GURL("ftp://host")));
 }
 
 TEST(FtpAuthCacheTest, EvictOldEntries) {
   FtpAuthCache cache;
 
+  scoped_refptr<AuthData> auth_data(new AuthData());
+
   for (size_t i = 0; i < FtpAuthCache::kMaxEntries; i++)
-    cache.Add(GURL("ftp://host" + IntToString(i)), L"username", L"password");
+    cache.Add(GURL("ftp://host" + IntToString(i)), auth_data.get());
 
   // No entries should be evicted before reaching the limit.
   for (size_t i = 0; i < FtpAuthCache::kMaxEntries; i++) {
-    EXPECT_TRUE(cache.Lookup(GURL("ftp://host" + IntToString(i))));
+    EXPECT_EQ(auth_data.get(),
+              cache.Lookup(GURL("ftp://host" + IntToString(i))));
   }
 
   // Adding one entry should cause eviction of the first entry.
-  cache.Add(GURL("ftp://last_host"), L"username", L"password");
+  cache.Add(GURL("ftp://last_host"), auth_data.get());
   EXPECT_EQ(NULL, cache.Lookup(GURL("ftp://host0")));
 
   // Remaining entries should not get evicted.
   for (size_t i = 1; i < FtpAuthCache::kMaxEntries; i++) {
-    EXPECT_TRUE(cache.Lookup(GURL("ftp://host" + IntToString(i))));
+    EXPECT_EQ(auth_data.get(),
+              cache.Lookup(GURL("ftp://host" + IntToString(i))));
   }
-  EXPECT_TRUE(cache.Lookup(GURL("ftp://last_host")));
+  EXPECT_EQ(auth_data.get(), cache.Lookup(GURL("ftp://last_host")));
 }