Implement utility-side proxy resolver factory Mojo service.

net/proxy/mojo_proxy_resolver_factory_impl.cc

Index: net/proxy/mojo_proxy_resolver_factory_impl.cc
diff --git a/net/proxy/mojo_proxy_resolver_factory_impl.cc b/net/proxy/mojo_proxy_resolver_factory_impl.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6c4c04a7bac66c05dcaad308095412943d22ee08
--- /dev/null
+++ b/net/proxy/mojo_proxy_resolver_factory_impl.cc
@@ -0,0 +1,82 @@
+// 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/mojo_proxy_resolver_factory_impl.h"
+
+#include "net/dns/host_resolver_mojo.h"
+#include "net/proxy/mojo_proxy_resolver_impl.h"
+#include "net/proxy/proxy_resolver_v8_tracing.h"
+#include "third_party/mojo/src/mojo/public/cpp/bindings/error_handler.h"
+
+namespace net {
+namespace {
+
+scoped_ptr<ProxyResolver> CreateDefaultProxyResolver(
+    HostResolver* host_resolver) {
+  return make_scoped_ptr(

[Anand Mistry (off Chromium), 2015/02/19 05:23:08]

ProxyResolverV8::EnsureIsolateCreated();

[Sam McNally, 2015/02/20 03:25:04]

Done.

+      new ProxyResolverV8Tracing(host_resolver, nullptr, nullptr));
+}
+
+// A class to manage the lifetime of a MojoProxyResolverHolder and a

[eroman, 2015/02/20 01:43:37]

Is this comment correct? MojoProxyResolverHolder --> MojoProxyResolver.

[Sam McNally, 2015/02/20 03:25:04]

Done.

+// HostResolverMojo. An instance will remain while the message pipes for both
+// mojo connections remain open.
+class MojoProxyResolverHolder : public mojo::ErrorHandler {
+ public:
+  MojoProxyResolverHolder(
+      const MojoProxyResolverFactoryImpl::Factory& proxy_resolver_factory,
+      mojo::InterfaceRequest<interfaces::ProxyResolver> request,
+      interfaces::HostResolverPtr host_resolver);
+
+ private:
+  // mojo::ErrorHandler override.
+  void OnConnectionError() override;
+
+  HostResolverMojo host_resolver_;
+  MojoProxyResolverImpl proxy_resolver_;
+  mojo::Binding<interfaces::ProxyResolver> binding_;
+
+  DISALLOW_COPY_AND_ASSIGN(MojoProxyResolverHolder);
+};
+
+MojoProxyResolverHolder::MojoProxyResolverHolder(
+    const MojoProxyResolverFactoryImpl::Factory& proxy_resolver_factory,
+    mojo::InterfaceRequest<interfaces::ProxyResolver> request,
+    interfaces::HostResolverPtr host_resolver)
+    : host_resolver_(host_resolver.Pass(),
+                     base::Bind(&MojoProxyResolverHolder::OnConnectionError,
+                                base::Unretained(this))),
+      proxy_resolver_(proxy_resolver_factory.Run(&host_resolver_)),
+      binding_(&proxy_resolver_, request.Pass()) {
+  binding_.set_error_handler(this);

[eroman, 2015/02/20 01:43:37]

what is the difference between the error handler on the binding and that placed
on the ProxyResolverPtr (by the unittest).

What would be a good place to read up on mojo concepts (header files, design
documents, or whatever).
I confess I am still quite confused between the various layerings here.

[Sam McNally, 2015/02/20 03:25:04]

They serve the same function, but operate on opposite ends of the message pipe.
This error handler will detect when the client has closed its ProxyResolverPtr;
the error handler on the ProxyResolverPtr will detect when the ProxyResolver
implementation has disconnected - outside of tests this would usually be due to
the utility process crashing.
The most up to date place is probably the comments in the bindings headers in
https://code.google.com/p/chromium/codesearch/#chromium/src/third_party/mojo/...
or
https://docs.google.com/document/d/1FqXnpW1oOENS5mgvCjupyiWs4wdtxPJ9IyCYK2WMl....

+}
+
+void MojoProxyResolverHolder::OnConnectionError() {
+  delete this;
+}
+
+}  // namespace
+
+MojoProxyResolverFactoryImpl::MojoProxyResolverFactoryImpl(
+    const MojoProxyResolverFactoryImpl::Factory& proxy_resolver_factory,
+    mojo::InterfaceRequest<interfaces::ProxyResolverFactory> request)
+    : proxy_resolver_impl_factory_(proxy_resolver_factory),
+      binding_(this, request.Pass()) {
+}
+
+MojoProxyResolverFactoryImpl::MojoProxyResolverFactoryImpl(
+    mojo::InterfaceRequest<interfaces::ProxyResolverFactory> request)
+    : MojoProxyResolverFactoryImpl(base::Bind(&CreateDefaultProxyResolver),
+                                   request.Pass()) {
+}
+
+MojoProxyResolverFactoryImpl::~MojoProxyResolverFactoryImpl() = default;
+
+void MojoProxyResolverFactoryImpl::CreateResolver(
+    mojo::InterfaceRequest<interfaces::ProxyResolver> request,
+    interfaces::HostResolverPtr host_resolver) {
+  new MojoProxyResolverHolder(proxy_resolver_impl_factory_, request.Pass(),

[eroman, 2015/02/20 01:43:37]

This still doesn't smell right to me.

At the very least there should be some comments explaining the expected
lifetime.

So OnConnectionError() can be called either by the host resolver, or by the
proxyresolver, and this results in the deletion of the "holder":
  - is it guaranteed that at least one of these will be called? Is it possible
to tear things down in a manner where neither connectionerror gets called and
things leak.
  - is it guaranteed that OnConnectionError() can be called at most once,
between both the callers

[Sam McNally, 2015/02/20 03:25:04]

Done.
It may be possible to leak during process shutdown. Otherwise, as long as the
client closes its connection, OnConnectionError() will be called.
Yes. When OnConnectionError() is called, the Binding and InterfacePtr are
deleted so nothing would be around to call it a second time.

+                              host_resolver.Pass());
+}
+
+}  // namespace net