Revert the revert... This has the cumulative effect of relanding 38694.

The chrome frame failure appears to be unrelated.

Add some tests to CookieMonster for overwriting persistent cookies, and checking that the PersistentCookieStore interface is exercised correctly.

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

TBR=eroman@chromium.org
Review URL: http://codereview.chromium.org/596048

TBR=eroman@chromium.org
Review URL: http://codereview.chromium.org/604012

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

1 files changed, 219 insertions, 40 deletions

diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc
index 735c182..2a86c6d 100644
--- a/net/base/cookie_monster_unittest.cc
+++ b/net/base/cookie_monster_unittest.cc
@@ -19,9 +19,71 @@ using base::Time;
 using base::TimeDelta;
 
 namespace {
-  class ParsedCookieTest : public testing::Test { };
-  class CookieMonsterTest : public testing::Test { };
-}
+
+class ParsedCookieTest : public testing::Test { };
+class CookieMonsterTest : public testing::Test { };
+
+// Describes a call to one of the 3 functions of PersistentCookieStore.
+struct CookieStoreCommand {
+  enum Type {
+    ADD,
+    UPDATE_ACCESS_TIME,
+    REMOVE,
+  };
+
+  CookieStoreCommand(Type type,
+                     const std::string& key,
+                     const net::CookieMonster::CanonicalCookie& cookie)
+      : type(type), key(key), cookie(cookie) {}
+
+  Type type;
+  std::string key;  // Only applicable to the ADD command.
+  net::CookieMonster::CanonicalCookie cookie;
+};
+
+// Implementation of PersistentCookieStore that captures the
+// received commands and saves them to a list.
+class MockPersistentCookieStore
+    : public net::CookieMonster::PersistentCookieStore {
+ public:
+  typedef std::vector<CookieStoreCommand> CommandList;
+
+  MockPersistentCookieStore() {}
+
+  virtual bool Load(
+      std::vector<net::CookieMonster::KeyedCanonicalCookie>* out_cookies) {
+    return true;
+  }
+
+  virtual void AddCookie(const std::string& key,
+                         const net::CookieMonster::CanonicalCookie& cookie) {
+    commands_.push_back(
+        CookieStoreCommand(CookieStoreCommand::ADD, key, cookie));
+  }
+
+  virtual void UpdateCookieAccessTime(
+      const net::CookieMonster::CanonicalCookie& cookie) {
+    commands_.push_back(CookieStoreCommand(
+        CookieStoreCommand::UPDATE_ACCESS_TIME, std::string(), cookie));
+  }
+
+  virtual void DeleteCookie(
+      const net::CookieMonster::CanonicalCookie& cookie) {
+    commands_.push_back(
+        CookieStoreCommand(CookieStoreCommand::REMOVE, std::string(), cookie));
+  }
+
+  const CommandList& commands() const {
+    return commands_;
+  }
+
+ private:
+  CommandList commands_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockPersistentCookieStore);
+};
+
+}  // namespace
 
 
 TEST(ParsedCookieTest, TestBasic) {
@@ -246,7 +308,9 @@ static const char kValidDomainCookieLine[] = "A=B; path=/; domain=google.izzle";
 TEST(CookieMonsterTest, DomainTest) {
   GURL url_google(kUrlGoogle);
 
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<MockPersistentCookieStore> store(
+      new MockPersistentCookieStore);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(store));
   EXPECT_TRUE(cm->SetCookie(url_google, "A=B"));
   EXPECT_EQ("A=B", cm->GetCookies(url_google));
   EXPECT_TRUE(cm->SetCookie(url_google, "C=D; domain=.google.izzle"));
@@ -272,23 +336,33 @@ TEST(CookieMonsterTest, DomainTest) {
   EXPECT_EQ("C=D; E=F; G=H",
             cm->GetCookies(GURL("http://bla.www.google.izzle")));
   EXPECT_EQ("A=B; C=D; E=F; G=H", cm->GetCookies(url_google));
+
+  // Nothing was persisted to the backing store.
+  EXPECT_EQ(0u, store->commands().size());
 }
 
 // FireFox recognizes domains containing trailing periods as valid.
 // IE and Safari do not. Assert the expected policy here.
 TEST(CookieMonsterTest, DomainWithTrailingDotTest) {
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<MockPersistentCookieStore> store(
+      new MockPersistentCookieStore);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(store));
   GURL url_google("http://www.google.com");
 
   EXPECT_FALSE(cm->SetCookie(url_google, "a=1; domain=.www.google.com."));
   EXPECT_FALSE(cm->SetCookie(url_google, "b=2; domain=.www.google.com.."));
   EXPECT_EQ("", cm->GetCookies(url_google));
+
+  // Nothing was persisted to the backing store.
+  EXPECT_EQ(0u, store->commands().size());
 }
 
 // Test that cookies can bet set on higher level domains.
 // http://b/issue?id=896491
 TEST(CookieMonsterTest, ValidSubdomainTest) {
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<MockPersistentCookieStore> store(
+      new MockPersistentCookieStore);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(store));
   GURL url_abcd("http://a.b.c.d.com");
   GURL url_bcd("http://b.c.d.com");
   GURL url_cd("http://c.d.com");
