summaryrefslogtreecommitdiffstats
path: root/net/url_request
diff options
context:
space:
mode:
Diffstat (limited to 'net/url_request')
-rw-r--r--net/url_request/url_request_throttler_manager.cc81
-rw-r--r--net/url_request/url_request_throttler_manager.h24
-rw-r--r--net/url_request/url_request_throttler_unittest.cc29
3 files changed, 24 insertions, 110 deletions
diff --git a/net/url_request/url_request_throttler_manager.cc b/net/url_request/url_request_throttler_manager.cc
index baeeb7e..635c716 100644
--- a/net/url_request/url_request_throttler_manager.cc
+++ b/net/url_request/url_request_throttler_manager.cc
@@ -4,36 +4,9 @@
#include "net/url_request/url_request_throttler_manager.h"
-#include <list>
-
#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 +18,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 +31,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 +64,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 +85,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,47 +98,21 @@ 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 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;
}
}
- CHECK(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());
diff --git a/net/url_request/url_request_throttler_manager.h b/net/url_request/url_request_throttler_manager.h
index 1cd2b5c..c48e917 100644
--- a/net/url_request/url_request_throttler_manager.h
+++ b/net/url_request/url_request_throttler_manager.h
@@ -12,7 +12,7 @@
#include "base/basictypes.h"
#include "base/scoped_ptr.h"
#include "base/singleton.h"
-#include "base/threading/thread_checker_impl.h"
+#include "base/threading/non_thread_safe.h"
#include "googleurl/src/gurl.h"
#include "net/url_request/url_request_throttler_entry.h"
@@ -28,12 +28,10 @@ namespace net {
// clean out outdated entries. URL ID consists of lowercased scheme, host, port
// and path. All URLs converted to the same ID will share the same entry.
//
-// NOTE: All usage of the singleton object of this class should be on the same
-// thread.
-//
-// TODO(joi): Switch back to NonThreadSafe (and remove checks in release builds)
-// once crbug.com/71721 has been tracked down.
-class URLRequestThrottlerManager {
+// NOTE: All usage of this singleton object must be on the same thread,
+// although to allow it to be used as a singleton, construction and destruction
+// can occur on a separate thread.
+class URLRequestThrottlerManager : public base::NonThreadSafe {
public:
static URLRequestThrottlerManager* GetInstance();
@@ -94,20 +92,10 @@ class URLRequestThrottlerManager {
// Number of requests that will be made between garbage collection.
static const unsigned int kRequestsBetweenCollecting;
- // Constructor copies the string "MAGICZZ\0" into this buffer; using it
- // to try to detect memory overwrites affecting url_entries_ in the wild.
- // TODO(joi): Remove once crbug.com/71721 is figured out.
- char magic_buffer_1_[8];
-
// Map that contains a list of URL ID and their matching
// URLRequestThrottlerEntry.
UrlEntryMap url_entries_;
- // Constructor copies the string "GOOGYZZ\0" into this buffer; using it
- // to try to detect memory overwrites affecting url_entries_ in the wild.
- // TODO(joi): Remove once crbug.com/71721 is figured out.
- char magic_buffer_2_[8];
-
// This keeps track of how many requests have been made. Used with
// GarbageCollectEntries.
unsigned int requests_since_last_gc_;
@@ -127,8 +115,6 @@ class URLRequestThrottlerManager {
// workaround.
bool being_tested_;
- base::ThreadCheckerImpl thread_checker_;
-
DISALLOW_COPY_AND_ASSIGN(URLRequestThrottlerManager);
};
diff --git a/net/url_request/url_request_throttler_unittest.cc b/net/url_request/url_request_throttler_unittest.cc
index 4aed38c..febc69c 100644
--- a/net/url_request/url_request_throttler_unittest.cc
+++ b/net/url_request/url_request_throttler_unittest.cc
@@ -333,26 +333,25 @@ TEST(URLRequestThrottlerManager, IsUrlStandardised) {
MockURLRequestThrottlerManager manager;
GurlAndString test_values[] = {
GurlAndString(GURL("http://www.example.com"),
- std::string("GOOGYhttp://www.example.com/MONSTA"),
+ std::string("http://www.example.com/"),
__LINE__),
GurlAndString(GURL("http://www.Example.com"),
- std::string("GOOGYhttp://www.example.com/MONSTA"),
+ std::string("http://www.example.com/"),
__LINE__),
GurlAndString(GURL("http://www.ex4mple.com/Pr4c71c41"),
- std::string("GOOGYhttp://www.ex4mple.com/pr4c71c41MONSTA"),
+ std::string("http://www.ex4mple.com/pr4c71c41"),
+ __LINE__),
+ GurlAndString(GURL("http://www.example.com/0/token/false"),
+ std::string("http://www.example.com/0/token/false"),
__LINE__),
- GurlAndString(
- GURL("http://www.example.com/0/token/false"),
- std::string("GOOGYhttp://www.example.com/0/token/falseMONSTA"),
- __LINE__),
GurlAndString(GURL("http://www.example.com/index.php?code=javascript"),
- std::string("GOOGYhttp://www.example.com/index.phpMONSTA"),
+ std::string("http://www.example.com/index.php"),
__LINE__),
GurlAndString(GURL("http://www.example.com/index.php?code=1#superEntry"),
- std::string("GOOGYhttp://www.example.com/index.phpMONSTA"),
+ std::string("http://www.example.com/index.php"),
__LINE__),
GurlAndString(GURL("http://www.example.com:1234/"),
- std::string("GOOGYhttp://www.example.com:1234/MONSTA"),
+ std::string("http://www.example.com:1234/"),
__LINE__)};
for (unsigned int i = 0; i < arraysize(test_values); ++i) {
@@ -390,13 +389,3 @@ TEST(URLRequestThrottlerManager, IsHostBeingRegistered) {
EXPECT_EQ(3, manager.GetNumberOfEntries());
}
-
-#if defined(GTEST_HAS_DEATH_TEST)
-TEST(URLRequestThrottlerManager, NullHandlingTest) {
- MockURLRequestThrottlerManager manager;
- manager.OverrideEntryForTests(GURL("http://www.foo.com/"), NULL);
- ASSERT_DEATH({
- manager.DoGarbageCollectEntries();
- }, "");
-}
-#endif // defined(GTEST_HAS_DEATH_TEST)
generated by cgit v1.1 at 2025-05-12 04:03:32 +0000