Add CHECKs to investigate ResourceThrottle crash.

content/browser/loader/throttling_resource_handler.cc

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..d27a7c59d553de98933a28bf2e6a276ceeb21294 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(1, 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(),
+                  sizeof(bad_throttle_name));

[Charlie Harrison, 2016/08/26 18:14:50]

Here you use sizeof but below you use arraysize. Let's be consistent.

[mmenke, 2016/08/26 18:17:57]

oops.  Switched to arraysize, seems more consistent with strlcpy.

+    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;