Remove minidump analysis aides from URLRequestThrottlerManager. New

net/url_request/url_request_throttler_manager.cc

Index: net/url_request/url_request_throttler_manager.cc
diff --git a/net/url_request/url_request_throttler_manager.cc b/net/url_request/url_request_throttler_manager.cc
index 330cc48a7b7674fcba9984d42508368487500a8c..383471fd47056293c3566cf77dfcac44837f9245 100644
--- a/net/url_request/url_request_throttler_manager.cc
+++ b/net/url_request/url_request_throttler_manager.cc
@@ -9,31 +9,6 @@
 #include "base/logging.h"
 #include "base/string_util.h"
 
-namespace {
-
-// TODO(joi): Remove after crbug.com/71721 is fixed.
-struct IteratorHistory {
-  // Copy of 'this' pointer at time of access; this helps both because
-  // the this pointer is often obfuscated (at least for this particular
-  // stack trace) in fully optimized builds, and possibly to detect
-  // changes in the this pointer during iteration over the map (e.g.
-  // from another thread overwriting memory).
-  net::URLRequestThrottlerManager* self;
-
-  // Copy of URL key.
-  char url[256];
-
-  // Not a refptr, we don't want to change behavior by keeping it alive.
-  net::URLRequestThrottlerEntryInterface* entry;
-
-  // Set to true if the entry gets erased. Helpful to verify that entries
-  // with 0 refcount (since we don't take a refcount above) have been
-  // erased from the map.
-  bool was_erased;
-};
-
-}  // namespace
-
 namespace net {
 
 const unsigned int URLRequestThrottlerManager::kMaximumNumberOfEntries = 1500;
@@ -45,7 +20,7 @@ URLRequestThrottlerManager* URLRequestThrottlerManager::GetInstance() {
 
 scoped_refptr<URLRequestThrottlerEntryInterface>
     URLRequestThrottlerManager::RegisterRequestUrl(const GURL &url) {
-  CHECK(being_tested_ || thread_checker_.CalledOnValidThread());
+  DCHECK(being_tested_ || CalledOnValidThread());
 
   // Normalize the url.
   std::string url_id = GetIdFromUrl(url);
@@ -58,9 +33,6 @@ scoped_refptr<URLRequestThrottlerEntryInterface>
   if (entry.get() == NULL)
     entry = new URLRequestThrottlerEntry();
 
-  // TODO(joi): Demote CHECKs in this file to DCHECKs (or remove them) once
-  // we fully understand crbug.com/71721
-  CHECK(entry.get());
   return entry;
 }
 
@@ -94,21 +66,17 @@ URLRequestThrottlerManager::URLRequestThrottlerManager()
   // Construction/destruction is on main thread (because BrowserMain
   // retrieves an instance to call InitializeOptions), but is from then on
   // used on I/O thread.
-  thread_checker_.DetachFromThread();
+  DetachFromThread();
 
   url_id_replacements_.ClearPassword();
   url_id_replacements_.ClearUsername();
   url_id_replacements_.ClearQuery();
   url_id_replacements_.ClearRef();
-
-  // TODO(joi): Remove after crbug.com/71721 is fixed.
-  base::strlcpy(magic_buffer_1_, "MAGICZZ", arraysize(magic_buffer_1_));
-  base::strlcpy(magic_buffer_2_, "GOOGYZZ", arraysize(magic_buffer_2_));
 }
 
 URLRequestThrottlerManager::~URLRequestThrottlerManager() {
   // Destruction is on main thread (AtExit), but real use is on I/O thread.
-  thread_checker_.DetachFromThread();
+  DetachFromThread();
 
   // Delete all entries.
   url_entries_.clear();
@@ -119,8 +87,7 @@ std::string URLRequestThrottlerManager::GetIdFromUrl(const GURL& url) const {
     return url.possibly_invalid_spec();
 
   GURL id = url.ReplaceComponents(url_id_replacements_);
-  // TODO(joi): Remove "GOOGY/MONSTA" stuff once crbug.com/71721 is done
-  return StringPrintf("GOOGY%sMONSTA", StringToLowerASCII(id.spec()).c_str());
+  return StringToLowerASCII(id.spec()).c_str();
 }
 
 void URLRequestThrottlerManager::GarbageCollectEntriesIfNecessary() {
@@ -133,48 +100,20 @@ void URLRequestThrottlerManager::GarbageCollectEntriesIfNecessary() {
 }
 
 void URLRequestThrottlerManager::GarbageCollectEntries() {
-  // TODO(joi): Remove these crash report aids once crbug.com/71721
-  // is figured out.
-  IteratorHistory history[32] = { { 0 } };
-  size_t history_ix = 0;
-  history[history_ix++].self = this;
-
-  int nulls_found = 0;
   UrlEntryMap::iterator i = url_entries_.begin();
   while (i != url_entries_.end()) {
-    if (i->second == NULL) {
-      ++nulls_found;
-    }
-
-    // Keep a log of the first 31 items accessed after the first
-    // NULL encountered (hypothesis is there are multiple NULLs,
-    // and we may learn more about pattern of memory overwrite).
-    // We also log when we access the first entry, to get an original
-    // value for our this pointer.
-    if (nulls_found > 0 && history_ix < arraysize(history)) {
-      history[history_ix].self = this;
-      base::strlcpy(history[history_ix].url, i->first.c_str(),
-                    arraysize(history[history_ix].url));
-      history[history_ix].entry = i->second.get();
-      history[history_ix].was_erased = false;
-      ++history_ix;
-    }
-
-    // TODO(joi): Remove first i->second check when crbug.com/71721 is fixed.
+    // TODO(joi): We know, as per crbug.com/71721, that values here can sometimes
+    // be NULL because of a a memory overwrite coming from somewhere else.
+    // Minidumps of the crash are at this point not giving us any new
+    // information so adding the [i->second == NULL] check lessens the impact
+    // on our users (it seems to generally avoid the crash).
     if (i->second == NULL || (i->second)->IsEntryOutdated()) {
       url_entries_.erase(i++);
-
-      if (nulls_found > 0 && (history_ix - 1) < arraysize(history)) {
-        history[history_ix - 1].was_erased = true;
-      }
     } else {
       ++i;
     }
   }
 
-  // TODO(joi): Make this a CHECK again after M11 branch point.
-  DCHECK(nulls_found == 0);
-
   // In case something broke we want to make sure not to grow indefinitely.
   while (url_entries_.size() > kMaximumNumberOfEntries) {
     url_entries_.erase(url_entries_.begin());