Add LoadState reporting to the mojo proxy resolver.

net/proxy/mojo_proxy_resolver_impl.cc

Index: net/proxy/mojo_proxy_resolver_impl.cc
diff --git a/net/proxy/mojo_proxy_resolver_impl.cc b/net/proxy/mojo_proxy_resolver_impl.cc
index 3568b202d98c07c90139883e9c629a536b16b5fa..9cba1db4a7377615864d4bbb4b573c0bce10d164 100644
--- a/net/proxy/mojo_proxy_resolver_impl.cc
+++ b/net/proxy/mojo_proxy_resolver_impl.cc
@@ -24,6 +24,9 @@ class MojoProxyResolverImpl::Job : public mojo::ErrorHandler {
 
   void Start();
 
+  // Invoked when the LoadState for this job changes.
+  void LoadStateChanged(LoadState load_state);
+
  private:
   // mojo::ErrorHandler override.
   // This is invoked in response to the client disconnecting, indicating
@@ -66,6 +69,16 @@ MojoProxyResolverImpl::~MojoProxyResolverImpl() {
   STLDeleteElements(&resolve_jobs_);
 }
 
+void MojoProxyResolverImpl::LoadStateChanged(
+    net::ProxyResolver::RequestHandle handle,
+    LoadState load_state) {
+  auto it = request_handle_to_job_.find(handle);
+  if (it == request_handle_to_job_.end())
+    return;
+
+  it->second->LoadStateChanged(load_state);
+}
+
 void MojoProxyResolverImpl::SetPacScript(
     const mojo::String& data,
     const mojo::Callback<void(int32_t)>& callback) {
@@ -86,7 +99,11 @@ void MojoProxyResolverImpl::GetProxyForUrl(
   job->Start();
 }
 
-void MojoProxyResolverImpl::DeleteJob(Job* job) {
+void MojoProxyResolverImpl::DeleteJob(
+    Job* job,
+    net::ProxyResolver::RequestHandle handle) {

[eroman, 2015/02/25 00:08:05]

Does this need another parameter? the handle is already part of the Job

[Sam McNally, 2015/02/26 04:00:00]

Done.

+  if (handle)
+    request_handle_to_job_.erase(handle);
   size_t num_erased = resolve_jobs_.erase(job);
   DCHECK(num_erased);
   delete job;
@@ -135,10 +152,15 @@ void MojoProxyResolverImpl::Job::Start() {
     return;
   }
   client_.set_error_handler(this);
+  resolver_->request_handle_to_job_.insert(
+      std::make_pair(request_handle_, this));
+}
+
+void MojoProxyResolverImpl::Job::LoadStateChanged(LoadState load_state) {
+  client_->LoadStateChanged(load_state);

[eroman, 2015/02/25 00:08:05]

What is the implication of calling this? Other callbacks on the
TracingProxyResolver are resilient to cancellation/deletion happening while
dispatching the callback, but it doesn't look like that is the case for this
loadstate change one.

What may client->LoadStateChanged() do in response?

[Anand Mistry (off Chromium), 2015/02/25 04:42:27]

As an optimisation, I wonder if we should try to coalesce state changes. The
case I'm thinking of is when a DNS resolution finishes. Then, one of two cases
is likely to happen:
1. The script executes for a short amount of time and fires another DNS request.
It's likely that the script is only executing for 10s of us before starting the
next request. In that case, the state changes and then changes back within a
very short amount of time.
2. The script executes for a short amount of time and finishes, returning the
result. In this case, the state is changed, but since the request has finished,
the state is then no longer valid.

In both cases, the intermediate state is unlikely to be observed. So instead of
changing the state and sending messages for updates, maybe set a 1ms timer
before sending the state change message, and cancel that timer if one of the two
events happen. That way, sending the Mojo messages is avoided entirely for these
cases. My suspicion is that case #2 is likely to be very common.

[Sam McNally, 2015/02/26 04:00:00]

client_ is the mojo binding so it only sends a message to the
ProxyResolverMojo::Job. Cancellation is signaled by closing the message pipe so
if the Job has been cancelled on the other end but this end hasn't noticed yet,
the message will be discarded.

 }
 
 void MojoProxyResolverImpl::Job::GetProxyDone(int error) {
-  request_handle_ = nullptr;
   DVLOG(1) << "GetProxyForUrl(" << url_ << ") finished with error " << error
            << ". " << result_.proxy_list().size() << " Proxies returned:";
   for (const auto& proxy : result_.proxy_list().GetAll()) {
@@ -150,11 +172,13 @@ void MojoProxyResolverImpl::Job::GetProxyDone(int error) {
         result_.proxy_list().GetAll());
   }
   client_->ReportResult(error, result.Pass());
-  resolver_->DeleteJob(this);
+  auto request_handle = request_handle_;
+  request_handle_ = nullptr;
+  resolver_->DeleteJob(this, request_handle);
 }
 
 void MojoProxyResolverImpl::Job::OnConnectionError() {
-  resolver_->DeleteJob(this);
+  resolver_->DeleteJob(this, request_handle_);
 }
 
 MojoProxyResolverImpl::SetPacScriptRequest::SetPacScriptRequest(