Handle ERR_PAC_SCRIPT_TERMINATED in ProxyService.

net/proxy/proxy_resolver_mojo.cc

Index: net/proxy/proxy_resolver_mojo.cc
diff --git a/net/proxy/proxy_resolver_mojo.cc b/net/proxy/proxy_resolver_mojo.cc
index 3801d41a093cb723ab7f21b87f3326ca48ed3f0a..6e12a43d317d21629e435673100ad4a1887e81be 100644
--- a/net/proxy/proxy_resolver_mojo.cc
+++ b/net/proxy/proxy_resolver_mojo.cc
@@ -100,9 +100,10 @@ void ProxyResolverMojo::Job::ReportResult(
     DVLOG(1) << "Servers: " << results_->ToPacString();
   }
 
-  callback_.Run(error);
+  CompletionCallback callback = callback_;
   callback_.Reset();
   resolver_->RemoveJob(this);
+  callback.Run(error);
 }
 
 void ProxyResolverMojo::Job::LoadStateChanged(int32_t load_state) {
@@ -114,7 +115,8 @@ ProxyResolverMojo::ProxyResolverMojo(
     HostResolver* host_resolver)
     : ProxyResolver(true /* |expects_pac_bytes| */),
       mojo_proxy_resolver_factory_(mojo_proxy_resolver_factory),
-      host_resolver_(host_resolver) {
+      host_resolver_(host_resolver),
+      weak_factory_(this) {
 }
 
 ProxyResolverMojo::~ProxyResolverMojo() {
@@ -163,8 +165,11 @@ void ProxyResolverMojo::OnSetPacScriptDone(
   DCHECK(!set_pac_script_callback_.IsCancelled());
   DVLOG(1) << "ProxyResolverMojo::OnSetPacScriptDone: " << result;
 
-  callback.Run(result);
+  // |callback| is owned by |set_pac_script_callback_|, so make a copy before
+  // cancelling.
+  auto callback_copy = callback;
   set_pac_script_callback_.Cancel();
+  callback_copy.Run(result);
 }
 
 void ProxyResolverMojo::SetUpServices() {
@@ -184,14 +189,15 @@ void ProxyResolverMojo::SetUpServices() {
 
 void ProxyResolverMojo::AbortPendingRequests() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (!set_pac_script_callback_.IsCancelled()) {
+  // |this| could be deleted as a result of one of these callbacks so use
+  // |weak_this| to check.
+  base::WeakPtr<ProxyResolverMojo> weak_this = weak_factory_.GetWeakPtr();
+  if (!set_pac_script_callback_.IsCancelled())
     set_pac_script_callback_.callback().Run(ERR_PAC_SCRIPT_TERMINATED);
-    set_pac_script_callback_.Cancel();
-  }
 
   // Need to use this loop because deleting a Job will cause its callback to be
   // run with a failure error code, which may cause other Jobs to be deleted.
-  while (!pending_jobs_.empty()) {
+  while (weak_this && !pending_jobs_.empty()) {

[eroman, 2015/04/15 00:56:52]

What is this needed for? This doesn't seem right, why would we be testing or
deletion of |this|? 

|this| being deleted is indicative of bigger problems no?

[Sam McNally, 2015/04/15 04:35:56]

Done. GetProxyForUrl requests will no longer immediately fail if the main mojo
connection disconnects; they'll fail once their individual request connections
disconnect.

     auto it = pending_jobs_.begin();
     Job* job = *it;
     pending_jobs_.erase(it);