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

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

10 files changed, 234 insertions, 65 deletions

diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 0fe660b..e206121 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -167,7 +167,7 @@ ChromeURLRequestContext* FactoryForOriginal::Create() {
 
   if (record_mode || playback_mode) {
     // Don't use existing cookies and use an in-memory store.
-    context->set_cookie_store(new net::CookieMonster());
+    context->set_cookie_store(new net::CookieMonster(NULL));
     cache->set_mode(
         record_mode ? net::HttpCache::RECORD : net::HttpCache::PLAYBACK);
   }
@@ -264,7 +264,7 @@ ChromeURLRequestContext* FactoryForOffTheRecord::Create() {
       new net::HttpCache(io_thread()->globals()->network_change_notifier.get(),
                          context->host_resolver(), context->proxy_service(),
                          context->ssl_config_service(), 0);
-  context->set_cookie_store(new net::CookieMonster);
+  context->set_cookie_store(new net::CookieMonster(NULL));
   context->set_cookie_policy(
       new ChromeCookiePolicy(host_content_settings_map_));
   context->set_http_transaction_factory(cache);
@@ -298,7 +298,7 @@ ChromeURLRequestContext* FactoryForOffTheRecordExtensions::Create() {
   ChromeURLRequestContext* context = new ChromeURLRequestContext;
   ApplyProfileParametersToContext(context);
 
-  net::CookieMonster* cookie_monster = new net::CookieMonster;
+  net::CookieMonster* cookie_monster = new net::CookieMonster(NULL);
 
   // Enable cookies for extension URLs only.
   const char* schemes[] = {chrome::kExtensionScheme};
diff --git a/chrome/browser/sync/glue/http_bridge.cc b/chrome/browser/sync/glue/http_bridge.cc
index 95d95ac..bf54eeb 100644
--- a/chrome/browser/sync/glue/http_bridge.cc
+++ b/chrome/browser/sync/glue/http_bridge.cc
@@ -65,7 +65,7 @@ HttpBridge::RequestContext::RequestContext(URLRequestContext* baseline_context)
     : baseline_context_(baseline_context) {
 
   // Create empty, in-memory cookie store.
-  cookie_store_ = new net::CookieMonster();
+  cookie_store_ = new net::CookieMonster(NULL);
 
   // We don't use a cache for bridged loads, but we do want to share proxy info.
   host_resolver_ = baseline_context->host_resolver();
diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc
index 94ddf97..5cae4f7 100644
--- a/chrome/test/testing_profile.cc
+++ b/chrome/test/testing_profile.cc
@@ -86,7 +86,7 @@ class BookmarkLoadObserver : public BookmarkModelObserver {
 class TestURLRequestContext : public URLRequestContext {
  public:
   TestURLRequestContext() {
-    cookie_store_ = new net::CookieMonster();
+    cookie_store_ = new net::CookieMonster(NULL);
   }
 };
 
diff --git a/chrome_frame/test/test_server_test.cc b/chrome_frame/test/test_server_test.cc
index bd5c0c3..d008b8c 100644
--- a/chrome_frame/test/test_server_test.cc
+++ b/chrome_frame/test/test_server_test.cc
@@ -70,7 +70,7 @@ class URLRequestTestContext : public URLRequestContext {
                                                ssl_config_service_),
           disk_cache::CreateInMemoryCacheBackend(0));
     // In-memory cookie store.
-    cookie_store_ = new net::CookieMonster();
+    cookie_store_ = new net::CookieMonster(NULL);
   }
 
   virtual ~URLRequestTestContext() {
diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index 595bf06..87ab1bd 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -90,14 +90,6 @@ void CookieMonster::EnableFileScheme() {
   enable_file_scheme_ = true;
 }
 
-CookieMonster::CookieMonster()
-    : initialized_(false),
-      store_(NULL),
-      last_access_threshold_(
-          TimeDelta::FromSeconds(kDefaultAccessUpdateThresholdSeconds)) {
-  SetDefaultCookieableSchemes();
-}
-
 CookieMonster::CookieMonster(PersistentCookieStore* store)
     : initialized_(false),
       store_(store),
diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h
index f73698d..6ae2903 100644
--- a/net/base/cookie_monster.h
+++ b/net/base/cookie_monster.h
@@ -50,19 +50,17 @@ class CookieMonster : public CookieStore {
   typedef std::pair<std::string, CanonicalCookie> CookieListPair;
   typedef std::vector<CookieListPair> CookieList;
 
-
-  CookieMonster();
-
   // 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 this
   // class, but it must remain valid for the duration of the cookie monster's
-  // existence.
-  CookieMonster(PersistentCookieStore* store);
+  // existence. If |store| is NULL, then no backing store will be updated.
+  explicit CookieMonster(PersistentCookieStore* store);
 
 #ifdef UNIT_TEST
-  CookieMonster(int last_access_threshold_milliseconds)
+  CookieMonster(PersistentCookieStore* store,
+                int last_access_threshold_milliseconds)
       : initialized_(false),
-        store_(NULL),
+        store_(store),
         last_access_threshold_(base::TimeDelta::FromMilliseconds(
             last_access_threshold_milliseconds)) {
     SetDefaultCookieableSchemes();
diff --git a/net/base/cookie_monster_perftest.cc b/net/base/cookie_monster_perftest.cc
index 9e76ac8..61d9f68 100644
--- a/net/base/cookie_monster_perftest.cc
+++ b/net/base/cookie_monster_perftest.cc
@@ -40,7 +40,7 @@ TEST(ParsedCookieTest, TestParseBigCookies) {
 static const GURL kUrlGoogle("http://www.google.izzle");
 
 TEST(CookieMonsterTest, TestAddCookiesOnSingleHost) {
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   std::vector<std::string> cookies;
   for (int i = 0; i < kNumCookies; i++) {
     cookies.push_back(StringPrintf("a%03d=b", i));
@@ -67,7 +67,7 @@ TEST(CookieMonsterTest, TestAddCookiesOnSingleHost) {
 }
 
 TEST(CookieMonsterTest, TestAddCookieOnManyHosts) {
-  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL));
   std::string cookie(kCookieLine);
   std::vector<GURL> gurls;  // just wanna have ffffuunnn
   for (int i = 0; i < kNumCookies; ++i) {
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")));
+}
diff --git a/net/url_request/url_request_unittest.h b/net/url_request/url_request_unittest.h
index 16116d9..6ea2c15 100644
--- a/net/url_request/url_request_unittest.h
+++ b/net/url_request/url_request_unittest.h
@@ -164,7 +164,7 @@ class TestURLRequestContext : public URLRequestContext {
                                                ssl_config_service_),
           disk_cache::CreateInMemoryCacheBackend(0));
     // In-memory cookie store.
-    cookie_store_ = new net::CookieMonster();
+    cookie_store_ = new net::CookieMonster(NULL);
     accept_language_ = "en-us,fr";
     accept_charset_ = "iso-8859-1,*,utf-8";
   }
diff --git a/webkit/tools/test_shell/test_shell_request_context.cc b/webkit/tools/test_shell/test_shell_request_context.cc
index a770b28..2e87202 100644
--- a/webkit/tools/test_shell/test_shell_request_context.cc
+++ b/webkit/tools/test_shell/test_shell_request_context.cc
@@ -32,7 +32,7 @@ void TestShellRequestContext::Init(
     const FilePath& cache_path,
     net::HttpCache::Mode cache_mode,
     bool no_proxy) {
-  cookie_store_ = new net::CookieMonster();
+  cookie_store_ = new net::CookieMonster(NULL);
   cookie_policy_ = new net::StaticCookiePolicy();
 
   // hard-code A-L and A-C for test shells