Re-order some declarations in ProxyService, to ensure that deletion of InitProxyResolver happens *after* the deletion of ProxyResolver.

BUG=24864
TEST=ProxyServiceTest.DeleteWhileInitProxyResolverHasOutstandingFetch, ProxyServiceTest.DeleteWhileInitProxyResolverHasOutstandingSet

Review URL: http://codereview.chromium.org/519060

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

3 files changed, 78 insertions, 6 deletions

diff --git a/net/proxy/mock_proxy_resolver.h b/net/proxy/mock_proxy_resolver.h
index 67eb925..c284eb1 100644
--- a/net/proxy/mock_proxy_resolver.h
+++ b/net/proxy/mock_proxy_resolver.h
@@ -124,6 +124,10 @@ class MockAsyncProxyResolverBase : public ProxyResolver {
     return ERR_IO_PENDING;
   }
 
+  virtual void CancelSetPacScript() {
+    // Do nothing (caller was responsible for completing the request).
+  }
+
   const RequestsList& pending_requests() const {
     return pending_requests_;
   }
diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h
index ed02b03..2c421d2 100644
--- a/net/proxy/proxy_service.h
+++ b/net/proxy/proxy_service.h
@@ -231,12 +231,6 @@ class ProxyService : public base::RefCountedThreadSafe<ProxyService> {
   // heuristic).
   static bool IsLocalName(const GURL& url);
 
-  // Helper to download the PAC script (wpad + custom) and apply fallback rules.
-  scoped_ptr<InitProxyResolver> init_proxy_resolver_;
-
-  // Log from the *last* time |init_proxy_resolver_.Init()| was called, or NULL.
-  scoped_refptr<LoadLog> init_proxy_resolver_log_;
-
   scoped_ptr<ProxyConfigService> config_service_;
   scoped_ptr<ProxyResolver> resolver_;
 
@@ -266,6 +260,16 @@ class ProxyService : public base::RefCountedThreadSafe<ProxyService> {
 
   // Callback for when |init_proxy_resolver_| is done.
   CompletionCallbackImpl<ProxyService> init_proxy_resolver_callback_;
+
+  // Helper to download the PAC script (wpad + custom) and apply fallback rules.
+  //
+  // Note that the declaration is important here: |proxy_script_fetcher_| and
+  // |proxy_resolver_| must outlive |init_proxy_resolver_|.
+  scoped_ptr<InitProxyResolver> init_proxy_resolver_;
+
+  // Log from the *last* time |init_proxy_resolver_.Init()| was called, or NULL.
+  scoped_refptr<LoadLog> init_proxy_resolver_log_;
+
   DISALLOW_COPY_AND_ASSIGN(ProxyService);
 };
 
diff --git a/net/proxy/proxy_service_unittest.cc b/net/proxy/proxy_service_unittest.cc
index ecf28c9..3240963 100644
--- a/net/proxy/proxy_service_unittest.cc
+++ b/net/proxy/proxy_service_unittest.cc
@@ -1535,6 +1535,70 @@ TEST(ProxyServiceTest, BypassDoesntApplyToPac) {
   EXPECT_EQ("request2:80", info2.proxy_server().ToURI());
 }
 
+// Delete the ProxyService while InitProxyResolver has an outstanding
+// request to the script fetcher. When run under valgrind, should not
+// have any memory errors (used to be that the ProxyScriptFetcher was
+// being deleted prior to the InitProxyResolver).
+TEST(ProxyServiceTest, DeleteWhileInitProxyResolverHasOutstandingFetch) {
+  ProxyConfig config;
+  config.pac_url = GURL("http://foopy/proxy.pac");
+
+  MockProxyConfigService* config_service = new MockProxyConfigService(config);
+  MockAsyncProxyResolverExpectsBytes* resolver =
+      new MockAsyncProxyResolverExpectsBytes;
+  scoped_refptr<ProxyService> service(
+      new ProxyService(config_service, resolver));
+
+  MockProxyScriptFetcher* fetcher = new MockProxyScriptFetcher;
+  service->SetProxyScriptFetcher(fetcher);
+
+  // Start 1 request.
+
+  ProxyInfo info1;
+  TestCompletionCallback callback1;
+  int rv = service->ResolveProxy(
+      GURL("http://www.google.com"), &info1, &callback1, NULL, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  // Check that nothing has been sent to the proxy resolver yet.
+  ASSERT_EQ(0u, resolver->pending_requests().size());
+
+  // InitProxyResolver should have issued a request to the ProxyScriptFetcher
+  // and be waiting on that to complete.
+  EXPECT_TRUE(fetcher->has_pending_request());
+  EXPECT_EQ(GURL("http://foopy/proxy.pac"), fetcher->pending_request_url());
+
+  // Delete the ProxyService
+  service = NULL;
+}
+
+// Delete the ProxyService while InitProxyResolver has an outstanding
+// request to the proxy resolver. When run under valgrind, should not
+// have any memory errors (used to be that the ProxyResolver was
+// being deleted prior to the InitProxyResolver).
+TEST(ProxyServiceTest, DeleteWhileInitProxyResolverHasOutstandingSet) {
+  MockProxyConfigService* config_service =
+      new MockProxyConfigService("http://foopy/proxy.pac");
+
+  MockAsyncProxyResolver* resolver = new MockAsyncProxyResolver;
+
+  scoped_refptr<ProxyService> service(
+      new ProxyService(config_service, resolver));
+
+  GURL url("http://www.google.com/");
+
+  ProxyInfo info;
+  TestCompletionCallback callback;
+  int rv = service->ResolveProxy(url, &info, &callback, NULL, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  EXPECT_EQ(GURL("http://foopy/proxy.pac"),
+            resolver->pending_set_pac_script_request()->pac_url());
+
+  // Delete the ProxyService.
+  service = NULL;
+}
+
 TEST(ProxyServiceTest, ResetProxyConfigService) {
   ProxyConfig config1;
   config1.proxy_rules.ParseFromString("foopy1:8080");