Implementation of ProxyResolver that uses a Mojo service.

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
new file mode 100644
index 0000000000000000000000000000000000000000..6bca3155fa576566dc209109703848839371b25a
--- /dev/null
+++ b/net/proxy/proxy_resolver_mojo.cc
@@ -0,0 +1,237 @@
+// 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"
+#include "net/base/net_errors.h"
+#include "net/dns/mojo_host_resolver_impl.h"
+#include "net/proxy/mojo_proxy_resolver_factory.h"
+#include "net/proxy/mojo_type_converters.h"
+#include "net/proxy/proxy_info.h"
+#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h"
+#include "third_party/mojo/src/mojo/public/cpp/bindings/error_handler.h"
+
+namespace net {
+
+class ProxyResolverMojo::Job : public interfaces::ProxyResolverRequestClient,
+                               public mojo::ErrorHandler {
+ public:
+  Job(ProxyResolverMojo* resolver,
+      const GURL& url,
+      ProxyInfo* results,
+      const net::CompletionCallback& callback);
+  ~Job() override;
+
+  // Start the job and return an error code.
+  int Start();

[Sam McNally, 2015/02/16 05:52:38]

This Job doesn't need a separate Start().

[Anand Mistry (off Chromium), 2015/02/17 05:49:21]

Done.

+
+  // Cancels the job and prevents the callback from being run.
+  void Cancel();
+
+ private:
+  // Overridden from mojo::ErrorHandler:
+  void OnConnectionError() override;
+
+  // Overridden from interfaces::ProxyResolverRequestClient:
+  void ReportResult(
+      int32_t error,
+      mojo::Array<interfaces::ProxyServerPtr> proxy_servers) override;
+
+  ProxyResolverMojo* resolver_;
+  const GURL url_;
+  ProxyInfo* results_;
+  net::CompletionCallback callback_;
+
+  base::ThreadChecker thread_checker_;
+  mojo::Binding<interfaces::ProxyResolverRequestClient> binding_;
+};
+
+ProxyResolverMojo::Job::Job(ProxyResolverMojo* resolver,
+                            const GURL& url,
+                            ProxyInfo* results,
+                            const net::CompletionCallback& callback)
+    : resolver_(resolver),
+      url_(url),
+      results_(results),
+      callback_(callback),
+      binding_(this) {
+  binding_.set_error_handler(this);
+}
+
+ProxyResolverMojo::Job::~Job() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!callback_.is_null())
+    callback_.Run(ERR_PAC_SCRIPT_FAILED);
+}
+
+int ProxyResolverMojo::Job::Start() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  // Should already have ensured the Mojo resolved is connected.

[Sam McNally, 2015/02/16 05:52:38]

I think this is missing a subject.

[Anand Mistry (off Chromium), 2015/02/17 05:49:21]

Removed the DCHECK since I don't think it's strictly true, or necessary.

+  DCHECK(resolver_->proxy_resolver_ptr_);
+
+  interfaces::ProxyResolverRequestClientPtr client_ptr;
+  binding_.Bind(mojo::GetProxy(&client_ptr));
+  resolver_->proxy_resolver_ptr_->GetProxyForUrl(mojo::String::From(url_),
+                                                 client_ptr.Pass());
+
+  return ERR_IO_PENDING;
+}
+
+void ProxyResolverMojo::Job::Cancel() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!callback_.is_null());
+  callback_.Reset();
+}
+
+void ProxyResolverMojo::Job::OnConnectionError() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(1) << "ProxyResolverMojo::Job::OnConnectionError";
+  resolver_->RemoveJob(this);
+}
+
+void ProxyResolverMojo::Job::ReportResult(
+    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);
+  callback_.Reset();
+  resolver_->RemoveJob(this);
+}
+
+ProxyResolverMojo::ProxyResolverMojo(
+    MojoProxyResolverFactory* proxy_resolver_factory,
+    HostResolver* host_resolver)
+    : ProxyResolver(true /* |expects_pac_bytes| */),
+      proxy_resolver_factory_(proxy_resolver_factory),
+      host_resolver_(host_resolver) {
+  SetupServices();
+}
+
+ProxyResolverMojo::~ProxyResolverMojo() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  // All pending requests should have been cancelled.
+  DCHECK(pending_jobs_.empty());
+}
+
+void ProxyResolverMojo::CancelSetPacScript() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!set_pac_script_callback_.is_null())
+    set_pac_script_callback_.Reset();
+}
+
+int ProxyResolverMojo::SetPacScript(
+    const scoped_refptr<ProxyResolverScriptData>& pac_script,
+    const net::CompletionCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(set_pac_script_callback_.is_null());
+  DCHECK(!callback.is_null());
+  if (pac_script->type() != ProxyResolverScriptData::TYPE_SCRIPT_CONTENTS ||
+      pac_script->utf16().empty())

[Sam McNally, 2015/02/16 05:52:38]

{

[Anand Mistry (off Chromium), 2015/02/17 05:49:21]

Done.

+    return ERR_PAC_SCRIPT_FAILED;
+
+  DVLOG(1) << "ProxyResolverMojo::SetPacScript: " << pac_script->utf16();
+  pac_script_ = pac_script;
+  set_pac_script_callback_ = callback;
+
+  proxy_resolver_ptr_->SetPacScript(
+      mojo::String::From(pac_script_->utf16()),
+      base::Bind(&ProxyResolverMojo::OnSetPacScriptDone,
+                 base::Unretained(this)));
+
+  return ERR_IO_PENDING;
+}
+
+void ProxyResolverMojo::OnSetPacScriptDone(int32_t result) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(1) << "ProxyResolverMojo::OnSetPacScriptDone: " << result;
+  if (!set_pac_script_callback_.is_null())
+    set_pac_script_callback_.Run(result);
+
+  set_pac_script_callback_.Reset();
+}
+
+void ProxyResolverMojo::SetupServices() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  host_resolver_binding_.reset(
+      new mojo::StrongBinding<interfaces::HostResolver>(
+          new MojoHostResolverImpl(host_resolver_)));

