Use a MojoProxyResolverFactory on Android.

chrome/browser/net/chrome_mojo_proxy_resolver_factory.cc

Index: chrome/browser/net/chrome_mojo_proxy_resolver_factory.cc
diff --git a/chrome/browser/net/utility_process_mojo_proxy_resolver_factory.cc b/chrome/browser/net/chrome_mojo_proxy_resolver_factory.cc
similarity index 63%
rename from chrome/browser/net/utility_process_mojo_proxy_resolver_factory.cc
rename to chrome/browser/net/chrome_mojo_proxy_resolver_factory.cc
index 4a88a57ccaf9971ae210721f58716c1ab6e32f48..60d43297ce13b490aedfe01efe9b2c16c9e9c294 100644
--- a/chrome/browser/net/utility_process_mojo_proxy_resolver_factory.cc
+++ b/chrome/browser/net/chrome_mojo_proxy_resolver_factory.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/net/utility_process_mojo_proxy_resolver_factory.h"
+#include "chrome/browser/net/chrome_mojo_proxy_resolver_factory.h"
 
 #include <utility>
 
@@ -11,61 +11,72 @@
 #include "base/memory/singleton.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "chrome/grit/generated_resources.h"
 #include "content/public/browser/browser_thread.h"
+
+#if !defined(OS_ANDROID)
+#include "chrome/grit/generated_resources.h"
 #include "content/public/browser/utility_process_host.h"
 #include "content/public/browser/utility_process_host_client.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "ui/base/l10n/l10n_util.h"
+#else  // defined(OS_ANDROID)
+#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "net/proxy/mojo_proxy_resolver_factory_impl.h"
+#endif  // !defined(OS_ANDROID)
 
 namespace {
 const int kUtilityProcessIdleTimeoutSeconds = 5;
 }
 
 // static
-UtilityProcessMojoProxyResolverFactory*
-UtilityProcessMojoProxyResolverFactory::GetInstance() {
+ChromeMojoProxyResolverFactory* ChromeMojoProxyResolverFactory::GetInstance() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  return base::Singleton<UtilityProcessMojoProxyResolverFactory,
-                         base::LeakySingletonTraits<
-                             UtilityProcessMojoProxyResolverFactory>>::get();
+  return base::Singleton<
+      ChromeMojoProxyResolverFactory,
+      base::LeakySingletonTraits<ChromeMojoProxyResolverFactory>>::get();
 }
 
-UtilityProcessMojoProxyResolverFactory::
-    UtilityProcessMojoProxyResolverFactory() {
+ChromeMojoProxyResolverFactory::ChromeMojoProxyResolverFactory() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 }
 