@@ -309,6 +383,9 @@ TEST(CookieMonsterTest, ValidSubdomainTest) {
   EXPECT_TRUE(cm->SetCookie(url_bcd, "X=cd; domain=.c.d.com"));
   EXPECT_EQ("b=2; c=3; d=4; X=bcd; X=cd", cm->GetCookies(url_bcd));
   EXPECT_EQ("c=3; d=4; X=cd", cm->GetCookies(url_cd));
+
+  // Nothing was persisted to the backing store.
+  EXPECT_EQ(0u, store->commands().size());
 }
 
 // Test that setting a cookie which specifies an invalid domain has
@@ -317,7 +394,10 @@ TEST(CookieMonsterTest, ValidSubdomainTest) {
 // http://b/issue?id=896472
 TEST(CookieMonsterTest, InvalidDomainTest) {
   {
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<MockPersistentCookieStore> store(
+        new MockPersistentCookieStore);
+
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(store));
     GURL url_foobar("http://foo.bar.com");
 
     // More specific sub-domain than allowed.
@@ -348,13 +428,16 @@ TEST(CookieMonsterTest, InvalidDomainTest) {
     EXPECT_FALSE(cm->SetCookie(url_foobar, "o=15; domain=.foo.bar.com#sup"));
 
     EXPECT_EQ("", cm->GetCookies(url_foobar));
+
+    // Nothing was persisted to the backing store.
+    EXPECT_EQ(0u, store->commands().size());
   }
 
   {
     // Make sure the cookie code hasn't gotten its subdomain string handling
     // reversed, missed a suffix check, etc.  It's important here that the two
     // hosts below have the same domain + registry.
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     GURL url_foocom("http://foo.com.com");
     EXPECT_FALSE(cm->SetCookie(url_foocom, "a=1; domain=.foo.com.com.com"));
     EXPECT_EQ("", cm->GetCookies(url_foocom));
@@ -366,7 +449,7 @@ TEST(CookieMonsterTest, InvalidDomainTest) {
 // http://b/issue?id=889898
 TEST(CookieMonsterTest, DomainWithoutLeadingDotTest) {
   {  // The omission of dot results in setting a domain cookie.
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     GURL url_hosted("http://manage.hosted.filefront.com");
     GURL url_filefront("http://www.filefront.com");
     EXPECT_TRUE(cm->SetCookie(url_hosted, "sawAd=1; domain=filefront.com"));
@@ -375,7 +458,7 @@ TEST(CookieMonsterTest, DomainWithoutLeadingDotTest) {
   }
 
   {  // Even when the domains match exactly, don't consider it host cookie.
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     GURL url("http://www.google.com");
     EXPECT_TRUE(cm->SetCookie(url, "a=1; domain=www.google.com"));
     EXPECT_EQ("a=1", cm->GetCookies(url));
@@ -387,7 +470,7 @@ TEST(CookieMonsterTest, DomainWithoutLeadingDotTest) {
 // Test that the domain specified in cookie string is treated case-insensitive
 // http://b/issue?id=896475.
 TEST(CookieMonsterTest, CaseInsensitiveDomainTest) {
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   GURL url_google("http://www.google.com");
   EXPECT_TRUE(cm->SetCookie(url_google, "a=1; domain=.GOOGLE.COM"));
   EXPECT_TRUE(cm->SetCookie(url_google, "b=2; domain=.wWw.gOOgLE.coM"));
@@ -397,13 +480,13 @@ TEST(CookieMonsterTest, CaseInsensitiveDomainTest) {
 TEST(CookieMonsterTest, TestIpAddress) {
   GURL url_ip("http://1.2.3.4/weee");
   {
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     EXPECT_TRUE(cm->SetCookie(url_ip, kValidCookieLine));
     EXPECT_EQ("A=B", cm->GetCookies(url_ip));
   }
 
   {  // IP addresses should not be able to set domain cookies.
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     EXPECT_FALSE(cm->SetCookie(url_ip, "b=2; domain=.1.2.3.4"));
     EXPECT_FALSE(cm->SetCookie(url_ip, "c=3; domain=.3.4"));
     EXPECT_EQ("", cm->GetCookies(url_ip));
@@ -419,7 +502,7 @@ TEST(CookieMonsterTest, TestIpAddress) {
 // Test host cookies, and setting of cookies on TLD.
 TEST(CookieMonsterTest, TestNonDottedAndTLD) {
   {
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     GURL url("http://com/");
     // Allow setting on "com", (but only as a host cookie).
     EXPECT_TRUE(cm->SetCookie(url, "a=1"));
@@ -433,7 +516,7 @@ TEST(CookieMonsterTest, TestNonDottedAndTLD) {
   }
 
   {  // http://com. should be treated the same as http://com.
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     GURL url("http://com./index.html");
     EXPECT_TRUE(cm->SetCookie(url, "a=1"));
     EXPECT_EQ("a=1", cm->GetCookies(url));
@@ -441,7 +524,7 @@ TEST(CookieMonsterTest, TestNonDottedAndTLD) {
   }
 
   {  // Should not be able to set host cookie from a subdomain.
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     GURL url("http://a.b");
     EXPECT_FALSE(cm->SetCookie(url, "a=1; domain=.b"));
     EXPECT_FALSE(cm->SetCookie(url, "b=2; domain=b"));
@@ -449,7 +532,7 @@ TEST(CookieMonsterTest, TestNonDottedAndTLD) {
   }
 
   {  // Same test as above, but explicitly on a known TLD (com).
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     GURL url("http://google.com");
     EXPECT_FALSE(cm->SetCookie(url, "a=1; domain=.com"));
     EXPECT_FALSE(cm->SetCookie(url, "b=2; domain=com"));
@@ -457,7 +540,7 @@ TEST(CookieMonsterTest, TestNonDottedAndTLD) {
   }
 
   {  // Make sure can't set cookie on TLD which is dotted.
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     GURL url("http://google.co.uk");
     EXPECT_FALSE(cm->SetCookie(url, "a=1; domain=.co.uk"));
     EXPECT_FALSE(cm->SetCookie(url, "b=2; domain=.uk"));
@@ -467,7 +550,7 @@ TEST(CookieMonsterTest, TestNonDottedAndTLD) {
   }
 
   {  // Intranet URLs should only be able to set host cookies.
-    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+    scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
     GURL url("http://b");
     EXPECT_TRUE(cm->SetCookie(url, "a=1"));
     EXPECT_FALSE(cm->SetCookie(url, "b=2; domain=.b"));
@@ -479,7 +562,7 @@ TEST(CookieMonsterTest, TestNonDottedAndTLD) {
 // Test reading/writing cookies when the domain ends with a period,
 // as in "www.google.com."
 TEST(CookieMonsterTest, TestHostEndsWithDot) {
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   GURL url("http://www.google.com");
   GURL url_with_dot("http://www.google.com.");
   EXPECT_TRUE(cm->SetCookie(url, "a=1"));
@@ -499,19 +582,19 @@ TEST(CookieMonsterTest, TestHostEndsWithDot) {
 }
 
 TEST(CookieMonsterTest, InvalidScheme) {
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   EXPECT_FALSE(cm->SetCookie(GURL(kUrlFtp), kValidCookieLine));
 }
 
 TEST(CookieMonsterTest, InvalidScheme_Read) {
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   EXPECT_TRUE(cm->SetCookie(GURL(kUrlGoogle), kValidDomainCookieLine));
   EXPECT_EQ("", cm->GetCookies(GURL(kUrlFtp)));
 }
 
 TEST(CookieMonsterTest, PathTest) {
   std::string url("http://www.google.izzle");
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   EXPECT_TRUE(cm->SetCookie(GURL(url), "A=B; path=/wee"));
   EXPECT_EQ("A=B", cm->GetCookies(GURL(url + "/wee")));
   EXPECT_EQ("A=B", cm->GetCookies(GURL(url + "/wee/")));
@@ -528,7 +611,7 @@ TEST(CookieMonsterTest, PathTest) {
 
 TEST(CookieMonsterTest, HttpOnlyTest) {
   GURL url_google(kUrlGoogle);
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   net::CookieOptions options;
   options.set_include_httponly();
 
@@ -650,7 +733,9 @@ TEST(CookieMonsterTest, TestCookieDateParsing) {
 
 TEST(CookieMonsterTest, TestCookieDeletion) {
   GURL url_google(kUrlGoogle);
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<MockPersistentCookieStore> store(
+      new MockPersistentCookieStore);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(store));
 
   // Create a session cookie.
   EXPECT_TRUE(cm->SetCookie(url_google, kValidCookieLine));
@@ -673,38 +758,53 @@ TEST(CookieMonsterTest, TestCookieDeletion) {
   EXPECT_TRUE(cm->SetCookie(url_google,
                            std::string(kValidCookieLine) +
                            "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
+  ASSERT_EQ(1u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[0].type);
+
   EXPECT_EQ("A=B", cm->GetCookies(url_google));
   // Delete it via Max-Age.
   EXPECT_TRUE(cm->SetCookie(url_google,
                            std::string(kValidCookieLine) + "; max-age=0"));
+  ASSERT_EQ(2u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[1].type);
   EXPECT_EQ("", cm->GetCookies(url_google));
 
   // Create a persistent cookie.
   EXPECT_TRUE(cm->SetCookie(url_google,
                            std::string(kValidCookieLine) +
                            "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
+  ASSERT_EQ(3u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[2].type);
   EXPECT_EQ("A=B", cm->GetCookies(url_google));
   // Delete it via Expires.
   EXPECT_TRUE(cm->SetCookie(url_google,
                            std::string(kValidCookieLine) +
                            "; expires=Mon, 18-Apr-1977 22:50:13 GMT"));
+  ASSERT_EQ(4u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[3].type);
   EXPECT_EQ("", cm->GetCookies(url_google));
 
   // Create a persistent cookie.
   EXPECT_TRUE(cm->SetCookie(url_google,
                            std::string(kValidCookieLine) +
                            "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
+  ASSERT_EQ(5u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[4].type);
   EXPECT_EQ("A=B", cm->GetCookies(url_google));
   // Delete it via Expires, with a unix epoch of 0.
   EXPECT_TRUE(cm->SetCookie(url_google,
                            std::string(kValidCookieLine) +
                            "; expires=Thu, 1-Jan-1970 00:00:00 GMT"));
+  ASSERT_EQ(6u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[5].type);
   EXPECT_EQ("", cm->GetCookies(url_google));
 }
 
 TEST(CookieMonsterTest, TestCookieDeleteAll) {
   GURL url_google(kUrlGoogle);
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<MockPersistentCookieStore> store(
+      new MockPersistentCookieStore);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(store));
   net::CookieOptions options;
   options.set_include_httponly();
 
@@ -716,11 +816,26 @@ TEST(CookieMonsterTest, TestCookieDeleteAll) {
 
   EXPECT_EQ(2, cm->DeleteAll(false));
   EXPECT_EQ("", cm->GetCookiesWithOptions(url_google, options));
+
+  EXPECT_EQ(0u, store->commands().size());
+
+  // Create a persistent cookie.
+  EXPECT_TRUE(cm->SetCookie(url_google,
+                            std::string(kValidCookieLine) +
+                            "; expires=Mon, 18-Apr-22 22:50:13 GMT"));
+  ASSERT_EQ(1u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[0].type);
+
+  EXPECT_EQ(1, cm->DeleteAll(true));  // sync_to_store = true.
+  ASSERT_EQ(2u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[1].type);
+
+  EXPECT_EQ("", cm->GetCookiesWithOptions(url_google, options));
 }
 
 TEST(CookieMonsterTest, TestCookieDeleteAllCreatedAfterTimestamp) {
   GURL url_google(kUrlGoogle);
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   Time now = Time::Now();
 
   // Nothing has been added so nothing should be deleted.
@@ -748,7 +863,7 @@ TEST(CookieMonsterTest, TestCookieDeleteAllCreatedAfterTimestamp) {
 
 TEST(CookieMonsterTest, TestCookieDeleteAllCreatedBetweenTimestamps) {
   GURL url_google(kUrlGoogle);
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   Time now = Time::Now();
 
   // Nothing has been added so nothing should be deleted.
@@ -791,7 +906,7 @@ TEST(CookieMonsterTest, TestCookieDeleteAllCreatedBetweenTimestamps) {
 TEST(CookieMonsterTest, TestSecure) {
   GURL url_google(kUrlGoogle);
   GURL url_google_secure(kUrlGoogleSecure);
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
 
   EXPECT_TRUE(cm->SetCookie(url_google, "A=B"));
   EXPECT_EQ("A=B", cm->GetCookies(url_google));
@@ -822,7 +937,7 @@ static const int kLastAccessThresholdMilliseconds = 200;
 TEST(CookieMonsterTest, TestLastAccess) {
   GURL url_google(kUrlGoogle);
   scoped_refptr<net::CookieMonster> cm(
-      new net::CookieMonster(kLastAccessThresholdMilliseconds));
+      new net::CookieMonster(NULL, kLastAccessThresholdMilliseconds));
 
   EXPECT_TRUE(cm->SetCookie(url_google, "A=B"));
   const Time last_access_date(GetFirstCookieAccessDate(cm));
@@ -850,7 +965,7 @@ static int CountInString(const std::string& str, char c) {
 
 TEST(CookieMonsterTest, TestHostGarbageCollection) {
   GURL url_google(kUrlGoogle);
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   // Add a bunch of cookies on a single host, should purge them.
   for (int i = 0; i < 101; i++) {
     std::string cookie = StringPrintf("a%03d=b", i);
@@ -865,7 +980,7 @@ TEST(CookieMonsterTest, TestHostGarbageCollection) {
 
 TEST(CookieMonsterTest, TestTotalGarbageCollection) {
   scoped_refptr<net::CookieMonster> cm(
-      new net::CookieMonster(kLastAccessThresholdMilliseconds));
+      new net::CookieMonster(NULL, kLastAccessThresholdMilliseconds));
 
   // Add a bunch of cookies on a bunch of host, some should get purged.
   const GURL sticky_cookie("http://a0000.izzle");
@@ -900,7 +1015,7 @@ TEST(CookieMonsterTest, TestTotalGarbageCollection) {
 TEST(CookieMonsterTest, NetUtilCookieTest) {
   const GURL test_url("http://mojo.jojo.google.izzle/");
 
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
 
   EXPECT_TRUE(cm->SetCookie(test_url, "foo=bar"));
   std::string value = cm->GetCookies(test_url);
@@ -930,7 +1045,7 @@ static bool FindAndDeleteCookie(net::CookieMonster* cm,
 TEST(CookieMonsterTest, TestDeleteSingleCookie) {
   GURL url_google(kUrlGoogle);
 
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
 
   EXPECT_TRUE(cm->SetCookie(url_google, "A=B"));
   EXPECT_TRUE(cm->SetCookie(url_google, "C=D"));
@@ -945,8 +1060,8 @@ TEST(CookieMonsterTest, TestDeleteSingleCookie) {
 }
 
 TEST(CookieMonsterTest, SetCookieableSchemes) {
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
-  scoped_refptr<net::CookieMonster> cm_foo(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
+  scoped_refptr<net::CookieMonster> cm_foo(new net::CookieMonster(NULL));
 
   // Only cm_foo should allow foo:// cookies.
   const char* kSchemes[] = {"foo"};
@@ -967,7 +1082,7 @@ TEST(CookieMonsterTest, GetAllCookiesForURL) {
   GURL url_google_secure(kUrlGoogleSecure);
 
   scoped_refptr<net::CookieMonster> cm(
-      new net::CookieMonster(kLastAccessThresholdMilliseconds));
+      new net::CookieMonster(NULL, kLastAccessThresholdMilliseconds));
 
   // Create an httponly cookie.
   net::CookieOptions options;
@@ -1027,7 +1142,7 @@ TEST(CookieMonsterTest, GetAllCookiesForURLPathMatching) {
   GURL url_google_foo("http://www.google.izzle/foo");
   GURL url_google_bar("http://www.google.izzle/bar");
 
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster());
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   net::CookieOptions options;
 
   EXPECT_TRUE(cm->SetCookieWithOptions(url_google_foo,
@@ -1070,7 +1185,7 @@ TEST(CookieMonsterTest, GetAllCookiesForURLPathMatching) {
 }
 
 TEST(CookieMonsterTest, DeleteCookieByName) {
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   GURL url_google(kUrlGoogle);
 
   EXPECT_TRUE(cm->SetCookie(url_google, "A=A1; path=/"));
@@ -1092,4 +1207,68 @@ TEST(CookieMonsterTest, DeleteCookieByName) {
   }
 }
 
-// TODO test overwrite cookie
+// Test that overwriting persistent cookies deletes the old one from the
+// backing store.
+TEST(CookieMonsterTest, OverwritePersistentCookie) {
+  GURL url_google("http://www.google.com/");
+  GURL url_chromium("http://chromium.org");
+  scoped_refptr<MockPersistentCookieStore> store(
+      new MockPersistentCookieStore);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(store));
+
+  // Insert a cookie "a" for path "/path1"
+  EXPECT_TRUE(
+      cm->SetCookie(url_google, "a=val1; path=/path1; "
+                                "expires=Mon, 18-Apr-22 22:50:13 GMT"));
+  ASSERT_EQ(1u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[0].type);
+
+  // Insert a cookie "b" for path "/path1"
+  EXPECT_TRUE(
+      cm->SetCookie(url_google, "b=val1; path=/path1; "
+                                "expires=Mon, 18-Apr-22 22:50:14 GMT"));
+  ASSERT_EQ(2u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[1].type);
+
+  // Insert a cookie "b" for path "/path1", that is httponly. This should
+  // overwrite the non-http-only version.
+  net::CookieOptions allow_httponly;
+  allow_httponly.set_include_httponly();
+  EXPECT_TRUE(
+    cm->SetCookieWithOptions(url_google,
+                             "b=val2; path=/path1; httponly; "
+                             "expires=Mon, 18-Apr-22 22:50:14 GMT",
+                             allow_httponly));
+  ASSERT_EQ(4u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[2].type);
+  EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[3].type);
+
+  // Insert a cookie "a" for path "/path1". This should overwrite.
+  EXPECT_TRUE(cm->SetCookie(url_google,
+                            "a=val33; path=/path1; "
+                            "expires=Mon, 18-Apr-22 22:50:14 GMT"));
+  ASSERT_EQ(6u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[4].type);
+  EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[5].type);
+
+  // Insert a cookie "a" for path "/path2". This should NOT overwrite
+  // cookie "a", since the path is different.
+  EXPECT_TRUE(cm->SetCookie(url_google,
+                            "a=val9; path=/path2; "
+                            "expires=Mon, 18-Apr-22 22:50:14 GMT"));
+  ASSERT_EQ(7u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[6].type);
+
+  // Insert a cookie "a" for path "/path1", but this time for "chromium.org".
+  // Although the name and path match, the hostnames do not, so shouldn't
+  // overwrite.
+  EXPECT_TRUE(cm->SetCookie(url_chromium,
+                            "a=val99; path=/path1; "
+                            "expires=Mon, 18-Apr-22 22:50:14 GMT"));
+  ASSERT_EQ(8u, store->commands().size());
+  EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[7].type);
+
+  EXPECT_EQ("a=val33", cm->GetCookies(GURL("http://www.google.com/path1")));
+  EXPECT_EQ("a=val9", cm->GetCookies(GURL("http://www.google.com/path2")));
+  EXPECT_EQ("a=val99", cm->GetCookies(GURL("http://chromium.org/path1")));
+}