Skip to content

Commit

Permalink
[BugBash] [DirectSockets] Simplify host resolving via SimpleHostResolver
Browse files Browse the repository at this point in the history
This CL removes the old custom ResolveHostAndOpenSocket class and
modernizes host resolving via the new SimpleHostResolver public API.

Bug: 1411522
Change-Id: Ib0dc139d52a40f85eef7878a1b26a0249540e138
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4217594
Reviewed-by: Sonja Laurila <laurila@google.com>
Commit-Queue: Andrew Rayskiy <greengrape@google.com>
Cr-Commit-Position: refs/heads/main@{#1100398}
  • Loading branch information
GrapeGreen authored and Chromium LUCI CQ committed Feb 2, 2023
1 parent ae5f437 commit bd7ec14
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 166 deletions.
2 changes: 0 additions & 2 deletions content/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3114,8 +3114,6 @@ source_set("browser") {
# The Direct Sockets API is not implemented on Android.
"direct_sockets/direct_sockets_service_impl.cc",
"direct_sockets/direct_sockets_service_impl.h",
"direct_sockets/resolve_host_and_open_socket.cc",
"direct_sockets/resolve_host_and_open_socket.h",

# Non-Android platforms that don't presently support
# enable_screen_capture, like Fuchsia, nevertheless compile
Expand Down
66 changes: 49 additions & 17 deletions content/browser/direct_sockets/direct_sockets_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "content/browser/direct_sockets/direct_sockets_service_impl.h"

#include "build/build_config.h"
#include "content/browser/direct_sockets/resolve_host_and_open_socket.h"
#include "content/browser/process_lock.h"
#include "content/browser/renderer_host/isolated_context_util.h"
#include "content/public/browser/browser_thread.h"
Expand All @@ -15,18 +14,18 @@
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/common/content_client.h"
#include "mojo/public/cpp/bindings/message.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/system/data_pipe.h"
#include "net/base/host_port_pair.h"
#include "net/base/ip_address.h"
#include "net/base/ip_endpoint.h"
#include "net/base/network_anonymization_key.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "services/network/public/cpp/simple_host_resolver.h"
#include "services/network/public/mojom/network_context.mojom.h"
#include "services/network/public/mojom/restricted_udp_socket.mojom.h"
#include "services/network/public/mojom/tcp_socket.mojom.h"
#include "services/network/public/mojom/udp_socket.mojom.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/public/mojom/direct_sockets/direct_sockets.mojom-shared.h"
#include "third_party/blink/public/mojom/direct_sockets/direct_sockets.mojom.h"
#include "third_party/blink/public/mojom/permissions_policy/permissions_policy_feature.mojom-shared.h"

Expand Down Expand Up @@ -98,12 +97,20 @@ content::DirectSocketsDelegate* GetDelegate() {
return GetContentClient()->browser()->GetDirectSocketsDelegate();
}

#if BUILDFLAG(ENABLE_MDNS)
bool ResemblesMulticastDNSName(base::StringPiece hostname) {
return base::EndsWith(hostname, ".local") ||
base::EndsWith(hostname, ".local.");
}
#endif // !BUILDFLAG(ENABLE_MDNS)

} // namespace

DirectSocketsServiceImpl::DirectSocketsServiceImpl(
RenderFrameHost* render_frame_host,
mojo::PendingReceiver<blink::mojom::DirectSocketsService> receiver)
: DocumentService(*render_frame_host, std::move(receiver)) {}
: DocumentService(*render_frame_host, std::move(receiver)),
resolver_(network::SimpleHostResolver::Create(GetNetworkContext())) {}

DirectSocketsServiceImpl::~DirectSocketsServiceImpl() = default;

Expand Down Expand Up @@ -145,13 +152,23 @@ void DirectSocketsServiceImpl::OpenTCPSocket(
return;
}

ResolveHostAndOpenSocket::Create(
std::move(remote_addr),
network::mojom::ResolveHostParametersPtr parameters =
network::mojom::ResolveHostParameters::New();
#if BUILDFLAG(ENABLE_MDNS)
if (ResemblesMulticastDNSName(remote_addr.host())) {
parameters->source = net::HostResolverSource::MULTICAST_DNS;
}
#endif // !BUILDFLAG(ENABLE_MDNS)

// Unretained(this) is safe here because the callback will be owned by
// |resolver_| which in turn is owned by |this|.
resolver_->ResolveHost(
network::mojom::HostResolverHost::NewHostPortPair(std::move(remote_addr)),
net::NetworkAnonymizationKey::CreateTransient(), std::move(parameters),
base::BindOnce(&DirectSocketsServiceImpl::OnResolveCompleteForTCPSocket,
weak_ptr_factory_.GetWeakPtr(), std::move(options),
base::Unretained(this), std::move(options),
std::move(receiver), std::move(observer),
std::move(callback)))
->Start(GetNetworkContext());
std::move(callback)));
}

