Convert HostResolverRequestClient to new Mojo types

This CL converts HostResolverRequestClientPtr and
HostResolverRequestClientRequest to new Mojo types.

It uses Remote or PendingRemote, PendingReceiver,
and Receiver instead of HostResolverRequestClientPtr,
HostResolverRequestClientRequest and Binding.

Bug: 955171
Change-Id: I11c270e26e77fee7391b64f898a0b10d8c66dc15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1811406Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#698748}

services/network/mojo_host_resolver_impl.cc

@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/ip_address.h"
 #include "net/base/net_errors.h"
@@ -27,7 +28,8 @@ class MojoHostResolverImpl::Job {
       const std::string& hostname,
       bool is_ex,
       const net::NetLogWithSource& net_log,
-      proxy_resolver::mojom::HostResolverRequestClientPtr client);
+      mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+          client);
   ~Job();
 
   void set_iter(std::list<Job>::iterator iter) { iter_ = iter; }
@@ -38,14 +40,14 @@ class MojoHostResolverImpl::Job {
   // Completion callback for the HostResolver::Resolve request.
   void OnResolveDone(int result);
 
-  // Mojo error handler.
-  void OnConnectionError();
+  // Mojo disconnect handler.
+  void OnMojoDisconnect();
 
   MojoHostResolverImpl* resolver_service_;
   // This Job's iterator in |resolver_service_|, so the Job may be removed on
   // completion.
   std::list<Job>::iterator iter_;
-  proxy_resolver::mojom::HostResolverRequestClientPtr client_;
+  mojo::Remote<proxy_resolver::mojom::HostResolverRequestClient> client_;
   const std::string hostname_;
   std::unique_ptr<net::HostResolver::ResolveHostRequest> request_;
   base::ThreadChecker thread_checker_;
@@ -62,7 +64,8 @@ MojoHostResolverImpl::~MojoHostResolverImpl() {
 void MojoHostResolverImpl::Resolve(
     const std::string& hostname,
     bool is_ex,
-    proxy_resolver::mojom::HostResolverRequestClientPtr client) {
+    mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+        client) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   pending_jobs_.emplace_front(this, resolver_, hostname, is_ex, net_log_,
@@ -83,12 +86,13 @@ MojoHostResolverImpl::Job::Job(
     const std::string& hostname,
     bool is_ex,
     const net::NetLogWithSource& net_log,
-    proxy_resolver::mojom::HostResolverRequestClientPtr client)
+    mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+        client)
     : resolver_service_(resolver_service),
       client_(std::move(client)),
       hostname_(hostname) {
-  client_.set_connection_error_handler(base::Bind(
-      &MojoHostResolverImpl::Job::OnConnectionError, base::Unretained(this)));
+  client_.set_disconnect_handler(base::Bind(
+      &MojoHostResolverImpl::Job::OnMojoDisconnect, base::Unretained(this)));
 
   net::HostResolver::ResolveHostParameters parameters;
   if (!is_ex)
@@ -133,11 +137,11 @@ void MojoHostResolverImpl::Job::OnResolveDone(int result) {
   resolver_service_->DeleteJob(iter_);
 }
 
-void MojoHostResolverImpl::Job::OnConnectionError() {
+void MojoHostResolverImpl::Job::OnMojoDisconnect() {
   DCHECK(thread_checker_.CalledOnValidThread());
   // |resolver_service_| should always outlive us.
   DCHECK(resolver_service_);
-  DVLOG(1) << "Connection error on request for " << hostname_;
+  DVLOG(1) << "Disconnection on request for " << hostname_;
   resolver_service_->DeleteJob(iter_);
 }
 

services/network/mojo_host_resolver_impl.h

@@ -12,6 +12,7 @@
 #include "base/component_export.h"
 #include "base/macros.h"
 #include "base/threading/thread_checker.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "net/log/net_log_with_source.h"
 #include "services/proxy_resolver/public/mojom/proxy_resolver.mojom.h"
 
@@ -37,9 +38,11 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) MojoHostResolverImpl {
                        const net::NetLogWithSource& net_log);
   ~MojoHostResolverImpl();
 
-  void Resolve(const std::string& hostname,
-               bool is_ex,
-               proxy_resolver::mojom::HostResolverRequestClientPtr client);
+  void Resolve(
+      const std::string& hostname,
+      bool is_ex,
+      mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+          client);
 
   bool request_in_progress() { return !pending_jobs_.empty(); }
 

services/network/mojo_host_resolver_impl_unittest.cc

@@ -13,8 +13,7 @@
 #include "base/run_loop.h"
 #include "base/test/task_environment.h"
 #include "base/time/time.h"
-#include "mojo/public/cpp/bindings/binding.h"
-#include "mojo/public/cpp/bindings/interface_request.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "net/base/address_family.h"
 #include "net/base/ip_address.h"
 #include "net/base/net_errors.h"
@@ -34,11 +33,11 @@ class TestRequestClient
     : public proxy_resolver::mojom::HostResolverRequestClient {
  public:
   explicit TestRequestClient(
-      mojo::InterfaceRequest<proxy_resolver::mojom::HostResolverRequestClient>
-          req)
-      : done_(false), binding_(this, std::move(req)) {
-    binding_.set_connection_error_handler(base::Bind(
-        &TestRequestClient::OnConnectionError, base::Unretained(this)));
+      mojo::PendingReceiver<proxy_resolver::mojom::HostResolverRequestClient>
+          receiver)
+      : done_(false), receiver_(this, std::move(receiver)) {
+    receiver_.set_disconnect_handler(base::Bind(
+        &TestRequestClient::OnMojoDisconnect, base::Unretained(this)));
   }
 
   void WaitForResult();
@@ -52,14 +51,14 @@ class TestRequestClient
   void ReportResult(int32_t error,
                     const std::vector<net::IPAddress>& results) override;
 
-  // Mojo error handler.
-  void OnConnectionError();
+  // Mojo disconnect handler.
+  void OnMojoDisconnect();
 
   bool done_;
   base::Closure run_loop_quit_closure_;
   base::Closure connection_error_quit_closure_;
 
-  mojo::Binding<proxy_resolver::mojom::HostResolverRequestClient> binding_;
+  mojo::Receiver<proxy_resolver::mojom::HostResolverRequestClient> receiver_;
 };
 
 void TestRequestClient::WaitForResult() {
@@ -90,7 +89,7 @@ void TestRequestClient::ReportResult(
   done_ = true;
 }
 
-void TestRequestClient::OnConnectionError() {
+void TestRequestClient::OnMojoDisconnect() {
   if (!connection_error_quit_closure_.is_null())
     connection_error_quit_closure_.Run();
 }
@@ -132,11 +131,12 @@ class MojoHostResolverImplTest : public testing::Test {
 };
 
 TEST_F(MojoHostResolverImplTest, Resolve) {
-  proxy_resolver::mojom::HostResolverRequestClientPtr client_ptr;
-  TestRequestClient client(mojo::MakeRequest(&client_ptr));
+  mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+      client_remote;
+  TestRequestClient client(client_remote.InitWithNewPipeAndPassReceiver());
 
   resolver_service_->Resolve("example.com", false /* is_ex */,
-                             std::move(client_ptr));
+                             std::move(client_remote));
   client.WaitForResult();
 
   EXPECT_THAT(client.error_, IsOk());
@@ -144,13 +144,14 @@ TEST_F(MojoHostResolverImplTest, Resolve) {
 }
 
 TEST_F(MojoHostResolverImplTest, ResolveSynchronous) {
-  proxy_resolver::mojom::HostResolverRequestClientPtr client_ptr;
-  TestRequestClient client(mojo::MakeRequest(&client_ptr));
+  mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+      client_remote;
+  TestRequestClient client(client_remote.InitWithNewPipeAndPassReceiver());
 
   mock_host_resolver_.set_synchronous_mode(true);
 
   resolver_service_->Resolve("example.com", false /* is_ex */,
-                             std::move(client_ptr));
+                             std::move(client_remote));
   client.WaitForResult();
 
   EXPECT_THAT(client.error_, IsOk());
@@ -158,17 +159,19 @@ TEST_F(MojoHostResolverImplTest, ResolveSynchronous) {
 }
 
 TEST_F(MojoHostResolverImplTest, ResolveMultiple) {
-  proxy_resolver::mojom::HostResolverRequestClientPtr client1_ptr;
-  TestRequestClient client1(mojo::MakeRequest(&client1_ptr));
-  proxy_resolver::mojom::HostResolverRequestClientPtr client2_ptr;
-  TestRequestClient client2(mojo::MakeRequest(&client2_ptr));
+  mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+      client1_remote;
+  TestRequestClient client1(client1_remote.InitWithNewPipeAndPassReceiver());
+  mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+      client2_remote;
+  TestRequestClient client2(client2_remote.InitWithNewPipeAndPassReceiver());
 
   mock_host_resolver_.set_ondemand_mode(true);
 
   resolver_service_->Resolve("example.com", false /* is_ex */,
-                             std::move(client1_ptr));
+                             std::move(client1_remote));
   resolver_service_->Resolve("chromium.org", false /* is_ex */,
-                             std::move(client2_ptr));
+                             std::move(client2_remote));
   WaitForRequests(2);
   mock_host_resolver_.ResolveAllPending();
 
@@ -182,17 +185,19 @@ TEST_F(MojoHostResolverImplTest, ResolveMultiple) {
 }
 
 TEST_F(MojoHostResolverImplTest, ResolveDuplicate) {
-  proxy_resolver::mojom::HostResolverRequestClientPtr client1_ptr;
-  TestRequestClient client1(mojo::MakeRequest(&client1_ptr));
-  proxy_resolver::mojom::HostResolverRequestClientPtr client2_ptr;
-  TestRequestClient client2(mojo::MakeRequest(&client2_ptr));
+  mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+      client1_remote;
+  TestRequestClient client1(client1_remote.InitWithNewPipeAndPassReceiver());
+  mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+      client2_remote;
+  TestRequestClient client2(client2_remote.InitWithNewPipeAndPassReceiver());
 
   mock_host_resolver_.set_ondemand_mode(true);
 
   resolver_service_->Resolve("example.com", false /* is_ex */,
-                             std::move(client1_ptr));
+                             std::move(client1_remote));
   resolver_service_->Resolve("example.com", false /* is_ex */,
-                             std::move(client2_ptr));
+                             std::move(client2_remote));
   WaitForRequests(2);
   mock_host_resolver_.ResolveAllPending();
 
@@ -206,11 +211,12 @@ TEST_F(MojoHostResolverImplTest, ResolveDuplicate) {
 }
 
 TEST_F(MojoHostResolverImplTest, ResolveFailure) {
-  proxy_resolver::mojom::HostResolverRequestClientPtr client_ptr;
-  TestRequestClient client(mojo::MakeRequest(&client_ptr));
+  mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+      client_remote;
+  TestRequestClient client(client_remote.InitWithNewPipeAndPassReceiver());
 
   resolver_service_->Resolve("failure.fail", false /* is_ex */,
-                             std::move(client_ptr));
+                             std::move(client_remote));
   client.WaitForResult();
 
   EXPECT_THAT(client.error_, IsError(net::ERR_NAME_NOT_RESOLVED));
@@ -218,11 +224,12 @@ TEST_F(MojoHostResolverImplTest, ResolveFailure) {
 }
 
 TEST_F(MojoHostResolverImplTest, ResolveEx) {
-  proxy_resolver::mojom::HostResolverRequestClientPtr client_ptr;
-  TestRequestClient client(mojo::MakeRequest(&client_ptr));
+  mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+      client_remote;
+  TestRequestClient client(client_remote.InitWithNewPipeAndPassReceiver());
 
   resolver_service_->Resolve("example.com", true /* is_ex */,
-                             std::move(client_ptr));
+                             std::move(client_remote));
   client.WaitForResult();
 
   EXPECT_THAT(client.error_, IsOk());
@@ -230,14 +237,15 @@ TEST_F(MojoHostResolverImplTest, ResolveEx) {
 }
 
 TEST_F(MojoHostResolverImplTest, DestroyClient) {
-  proxy_resolver::mojom::HostResolverRequestClientPtr client_ptr;
+  mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+      client_remote;
   std::unique_ptr<TestRequestClient> client(
-      new TestRequestClient(mojo::MakeRequest(&client_ptr)));
+      new TestRequestClient(client_remote.InitWithNewPipeAndPassReceiver()));
 
   mock_host_resolver_.set_ondemand_mode(true);
 
   resolver_service_->Resolve("example.com", false /* is_ex */,
-                             std::move(client_ptr));
+                             std::move(client_remote));
   WaitForRequests(1);
 
   client.reset();

services/network/proxy_resolver_factory_mojo.cc

@@ -53,13 +53,14 @@ base::Value NetLogErrorParams(int line_number, const std::string& message) {
 // on a worker thread. Will notify |client| on completion.
 void DoMyIpAddressOnWorker(
     bool is_ex,
-    proxy_resolver::mojom::HostResolverRequestClientPtrInfo client_info) {
+    mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+        client_remote) {
   // Resolve the list of IP addresses.
   std::vector<net::IPAddress> my_ip_addresses =
       is_ex ? net::PacMyIpAddressEx() : net::PacMyIpAddress();
 
-  proxy_resolver::mojom::HostResolverRequestClientPtr client;
-  client.Bind(std::move(client_info));
+  mojo::Remote<proxy_resolver::mojom::HostResolverRequestClient> client(
+      std::move(client_remote));
 
   // TODO(eroman): Note that this code always returns a success response (with
   // loopback) rather than passing forward the error. This is to ensure that the
@@ -129,10 +130,7 @@ class ClientMixin : public ClientInterface {
           base::BindOnce(&DoMyIpAddressOnWorker, is_ex, std::move(client)));
     } else {
       // Request was for dnsResolve() or dnsResolveEx().
-      host_resolver_.Resolve(
-          hostname, is_ex,
-          proxy_resolver::mojom::HostResolverRequestClientPtr(
-              std::move(client)));
+      host_resolver_.Resolve(hostname, is_ex, std::move(client));
     }
   }
 

services/network/proxy_resolver_factory_mojo_unittest.cc

@@ -282,11 +282,12 @@ void MockMojoProxyResolver::GetProxyForUrl(
       break;
     }
     case GetProxyForUrlAction::MAKE_DNS_REQUEST: {
-      proxy_resolver::mojom::HostResolverRequestClientPtr dns_client;
-      mojo::MakeRequest(&dns_client);
+      mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+          dns_client;
+      ignore_result(dns_client.InitWithNewPipeAndPassReceiver());
       client->ResolveDns(url.host(),
                          net::ProxyResolveDnsOperation::DNS_RESOLVE_EX,
-                         dns_client.PassInterface());
+                         std::move(dns_client));
       blocked_clients_.push_back(
           std::make_unique<
               proxy_resolver::mojom::ProxyResolverRequestClientPtr>(
@@ -467,11 +468,12 @@ void MockMojoProxyResolverFactory::CreateResolver(
       break;
     }
     case CreateProxyResolverAction::MAKE_DNS_REQUEST: {
-      proxy_resolver::mojom::HostResolverRequestClientPtr dns_client;
-      mojo::MakeRequest(&dns_client);
+      mojo::PendingRemote<proxy_resolver::mojom::HostResolverRequestClient>
+          dns_client;
+      ignore_result(dns_client.InitWithNewPipeAndPassReceiver());
       client->ResolveDns(pac_script,
                          net::ProxyResolveDnsOperation::DNS_RESOLVE_EX,
-                         dns_client.PassInterface());
+                         std::move(dns_client));
       blocked_clients_.push_back(
           std::make_unique<
               proxy_resolver::mojom::ProxyResolverFactoryRequestClientPtr>(