Fix HttpPipeliningCompatibilityClient ownership.
Previously it was unowned. Now IOThread::Globals owns it. Due to this, move the CHECK for no leaking URLRequests to the IOThread::Globals destructor.
Also, fix up some lifetime ordering issues with HttpPipeliningCompatibilityClient. This lets us delete the HttpPipeliningCompatibilityClient synchronously from within the finishing callback.
BUG=123830
TEST=none
Review URL: http://codereview.chromium.org/10041008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@132715 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 1e978791..15361ea5 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -24,6 +24,7 @@
#include "chrome/browser/net/chrome_network_delegate.h"
#include "chrome/browser/net/chrome_url_request_context.h"
#include "chrome/browser/net/connect_interceptor.h"
+#include "chrome/browser/net/http_pipelining_compatibility_client.h"
#include "chrome/browser/net/pref_proxy_config_tracker.h"
#include "chrome/browser/net/proxy_service_factory.h"
#include "chrome/browser/net/sdch_dictionary_fetcher.h"
@@ -286,7 +287,9 @@
IOThread::Globals::Globals() {}
-IOThread::Globals::~Globals() {}
+IOThread::Globals::~Globals() {
+ system_request_context->AssertNoURLRequests();
+}
// |local_state| is passed in explicitly in order to (1) reduce implicit
// dependencies and (2) make IOThread more flexible for testing.
@@ -465,8 +468,6 @@
net::ShutdownNSSHttpIO();
#endif // defined(USE_NSS)
- globals_->system_request_context->AssertNoURLRequests();
-
system_url_request_context_getter_ = NULL;
// Release objects that the net::URLRequestContext could have been pointing
diff --git a/chrome/browser/io_thread.h b/chrome/browser/io_thread.h
index 2cffacfa..5bf248a 100644
--- a/chrome/browser/io_thread.h
+++ b/chrome/browser/io_thread.h
@@ -25,6 +25,10 @@
class PrefService;
class SystemURLRequestContextGetter;
+namespace chrome_browser_net {
+class HttpPipeliningCompatibilityClient;
+}
+
namespace net {
class CertVerifier;
class CookieStore;
@@ -91,6 +95,8 @@
scoped_ptr<net::ServerBoundCertService> system_server_bound_cert_service;
scoped_refptr<ExtensionEventRouterForwarder>
extension_event_router_forwarder;
+ scoped_ptr<chrome_browser_net::HttpPipeliningCompatibilityClient>
+ http_pipelining_compatibility_client;
};
// |net_log| must either outlive the IOThread or be NULL.
diff --git a/chrome/browser/net/http_pipelining_compatibility_client.cc b/chrome/browser/net/http_pipelining_compatibility_client.cc
index 1fc294fb..0708404 100644
--- a/chrome/browser/net/http_pipelining_compatibility_client.cc
+++ b/chrome/browser/net/http_pipelining_compatibility_client.cc
@@ -437,24 +437,18 @@
namespace {
-void DeleteClient(HttpPipeliningCompatibilityClient* client) {
- delete client;
-}
-
-void DelayedDeleteClient(HttpPipeliningCompatibilityClient* client, int rv) {
- content::BrowserThread::PostTask(
- content::BrowserThread::IO,
- FROM_HERE,
- base::Bind(&DeleteClient, client));
+void DeleteClient(IOThread* io_thread, int /* rv */) {
+ DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+ io_thread->globals()->http_pipelining_compatibility_client.reset();
}
void CollectPipeliningCapabilityStatsOnIOThread(
const std::string& pipeline_test_server,
- net::URLRequestContextGetter* url_request_context_getter) {
+ IOThread* io_thread) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
net::URLRequestContext* url_request_context =
- url_request_context_getter->GetURLRequestContext();
+ io_thread->globals()->system_request_context.get();
if (!url_request_context->proxy_service()->config().proxy_rules().empty()) {
// Pipelining with explicitly configured proxies is disabled for now.
return;
@@ -516,8 +510,9 @@
new HttpPipeliningCompatibilityClient(NULL);
client->Start(pipeline_test_server, requests,
HttpPipeliningCompatibilityClient::PIPE_TEST_CANARY_AND_STATS,
- base::Bind(&DelayedDeleteClient, client),
- url_request_context_getter->GetURLRequestContext());
+ base::Bind(&DeleteClient, io_thread),
+ url_request_context);
+ io_thread->globals()->http_pipelining_compatibility_client.reset(client);
}
} // anonymous namespace
@@ -533,8 +528,7 @@
FROM_HERE,
base::Bind(&CollectPipeliningCapabilityStatsOnIOThread,
pipeline_test_server,
- make_scoped_refptr(
- io_thread->system_url_request_context_getter())));
+ io_thread));
}
} // namespace chrome_browser_net
diff --git a/chrome/browser/net/http_pipelining_compatibility_client.h b/chrome/browser/net/http_pipelining_compatibility_client.h
index 88159b30..b50f9d3 100644
--- a/chrome/browser/net/http_pipelining_compatibility_client.h
+++ b/chrome/browser/net/http_pipelining_compatibility_client.h
@@ -141,14 +141,14 @@
virtual void ReportNetworkError(int request_id, int error_code) OVERRIDE;
virtual void ReportResponseCode(int request_id, int response_code) OVERRIDE;
+ scoped_refptr<net::URLRequestContext> url_request_context_;
+ scoped_ptr<net::HttpTransactionFactory> http_transaction_factory_;
scoped_ptr<internal::PipelineTestRequest::Factory> factory_;
ScopedVector<internal::PipelineTestRequest> requests_;
scoped_ptr<internal::PipelineTestRequest> canary_request_;
net::CompletionCallback finished_callback_;
size_t num_finished_;
size_t num_succeeded_;
- scoped_ptr<net::HttpTransactionFactory> http_transaction_factory_;
- scoped_refptr<net::URLRequestContext> url_request_context_;
};
void CollectPipeliningCapabilityStatsOnUIThread(
