Fix bug in DeleteAllForURL; deletes entire store instead of just

cookies related to URL (found by inspection.)  Also changed name
and semantics to more closely reflect usage of primary caller (extension
data deleter), and added test for that set of semantics.

BUG=none
TEST=Linux CookieMonsterTest.*:ParsedCookieTest.* (especially new CookieMonsterTest.DeleteAllHost)

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

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

3 files changed, 169 insertions, 9 deletions

diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index f6d3a54..86d71b8 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -901,17 +901,24 @@ int CookieMonster::DeleteAllCreatedAfter(const Time& delete_begin,
   return DeleteAllCreatedBetween(delete_begin, Time(), sync_to_store);
 }
 
-int CookieMonster::DeleteAllForURL(const GURL& url,
-                                   bool sync_to_store) {
+int CookieMonster::DeleteAllForHost(const GURL& url) {
   AutoLock autolock(lock_);
   InitIfNecessary();
 
-  CookieList cookies = InternalGetAllCookiesForURL(url);
+  if (!HasCookieableScheme(url))
+    return 0;
+
+  // We store host cookies in the store by their canonical host name;
+  // domain cookies are stored with a leading ".".  So this is a pretty
+  // simple lookup and per-cookie delete.
   int num_deleted = 0;
-  for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end();) {
-    CookieMap::iterator curit = it;
-    ++it;
-    InternalDeleteCookie(curit, sync_to_store, kDeleteCookieExplicit);
+  for (CookieMapItPair its = cookies_.equal_range(url.host());
+       its.first != its.second;) {
+    CookieMap::iterator curit = its.first;
+    ++its.first;
+    num_deleted++;
+
+    InternalDeleteCookie(curit, true, kDeleteCookieExplicit);
   }
   return num_deleted;
 }
diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h
index 43943a0..eadc59a 100644
--- a/net/base/cookie_monster.h
+++ b/net/base/cookie_monster.h
@@ -138,8 +138,11 @@ class CookieMonster : public CookieStore {
   // one passed into the function via |delete_after|.
   int DeleteAllCreatedAfter(const base::Time& delete_begin, bool sync_to_store);
 
-  // Delete all cookies that match the given URL.
-  int DeleteAllForURL(const GURL& url, bool sync_to_store);
+  // Delete all cookies that match the host of the given URL
+  // regardless of path.  This includes all http_only and secure cookies,
+  // but does not include any domain cookies that may apply to this host.
+  // Returns the number of cookies deleted.
+  int DeleteAllForHost(const GURL& url);
 
   // Delete one specific cookie.
   bool DeleteCookie(const std::string& domain,
diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc
index a814480..21d6df8 100644
--- a/net/base/cookie_monster_unittest.cc
+++ b/net/base/cookie_monster_unittest.cc
@@ -163,6 +163,91 @@ void AddKeyedCookieToList(
           key, cookie.release()));
 }
 
+// Helper for DeleteAllForHost test; repopulates CM with same layout
+// each time.
+const char* kTopLevelDomainPlus1 = "http://www.harvard.edu";
+const char* kTopLevelDomainPlus2 = "http://www.math.harvard.edu";
+const char* kTopLevelDomainPlus2Secure = "https://www.math.harvard.edu";
+const char* kTopLevelDomainPlus3 =
+    "http://www.bourbaki.math.harvard.edu";
+const char* kOtherDomain = "http://www.mit.edu";
+
+void PopulateCmForDeleteAllForHost(scoped_refptr<net::CookieMonster> cm) {
+  GURL url_top_level_domain_plus_1(kTopLevelDomainPlus1);
+  GURL url_top_level_domain_plus_2(kTopLevelDomainPlus2);
+  GURL url_top_level_domain_plus_2_secure(kTopLevelDomainPlus2Secure);
+  GURL url_top_level_domain_plus_3(kTopLevelDomainPlus3);
+  GURL url_other(kOtherDomain);
+
+  cm->DeleteAll(true);
+
+  // Static population for probe:
+  //    * Three levels of domain cookie (.b.a, .c.b.a, .d.c.b.a)
+  //    * Three levels of host cookie (w.b.a, w.c.b.a, w.d.c.b.a)
+  //    * http_only cookie (w.c.b.a)
+  //    * Two secure cookies (.c.b.a, w.c.b.a)
+  //    * Two domain path cookies (.c.b.a/dir1, .c.b.a/dir1/dir2)
+  //    * Two host path cookies (w.c.b.a/dir1, w.c.b.a/dir1/dir2)
+
+  // Domain cookies
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_1,
+                                       "dom_1", "X", ".harvard.edu", "/",
+                                       base::Time(), false, false));
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_2,
+                                       "dom_2", "X", ".math.harvard.edu", "/",
+                                       base::Time(), false, false));
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_3,
+                                       "dom_3", "X",
+                                       ".bourbaki.math.harvard.edu", "/",
+                                       base::Time(), false, false));
+
+  // Host cookies
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_1,
+                                       "host_1", "X", "", "/",
+                                       base::Time(), false, false));
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_2,
+                                       "host_2", "X", "", "/",
+                                       base::Time(), false, false));
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_3,
+                                       "host_3", "X", "", "/",
+                                       base::Time(), false, false));
+
+  // Http_only cookie
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_2,
+                                       "httpo_check", "X", "", "/",
+                                       base::Time(), false, true));
+
+  // Secure cookies
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_2_secure,
+                                       "sec_dom", "X", ".math.harvard.edu",
+                                       "/", base::Time(), true, false));
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_2_secure,
+                                       "sec_host", "X", "", "/",
+                                       base::Time(), true, false));
+
+  // Domain path cookies
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_2,
+                                       "dom_path_1", "X",
+                                       ".math.harvard.edu", "/dir1",
+                                       base::Time(), false, false));
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_2,
+                                       "dom_path_2", "X",
+                                       ".math.harvard.edu", "/dir1/dir2",
+                                       base::Time(), false, false));
+
+  // Host path cookies
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_2,
+                                       "host_path_1", "X",
+                                       "", "/dir1",
+                                       base::Time(), false, false));
+  EXPECT_TRUE(cm->SetCookieWithDetails(url_top_level_domain_plus_2,
+                                       "host_path_2", "X",
+                                       "", "/dir1/dir2",
+                                       base::Time(), false, false));
+
+  EXPECT_EQ(13U, cm->GetAllCookies().size());
+}
+
 }  // namespace
 
 
