Changed type CookieList to being a vector CanonicalCookies.

Originally, it was a vector of pair<domain_string, CanonicalCookie>.

TEST=Refactor; all relevant unit tests should still pass.
BUG=8850

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

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

4 files changed, 81 insertions, 87 deletions

diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index 183ca96..e79cf47 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -268,11 +268,11 @@ int CookieMonster::TrimDuplicateCookiesForHost(
     CookieMap::iterator end) {
   lock_.AssertAcquired();
 
-  // List of cookies ordered by creation time.
-  typedef std::set<CookieMap::iterator, OrderByCreationTimeDesc> CookieList;
+  // Set of cookies ordered by creation time.
+  typedef std::set<CookieMap::iterator, OrderByCreationTimeDesc> CookieSet;
 
   // Helper map we populate to find the duplicates.
-  typedef std::map<CookieSignature, CookieList> EquivalenceMap;
+  typedef std::map<CookieSignature, CookieSet> EquivalenceMap;
   EquivalenceMap equivalent_cookies;
 
   // The number of duplicate cookies that have been found.
@@ -286,7 +286,7 @@ int CookieMonster::TrimDuplicateCookiesForHost(
 
     CookieSignature signature(cookie->Name(), cookie->Domain(),
                               cookie->Path());
-    CookieList& list = equivalent_cookies[signature];
+    CookieSet& list = equivalent_cookies[signature];
 
     // We found a duplicate!
     if (!list.empty())
@@ -307,7 +307,7 @@ int CookieMonster::TrimDuplicateCookiesForHost(
        it != equivalent_cookies.end();
        ++it) {
     const CookieSignature& signature = it->first;
-    CookieList& dupes = it->second;
+    CookieSet& dupes = it->second;
 
     if (dupes.size() <= 1)
       continue;  // This cookiename/path has no duplicates.
@@ -327,7 +327,7 @@ int CookieMonster::TrimDuplicateCookiesForHost(
     // Remove all the cookies identified by |dupes|. It is valid to delete our
     // list of iterators one at a time, since |cookies_| is a multimap (they
     // don't invalidate existing iterators following deletion).
-    for (CookieList::iterator dupes_it = dupes.begin();
+    for (CookieSet::iterator dupes_it = dupes.begin();
          dupes_it != dupes.end();
          ++dupes_it) {
       InternalDeleteCookie(*dupes_it, true /*sync_to_store*/,
@@ -775,7 +775,7 @@ void CookieMonster::InternalInsertCookie(const std::string& key,
     store_->AddCookie(key, *cc);
   cookies_.insert(CookieMap::value_type(key, cc));
   if (delegate_.get())
-    delegate_->OnCookieChanged(key, *cc, false);
+    delegate_->OnCookieChanged(*cc, false);
 }
 
 void CookieMonster::InternalUpdateCookieAccessTime(CanonicalCookie* cc) {
@@ -811,7 +811,7 @@ void CookieMonster::InternalDeleteCookie(CookieMap::iterator it,
   if (cc->IsPersistent() && store_ && sync_to_store)
     store_->DeleteCookie(*cc);
   if (delegate_.get())
-    delegate_->OnCookieChanged(it->first, *cc, true);
+    delegate_->OnCookieChanged(*cc, true);
   cookies_.erase(it);
   delete cc;
 }
@@ -1129,7 +1129,7 @@ CookieMonster::CookieList CookieMonster::GetAllCookies() {
 
   CookieList cookie_list;
   for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end(); ++it)
-    cookie_list.push_back(CookieListPair(it->first, *it->second));
+    cookie_list.push_back(*it->second);
 
   return cookie_list;
 }
@@ -1231,7 +1231,7 @@ void CookieMonster::FindRawCookies(const std::string& key,
       continue;
     if (!cc->IsOnPath(path))
       continue;
-    list->push_back(CookieListPair(key, *cc));
+    list->push_back(*cc);
   }
 }
 
@@ -1682,3 +1682,4 @@ std::string CookieMonster::CanonicalCookie::DebugString() const {
 }
 
 }  // namespace
+
diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h
index 96d98b3..623363f 100644
--- a/net/base/cookie_monster.h
+++ b/net/base/cookie_monster.h
@@ -51,8 +51,7 @@ class CookieMonster : public CookieStore {
   typedef std::multimap<std::string, CanonicalCookie*> CookieMap;
   typedef std::pair<CookieMap::iterator, CookieMap::iterator> CookieMapItPair;
   typedef std::pair<std::string, CanonicalCookie*> KeyedCanonicalCookie;
-  typedef std::pair<std::string, CanonicalCookie> CookieListPair;
-  typedef std::vector<CookieListPair> CookieList;
+  typedef std::vector<CanonicalCookie> CookieList;
 
   // The store passed in should not have had Init() called on it yet. This
   // class will take care of initializing it. The backing store is NOT owned by
@@ -430,10 +429,9 @@ class CookieMonster::Delegate
     : public base::RefCountedThreadSafe<CookieMonster::Delegate> {
  public:
   // Will be called when a cookie is added or removed. The function is passed
-  // the respective |cookie| which was added to or removed from the cookies for
-  // |domain_key|. If |removed| is true, the cookie was deleted.
-  virtual void OnCookieChanged(const std::string& domain_key,
-                               const CookieMonster::CanonicalCookie& cookie,
+  // the respective |cookie| which was added to or removed from the cookies.
+  // If |removed| is true, the cookie was deleted.
+  virtual void OnCookieChanged(const CookieMonster::CanonicalCookie& cookie,
                                bool removed) = 0;
  protected:
   friend class base::RefCountedThreadSafe<CookieMonster::Delegate>;
diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc
index e9ffc7b..c61a782 100644
--- a/net/base/cookie_monster_unittest.cc
+++ b/net/base/cookie_monster_unittest.cc
@@ -103,18 +103,15 @@ class MockPersistentCookieStore
 // Mock for CookieMonster::Delegate
 class MockCookieMonsterDelegate : public net::CookieMonster::Delegate {
  public:
-  typedef std::pair<net::CookieMonster::CookieListPair, bool>
+  typedef std::pair<net::CookieMonster::CanonicalCookie, bool>
       CookieNotification;
 
   MockCookieMonsterDelegate() {}
 
   virtual void OnCookieChanged(
-      const std::string& domain_key,
       const net::CookieMonster::CanonicalCookie& cookie,
       bool removed) {
-    CookieNotification notification(
-        net::CookieMonster::CookieListPair(domain_key, cookie),
-        removed);
+    CookieNotification notification(cookie, removed);
     changes_.push_back(notification);
   }
 
@@ -1139,7 +1136,7 @@ TEST(CookieMonsterTest, TestSecure) {
 
 static Time GetFirstCookieAccessDate(net::CookieMonster* cm) {
   const net::CookieMonster::CookieList all_cookies(cm->GetAllCookies());
-  return all_cookies.front().second.LastAccessDate();
+  return all_cookies.front().LastAccessDate();
 }
 
 static const int kLastAccessThresholdMilliseconds = 200;
@@ -1247,8 +1244,8 @@ static bool FindAndDeleteCookie(net::CookieMonster* cm,
   net::CookieMonster::CookieList cookies = cm->GetAllCookies();
   for (net::CookieMonster::CookieList::iterator it = cookies.begin();
        it != cookies.end(); ++it)
-    if (it->first == domain && it->second.Name() == name)
-      return cm->DeleteCookie(domain, it->second, false);
+    if (it->Domain() == domain && it->Name() == name)
+      return cm->DeleteCookie(domain, *it, false);
   return false;
 }
 
@@ -1315,12 +1312,12 @@ TEST(CookieMonsterTest, GetAllCookiesForURL) {
   net::CookieMonster::CookieList::iterator it = cookies.begin();
 
   ASSERT_TRUE(it != cookies.end());
-  EXPECT_EQ("www.google.izzle", it->first);
-  EXPECT_EQ("A", it->second.Name());
+  EXPECT_EQ("www.google.izzle", it->Domain());
+  EXPECT_EQ("A", it->Name());
 
   ASSERT_TRUE(++it != cookies.end());
-  EXPECT_EQ(".google.izzle", it->first);
-  EXPECT_EQ("C", it->second.Name());
+  EXPECT_EQ(".google.izzle", it->Domain());
+  EXPECT_EQ("C", it->Name());
 
   ASSERT_TRUE(++it == cookies.end());
 
@@ -1329,16 +1326,16 @@ TEST(CookieMonsterTest, GetAllCookiesForURL) {
   it = cookies.begin();
 
   ASSERT_TRUE(it != cookies.end());
-  EXPECT_EQ("www.google.izzle", it->first);
-  EXPECT_EQ("A", it->second.Name());
+  EXPECT_EQ("www.google.izzle", it->Domain());
+  EXPECT_EQ("A", it->Name());
 
   ASSERT_TRUE(++it != cookies.end());
-  EXPECT_EQ(".google.izzle", it->first);
-  EXPECT_EQ("C", it->second.Name());
+  EXPECT_EQ(".google.izzle", it->Domain());
+  EXPECT_EQ("C", it->Name());
 
   ASSERT_TRUE(++it != cookies.end());
-  EXPECT_EQ(".google.izzle", it->first);
-  EXPECT_EQ("E", it->second.Name());
+  EXPECT_EQ(".google.izzle", it->Domain());
+  EXPECT_EQ("E", it->Name());
 
   ASSERT_TRUE(++it == cookies.end());
 
@@ -1369,12 +1366,12 @@ TEST(CookieMonsterTest, GetAllCookiesForURLPathMatching) {
   net::CookieMonster::CookieList::iterator it = cookies.begin();
 
   ASSERT_TRUE(it != cookies.end());
-  EXPECT_EQ("A", it->second.Name());
-  EXPECT_EQ("/foo", it->second.Path());
+  EXPECT_EQ("A", it->Name());
+  EXPECT_EQ("/foo", it->Path());
 
   ASSERT_TRUE(++it != cookies.end());
-  EXPECT_EQ("E", it->second.Name());
-  EXPECT_EQ("/", it->second.Path());
+  EXPECT_EQ("E", it->Name());
+  EXPECT_EQ("/", it->Path());
 
   ASSERT_TRUE(++it == cookies.end());
 
@@ -1382,12 +1379,12 @@ TEST(CookieMonsterTest, GetAllCookiesForURLPathMatching) {
   it = cookies.begin();
 
   ASSERT_TRUE(it != cookies.end());
-  EXPECT_EQ("C", it->second.Name());
-  EXPECT_EQ("/bar", it->second.Path());
+  EXPECT_EQ("C", it->Name());
+  EXPECT_EQ("/bar", it->Path());
 
   ASSERT_TRUE(++it != cookies.end());
-  EXPECT_EQ("E", it->second.Name());
-  EXPECT_EQ("/", it->second.Path());
+  EXPECT_EQ("E", it->Name());
+  EXPECT_EQ("/", it->Path());
 
   ASSERT_TRUE(++it == cookies.end());
 }
@@ -1410,8 +1407,8 @@ TEST(CookieMonsterTest, DeleteCookieByName) {
   EXPECT_EQ(expected_size, cookies.size());
   for (net::CookieMonster::CookieList::iterator it = cookies.begin();
        it != cookies.end(); ++it) {
-    EXPECT_NE("A1", it->second.Value());
-    EXPECT_NE("A2", it->second.Value());
+    EXPECT_NE("A1", it->Value());
+    EXPECT_NE("A2", it->Value());
   }
 }
 
@@ -1578,26 +1575,26 @@ TEST(CookieMonsterTest, Delegate) {
   EXPECT_EQ("A=B; C=D; E=F", cm->GetCookies(url_google));
   ASSERT_EQ(3u, delegate->changes().size());
   EXPECT_EQ(false, delegate->changes()[0].second);
-  EXPECT_EQ(url_google.host(), delegate->changes()[0].first.first);
-  EXPECT_EQ("A", delegate->changes()[0].first.second.Name());
-  EXPECT_EQ("B", delegate->changes()[0].first.second.Value());
-  EXPECT_EQ(url_google.host(), delegate->changes()[1].first.first);
+  EXPECT_EQ(url_google.host(), delegate->changes()[0].first.Domain());
+  EXPECT_EQ("A", delegate->changes()[0].first.Name());
+  EXPECT_EQ("B", delegate->changes()[0].first.Value());
+  EXPECT_EQ(url_google.host(), delegate->changes()[1].first.Domain());
   EXPECT_EQ(false, delegate->changes()[1].second);
-  EXPECT_EQ("C", delegate->changes()[1].first.second.Name());
-  EXPECT_EQ("D", delegate->changes()[1].first.second.Value());
-  EXPECT_EQ(url_google.host(), delegate->changes()[2].first.first);
+  EXPECT_EQ("C", delegate->changes()[1].first.Name());
+  EXPECT_EQ("D", delegate->changes()[1].first.Value());
+  EXPECT_EQ(url_google.host(), delegate->changes()[2].first.Domain());
   EXPECT_EQ(false, delegate->changes()[2].second);
-  EXPECT_EQ("E", delegate->changes()[2].first.second.Name());
-  EXPECT_EQ("F", delegate->changes()[2].first.second.Value());
+  EXPECT_EQ("E", delegate->changes()[2].first.Name());
+  EXPECT_EQ("F", delegate->changes()[2].first.Value());
   delegate->reset();
 
   EXPECT_TRUE(FindAndDeleteCookie(cm, url_google.host(), "C"));
   EXPECT_EQ("A=B; E=F", cm->GetCookies(url_google));
   ASSERT_EQ(1u, delegate->changes().size());
-  EXPECT_EQ(url_google.host(), delegate->changes()[0].first.first);
+  EXPECT_EQ(url_google.host(), delegate->changes()[0].first.Domain());
   EXPECT_EQ(true, delegate->changes()[0].second);
-  EXPECT_EQ("C", delegate->changes()[0].first.second.Name());
-  EXPECT_EQ("D", delegate->changes()[0].first.second.Value());
+  EXPECT_EQ("C", delegate->changes()[0].first.Name());
+  EXPECT_EQ("D", delegate->changes()[0].first.Value());
   delegate->reset();
 
   EXPECT_FALSE(FindAndDeleteCookie(cm, "random.host", "E"));
@@ -1612,9 +1609,9 @@ TEST(CookieMonsterTest, Delegate) {
   EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[0].type);
   ASSERT_EQ(1u, delegate->changes().size());
   EXPECT_EQ(false, delegate->changes()[0].second);
-  EXPECT_EQ(url_google.host(), delegate->changes()[0].first.first);
-  EXPECT_EQ("a", delegate->changes()[0].first.second.Name());
-  EXPECT_EQ("val1", delegate->changes()[0].first.second.Value());
+  EXPECT_EQ(url_google.host(), delegate->changes()[0].first.Domain());
+  EXPECT_EQ("a", delegate->changes()[0].first.Name());
+  EXPECT_EQ("val1", delegate->changes()[0].first.Value());
   delegate->reset();
 
   // Insert a cookie "a" for path "/path1", that is httponly. This should
@@ -1630,14 +1627,14 @@ TEST(CookieMonsterTest, Delegate) {
   EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[1].type);
   EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[2].type);
   ASSERT_EQ(2u, delegate->changes().size());
-  EXPECT_EQ(url_google.host(), delegate->changes()[0].first.first);
+  EXPECT_EQ(url_google.host(), delegate->changes()[0].first.Domain());
   EXPECT_EQ(true, delegate->changes()[0].second);
-  EXPECT_EQ("a", delegate->changes()[0].first.second.Name());
-  EXPECT_EQ("val1", delegate->changes()[0].first.second.Value());
-  EXPECT_EQ(url_google.host(), delegate->changes()[1].first.first);
+  EXPECT_EQ("a", delegate->changes()[0].first.Name());
+  EXPECT_EQ("val1", delegate->changes()[0].first.Value());
+  EXPECT_EQ(url_google.host(), delegate->changes()[1].first.Domain());
   EXPECT_EQ(false, delegate->changes()[1].second);
-  EXPECT_EQ("a", delegate->changes()[1].first.second.Name());
-  EXPECT_EQ("val2", delegate->changes()[1].first.second.Value());
+  EXPECT_EQ("a", delegate->changes()[1].first.Name());
+  EXPECT_EQ("val2", delegate->changes()[1].first.Value());
   delegate->reset();
 }
 
@@ -1681,14 +1678,13 @@ TEST(CookieMonsterTest, SetCookieWithDetails) {
   net::CookieMonster::CookieList::iterator it = cookies.begin();
 
   ASSERT_TRUE(it != cookies.end());
-  EXPECT_EQ("A", it->second.Name());
-  EXPECT_EQ("B", it->second.Value());
-  EXPECT_EQ("www.google.izzle", it->first);
-  EXPECT_EQ("www.google.izzle", it->second.Domain());
-  EXPECT_EQ("/foo", it->second.Path());
-  EXPECT_FALSE(it->second.DoesExpire());
-  EXPECT_FALSE(it->second.IsSecure());
-  EXPECT_FALSE(it->second.IsHttpOnly());
+  EXPECT_EQ("A", it->Name());
+  EXPECT_EQ("B", it->Value());
+  EXPECT_EQ("www.google.izzle", it->Domain());
+  EXPECT_EQ("/foo", it->Path());
+  EXPECT_FALSE(it->DoesExpire());
+  EXPECT_FALSE(it->IsSecure());
+  EXPECT_FALSE(it->IsHttpOnly());
 
   ASSERT_TRUE(++it == cookies.end());
 
@@ -1696,13 +1692,12 @@ TEST(CookieMonsterTest, SetCookieWithDetails) {
   it = cookies.begin();
 
   ASSERT_TRUE(it != cookies.end());
-  EXPECT_EQ("C", it->second.Name());
-  EXPECT_EQ("D", it->second.Value());
-  EXPECT_EQ(".google.izzle", it->first);
-  EXPECT_EQ(".google.izzle", it->second.Domain());
-  EXPECT_EQ("/bar", it->second.Path());
-  EXPECT_FALSE(it->second.IsSecure());
-  EXPECT_TRUE(it->second.IsHttpOnly());
+  EXPECT_EQ("C", it->Name());
+  EXPECT_EQ("D", it->Value());
+  EXPECT_EQ(".google.izzle", it->Domain());
+  EXPECT_EQ("/bar", it->Path());
+  EXPECT_FALSE(it->IsSecure());
+  EXPECT_TRUE(it->IsHttpOnly());
 
   ASSERT_TRUE(++it == cookies.end());
 
@@ -1710,12 +1705,12 @@ TEST(CookieMonsterTest, SetCookieWithDetails) {
   it = cookies.begin();
 
   ASSERT_TRUE(it != cookies.end());
-  EXPECT_EQ("E", it->second.Name());
-  EXPECT_EQ("F", it->second.Value());
-  EXPECT_EQ("/", it->second.Path());
-  EXPECT_EQ("www.google.izzle", it->second.Domain());
-  EXPECT_TRUE(it->second.IsSecure());
-  EXPECT_FALSE(it->second.IsHttpOnly());
+  EXPECT_EQ("E", it->Name());
+  EXPECT_EQ("F", it->Value());
+  EXPECT_EQ("/", it->Path());
+  EXPECT_EQ("www.google.izzle", it->Domain());
+  EXPECT_TRUE(it->IsSecure());
+  EXPECT_FALSE(it->IsHttpOnly());
 
   ASSERT_TRUE(++it == cookies.end());
 }
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index d1a542a..bd4e56a 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -1637,7 +1637,7 @@ TEST_F(URLRequestTest, CookiePolicy_ForceSession) {
   net::CookieMonster::CookieList cookies =
       context->cookie_store()->GetCookieMonster()->GetAllCookies();
   EXPECT_EQ(1U, cookies.size());
-  EXPECT_FALSE(cookies[0].second.IsPersistent());
+  EXPECT_FALSE(cookies[0].IsPersistent());
 
   context->set_cookie_policy(NULL);
 }