Stop refcounting URLRequestContext.

While doing so, fix a few issues with the code like ordering of URLRequestContext to ensure correct destruction order. Also fix const correctness in some places.

BUG=58859
TEST=none
TBR=willchan

Review URL: https://chromiumcodereview.appspot.com/10299002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@136497 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 32 insertions, 40 deletions

diff --git a/net/proxy/dhcp_proxy_script_adapter_fetcher_win_unittest.cc b/net/proxy/dhcp_proxy_script_adapter_fetcher_win_unittest.cc
index 9c66d11..0c612eb 100644
--- a/net/proxy/dhcp_proxy_script_adapter_fetcher_win_unittest.cc
+++ b/net/proxy/dhcp_proxy_script_adapter_fetcher_win_unittest.cc
@@ -150,7 +150,7 @@ class FetcherClient {
   }
 
   TestCompletionCallback callback_;
-  scoped_refptr<URLRequestContext> url_request_context_;
+  scoped_ptr<URLRequestContext> url_request_context_;
   scoped_ptr<MockDhcpProxyScriptAdapterFetcher> fetcher_;
   string16 pac_text_;
 };
@@ -278,11 +278,10 @@ TEST(DhcpProxyScriptAdapterFetcher, MockDhcpRealFetch) {
   GURL configured_url = test_server.GetURL("files/downloadable.pac");
 
   FetcherClient client;
-  scoped_refptr<URLRequestContext> url_request_context(
-      new TestURLRequestContext());
+  TestURLRequestContext url_request_context;
   client.fetcher_.reset(
       new MockDhcpRealFetchProxyScriptAdapterFetcher(
-          url_request_context.get()));
+          &url_request_context));
   client.fetcher_->configured_url_ = configured_url.spec();
   client.RunTest();
   client.WaitForResult(OK);
diff --git a/net/proxy/dhcp_proxy_script_fetcher_factory_unittest.cc b/net/proxy/dhcp_proxy_script_fetcher_factory_unittest.cc
index 97730bc..9eb7c67 100644
--- a/net/proxy/dhcp_proxy_script_fetcher_factory_unittest.cc
+++ b/net/proxy/dhcp_proxy_script_fetcher_factory_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -22,8 +22,8 @@ TEST(DhcpProxyScriptFetcherFactoryTest, WindowsFetcherOnWindows) {
   DhcpProxyScriptFetcherFactory factory;
   factory.set_enabled(true);
 
-  scoped_refptr<TestURLRequestContext> context(new TestURLRequestContext());
-  scoped_ptr<DhcpProxyScriptFetcher> fetcher(factory.Create(context));
+  scoped_ptr<TestURLRequestContext> context(new TestURLRequestContext());
+  scoped_ptr<DhcpProxyScriptFetcher> fetcher(factory.Create(context.get()));
   EXPECT_EQ("win", fetcher->GetFetcherName());
 }
 #endif  // defined(OS_WIN)