void DirectSocketsServiceImpl::OpenUDPSocket(
Expand Down Expand Up @@ -184,13 +201,24 @@ void DirectSocketsServiceImpl::OpenUDPSocket(
return;
}

ResolveHostAndOpenSocket::Create(
std::move(remote_addr),
network::mojom::ResolveHostParametersPtr parameters =
network::mojom::ResolveHostParameters::New();
#if BUILDFLAG(ENABLE_MDNS)
if (ResemblesMulticastDNSName(remote_addr.host())) {
parameters->source = net::HostResolverSource::MULTICAST_DNS;
}
#endif // !BUILDFLAG(ENABLE_MDNS)

// Unretained(this) is safe here because the callback will be owned by
// |resolver_| which in turn is owned by |this|.
resolver_->ResolveHost(
network::mojom::HostResolverHost::NewHostPortPair(
std::move(remote_addr)),
net::NetworkAnonymizationKey::CreateTransient(), std::move(parameters),
base::BindOnce(&DirectSocketsServiceImpl::OnResolveCompleteForUDPSocket,
weak_ptr_factory_.GetWeakPtr(), std::move(options),
base::Unretained(this), std::move(options),
std::move(receiver), std::move(listener),
std::move(callback)))
->Start(GetNetworkContext());
std::move(callback)));
} else {
// Handle BOUND mode request.
DCHECK(options->local_addr);
Expand Down Expand Up @@ -243,7 +271,9 @@ void DirectSocketsServiceImpl::OnResolveCompleteForTCPSocket(
mojo::PendingRemote<network::mojom::SocketObserver> observer,
OpenTCPSocketCallback callback,
int result,
const absl::optional<net::AddressList>& resolved_addresses) {
const net::ResolveErrorInfo&,
const absl::optional<net::AddressList>& resolved_addresses,
const absl::optional<net::HostResolverEndpointResults>&) {
if (result != net::OK) {
std::move(callback).Run(result, absl::nullopt, absl::nullopt,
mojo::ScopedDataPipeConsumerHandle(),
Expand All @@ -269,7 +299,9 @@ void DirectSocketsServiceImpl::OnResolveCompleteForUDPSocket(
mojo::PendingRemote<network::mojom::UDPSocketListener> listener,
OpenUDPSocketCallback callback,
int result,
const absl::optional<net::AddressList>& resolved_addresses) {
const net::ResolveErrorInfo&,
const absl::optional<net::AddressList>& resolved_addresses,
const absl::optional<net::HostResolverEndpointResults>&) {
if (result != net::OK) {
std::move(callback).Run(result, absl::nullopt, absl::nullopt);
return;
Expand Down
22 changes: 13 additions & 9 deletions content/browser/direct_sockets/direct_sockets_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,21 @@
#ifndef CONTENT_BROWSER_DIRECT_SOCKETS_DIRECT_SOCKETS_SERVICE_IMPL_H_
#define CONTENT_BROWSER_DIRECT_SOCKETS_DIRECT_SOCKETS_SERVICE_IMPL_H_

#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "content/common/content_export.h"
#include "content/public/browser/document_service.h"
#include "content/public/browser/render_frame_host.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/unique_receiver_set.h"
#include "net/base/address_list.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "net/dns/public/host_resolver_results.h"
#include "third_party/blink/public/mojom/direct_sockets/direct_sockets.mojom.h"

namespace network::mojom {
namespace network {
class SimpleHostResolver;
namespace mojom {
class NetworkContext;
} // namespace network::mojom
} // namespace mojom
} // namespace network

namespace content {

Expand Down Expand Up @@ -63,17 +63,21 @@ class CONTENT_EXPORT DirectSocketsServiceImpl
mojo::PendingRemote<network::mojom::SocketObserver>,
OpenTCPSocketCallback,
int result,
const absl::optional<net::AddressList>& resolved_addresses);
const net::ResolveErrorInfo&,
const absl::optional<net::AddressList>& resolved_addresses,
const absl::optional<net::HostResolverEndpointResults>&);

void OnResolveCompleteForUDPSocket(
blink::mojom::DirectUDPSocketOptionsPtr,
mojo::PendingReceiver<network::mojom::RestrictedUDPSocket>,
mojo::PendingRemote<network::mojom::UDPSocketListener>,
OpenUDPSocketCallback,
int result,
const absl::optional<net::AddressList>& resolved_addresses);
const net::ResolveErrorInfo&,
const absl::optional<net::AddressList>& resolved_addresses,
const absl::optional<net::HostResolverEndpointResults>&);

base::WeakPtrFactory<DirectSocketsServiceImpl> weak_ptr_factory_{this};
std::unique_ptr<network::SimpleHostResolver> resolver_;
};

} // namespace content
Expand Down
81 changes: 0 additions & 81 deletions content/browser/direct_sockets/resolve_host_and_open_socket.cc

This file was deleted.

57 changes: 0 additions & 57 deletions content/browser/direct_sockets/resolve_host_and_open_socket.h

This file was deleted.

0 comments on commit bd7ec14

Please sign in to comment.