Add optional parameters struct to DNS Mojo API.

Everything in the struct should have a reasonable default value and the
struct param itself is optional.

Added DNS qtype and initial priority parameters. Qtype uses a new mojo
enum with mojo EnumTraits to convert to and from the net::HostResolver
enum. RequestPriority already has a mojoified version in the network
service.

Also moved the control handle parameter to the new struct as it is an
optional parameter.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I1b1b9e6d77d23216b210b56885b426bc2b710155
Reviewed-on: https://chromium-review.googlesource.com/1173337
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583518}

services/network/host_resolver.cc

@@ -21,6 +21,20 @@ static base::LazyInstance<HostResolver::ResolveHostCallback>::Leaky
     resolve_host_callback;
 }
 
+namespace {
+base::Optional<net::HostResolver::ResolveHostParameters>
+ConvertOptionalParameters(
+    const mojom::ResolveHostParametersPtr& mojo_parameters) {
+  if (!mojo_parameters)
+    return base::nullopt;
+
+  net::HostResolver::ResolveHostParameters parameters;
+  parameters.dns_query_type = mojo_parameters->dns_query_type;
+  parameters.initial_priority = mojo_parameters->initial_priority;
+  return parameters;
+}
+}  // namespace
+
 HostResolver::HostResolver(
     mojom::HostResolverRequest resolver_request,
     ConnectionShutdownCallback connection_shutdown_callback,
@@ -45,18 +59,25 @@ HostResolver::~HostResolver() {
     binding_.Close();
 }
 
-void HostResolver::ResolveHost(const net::HostPortPair& host,
-                               mojom::ResolveHostHandleRequest control_handle,
-                               mojom::ResolveHostClientPtr response_client) {
+void HostResolver::ResolveHost(
+    const net::HostPortPair& host,
+    mojom::ResolveHostParametersPtr optional_parameters,
+    mojom::ResolveHostClientPtr response_client) {
   if (resolve_host_callback.Get())
     resolve_host_callback.Get().Run(host.host());
-  auto request =
-      std::make_unique<ResolveHostRequest>(internal_resolver_, host, net_log_);
 
-  int rv =
-      request->Start(std::move(control_handle), std::move(response_client),
-                     base::BindOnce(&HostResolver::OnResolveHostComplete,
-                                    base::Unretained(this), request.get()));
+  auto request = std::make_unique<ResolveHostRequest>(
+      internal_resolver_, host, ConvertOptionalParameters(optional_parameters),
+      net_log_);
+
+  mojom::ResolveHostHandleRequest control_handle_request;
+  if (optional_parameters)
+    control_handle_request = std::move(optional_parameters->control_handle);
+
+  int rv = request->Start(
+      std::move(control_handle_request), std::move(response_client),
+      base::BindOnce(&HostResolver::OnResolveHostComplete,
+                     base::Unretained(this), request.get()));
   if (rv != net::ERR_IO_PENDING)
     return;
 

services/network/host_resolver.h

@@ -45,7 +45,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) HostResolver
   ~HostResolver() override;
 
   void ResolveHost(const net::HostPortPair& host,
-                   mojom::ResolveHostHandleRequest control_handle,
+                   mojom::ResolveHostParametersPtr optional_parameters,
                    mojom::ResolveHostClientPtr response_client) override;
 
   size_t GetNumOutstandingRequestsForTesting() const;

services/network/network_context.cc

@@ -808,15 +808,16 @@ void NetworkContext::CreateNetLogExporter(
                                         std::move(request));
 }
 
-void NetworkContext::ResolveHost(const net::HostPortPair& host,
-                                 mojom::ResolveHostHandleRequest control_handle,
-                                 mojom::ResolveHostClientPtr response_client) {
+void NetworkContext::ResolveHost(
+    const net::HostPortPair& host,
+    mojom::ResolveHostParametersPtr optional_parameters,
+    mojom::ResolveHostClientPtr response_client) {
   if (!internal_host_resolver_) {
     internal_host_resolver_ = std::make_unique<HostResolver>(
         url_request_context_->host_resolver(), network_service_->net_log());
   }
 
-  internal_host_resolver_->ResolveHost(host, std::move(control_handle),
+  internal_host_resolver_->ResolveHost(host, std::move(optional_parameters),
                                        std::move(response_client));
 }
 

services/network/network_context.h

@@ -208,7 +208,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
       mojom::ProxyLookupClientPtr proxy_lookup_client) override;
   void CreateNetLogExporter(mojom::NetLogExporterRequest request) override;
   void ResolveHost(const net::HostPortPair& host,
-                   mojom::ResolveHostHandleRequest control_handle,
+                   mojom::ResolveHostParametersPtr optional_parameters,
                    mojom::ResolveHostClientPtr response_client) override;
   void CreateHostResolver(mojom::HostResolverRequest request) override;
   void AddHSTSForTesting(const std::string& host,

services/network/network_context_unittest.cc

@@ -2620,12 +2620,15 @@ TEST_F(NetworkContextTest, ResolveHost_Sync) {
   resolver->set_synchronous_mode(true);
 
   base::RunLoop run_loop;
+  mojom::ResolveHostHandlePtr control_handle;
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->control_handle = mojo::MakeRequest(&control_handle);
   mojom::ResolveHostClientPtr response_client_ptr;
   TestResolveHostClient response_client(&response_client_ptr, &run_loop);
 
-  mojom::ResolveHostHandlePtr control_handle;
   network_context->ResolveHost(net::HostPortPair("localhost", 160),
-                               mojo::MakeRequest(&control_handle),
+                               std::move(optional_parameters),
                                std::move(response_client_ptr));
   run_loop.Run();
 
@@ -2646,12 +2649,15 @@ TEST_F(NetworkContextTest, ResolveHost_Async) {
   resolver->set_synchronous_mode(false);
 
   base::RunLoop run_loop;
+  mojom::ResolveHostHandlePtr control_handle;
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->control_handle = mojo::MakeRequest(&control_handle);
   mojom::ResolveHostClientPtr response_client_ptr;
   TestResolveHostClient response_client(&response_client_ptr, &run_loop);
 
-  mojom::ResolveHostHandlePtr control_handle;
   network_context->ResolveHost(net::HostPortPair("localhost", 160),
-                               mojo::MakeRequest(&control_handle),
+                               std::move(optional_parameters),
                                std::move(response_client_ptr));
 
   bool control_handle_closed = false;
@@ -2679,12 +2685,15 @@ TEST_F(NetworkContextTest, ResolveHost_Failure_Sync) {
   resolver->set_synchronous_mode(true);
 
   base::RunLoop run_loop;
+  mojom::ResolveHostHandlePtr control_handle;
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->control_handle = mojo::MakeRequest(&control_handle);
   mojom::ResolveHostClientPtr response_client_ptr;
   TestResolveHostClient response_client(&response_client_ptr, &run_loop);
 
-  mojom::ResolveHostHandlePtr control_handle;
   network_context->ResolveHost(net::HostPortPair("example.com", 160),
-                               mojo::MakeRequest(&control_handle),
+                               std::move(optional_parameters),
                                std::move(response_client_ptr));
   run_loop.Run();
 
@@ -2704,12 +2713,15 @@ TEST_F(NetworkContextTest, ResolveHost_Failure_Async) {
   resolver->set_synchronous_mode(false);
 
   base::RunLoop run_loop;
+  mojom::ResolveHostHandlePtr control_handle;
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->control_handle = mojo::MakeRequest(&control_handle);
   mojom::ResolveHostClientPtr response_client_ptr;
   TestResolveHostClient response_client(&response_client_ptr, &run_loop);
 
-  mojom::ResolveHostHandlePtr control_handle;
   network_context->ResolveHost(net::HostPortPair("example.com", 160),
-                               mojo::MakeRequest(&control_handle),
+                               std::move(optional_parameters),
                                std::move(response_client_ptr));
 
   bool control_handle_closed = false;
@@ -2753,14 +2765,17 @@ TEST_F(NetworkContextTest, ResolveHost_CloseControlHandle) {
       CreateContextWithParams(CreateContextParams());
 
   base::RunLoop run_loop;
+  mojom::ResolveHostHandlePtr control_handle;
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->control_handle = mojo::MakeRequest(&control_handle);
   mojom::ResolveHostClientPtr response_client_ptr;
   TestResolveHostClient response_client(&response_client_ptr, &run_loop);
 
   // Resolve "localhost" because it should always resolve fast and locally, even
   // when using a real HostResolver.
-  mojom::ResolveHostHandlePtr control_handle;
   network_context->ResolveHost(net::HostPortPair("localhost", 160),
-                               mojo::MakeRequest(&control_handle),
+                               std::move(optional_parameters),
                                std::move(response_client_ptr));
   control_handle = nullptr;
   run_loop.Run();
@@ -2786,12 +2801,15 @@ TEST_F(NetworkContextTest, ResolveHost_Cancellation) {
   ASSERT_EQ(0, resolver->num_cancellations());
 
   base::RunLoop run_loop;
+  mojom::ResolveHostHandlePtr control_handle;
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->control_handle = mojo::MakeRequest(&control_handle);
   mojom::ResolveHostClientPtr response_client_ptr;
   TestResolveHostClient response_client(&response_client_ptr, &run_loop);
 
-  mojom::ResolveHostHandlePtr control_handle;
   network_context->ResolveHost(net::HostPortPair("localhost", 80),
-                               mojo::MakeRequest(&control_handle),
+                               std::move(optional_parameters),
                                std::move(response_client_ptr));
   bool control_handle_closed = false;
   auto connection_error_callback =
@@ -2823,12 +2841,15 @@ TEST_F(NetworkContextTest, ResolveHost_DestroyContext) {
   ASSERT_EQ(0, resolver->num_cancellations());
 
   base::RunLoop run_loop;
+  mojom::ResolveHostHandlePtr control_handle;
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->control_handle = mojo::MakeRequest(&control_handle);
   mojom::ResolveHostClientPtr response_client_ptr;
   TestResolveHostClient response_client(&response_client_ptr, &run_loop);
 
-  mojom::ResolveHostHandlePtr control_handle;
   network_context->ResolveHost(net::HostPortPair("localhost", 80),
-                               mojo::MakeRequest(&control_handle),
+                               std::move(optional_parameters),
                                std::move(response_client_ptr));
   bool control_handle_closed = false;
   auto connection_error_callback =
@@ -2858,12 +2879,15 @@ TEST_F(NetworkContextTest, ResolveHost_CloseClient) {
   ASSERT_EQ(0, resolver->num_cancellations());
 
   base::RunLoop run_loop;
+  mojom::ResolveHostHandlePtr control_handle;
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->control_handle = mojo::MakeRequest(&control_handle);
   mojom::ResolveHostClientPtr response_client_ptr;
   TestResolveHostClient response_client(&response_client_ptr, &run_loop);
 
-  mojom::ResolveHostHandlePtr control_handle;
   network_context->ResolveHost(net::HostPortPair("localhost", 80),
-                               mojo::MakeRequest(&control_handle),
+                               std::move(optional_parameters),
                                std::move(response_client_ptr));
   bool control_handle_closed = false;
   auto connection_error_callback =
@@ -2922,12 +2946,15 @@ TEST_F(NetworkContextTest, CreateHostResolver_CloseResolver) {
   ASSERT_EQ(0, internal_resolver->num_cancellations());
 
   base::RunLoop run_loop;
+  mojom::ResolveHostHandlePtr control_handle;
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->control_handle = mojo::MakeRequest(&control_handle);
   mojom::ResolveHostClientPtr response_client_ptr;
   TestResolveHostClient response_client(&response_client_ptr, &run_loop);
 
-  mojom::ResolveHostHandlePtr control_handle;
   resolver->ResolveHost(net::HostPortPair("localhost", 80),
-                        mojo::MakeRequest(&control_handle),
+                        std::move(optional_parameters),
                         std::move(response_client_ptr));
   bool control_handle_closed = false;
   auto connection_error_callback =
@@ -2961,12 +2988,15 @@ TEST_F(NetworkContextTest, CreateHostResolver_CloseContext) {
   ASSERT_EQ(0, internal_resolver->num_cancellations());
 
   base::RunLoop run_loop;
+  mojom::ResolveHostHandlePtr control_handle;
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->control_handle = mojo::MakeRequest(&control_handle);
   mojom::ResolveHostClientPtr response_client_ptr;
   TestResolveHostClient response_client(&response_client_ptr, &run_loop);
 
-  mojom::ResolveHostHandlePtr control_handle;
   resolver->ResolveHost(net::HostPortPair("localhost", 80),
-                        mojo::MakeRequest(&control_handle),
+                        std::move(optional_parameters),
                         std::move(response_client_ptr));
   // Run a bit to ensure the resolve request makes it to the resolver. Otherwise
   // the resolver will be destroyed and close its pipe before it even knows

services/network/public/cpp/host_resolver.typemap

+# Copyright 2018 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.
+
+mojom = "//services/network/public/mojom/host_resolver.mojom"
+public_headers = [ "//net/dns/host_resolver.h" ]
+traits_headers =
+    [ "//services/network/public/cpp/host_resolver_mojom_traits.h" ]
+sources = [
+  "//services/network/public/cpp/host_resolver_mojom_traits.cc",
+]
+public_deps = [
+  "//net",
+]
+type_mappings = [ "network.mojom.ResolveHostParameters.DnsQueryType=net::HostResolver::DnsQueryType" ]

services/network/public/cpp/host_resolver_mojom_traits.cc

+// Copyright 2018 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 "services/network/public/cpp/host_resolver_mojom_traits.h"
+
+namespace mojo {
+
+using network::mojom::ResolveHostParameters;
+
+// static
+ResolveHostParameters::DnsQueryType EnumTraits<
+    ResolveHostParameters::DnsQueryType,
+    net::HostResolver::DnsQueryType>::ToMojom(net::HostResolver::DnsQueryType
+                                                  input) {
+  switch (input) {
+    case net::HostResolver::DnsQueryType::UNSPECIFIED:
+      return ResolveHostParameters::DnsQueryType::UNSPECIFIED;
+    case net::HostResolver::DnsQueryType::A:
+      return ResolveHostParameters::DnsQueryType::A;
+    case net::HostResolver::DnsQueryType::AAAA:
+      return ResolveHostParameters::DnsQueryType::AAAA;
+  }
+}
+
+// static
+bool EnumTraits<ResolveHostParameters::DnsQueryType,
+                net::HostResolver::DnsQueryType>::
+    FromMojom(ResolveHostParameters::DnsQueryType input,
+              net::HostResolver::DnsQueryType* output) {
+  switch (input) {
+    case ResolveHostParameters::DnsQueryType::UNSPECIFIED:
+      *output = net::HostResolver::DnsQueryType::UNSPECIFIED;
+      return true;
+    case ResolveHostParameters::DnsQueryType::A:
+      *output = net::HostResolver::DnsQueryType::A;
+      return true;
+    case ResolveHostParameters::DnsQueryType::AAAA:
+      *output = net::HostResolver::DnsQueryType::AAAA;
+      return true;
+  }
+}
+
+}  // namespace mojo

services/network/public/cpp/host_resolver_mojom_traits.h

+// Copyright 2018 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.
+
+#ifndef SERVICES_NETWORK_PUBLIC_CPP_HOST_RESOLVER_MOJOM_TRAITS_H_
+#define SERVICES_NETWORK_PUBLIC_CPP_HOST_RESOLVER_MOJOM_TRAITS_H_
+
+#include "mojo/public/cpp/bindings/enum_traits.h"
+#include "net/dns/host_resolver.h"
+#include "services/network/public/mojom/host_resolver.mojom.h"
+
+namespace mojo {
+
+template <>
+struct EnumTraits<network::mojom::ResolveHostParameters::DnsQueryType,
+                  net::HostResolver::DnsQueryType> {
+  static network::mojom::ResolveHostParameters::DnsQueryType ToMojom(
+      net::HostResolver::DnsQueryType input);
+  static bool FromMojom(
+      network::mojom::ResolveHostParameters::DnsQueryType input,
+      net::HostResolver::DnsQueryType* output);
+};
+
+}  // namespace mojo
+
+#endif  // SERVICES_NETWORK_PUBLIC_CPP_HOST_RESOLVER_MOJOM_TRAITS_H_

services/network/public/cpp/typemaps.gni

@@ -7,6 +7,7 @@ typemaps = [
   "//services/network/public/cpp/cors_error_status.typemap",
   "//services/network/public/cpp/digitally_signed.typemap",
   "//services/network/public/cpp/http_request_headers.typemap",
+  "//services/network/public/cpp/host_resolver.typemap",
   "//services/network/public/cpp/mutable_network_traffic_annotation_tag.typemap",
   "//services/network/public/cpp/mutable_partial_network_traffic_annotation_tag.typemap",
   "//services/network/public/cpp/network_param.typemap",

services/network/public/mojom/host_resolver.mojom

@@ -6,6 +6,7 @@ module network.mojom;
 
 import "net/interfaces/address_list.mojom";
 import "services/network/public/mojom/network_param.mojom";
+import "services/network/public/mojom/url_loader.mojom";
 
 // Control handle used to control outstanding NetworkContext::ResolveHost
 // requests. Handle is optional for all requests, and may be closed at any time
@@ -31,6 +32,31 @@ interface ResolveHostClient {
   OnComplete(int32 result, net.interfaces.AddressList? resolved_addresses);
 };
 
+// Parameter-grouping struct for additional optional parameters for
+// HostResolver::CreateRequest() calls. All fields are optional and have a
+// reasonable default.
+struct ResolveHostParameters {
+  // DNS query type for a ResolveHostRequest.
+  // See:
+  // https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-4
+  enum DnsQueryType {
+    UNSPECIFIED,
+    A,
+    AAAA,
+  };
+
+  // Requested DNS query type. If UNSPECIFIED, resolver will pick A or AAAA (or
+  // both) based on IPv4/IPv6 settings.
+  DnsQueryType dns_query_type = DnsQueryType.UNSPECIFIED;
+
+  // The initial net priority for the host resolution request.
+  RequestPriority initial_priority = RequestPriority.kLowest;
+
+  // If set, the outstanding request can be controlled, eg cancelled, via the
+  // handle.
+  ResolveHostHandle&? control_handle;
+};
+
 // Interface that can be passed to code/processes without direct access to
 // NetworkContext to make ResolveHost requests. If destroyed, all outstanding
 // ResolveHost requests from the destroyed interface will be cancelled.
@@ -39,12 +65,14 @@ interface HostResolver {
   // error code, and on success (network error code OK), an AddressList. All
   // results are sent via the passed |response_client|.
   //
+  // Additional optional parameters may be set using |optional_parameters|. If
+  // unset, reasonable defaults will be used, equivalent to using a
+  // ResolveHostParameters struct without changing any fields from their default
+  // values.
+  //
   // Results in ERR_NAME_NOT_RESOLVED if hostname is invalid, or if it is an
   // incompatible IP literal (e.g. IPv6 is disabled and it is an IPv6 literal).
   //
-  // If passed an optional |control_handle|, the outstanding request can be
-  // controlled, eg cancelled, via the handle.
-  //
   // All outstanding requests are cancelled if the HostResolver or parent
   // NetworkContext are destroyed. Such requests will receive ERR_FAILED via
   // |response_client|.
@@ -52,6 +80,6 @@ interface HostResolver {
   // TODO(crbug.com/821021): Implement more complex functionality to meet full
   // capabilities of Resolve() and DnsClient/MDnsClient functionality.
   ResolveHost(HostPortPair host,
-              ResolveHostHandle&? control_handle,
+              ResolveHostParameters? optional_parameters,
               ResolveHostClient response_client);
 };

services/network/public/mojom/network_context.mojom

@@ -555,7 +555,7 @@ interface NetworkContext {
   // through CreateHostResolver, but most likely most usage except proxy
   // resolver will directly use this method.
   ResolveHost(HostPortPair host,
-              ResolveHostHandle&? control_handle,
+              ResolveHostParameters? optional_parameters,
               ResolveHostClient response_client);
 
   // Creates a HostResolver interface that can be passed to code/processes

services/network/resolve_host_request.cc

@@ -15,15 +15,18 @@
 
 namespace network {
 
-ResolveHostRequest::ResolveHostRequest(net::HostResolver* resolver,
-                                       const net::HostPortPair& host,
-                                       net::NetLog* net_log) {
+ResolveHostRequest::ResolveHostRequest(
+    net::HostResolver* resolver,
+    const net::HostPortPair& host,
+    const base::Optional<net::HostResolver::ResolveHostParameters>&
+        optional_parameters,
+    net::NetLog* net_log) {
   DCHECK(resolver);
   DCHECK(net_log);
 
   internal_request_ = resolver->CreateRequest(
       host, net::NetLogWithSource::Make(net_log, net::NetLogSourceType::NONE),
-      base::nullopt);
+      optional_parameters);
 }
 
 ResolveHostRequest::~ResolveHostRequest() {

services/network/resolve_host_request.h

@@ -26,9 +26,12 @@ namespace network {
 // lifetime and cancellation.
 class ResolveHostRequest : public mojom::ResolveHostHandle {
  public:
-  ResolveHostRequest(net::HostResolver* resolver,
-                     const net::HostPortPair& host,
-                     net::NetLog* net_log);
+  ResolveHostRequest(
+      net::HostResolver* resolver,
+      const net::HostPortPair& host,
+      const base::Optional<net::HostResolver::ResolveHostParameters>&
+          optional_parameters,
+      net::NetLog* net_log);
   ~ResolveHostRequest() override;
 
   int Start(mojom::ResolveHostHandleRequest control_handle_request,

services/network/test/test_network_context.h

@@ -106,7 +106,7 @@ class TestNetworkContext : public mojom::NetworkContext {
       ::network::mojom::ProxyLookupClientPtr proxy_lookup_client) override {}
   void CreateNetLogExporter(mojom::NetLogExporterRequest exporter) override {}
   void ResolveHost(const net::HostPortPair& host,
-                   mojom::ResolveHostHandleRequest control_handle,
+                   mojom::ResolveHostParametersPtr optional_parameters,
                    mojom::ResolveHostClientPtr response_client) override {}
   void CreateHostResolver(mojom::HostResolverRequest request) override {}
   void AddHSTSForTesting(const std::string& host,