-UtilityProcessMojoProxyResolverFactory::
-    ~UtilityProcessMojoProxyResolverFactory() {
+ChromeMojoProxyResolverFactory::~ChromeMojoProxyResolverFactory() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
-void UtilityProcessMojoProxyResolverFactory::CreateProcessAndConnect() {
+void ChromeMojoProxyResolverFactory::CreateProcessAndConnect() {

[eroman, 2017/06/08 23:01:42]

This function name is not really applicable on the android version now (i.e.
doesn't create process).

[mmenke, 2017/06/09 15:20:40]

Good point.  Renamed.  (Also reordered things so they match the order in the
header)

   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!resolver_factory_);
+
+#if defined(OS_ANDROID)
+  mojo::MakeStrongBinding(base::MakeUnique<net::MojoProxyResolverFactoryImpl>(),
+                          mojo::MakeRequest(&resolver_factory_));
+#else   // !defined(OS_ANDROID)
   DCHECK(!weak_utility_process_host_);
+
   DVLOG(1) << "Attempting to create utility process for proxy resolver";
   content::UtilityProcessHost* utility_process_host =
       content::UtilityProcessHost::Create(
           scoped_refptr<content::UtilityProcessHostClient>(),
           base::ThreadTaskRunnerHandle::Get());
-  utility_process_host->SetName(l10n_util::GetStringUTF16(
-      IDS_UTILITY_PROCESS_PROXY_RESOLVER_NAME));
+  utility_process_host->SetName(
+      l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_PROXY_RESOLVER_NAME));
   bool process_started = utility_process_host->Start();
   if (process_started) {
     BindInterface(utility_process_host, &resolver_factory_);
-    resolver_factory_.set_connection_error_handler(
-        base::Bind(&UtilityProcessMojoProxyResolverFactory::OnConnectionError,
-                   base::Unretained(this)));
     weak_utility_process_host_ = utility_process_host->AsWeakPtr();
   } else {
     LOG(ERROR) << "Unable to connect to utility process";
+    return;
   }
+#endif  // defined(OS_ANDROID)
+
+  resolver_factory_.set_connection_error_handler(base::Bind(
+      &ChromeMojoProxyResolverFactory::DestroyFactory, base::Unretained(this)));

[mmenke, 2017/06/08 22:32:41]

It's not really clear to me if a "connection error" can happen to same-process
Mojo interfaces (Run out of memory/handles or something?  I dunno).  Since it's
no extra code, figure I'll play it safe for now, and worry about that again when
it comes to actually hooking up a ProxyResolverFactory to the NetworkService.

If you prefer, though, I can just make a separate MojoProxyResolverFactory for
Android (May even just be able to share and rename the
TestMojoProxyResolverFactory)

[eroman, 2017/06/08 23:01:43]

From an abstraction perspective seems like should always handle connection
errors, so I would say this is desirable as written.

[mmenke, 2017/06/09 15:26:50]

I don't think it matters from an abstraction perspective, actually - this class
is the one that chooses to run it in process, so seems like it can strongly
depend on running it in-process on Android.

Other classes, of course, should not depend on that.

 }
 
 std::unique_ptr<base::ScopedClosureRunner>
-UtilityProcessMojoProxyResolverFactory::CreateResolver(
+ChromeMojoProxyResolverFactory::CreateResolver(
     const std::string& pac_script,
     mojo::InterfaceRequest<net::interfaces::ProxyResolver> req,
     net::interfaces::ProxyResolverFactoryRequestClientPtr client) {
@@ -74,7 +85,7 @@ UtilityProcessMojoProxyResolverFactory::CreateResolver(
     CreateProcessAndConnect();
 
   if (!resolver_factory_) {
-    // If there's still no factory, then utility process creation failed so
+    // If there's still no factory, then factor creation failed so

[eroman, 2017/06/08 23:01:43]

factory

[mmenke, 2017/06/09 15:20:40]

Done.  Also shorted the sentence a bit (4 clauses is a bit much).

     // close |req|'s message pipe, which should cause a connection error.
     req = nullptr;
     return nullptr;
@@ -84,38 +95,39 @@ UtilityProcessMojoProxyResolverFactory::CreateResolver(
   resolver_factory_->CreateResolver(pac_script, std::move(req),
                                     std::move(client));
   return base::MakeUnique<base::ScopedClosureRunner>(
-      base::Bind(&UtilityProcessMojoProxyResolverFactory::OnResolverDestroyed,
+      base::Bind(&ChromeMojoProxyResolverFactory::OnResolverDestroyed,
                  base::Unretained(this)));
 }
 
-void UtilityProcessMojoProxyResolverFactory::OnConnectionError() {
-  DVLOG(1) << "Disconnection from utility process detected";
+void ChromeMojoProxyResolverFactory::DestroyFactory() {
   resolver_factory_.reset();
+#if !defined(OS_ANDROID)
   delete weak_utility_process_host_.get();
   weak_utility_process_host_.reset();
+#endif
 }
 
-void UtilityProcessMojoProxyResolverFactory::OnResolverDestroyed() {
+void ChromeMojoProxyResolverFactory::OnResolverDestroyed() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_GT(num_proxy_resolvers_, 0u);
   if (--num_proxy_resolvers_ == 0) {
-    // When all proxy resolvers have been destroyed, the proxy resolver utility
-    // process is no longer needed. However, new proxy resolvers may be created
-    // shortly after being destroyed (e.g. due to a network change). If the
+    // When all proxy resolvers have been destroyed, the proxy resolver factory
+    // is no longer needed. However, new proxy resolvers may be created
+    // shortly after being destroyed (e.g. due to a network change).
+    //
+    // On desktop, where a utliity process is used, if the

[eroman, 2017/06/08 23:01:42]

utility

[mmenke, 2017/06/09 15:20:40]

Done.  Also re-flowed comment (Not sure why git cl format didn't do that itself)

     // utility process is shut down immediately, this would cause unnecessary
     // process churn, so wait for an idle timeout before shutting down the
     // proxy resolver utility process.
     idle_timer_.Start(
         FROM_HERE,
         base::TimeDelta::FromSeconds(kUtilityProcessIdleTimeoutSeconds), this,
-        &UtilityProcessMojoProxyResolverFactory::OnIdleTimeout);
+        &ChromeMojoProxyResolverFactory::OnIdleTimeout);
   }
 }
 
-void UtilityProcessMojoProxyResolverFactory::OnIdleTimeout() {
+void ChromeMojoProxyResolverFactory::OnIdleTimeout() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_EQ(num_proxy_resolvers_, 0u);
-  delete weak_utility_process_host_.get();
-  weak_utility_process_host_.reset();
-  resolver_factory_.reset();
+  DestroyFactory();
 }