Remove CookieMonster bug catching stuff.

BUG=74585
TEST=net_unittests, try bots.

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

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

4 files changed, 3 insertions, 153 deletions

diff --git a/chrome/browser/tab_contents/tab_specific_content_settings.cc b/chrome/browser/tab_contents/tab_specific_content_settings.cc
index 7dcc0ab..113d528 100644
--- a/chrome/browser/tab_contents/tab_specific_content_settings.cc
+++ b/chrome/browser/tab_contents/tab_specific_content_settings.cc
@@ -10,7 +10,6 @@
 #include "chrome/browser/browsing_data_indexed_db_helper.h"
 #include "chrome/browser/browsing_data_local_storage_helper.h"
 #include "chrome/browser/cookies_tree_model.h"
-#include "chrome/browser/prerender/prerender_manager.h"
 #include "net/base/cookie_monster.h"
 
 bool TabSpecificContentSettings::LocalSharedObjectsContainer::empty() const {
@@ -115,7 +114,6 @@ void TabSpecificContentSettings::OnCookiesRead(
   typedef net::CookieList::const_iterator cookie_iterator;
   for (cookie_iterator cookie = cookie_list.begin();
        cookie != cookie_list.end(); ++cookie) {
-    container.cookies()->ValidateMap(prerender::PrerenderManager::GetMode());
     container.cookies()->SetCookieWithDetails(url,
                                               cookie->Name(),
                                               cookie->Value(),
@@ -124,7 +122,6 @@ void TabSpecificContentSettings::OnCookiesRead(
                                               cookie->ExpiryDate(),
                                               cookie->IsSecure(),
                                               cookie->IsHttpOnly());
-    container.cookies()->ValidateMap(prerender::PrerenderManager::GetMode());
   }
   if (blocked_by_policy)
     OnContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES, std::string());
@@ -138,20 +135,12 @@ void TabSpecificContentSettings::OnCookieChanged(
     const net::CookieOptions& options,
     bool blocked_by_policy) {
   if (blocked_by_policy) {
-    blocked_local_shared_objects_.cookies()->ValidateMap(
-        prerender::PrerenderManager::GetMode());
     blocked_local_shared_objects_.cookies()->SetCookieWithOptions(
         url, cookie_line, options);
-    blocked_local_shared_objects_.cookies()->ValidateMap(
-        prerender::PrerenderManager::GetMode());
     OnContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES, std::string());
   } else {
-    allowed_local_shared_objects_.cookies()->ValidateMap(
-        prerender::PrerenderManager::GetMode());
     allowed_local_shared_objects_.cookies()->SetCookieWithOptions(
         url, cookie_line, options);
-    allowed_local_shared_objects_.cookies()->ValidateMap(
-        prerender::PrerenderManager::GetMode());
     OnContentAccessed(CONTENT_SETTINGS_TYPE_COOKIES);
   }
 }
@@ -301,7 +290,6 @@ TabSpecificContentSettings::LocalSharedObjectsContainer::
 
 TabSpecificContentSettings::LocalSharedObjectsContainer::
     ~LocalSharedObjectsContainer() {
-  cookies_->ValidateMap(prerender::PrerenderManager::GetMode());
 }
 
 void TabSpecificContentSettings::LocalSharedObjectsContainer::Reset() {
diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index b850bbe..46d21b5 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -65,8 +65,6 @@ using base::TimeDelta;
 using base::TimeTicks;
 
 static const int kMinutesInTenYears = 10 * 365 * 24 * 60;
-static const int kMonsterAlive = 0xBBEEEEFF;
-static const int kMonsterDead = 0xFFEEEEDD;
 
 namespace net {
 
@@ -395,8 +393,7 @@ CookieMonster::CookieMonster(PersistentCookieStore* store, Delegate* delegate)
           TimeDelta::FromSeconds(kDefaultAccessUpdateThresholdSeconds)),
       delegate_(delegate),
       last_statistic_record_time_(Time::Now()),
-      keep_expired_cookies_(false),
-      monster_alive_(kMonsterAlive) {
+      keep_expired_cookies_(false) {
   InitializeHistograms();
   SetDefaultCookieableSchemes();
 }
@@ -411,8 +408,7 @@ CookieMonster::CookieMonster(PersistentCookieStore* store,
           last_access_threshold_milliseconds)),
       delegate_(delegate),
       last_statistic_record_time_(base::Time::Now()),
-      keep_expired_cookies_(false),
-      monster_alive_(kMonsterAlive) {
+      keep_expired_cookies_(false) {
   InitializeHistograms();
   SetDefaultCookieableSchemes();
 }
@@ -544,7 +540,6 @@ bool CookieMonster::SetCookieWithDetails(
     const std::string& domain, const std::string& path,
     const base::Time& expiration_time, bool secure, bool http_only) {
   base::AutoLock autolock(lock_);
-  ValidateMapWhileLockHeld(0);
 
   if (!HasCookieableScheme(url))
     return false;
@@ -571,7 +566,6 @@ bool CookieMonster::SetCookieWithDetails(
 
 CookieList CookieMonster::GetAllCookies() {
   base::AutoLock autolock(lock_);
-  ValidateMapWhileLockHeld(0);
   InitIfNecessary();
 
   // This function is being called to scrape the cookie list for management UI
@@ -600,7 +594,6 @@ CookieList CookieMonster::GetAllCookies() {
        it != cookie_ptrs.end(); ++it)
     cookie_list.push_back(**it);
 
-  ValidateMapWhileLockHeld(0);
   return cookie_list;
 }
 
@@ -608,7 +601,6 @@ CookieList CookieMonster::GetAllCookiesForURLWithOptions(
     const GURL& url,
     const CookieOptions& options) {
   base::AutoLock autolock(lock_);
-  ValidateMapWhileLockHeld(0);
   InitIfNecessary();
 
   std::vector<CanonicalCookie*> cookie_ptrs;
@@ -632,7 +624,6 @@ CookieList CookieMonster::GetAllCookiesForURL(const GURL& url) {
 
 int CookieMonster::DeleteAll(bool sync_to_store) {
   base::AutoLock autolock(lock_);
-  ValidateMapWhileLockHeld(0);
   if (sync_to_store)
     InitIfNecessary();
 
@@ -653,7 +644,6 @@ int CookieMonster::DeleteAllCreatedBetween(const Time& delete_begin,
                                            const Time& delete_end,
                                            bool sync_to_store) {
   base::AutoLock autolock(lock_);
-  ValidateMapWhileLockHeld(0);
   InitIfNecessary();
 
   int num_deleted = 0;
@@ -679,7 +669,6 @@ int CookieMonster::DeleteAllCreatedAfter(const Time& delete_begin,
 
 int CookieMonster::DeleteAllForHost(const GURL& url) {
   base::AutoLock autolock(lock_);
-  ValidateMapWhileLockHeld(0);
   InitIfNecessary();
 
   if (!HasCookieableScheme(url))
@@ -711,7 +700,6 @@ int CookieMonster::DeleteAllForHost(const GURL& url) {
 
 bool CookieMonster::DeleteCanonicalCookie(const CanonicalCookie& cookie) {
   base::AutoLock autolock(lock_);
-  ValidateMapWhileLockHeld(0);
   InitIfNecessary();
 
   for (CookieMapItPair its = cookies_.equal_range(GetKey(cookie.Domain()));
@@ -851,106 +839,14 @@ CookieMonster* CookieMonster::GetCookieMonster() {
   return this;
 }
 
-void CookieMonster::ValidateMap(int arg) {
-  base::AutoLock autolock(lock_);
-  ValidateMapWhileLockHeld(arg);
-}
-
-// TODO(eroman): Get rid of this once 74585 is fixed.
-void AskUserToReportIssue() {
-#if defined(OS_WIN)
-  MessageBox(
-      NULL,
-      L"Chrome has crashed due to issue 74585.\n"
-      L"Please help us fix the problem by adding a comment on:\n\n"
-      L"    http://crbug.com/74585\n\n"
-      L"(which explains how you triggered this.) Thanks!",
-      L"OOPS!",
-      MB_ICONERROR | MB_SETFOREGROUND | MB_TOPMOST);
-#endif  // defined(OS_WIN)
-}
-
-
-void CookieMonster::ValidateMapWhileLockHeld(int arg) {
-  lock_.AssertAcquired();
-
-  // Skipping InitIfNecessary() to allow validation before first use.
-  CHECK_EQ(kMonsterAlive, monster_alive_);
-
-  // Check if any of the pointers in the map are NULL (they shouldn't be).
-  CookieMap::iterator it = cookies_.begin();
-  int i = 0;
-
-  for (; it != cookies_.end(); ++it, ++i) {
-    if (!it->second) {
-      // The map is invalid!
-      AskUserToReportIssue();
-      CrashBecauseCookieMapIsInvalid(cookies_, it, i);
-    }
-  }
-}
-
-// Disable optimizations so that our variables will be stack-resident in the
-// minidumps.
-#if defined(OS_WIN)
-#pragma warning (disable: 4748)
-#pragma optimize( "", off )
-#endif
-
-// TODO(eroman): Get rid of this once 74585 is fixed.
-void CookieMonster::CrashBecauseCookieMapIsInvalid(
-    const CookieMap& cookies,
-    const CookieMap::iterator& null_value_it,
-    int null_value_pos) {
-  int num_null_values = 0;
-
-  // Check how many other NULL values the map contains (in case there is more
-  // than one).
-  for (CookieMap::const_iterator it = cookies.begin(); it != cookies.end();
-       ++it) {
-    if (!it->second) {
-      num_null_values += 1;
-    }
-  }
-
-  // Keep track of how big the map was.
-  int size = cookies.size();
-
-  // Check that the non-NULL pointers are not garbage memory. If they are,
-  // the following loop will likely crash when calling IsEquivalent()
-  CanonicalCookie cc(
-      GURL("http://bogus-cc/"), "bogus name", "bogus value", "bogus domain",
-      "path", true, true, base::Time(), base::Time(), false, base::Time());
-  for (CookieMap::const_iterator it = cookies.begin(); it != cookies.end();
-       ++it) {
-    if (it->second) {
-      CHECK(!cc.IsEquivalent(*it->second));
-    }
-  }
-
-  // Crash!
-  CHECK(false);
-
-  // The following shouldn't be necessary since we disabled optimizations, but
-  // it can't hurt ;)
-  LOG(INFO) << num_null_values << " " << size << " " << null_value_pos;
-}
-
-#if defined(OS_WIN)
-#pragma optimize( "", on )
-#pragma warning (default: 4748)
-#endif
-
 CookieMonster::~CookieMonster() {
   DeleteAll(false);
-  monster_alive_ = kMonsterDead;
 }
 
 bool CookieMonster::SetCookieWithCreationTime(const GURL& url,
                                               const std::string& cookie_line,
                                               const base::Time& creation_time) {
   base::AutoLock autolock(lock_);
-  CHECK_EQ(kMonsterAlive, monster_alive_);
 
   if (!HasCookieableScheme(url)) {
     return false;
@@ -1018,7 +914,6 @@ void CookieMonster::InitStore() {
 
 void CookieMonster::EnsureCookiesMapIsValid() {
   lock_.AssertAcquired();
-  ValidateMapWhileLockHeld(0);
 
   int num_duplicates_trimmed = 0;
 
@@ -1045,7 +940,6 @@ int CookieMonster::TrimDuplicateCookiesForKey(
     CookieMap::iterator begin,
     CookieMap::iterator end) {
   lock_.AssertAcquired();
-  ValidateMapWhileLockHeld(0);
 
   // Set of cookies ordered by creation time.
   typedef std::set<CookieMap::iterator, OrderByCreationTimeDesc> CookieSet;
@@ -1144,7 +1038,6 @@ void CookieMonster::FindCookiesForHostAndDomain(
     bool update_access_time,
     std::vector<CanonicalCookie*>* cookies) {
   lock_.AssertAcquired();
-  ValidateMapWhileLockHeld(0);
 
   const Time current_time(CurrentTime());
 
@@ -1244,7 +1137,6 @@ bool CookieMonster::DeleteAnyEquivalentCookie(const std::string& key,
                                               bool skip_httponly,
                                               bool already_expired) {
   lock_.AssertAcquired();
-  ValidateMapWhileLockHeld(0);
 
   bool found_equivalent_cookie = false;
   bool skipped_httponly = false;
@@ -1275,7 +1167,6 @@ void CookieMonster::InternalInsertCookie(const std::string& key,
                                          CanonicalCookie* cc,
                                          bool sync_to_store) {
   lock_.AssertAcquired();
-  ValidateMapWhileLockHeld(0);
 
   if (cc->IsPersistent() && store_ && sync_to_store)
     store_->AddCookie(*cc);
@@ -1292,7 +1183,6 @@ bool CookieMonster::SetCookieWithCreationTimeAndOptions(
     const Time& creation_time_or_null,
     const CookieOptions& options) {
   lock_.AssertAcquired();
-  ValidateMapWhileLockHeld(0);
 
   VLOG(kVlogSetCookies) << "SetCookie() line: " << cookie_line;
 
@@ -1377,7 +1267,6 @@ bool CookieMonster::SetCanonicalCookie(scoped_ptr<CanonicalCookie>* cc,
 void CookieMonster::InternalUpdateCookieAccessTime(CanonicalCookie* cc,
                                                    const Time& current) {
   lock_.AssertAcquired();
-  ValidateMapWhileLockHeld(0);
 
   // Based off the Mozilla code.  When a cookie has been accessed recently,
   // don't bother updating its access time again.  This reduces the number of
@@ -1423,8 +1312,6 @@ void CookieMonster::InternalDeleteCookie(CookieMap::iterator it,
   }
   cookies_.erase(it);
   delete cc;
-
-  ValidateMapWhileLockHeld(0);
 }
 
 // Domain expiry behavior is unchanged by key/expiry scheme (the
@@ -1435,7 +1322,6 @@ void CookieMonster::InternalDeleteCookie(CookieMap::iterator it,
 int CookieMonster::GarbageCollect(const Time& current,
                                   const std::string& key) {
   lock_.AssertAcquired();
-  ValidateMapWhileLockHeld(0);
 
   int num_deleted = 0;
 
@@ -1509,8 +1395,6 @@ int CookieMonster::GarbageCollect(const Time& current,
     }
   }
 
-  ValidateMapWhileLockHeld(0);
-
   return num_deleted;
 }
 
diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h
index 869b0f5..8a0d591 100644
--- a/net/base/cookie_monster.h
+++ b/net/base/cookie_monster.h
@@ -300,14 +300,6 @@ class CookieMonster : public CookieStore {
     DELETE_COOKIE_LAST_ENTRY
   };
 
-  // Debugging methods
-  // TODO(eroman): Delete these when 74585 is sorted out.
-  void ValidateMapWhileLockHeld(int arg);
-  static void CrashBecauseCookieMapIsInvalid(
-      const CookieMap& cookies,
-      const CookieMap::iterator& null_value_it,
-      int null_value_pos);
-
   // Cookie garbage collection thresholds.  Based off of the Mozilla defaults.
   // When the number of cookies gets to k{Domain,}MaxCookies
   // purge down to k{Domain,}MaxCookies - k{Domain,}PurgeCookies.
@@ -531,10 +523,6 @@ class CookieMonster : public CookieStore {
 
   static bool enable_file_scheme_;
 
-  // For temporary debugging; allows for an assert in ValidateMap()
-  // if that function is called post-destruction.
-  int monster_alive_;
-
   DISALLOW_COPY_AND_ASSIGN(CookieMonster);
 };
 
diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc
index d56c8a1..a430833 100644
--- a/net/base/cookie_monster_unittest.cc
+++ b/net/base/cookie_monster_unittest.cc
@@ -1945,7 +1945,6 @@ TEST(CookieMonsterTest, CookieOrdering) {
     EXPECT_EQ("b", cookies[i++].Name());
     EXPECT_EQ("c", cookies[i++].Name());
   }
-  cm->ValidateMap(0);  // Quick check that ValidateMap() doesn't fail.
 }
 
 
@@ -1964,13 +1963,7 @@ static CookieMonster* CreateMonsterForGC(int num_cookies) {
 // get rid of cookies when we should).  The perftest is probing for
 // whether garbage collection happens when it shouldn't.  See comments
 // before that test for more details.
-
-// TODO(eroman): Re-enable this test! I disabled this test because after
-// committing r77790 (which adds some validation checks), the test started
-// timing out on tsan bots. This test was already running pretty slowly
-// (3 minutes), but my change must have put it over the edge. r77790 is
-// only temporary, so once it is reverted this can be re-enabled.
-TEST(CookieMonsterTest, DISABLED_GarbageCollectionTriggers) {
+TEST(CookieMonsterTest, GarbageCollectionTriggers) {
   // First we check to make sure that a whole lot of recent cookies
   // doesn't get rid of anything after garbage collection is checked for.
   {
@@ -2038,14 +2031,11 @@ TEST(CookieMonsterTest, DISABLED_GarbageCollectionTriggers) {
               test_case->num_cookies, test_case->num_old_cookies,
               CookieMonster::kSafeFromGlobalPurgeDays * 2));
       cm->SetExpiryAndKeyScheme(schemes[recent_scheme]);
-      cm->ValidateMap(0);
       EXPECT_EQ(test_case->expected_initial_cookies,
                 static_cast<int>(cm->GetAllCookies().size()))
           << "For test case " << ci;
       // Will trigger GC
-      cm->ValidateMap(0);
       cm->SetCookie(GURL("http://newdomain.com"), "b=2");
-      cm->ValidateMap(0);
       EXPECT_EQ(test_case->expected_cookies_after_set[recent_scheme],
                 static_cast<int>((cm->GetAllCookies().size())))
           << "For test case (" << ci << ", " << recent_scheme << ")";