From 62c4e64828e906b7597eae4418b6cf3c93502d49 Mon Sep 17 00:00:00 2001 From: "rdsmith@google.com" Date: Fri, 2 Jul 2010 21:23:41 +0000 Subject: Revert 51544 - I think the chances are noticeably above zero that this change is responsible for the XP tests (dbg)(4) browser_tests going red, and we're heading into a long weekend. I'll revert, and resubmit next week if it turns out that it wasn't my problem. Original description: 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 TBR=rdsmith@google.com Review URL: http://codereview.chromium.org/2858046 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51567 0039d316-1c4b-4281-b951-d872f2087c98 --- net/base/cookie_monster.cc | 21 ++--- net/base/cookie_monster.h | 7 +- net/base/cookie_monster_unittest.cc | 150 ------------------------------------ 3 files changed, 9 insertions(+), 169 deletions(-) (limited to 'net') diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc index 86d71b8..f6d3a54 100644 --- a/net/base/cookie_monster.cc +++ b/net/base/cookie_monster.cc @@ -901,24 +901,17 @@ int CookieMonster::DeleteAllCreatedAfter(const Time& delete_begin, return DeleteAllCreatedBetween(delete_begin, Time(), sync_to_store); } -int CookieMonster::DeleteAllForHost(const GURL& url) { +int CookieMonster::DeleteAllForURL(const GURL& url, + bool sync_to_store) { AutoLock autolock(lock_); InitIfNecessary(); - 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. + CookieList cookies = InternalGetAllCookiesForURL(url); int num_deleted = 0; - 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); + for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end();) { + CookieMap::iterator curit = it; + ++it; + InternalDeleteCookie(curit, sync_to_store, kDeleteCookieExplicit); } return num_deleted; } diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h index eadc59a..43943a0 100644 --- a/net/base/cookie_monster.h +++ b/net/base/cookie_monster.h @@ -138,11 +138,8 @@ 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 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 all cookies that match the given URL. + int DeleteAllForURL(const GURL& url, bool sync_to_store); // 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 21d6df8..a814480 100644 --- a/net/base/cookie_monster_unittest.cc +++ b/net/base/cookie_monster_unittest.cc @@ -163,91 +163,6 @@ 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 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 @@ -1716,68 +1631,3 @@ TEST(CookieMonsterTest, SetCookieWithDetails) { ASSERT_TRUE(++it == cookies.end()); } - - - -TEST(CookieMonsterTest, DeleteAllForHost) { - scoped_refptr 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")))); - -} -- cgit v1.1