Handle ERR_PAC_SCRIPT_TERMINATED in ProxyService.

net/proxy/proxy_resolver_mojo.cc

 // Copyright 2015 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.
 
 #include "net/proxy/proxy_resolver_mojo.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "mojo/common/common_type_converters.h"
@@ skipping 82 matching lines @@
     int32_t error,
     mojo::Array<interfaces::ProxyServerPtr> proxy_servers) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) << "ProxyResolverMojo::Job::ReportResult: " << error;
 
   if (error == OK) {
     *results_ = proxy_servers.To<ProxyInfo>();
     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) {
   load_state_ = static_cast<LoadState>(load_state);
 }
 
 ProxyResolverMojo::ProxyResolverMojo(
     MojoProxyResolverFactory* mojo_proxy_resolver_factory,
     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() {
   DCHECK(thread_checker_.CalledOnValidThread());
   // All pending requests should have been cancelled.
   DCHECK(pending_jobs_.empty());
   DCHECK(set_pac_script_callback_.IsCancelled());
 }
 
 void ProxyResolverMojo::CancelSetPacScript() {
@@ skipping 28 matching lines @@
 }
 
 void ProxyResolverMojo::OnSetPacScriptDone(
     const scoped_refptr<ProxyResolverScriptData>& pac_script,
     const net::CompletionCallback& callback,
     int32_t result) {
   DCHECK(thread_checker_.CalledOnValidThread());
   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() {
   DCHECK(thread_checker_.CalledOnValidThread());
   // A Mojo service implementation must outlive its binding.
   mojo_host_resolver_binding_.reset();
 
   interfaces::HostResolverPtr mojo_host_resolver_ptr;
   mojo_host_resolver_.reset(new MojoHostResolverImpl(host_resolver_));
   mojo_host_resolver_binding_.reset(new mojo::Binding<interfaces::HostResolver>(
       mojo_host_resolver_.get(), mojo::GetProxy(&mojo_host_resolver_ptr)));
   mojo_proxy_resolver_ptr_.reset();
   mojo_proxy_resolver_factory_->Create(
       mojo::GetProxy(&mojo_proxy_resolver_ptr_), mojo_host_resolver_ptr.Pass());
   mojo_proxy_resolver_ptr_.set_error_handler(this);
 }
 
 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);
 
     // Deleting the job will cause its completion callback to be run with an
     // ERR_PAC_SCRIPT_TERMINATED error.
     delete job;
   }
 }
 
@@ skipping 50 matching lines @@
   RemoveJob(job);
 }
 
 LoadState ProxyResolverMojo::GetLoadState(RequestHandle request) const {
   Job* job = static_cast<Job*>(request);
   CHECK_EQ(1u, pending_jobs_.count(job));
   return job->load_state();
 }
 
 }  // namespace net