@@ -1631,3 +1716,68 @@ TEST(CookieMonsterTest, SetCookieWithDetails) {
 
   ASSERT_TRUE(++it == cookies.end());
 }
+
+
+
+TEST(CookieMonsterTest, DeleteAllForHost) {
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL, NULL));
+
+  // Test probes:
+  //    * Non-secure URL, mid-level (http://w.c.b.a)
+  //    * Secure URL, mid-level (https://w.c.b.a)
+  //    * URL with path, mid-level (https:/w.c.b.a/dir1/xx)
+  // All three tests should nuke only the midlevel host cookie,
+  // the http_only cookie, the host secure cookie, and the two host
+  // path cookies.  http_only, secure, and paths are ignored by
+  // this call, and domain cookies arent touched.
+  PopulateCmForDeleteAllForHost(cm);
+  EXPECT_EQ("dom_1=X; dom_2=X; dom_3=X; host_3=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus3)));
+  EXPECT_EQ("dom_1=X; dom_2=X; host_2=X; sec_dom=X; sec_host=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus2Secure)));
+  EXPECT_EQ("dom_1=X; host_1=X", cm->GetCookies(GURL(kTopLevelDomainPlus1)));
+  EXPECT_EQ("dom_path_2=X; host_path_2=X; dom_path_1=X; host_path_1=X; "
+            "dom_1=X; dom_2=X; host_2=X; sec_dom=X; sec_host=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus2Secure +
+                                std::string("/dir1/dir2/xxx"))));
+
+  EXPECT_EQ(5, cm->DeleteAllForHost(GURL(kTopLevelDomainPlus2)));
+  EXPECT_EQ(8U, cm->GetAllCookies().size());
+
+  EXPECT_EQ("dom_1=X; dom_2=X; dom_3=X; host_3=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus3)));
+  EXPECT_EQ("dom_1=X; dom_2=X; sec_dom=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus2Secure)));
+  EXPECT_EQ("dom_1=X; host_1=X", cm->GetCookies(GURL(kTopLevelDomainPlus1)));
+  EXPECT_EQ("dom_path_2=X; dom_path_1=X; dom_1=X; dom_2=X; sec_dom=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus2Secure +
+                                std::string("/dir1/dir2/xxx"))));
+
+  PopulateCmForDeleteAllForHost(cm);
+  EXPECT_EQ(5, cm->DeleteAllForHost(GURL(kTopLevelDomainPlus2Secure)));
+  EXPECT_EQ(8U, cm->GetAllCookies().size());
+
+  EXPECT_EQ("dom_1=X; dom_2=X; dom_3=X; host_3=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus3)));
+  EXPECT_EQ("dom_1=X; dom_2=X; sec_dom=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus2Secure)));
+  EXPECT_EQ("dom_1=X; host_1=X", cm->GetCookies(GURL(kTopLevelDomainPlus1)));
+  EXPECT_EQ("dom_path_2=X; dom_path_1=X; dom_1=X; dom_2=X; sec_dom=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus2Secure +
+                                std::string("/dir1/dir2/xxx"))));
+
+  PopulateCmForDeleteAllForHost(cm);
+  EXPECT_EQ(5, cm->DeleteAllForHost(GURL(kTopLevelDomainPlus2Secure +
+                                         std::string("/dir1/xxx"))));
+  EXPECT_EQ(8U, cm->GetAllCookies().size());
+
+  EXPECT_EQ("dom_1=X; dom_2=X; dom_3=X; host_3=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus3)));
+  EXPECT_EQ("dom_1=X; dom_2=X; sec_dom=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus2Secure)));
+  EXPECT_EQ("dom_1=X; host_1=X", cm->GetCookies(GURL(kTopLevelDomainPlus1)));
+  EXPECT_EQ("dom_path_2=X; dom_path_1=X; dom_1=X; dom_2=X; sec_dom=X",
+            cm->GetCookies(GURL(kTopLevelDomainPlus2Secure +
+                                std::string("/dir1/dir2/xxx"))));
+
+}