[Sam McNally, 2015/02/16 05:52:38]

This will leak if the host resolver pipe isn't closed before
host_resolver_binding_ is cleared. StrongBinding really does expect to be a
member variable.

[Anand Mistry (off Chromium), 2015/02/17 05:49:21]

Done.

+  interfaces::HostResolverPtr host_resolver_ptr;
+  host_resolver_binding_->Bind(&host_resolver_ptr);

[Sam McNally, 2015/02/16 05:52:38]

Binding can take this as a constructor arg.

[Anand Mistry (off Chromium), 2015/02/17 05:49:21]

Done.

+  proxy_resolver_ptr_.reset();
+  proxy_resolver_factory_->Create(mojo::GetProxy(&proxy_resolver_ptr_),
+                                  host_resolver_ptr.Pass());
+  proxy_resolver_ptr_.set_error_handler(this);
+
+  if (pac_script_) {
+    proxy_resolver_ptr_->SetPacScript(mojo::String::From(pac_script_->utf16()),

[Sam McNally, 2015/02/16 05:52:38]

This and CancelSetPacScript can result in multiple SetPacScript requests
reaching the other side; the current implementation doesn't cope well with that.
We need to decide whether the mojom interface should allow this and fix one
side.

[Anand Mistry (off Chromium), 2015/02/17 05:49:21]

Fixed this side to deal with multiple outstanding SetPacScript requests.

+                                      mojo::Callback<void(int32_t)>());
+  }
+}
+
+void ProxyResolverMojo::CancelPendingRequests() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!set_pac_script_callback_.is_null())
+    set_pac_script_callback_.Run(ERR_PAC_SCRIPT_FAILED);
+  set_pac_script_callback_.Reset();
+
+  // Deleting a Job will automatically fail it.
+  STLDeleteElements(&pending_jobs_);
+}
+
+void ProxyResolverMojo::OnConnectionError() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(1) << "ProxyResolverMojo::OnConnectionError";
+  CancelPendingRequests();
+
+  // Restart.
+  SetupServices();
+}
+
+void ProxyResolverMojo::RemoveJob(Job* job) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  size_t num_erased = pending_jobs_.erase(job);
+  DCHECK(num_erased);
+  delete job;
+}
+
+int ProxyResolverMojo::GetProxyForURL(const GURL& url,
+                                      ProxyInfo* results,
+                                      const net::CompletionCallback& callback,
+                                      RequestHandle* request,
+                                      const BoundNetLog& net_log) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  Job* job = new Job(this, url, results, callback);
+  bool inserted = pending_jobs_.insert(job).second;
+  DCHECK(inserted);
+  *request = job;
+
+  return job->Start();

[Sam McNally, 2015/02/16 05:52:38]

Why not return ERR_IO_PENDING directly?

To be strictly correct, you should be checking the return value before setting
*request.

[Anand Mistry (off Chromium), 2015/02/17 05:49:21]

Done.

+}
+
+void ProxyResolverMojo::CancelRequest(RequestHandle request) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  Job* job = static_cast<Job*>(request);
+  DCHECK(job);
+  job->Cancel();
+  RemoveJob(job);
+}
+
+LoadState ProxyResolverMojo::GetLoadState(RequestHandle request) const {
+  // TODO(amistry): Implement real LoadState.
+  return LOAD_STATE_RESOLVING_PROXY_FOR_URL;
+}
+
+}  // namespace net