Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1507)

Unified Diff: content/browser/loader/throttling_resource_handler.cc

Issue 2271403002: Add CHECKs to investigate ResourceThrottle crash. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Oops Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/browser/loader/throttling_resource_handler.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/loader/throttling_resource_handler.cc
diff --git a/content/browser/loader/throttling_resource_handler.cc b/content/browser/loader/throttling_resource_handler.cc
index cf8ad2247c1f47a9dceb633080ab22195ab27a2f..1bbf792fa9e83be0ca09b986b101efdf582f98f6 100644
--- a/content/browser/loader/throttling_resource_handler.cc
+++ b/content/browser/loader/throttling_resource_handler.cc
@@ -6,6 +6,8 @@
#include <utility>
+#include "base/debug/alias.h"
+#include "base/strings/string_util.h"
#include "content/browser/loader/resource_request_info_impl.h"
#include "content/public/browser/resource_throttle.h"
#include "content/public/common/resource_response.h"
@@ -21,7 +23,8 @@ ThrottlingResourceHandler::ThrottlingResourceHandler(
deferred_stage_(DEFERRED_NONE),
throttles_(std::move(throttles)),
next_index_(0),
- cancelled_by_resource_throttle_(false) {
+ cancelled_by_resource_throttle_(false),
+ currently_calling_throttle_(false) {
for (size_t i = 0; i < throttles_.size(); ++i) {
throttles_[i]->set_controller(this);
// Throttles must have a name, as otherwise, bugs where a throttle fails
@@ -31,18 +34,41 @@ ThrottlingResourceHandler::ThrottlingResourceHandler(
}
ThrottlingResourceHandler::~ThrottlingResourceHandler() {
+ // Check if |this| is being destroyed in a reentrant call from a throttle.
+ if (currently_calling_throttle_) {
+ CHECK_LE(1u, next_index_);
+ CHECK_LE(next_index_, throttles_.size());
+
+ // Stick some information on the stack that may be useful in debugging.
+ char bad_throttle_name[100];
+ base::strlcpy(bad_throttle_name,
+ throttles_[next_index_ - 1]->GetNameForLogging(),
+ arraysize(bad_throttle_name));
+ DeferredStage deferred_stage = deferred_stage_;
+ char url[128];
+ // The request should still be valid at this point...
+ base::strlcpy(url, request()->url().spec().c_str(), arraysize(url));
+ base::debug::Alias(bad_throttle_name);
+ base::debug::Alias(&deferred_stage);
+ base::debug::Alias(url);
+
+ CHECK(false);
+ }
}
bool ThrottlingResourceHandler::OnRequestRedirected(
const net::RedirectInfo& redirect_info,
ResourceResponse* response,
bool* defer) {
+ CHECK(!currently_calling_throttle_);
DCHECK(!cancelled_by_resource_throttle_);
*defer = false;
while (next_index_ < throttles_.size()) {
int index = next_index_;
+ currently_calling_throttle_ = true;
throttles_[index]->WillRedirectRequest(redirect_info, defer);
+ currently_calling_throttle_ = false;
next_index_++;
if (cancelled_by_resource_throttle_)
return false;
@@ -61,12 +87,15 @@ bool ThrottlingResourceHandler::OnRequestRedirected(
}
bool ThrottlingResourceHandler::OnWillStart(const GURL& url, bool* defer) {
+ CHECK(!currently_calling_throttle_);
DCHECK(!cancelled_by_resource_throttle_);
*defer = false;
while (next_index_ < throttles_.size()) {
int index = next_index_;
+ currently_calling_throttle_ = true;
throttles_[index]->WillStartRequest(defer);
+ currently_calling_throttle_ = false;
next_index_++;
if (cancelled_by_resource_throttle_)
return false;
@@ -85,11 +114,14 @@ bool ThrottlingResourceHandler::OnWillStart(const GURL& url, bool* defer) {
bool ThrottlingResourceHandler::OnResponseStarted(ResourceResponse* response,
bool* defer) {
+ CHECK(!currently_calling_throttle_);
DCHECK(!cancelled_by_resource_throttle_);
while (next_index_ < throttles_.size()) {
int index = next_index_;
+ currently_calling_throttle_ = true;
throttles_[index]->WillProcessResponse(defer);
+ currently_calling_throttle_ = false;
next_index_++;
if (cancelled_by_resource_throttle_)
return false;
@@ -122,7 +154,11 @@ void ThrottlingResourceHandler::CancelWithError(int error_code) {
}
void ThrottlingResourceHandler::Resume() {
+ CHECK(!currently_calling_throttle_);
DCHECK(!cancelled_by_resource_throttle_);
+ // TODO(mmenke): Remove CHECK once https://crbug.com/640545 is resolved, as
+ // it's redundant with the NOTREACHED() below.
+ CHECK_NE(DEFERRED_NONE, deferred_stage_);
DeferredStage last_deferred_stage = deferred_stage_;
deferred_stage_ = DEFERRED_NONE;
@@ -145,6 +181,7 @@ void ThrottlingResourceHandler::Resume() {
}
void ThrottlingResourceHandler::ResumeStart() {
+ CHECK(!currently_calling_throttle_);
DCHECK(!cancelled_by_resource_throttle_);
GURL url = deferred_url_;
@@ -159,6 +196,7 @@ void ThrottlingResourceHandler::ResumeStart() {
}
void ThrottlingResourceHandler::ResumeRedirect() {
+ CHECK(!currently_calling_throttle_);
DCHECK(!cancelled_by_resource_throttle_);
net::RedirectInfo redirect_info = deferred_redirect_;
@@ -175,6 +213,7 @@ void ThrottlingResourceHandler::ResumeRedirect() {
}
void ThrottlingResourceHandler::ResumeResponse() {
+ CHECK(!currently_calling_throttle_);
DCHECK(!cancelled_by_resource_throttle_);
scoped_refptr<ResourceResponse> response;
« no previous file with comments | « content/browser/loader/throttling_resource_handler.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698