diff --git a/net/proxy/dhcp_proxy_script_fetcher_win_unittest.cc b/net/proxy/dhcp_proxy_script_fetcher_win_unittest.cc
index 644b533..e05f7c1 100644
--- a/net/proxy/dhcp_proxy_script_fetcher_win_unittest.cc
+++ b/net/proxy/dhcp_proxy_script_fetcher_win_unittest.cc
@@ -48,7 +48,7 @@ TEST(DhcpProxyScriptFetcherWin, AdapterNamesAndPacURLFromDhcp) {
 class RealFetchTester {
  public:
   RealFetchTester()
-      : context_((new TestURLRequestContext())),
+      : context_(new TestURLRequestContext),
         fetcher_(new DhcpProxyScriptFetcherWin(context_.get())),
         finished_(false),
         on_completion_is_error_(false) {
@@ -114,7 +114,7 @@ class RealFetchTester {
     base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(30));
   }
 
-  scoped_refptr<URLRequestContext> context_;
+  scoped_ptr<URLRequestContext> context_;
   scoped_ptr<DhcpProxyScriptFetcherWin> fetcher_;
   bool finished_;
   string16 pac_text_;
@@ -200,7 +200,7 @@ TEST(DhcpProxyScriptFetcherWin, RealFetchWithDeferredCancel) {
   // the cancel is called before they complete.
   RealFetchTester fetcher;
   fetcher.fetcher_.reset(
-      new DelayingDhcpProxyScriptFetcherWin(fetcher.context_));
+      new DelayingDhcpProxyScriptFetcherWin(fetcher.context_.get()));
   fetcher.on_completion_is_error_ = true;
   fetcher.RunTestWithDeferredCancel();
   fetcher.WaitUntilDone();
@@ -372,7 +372,7 @@ class FetcherClient {
 public:
   FetcherClient()
       : context_(new TestURLRequestContext),
-        fetcher_(context_),
+        fetcher_(context_.get()),
         finished_(false),
         result_(ERR_UNEXPECTED) {
   }
@@ -411,7 +411,7 @@ public:
     fetcher_.ResetTestState();
   }
 
-  scoped_refptr<URLRequestContext> context_;
+  scoped_ptr<URLRequestContext> context_;
   MockDhcpProxyScriptFetcherWin fetcher_;
   bool finished_;
   int result_;
@@ -421,9 +421,9 @@ public:
 // We separate out each test's logic so that we can easily implement
 // the ReuseFetcher test at the bottom.
 void TestNormalCaseURLConfiguredOneAdapter(FetcherClient* client) {
-  scoped_refptr<URLRequestContext> context(new TestURLRequestContext);
+  TestURLRequestContext context;
   scoped_ptr<DummyDhcpProxyScriptAdapterFetcher> adapter_fetcher(
-      new DummyDhcpProxyScriptAdapterFetcher(context));
+      new DummyDhcpProxyScriptAdapterFetcher(&context));
   adapter_fetcher->Configure(true, OK, L"bingo", 1);
   client->fetcher_.PushBackAdapter("a", adapter_fetcher.release());
   client->RunTest();
@@ -573,9 +573,9 @@ TEST(DhcpProxyScriptFetcherWin, ShortCircuitLessPreferredAdapters) {
 }
 
 void TestImmediateCancel(FetcherClient* client) {
-  scoped_refptr<URLRequestContext> context(new TestURLRequestContext);
+  TestURLRequestContext context;
   scoped_ptr<DummyDhcpProxyScriptAdapterFetcher> adapter_fetcher(
-      new DummyDhcpProxyScriptAdapterFetcher(context));
+      new DummyDhcpProxyScriptAdapterFetcher(&context));
   adapter_fetcher->Configure(true, OK, L"bingo", 1);
   client->fetcher_.PushBackAdapter("a", adapter_fetcher.release());
   client->RunTest();
diff --git a/net/proxy/proxy_script_fetcher_impl.cc b/net/proxy/proxy_script_fetcher_impl.cc
index 32f6542..d69200f 100644
--- a/net/proxy/proxy_script_fetcher_impl.cc
+++ b/net/proxy/proxy_script_fetcher_impl.cc
@@ -295,9 +295,6 @@ void ProxyScriptFetcherImpl::FetchCompleted() {
   int result_code = result_code_;
   CompletionCallback callback = callback_;
 
-  // Hold a reference to the URLRequestContext to prevent re-entrancy from
-  // ~URLRequestContext.
-  scoped_refptr<const URLRequestContext> context(cur_request_->context());
   ResetCurRequestState();
 
   callback.Run(result_code);
diff --git a/net/proxy/proxy_script_fetcher_impl.h b/net/proxy/proxy_script_fetcher_impl.h
index 3774748..119ace1 100644
--- a/net/proxy/proxy_script_fetcher_impl.h
+++ b/net/proxy/proxy_script_fetcher_impl.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -85,7 +85,7 @@ class NET_EXPORT ProxyScriptFetcherImpl : public ProxyScriptFetcher,
   base::WeakPtrFactory<ProxyScriptFetcherImpl> weak_factory_;
 
   // The context used for making network requests.
-  URLRequestContext* url_request_context_;
+  URLRequestContext* const url_request_context_;
 
   // Buffer that URLRequest writes into.
   scoped_refptr<IOBuffer> buf_;
diff --git a/net/proxy/proxy_script_fetcher_impl_unittest.cc b/net/proxy/proxy_script_fetcher_impl_unittest.cc
index 45fcc07..dd95e7e 100644
--- a/net/proxy/proxy_script_fetcher_impl_unittest.cc
+++ b/net/proxy/proxy_script_fetcher_impl_unittest.cc
@@ -96,10 +96,10 @@ class RequestContext : public URLRequestContext {
         new CheckNoRevocationFlagSetInterceptor);
   }
 
- private:
-  ~RequestContext() {
+  virtual ~RequestContext() {
   }
 
+ private:
   URLRequestContextStorage storage_;
   scoped_ptr<URLRequestJobFactory> url_request_job_factory_;
 };
@@ -197,22 +197,17 @@ class ProxyScriptFetcherImplTest : public PlatformTest {
       : test_server_(TestServer::TYPE_HTTP,
                      net::TestServer::kLocalhost,
                      FilePath(kDocRoot)) {
-  }
-
-  // testing::Test overrides
-  virtual void SetUp() OVERRIDE {
-    context_ = new RequestContext;
-    context_->set_network_delegate(&network_delegate_);
+    context_.set_network_delegate(&network_delegate_);
   }
 
  protected:
   TestServer test_server_;
   BasicNetworkDelegate network_delegate_;
-  scoped_refptr<URLRequestContext> context_;
+  RequestContext context_;
 };
 
 TEST_F(ProxyScriptFetcherImplTest, FileUrl) {
-  ProxyScriptFetcherImpl pac_fetcher(context_.get());
+  ProxyScriptFetcherImpl pac_fetcher(&context_);
 
   { // Fetch a non-existent file.
     string16 text;
@@ -239,7 +234,7 @@ TEST_F(ProxyScriptFetcherImplTest, FileUrl) {
 TEST_F(ProxyScriptFetcherImplTest, HttpMimeType) {
   ASSERT_TRUE(test_server_.Start());
 
-  ProxyScriptFetcherImpl pac_fetcher(context_.get());
+  ProxyScriptFetcherImpl pac_fetcher(&context_);
 
   { // Fetch a PAC with mime type "text/plain"
     GURL url(test_server_.GetURL("files/pac.txt"));
@@ -273,7 +268,7 @@ TEST_F(ProxyScriptFetcherImplTest, HttpMimeType) {
 TEST_F(ProxyScriptFetcherImplTest, HttpStatusCode) {
   ASSERT_TRUE(test_server_.Start());
 
-  ProxyScriptFetcherImpl pac_fetcher(context_.get());
+  ProxyScriptFetcherImpl pac_fetcher(&context_);
 
   { // Fetch a PAC which gives a 500 -- FAIL
     GURL url(test_server_.GetURL("files/500.pac"));
@@ -298,7 +293,7 @@ TEST_F(ProxyScriptFetcherImplTest, HttpStatusCode) {
 TEST_F(ProxyScriptFetcherImplTest, ContentDisposition) {
   ASSERT_TRUE(test_server_.Start());
 
-  ProxyScriptFetcherImpl pac_fetcher(context_.get());
+  ProxyScriptFetcherImpl pac_fetcher(&context_);
 
   // Fetch PAC scripts via HTTP with a Content-Disposition header -- should
   // have no effect.
@@ -314,7 +309,7 @@ TEST_F(ProxyScriptFetcherImplTest, ContentDisposition) {
 TEST_F(ProxyScriptFetcherImplTest, NoCache) {
   ASSERT_TRUE(test_server_.Start());
 
-  ProxyScriptFetcherImpl pac_fetcher(context_.get());
+  ProxyScriptFetcherImpl pac_fetcher(&context_);
 
   // Fetch a PAC script whose HTTP headers make it cacheable for 1 hour.
   GURL url(test_server_.GetURL("files/cacheable_1hr.pac"));
@@ -345,7 +340,7 @@ TEST_F(ProxyScriptFetcherImplTest, NoCache) {
 TEST_F(ProxyScriptFetcherImplTest, TooLarge) {
   ASSERT_TRUE(test_server_.Start());
 
-  ProxyScriptFetcherImpl pac_fetcher(context_.get());
+  ProxyScriptFetcherImpl pac_fetcher(&context_);
 
   // Set the maximum response size to 50 bytes.
   int prev_size = pac_fetcher.SetSizeConstraint(50);
@@ -385,7 +380,7 @@ TEST_F(ProxyScriptFetcherImplTest, TooLarge) {
 TEST_F(ProxyScriptFetcherImplTest, Hang) {
   ASSERT_TRUE(test_server_.Start());
 
-  ProxyScriptFetcherImpl pac_fetcher(context_.get());
+  ProxyScriptFetcherImpl pac_fetcher(&context_);
 
   // Set the timeout period to 0.5 seconds.
   base::TimeDelta prev_timeout = pac_fetcher.SetTimeoutConstraint(
@@ -393,7 +388,8 @@ TEST_F(ProxyScriptFetcherImplTest, Hang) {
 
   // Try fetching a URL which takes 1.2 seconds. We should abort the request
   // after 500 ms, and fail with a timeout error.
-  { GURL url(test_server_.GetURL("slow/proxy.pac?1.2"));
+  {
+    GURL url(test_server_.GetURL("slow/proxy.pac?1.2"));
     string16 text;
     TestCompletionCallback callback;
     int result = pac_fetcher.Fetch(url, &text, callback.callback());
@@ -422,7 +418,7 @@ TEST_F(ProxyScriptFetcherImplTest, Hang) {
 TEST_F(ProxyScriptFetcherImplTest, Encodings) {
   ASSERT_TRUE(test_server_.Start());
 
-  ProxyScriptFetcherImpl pac_fetcher(context_.get());
+  ProxyScriptFetcherImpl pac_fetcher(&context_);
 
   // Test a response that is gzip-encoded -- should get inflated.
   {
@@ -449,7 +445,7 @@ TEST_F(ProxyScriptFetcherImplTest, Encodings) {
 }
 
 TEST_F(ProxyScriptFetcherImplTest, DataURLs) {
-  ProxyScriptFetcherImpl pac_fetcher(context_.get());
+  ProxyScriptFetcherImpl pac_fetcher(&context_);
 
   const char kEncodedUrl[] =
       "data:application/x-ns-proxy-autoconfig;base64,ZnVuY3Rpb24gRmluZFByb3h5R"