Commit a3ef4383 authored by davidben's avatar davidben Committed by Commit bot

CHECK that URLRequestJobs are not orphaned while blocked by extensions.

This is to help debug https://crbug.com/289715 and try to get a stack trace
earlier.

BUG=289715

Review URL: https://codereview.chromium.org/1337113004

Cr-Commit-Position: refs/heads/master@{#348745}
parent af542442
......@@ -109,6 +109,7 @@ class ChromeExtensionsNetworkDelegateImpl
void OnResponseStarted(net::URLRequest* request) override;
void OnCompleted(net::URLRequest* request, bool started) override;
void OnURLRequestDestroyed(net::URLRequest* request) override;
void OnURLRequestJobOrphaned(net::URLRequest* request) override;
void OnPACScriptError(int line_number, const base::string16& error) override;
net::NetworkDelegate::AuthRequiredResponse OnAuthRequired(
net::URLRequest* request,
......@@ -236,6 +237,12 @@ void ChromeExtensionsNetworkDelegateImpl::OnURLRequestDestroyed(
profile_, request);
}
void ChromeExtensionsNetworkDelegateImpl::OnURLRequestJobOrphaned(
net::URLRequest* request) {
ExtensionWebRequestEventRouter::GetInstance()->OnURLRequestJobOrphaned(
profile_, request);
}
void ChromeExtensionsNetworkDelegateImpl::OnPACScriptError(
int line_number,
const base::string16& error) {
......@@ -340,6 +347,10 @@ void ChromeExtensionsNetworkDelegate::OnURLRequestDestroyed(
net::URLRequest* request) {
}
void ChromeExtensionsNetworkDelegate::OnURLRequestJobOrphaned(
net::URLRequest* request) {
}
void ChromeExtensionsNetworkDelegate::OnPACScriptError(
int line_number,
const base::string16& error) {
......
......@@ -63,6 +63,7 @@ class ChromeExtensionsNetworkDelegate : public net::NetworkDelegateImpl {
void OnResponseStarted(net::URLRequest* request) override;
void OnCompleted(net::URLRequest* request, bool started) override;
void OnURLRequestDestroyed(net::URLRequest* request) override;
void OnURLRequestJobOrphaned(net::URLRequest* request) override;
void OnPACScriptError(int line_number, const base::string16& error) override;
net::NetworkDelegate::AuthRequiredResponse OnAuthRequired(
net::URLRequest* request,
......
......@@ -542,6 +542,10 @@ void ChromeNetworkDelegate::OnURLRequestDestroyed(net::URLRequest* request) {
extensions_delegate_->OnURLRequestDestroyed(request);
}
void ChromeNetworkDelegate::OnURLRequestJobOrphaned(net::URLRequest* request) {
extensions_delegate_->OnURLRequestJobOrphaned(request);
}
void ChromeNetworkDelegate::OnPACScriptError(int line_number,
const base::string16& error) {
extensions_delegate_->OnPACScriptError(line_number, error);
......
......@@ -157,6 +157,7 @@ class ChromeNetworkDelegate : public net::NetworkDelegateImpl {
int64_t bytes_received) override;
void OnCompleted(net::URLRequest* request, bool started) override;
void OnURLRequestDestroyed(net::URLRequest* request) override;
void OnURLRequestJobOrphaned(net::URLRequest* request) override;
void OnPACScriptError(int line_number, const base::string16& error) override;
net::NetworkDelegate::AuthRequiredResponse OnAuthRequired(
net::URLRequest* request,
......
......@@ -8,6 +8,7 @@
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/debug/alias.h"
#include "base/json/json_writer.h"
#include "base/lazy_instance.h"
#include "base/macros.h"
......@@ -1226,6 +1227,20 @@ void ExtensionWebRequestEventRouter::OnURLRequestDestroyed(
base::Time::Now());
}
void ExtensionWebRequestEventRouter::OnURLRequestJobOrphaned(
void* browser_context,
const net::URLRequest* request) {
// See https://crbug.com/289715. While a URLRequest is blocking on an
// extension, it may not orphan jobs unless OnURLRequestDestroyed is called
// first.
//
// TODO(davidben): Remove this when the crash has been diagnosed.
char url_buf[128];
base::strlcpy(url_buf, request->url().spec().c_str(), arraysize(url_buf));
base::debug::Alias(url_buf);
CHECK_EQ(0u, blocked_requests_.count(request->identifier()));
}
void ExtensionWebRequestEventRouter::ClearPendingCallbacks(
const net::URLRequest* request) {
blocked_requests_.erase(request->identifier());
......
......@@ -261,6 +261,10 @@ class ExtensionWebRequestEventRouter
void OnURLRequestDestroyed(void* browser_context,
const net::URLRequest* request);
// See https://crbug.com/289715.
void OnURLRequestJobOrphaned(void* browser_context,
const net::URLRequest* request);
// Called when an event listener handles a blocking event and responds.
void OnEventHandled(void* browser_context,
const std::string& extension_id,
......
......@@ -166,6 +166,12 @@ void LayeredNetworkDelegate::OnURLRequestDestroyedInternal(
URLRequest* request) {
}
void LayeredNetworkDelegate::OnURLRequestJobOrphaned(URLRequest* request) {
// This hook is only added to debug https://crbug.com/289715, so there is no
// need for a OnURLRequestJobOrphanedInternal hook.
nested_network_delegate_->NotifyURLRequestJobOrphaned(request);
}
void LayeredNetworkDelegate::OnPACScriptError(int line_number,
const base::string16& error) {
OnPACScriptErrorInternal(line_number, error);
......
......@@ -68,6 +68,7 @@ class NET_EXPORT LayeredNetworkDelegate : public NetworkDelegate {
int64_t bytes_received) final;
void OnCompleted(URLRequest* request, bool started) final;
void OnURLRequestDestroyed(URLRequest* request) final;
void OnURLRequestJobOrphaned(URLRequest* request) final;
void OnPACScriptError(int line_number, const base::string16& error) final;
AuthRequiredResponse OnAuthRequired(URLRequest* request,
const AuthChallengeInfo& auth_info,
......
......@@ -118,6 +118,12 @@ void NetworkDelegate::NotifyURLRequestDestroyed(URLRequest* request) {
OnURLRequestDestroyed(request);
}
void NetworkDelegate::NotifyURLRequestJobOrphaned(URLRequest* request) {
DCHECK(CalledOnValidThread());
DCHECK(request);
OnURLRequestJobOrphaned(request);
}
void NetworkDelegate::NotifyPACScriptError(int line_number,
const base::string16& error) {
DCHECK(CalledOnValidThread());
......
......@@ -91,6 +91,7 @@ class NET_EXPORT NetworkDelegate : public base::NonThreadSafe {
int64_t bytes_received);
void NotifyCompleted(URLRequest* request, bool started);
void NotifyURLRequestDestroyed(URLRequest* request);
void NotifyURLRequestJobOrphaned(URLRequest* request);
void NotifyPACScriptError(int line_number, const base::string16& error);
AuthRequiredResponse NotifyAuthRequired(URLRequest* request,
const AuthChallengeInfo& auth_info,
......@@ -222,6 +223,13 @@ class NET_EXPORT NetworkDelegate : public base::NonThreadSafe {
// a virtual method call.
virtual void OnURLRequestDestroyed(URLRequest* request) = 0;
// Called when the current job for |request| is orphaned. This is a temporary
// callback to diagnose https://crbug.com/289715 and may not be used for other
// purposes. Note that it may be called after OnURLRequestDestroyed.
//
// TODO(davidben): Remove this once data has been gathered.
virtual void OnURLRequestJobOrphaned(URLRequest* request) = 0;
// Corresponds to ProxyResolverJSBindings::OnError.
virtual void OnPACScriptError(int line_number,
const base::string16& error) = 0;
......
......@@ -65,6 +65,8 @@ void NetworkDelegateImpl::OnCompleted(URLRequest* request, bool started) {
void NetworkDelegateImpl::OnURLRequestDestroyed(URLRequest* request) {
}
void NetworkDelegateImpl::OnURLRequestJobOrphaned(URLRequest* request) {}
void NetworkDelegateImpl::OnPACScriptError(int line_number,
const base::string16& error) {
}
......
......@@ -138,6 +138,13 @@ class NET_EXPORT NetworkDelegateImpl : public NetworkDelegate {
// a virtual method call.
void OnURLRequestDestroyed(URLRequest* request) override;
// Called when the current job for |request| is orphaned. This is a temporary
// callback to diagnose https://crbug.com/289715 and may not be used for other
// purposes. Note that it may be called after OnURLRequestDestroyed.
//
// TODO(davidben): Remove this once data has been gathered.
void OnURLRequestJobOrphaned(URLRequest* request) override;
// Corresponds to ProxyResolverJSBindings::OnError.
void OnPACScriptError(int line_number, const base::string16& error) override;
......
......@@ -882,6 +882,9 @@ void URLRequest::PrepareToRestart() {
}
void URLRequest::OrphanJob() {
if (network_delegate_)
network_delegate_->NotifyURLRequestJobOrphaned(this);
// When calling this function, please check that URLRequestHttpJob is
// not in between calling NetworkDelegate::NotifyHeadersReceived receiving
// the call back. This is currently guaranteed by the following strategies:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment