diff options
author | eroman@chromium.org <eroman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-16 00:34:13 +0000 |
---|---|---|
committer | eroman@chromium.org <eroman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-16 00:34:13 +0000 |
commit | bfb95f5dc2660a179c45066225a366b1ade2a81f (patch) | |
tree | 735716697a61a87e9d63de26ea49bf9b32d1d7e7 /net | |
parent | 46c39f3ba687831eb0bdee81d7b893af50235110 (diff) | |
download | chromium_src-bfb95f5dc2660a179c45066225a366b1ade2a81f.zip chromium_src-bfb95f5dc2660a179c45066225a366b1ade2a81f.tar.gz chromium_src-bfb95f5dc2660a179c45066225a366b1ade2a81f.tar.bz2 |
Add some additional instrumention for bug 74585. It does the following:
(1) On Windows when the corruption is detected, prompts the user asking them to report it on http://crbug.com/74585.
This message is NOT internationalized, it is english only. But hopefully we will get some feedback from it.
(2) Before crashing, we do some additional checks in the map to see:
- how many pointers were NULL
- how many entries the map had
- dereference all the non-NULL values in the map to make sure they aren't garbage memory.
BUG=74585
Review URL: http://codereview.chromium.org/6693009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@78310 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/base/cookie_monster.cc | 82 | ||||
-rw-r--r-- | net/base/cookie_monster.h | 8 |
2 files changed, 84 insertions, 6 deletions
diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc index 1117add..9de1296c 100644 --- a/net/base/cookie_monster.cc +++ b/net/base/cookie_monster.cc @@ -822,17 +822,91 @@ void CookieMonster::ValidateMap(int arg) { 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_); - for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end(); ++it) - CHECK(it->second); - // Keep routine from optimizing away. - VLOG(1) << "ValidateMap arg was " << arg; + + // 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; diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h index 0155735..b6649715 100644 --- a/net/base/cookie_monster.h +++ b/net/base/cookie_monster.h @@ -288,9 +288,13 @@ class CookieMonster : public CookieStore { DELETE_COOKIE_LAST_ENTRY }; - // Debugging method - // TODO(eroman): Delete this when 74585 is sorted out. + // 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 |