Skip to content

Commit

Permalink
Add test for URLRequestContextBuilder::BindToNetwork
Browse files Browse the repository at this point in the history
Add a new test that checks that a network-bound URLRequestContext has
all the needed pieces in place.

To check that the underlying ContextHostResolver has been bound to the
correct network, extend the target network concept to HostResolver (for
testing purposes only).

Bug: 1284972
Change-Id: I07be7b6cdc32dc540b5b3396153e53e81f5c81db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3572626
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Stefano Duo <stefanoduo@google.com>
Cr-Commit-Position: refs/heads/main@{#995154}
  • Loading branch information
StefanoDuo authored and Chromium LUCI CQ committed Apr 22, 2022
1 parent 13032f2 commit bf5a8a9
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 1 deletion.
6 changes: 6 additions & 0 deletions net/dns/context_host_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ const URLRequestContext* ContextHostResolver::GetContextForTesting() const {
return resolve_context_ ? resolve_context_->url_request_context() : nullptr;
}

NetworkChangeNotifier::NetworkHandle
ContextHostResolver::GetTargetNetworkForTesting() const {
return resolve_context_ ? resolve_context_->GetTargetNetwork()
: NetworkChangeNotifier::kInvalidNetworkHandle;
}

size_t ContextHostResolver::LastRestoredCacheSize() const {
return resolve_context_->host_cache()
? resolve_context_->host_cache()->last_restore_size()
Expand Down
2 changes: 2 additions & 0 deletions net/dns/context_host_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ class NET_EXPORT ContextHostResolver : public HostResolver {
void SetRequestContext(URLRequestContext* request_context) override;
HostResolverManager* GetManagerForTesting() override;
const URLRequestContext* GetContextForTesting() const override;
NetworkChangeNotifier::NetworkHandle GetTargetNetworkForTesting()
const override;

// Returns the number of host cache entries that were restored, or 0 if there
// is no cache.
Expand Down
6 changes: 5 additions & 1 deletion net/dns/host_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "base/values.h"
#include "net/base/address_list.h"
#include "net/base/net_errors.h"
#include "net/base/network_change_notifier.h"
#include "net/dns/context_host_resolver.h"
#include "net/dns/dns_client.h"
#include "net/dns/dns_util.h"
Expand Down Expand Up @@ -173,6 +172,11 @@ const URLRequestContext* HostResolver::GetContextForTesting() const {
return nullptr;
}

NetworkChangeNotifier::NetworkHandle HostResolver::GetTargetNetworkForTesting()
const {
return NetworkChangeNotifier::kInvalidNetworkHandle;
}

// static
std::unique_ptr<HostResolver> HostResolver::CreateResolver(
HostResolverManager* manager,
Expand Down
2 changes: 2 additions & 0 deletions net/dns/host_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,8 @@ class NET_EXPORT HostResolver {

virtual HostResolverManager* GetManagerForTesting();
virtual const URLRequestContext* GetContextForTesting() const;
virtual NetworkChangeNotifier::NetworkHandle GetTargetNetworkForTesting()
const;

// Creates a new HostResolver. |manager| must outlive the returned resolver.
//
Expand Down
4 changes: 4 additions & 0 deletions net/dns/host_resolver_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,10 @@ class NET_EXPORT HostResolverManager

bool check_ipv6_on_wifi_for_testing() const { return check_ipv6_on_wifi_; }

NetworkChangeNotifier::NetworkHandle target_network_for_testing() const {
return target_network_;
}

// Public to be called from std::make_unique. Not to be called directly.
HostResolverManager(base::PassKey<HostResolverManager>,
const HostResolver::ManagerOptions& options,
Expand Down
51 changes: 51 additions & 0 deletions net/url_request/url_request_context_builder_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/run_loop.h"
#include "base/task/thread_pool.h"
#include "build/build_config.h"
#include "net/base/mock_network_change_notifier.h"
#include "net/base/network_isolation_key.h"
#include "net/base/request_priority.h"
#include "net/base/test_completion_callback.h"
Expand All @@ -19,6 +20,7 @@
#include "net/http/http_auth_handler_factory.h"
#include "net/log/net_log_with_source.h"
#include "net/proxy_resolution/configured_proxy_resolution_service.h"
#include "net/socket/client_socket_factory.h"
#include "net/ssl/ssl_info.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/gtest_util.h"
Expand All @@ -37,6 +39,10 @@
#endif // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) ||
// BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_ANDROID)
#include "base/android/build_info.h"
#endif // BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(ENABLE_REPORTING)
#include "base/files/scoped_temp_dir.h"
#include "base/threading/thread_task_runner_handle.h"
Expand Down Expand Up @@ -274,6 +280,51 @@ TEST_F(URLRequestContextBuilderTest, CustomHostResolver) {
EXPECT_EQ(context.get(), context->host_resolver()->GetContextForTesting());
}

TEST_F(URLRequestContextBuilderTest, BindToNetworkFinalConfiguration) {
#if BUILDFLAG(IS_ANDROID)
if (base::android::BuildInfo::GetInstance()->sdk_int() <
base::android::SDK_VERSION_MARSHMALLOW) {
GTEST_SKIP()
<< "BindToNetwork is supported starting from Android Marshmallow";
}

// The actual network handle doesn't really matter, this test just wants to
// check that all the pieces are in place and configured correctly.
constexpr NetworkChangeNotifier::NetworkHandle network = 2;
auto scoped_mock_network_change_notifier =
std::make_unique<test::ScopedMockNetworkChangeNotifier>();
test::MockNetworkChangeNotifier* mock_ncn =
scoped_mock_network_change_notifier->mock_network_change_notifier();
mock_ncn->ForceNetworkHandlesSupported();

builder_.BindToNetwork(network);
std::unique_ptr<URLRequestContext> context = builder_.Build();

EXPECT_EQ(context->bound_network(), network);
EXPECT_EQ(context->host_resolver()->GetTargetNetworkForTesting(), network);
EXPECT_EQ(context->host_resolver()
->GetManagerForTesting()
->target_network_for_testing(),
network);
ASSERT_TRUE(context->GetNetworkSessionContext());
// A special factory that bind sockets to `network` is needed. We don't need
// to check exactly for that, the fact that we are not using the default one
// should be good enough.
EXPECT_NE(context->GetNetworkSessionContext()->client_socket_factory,
ClientSocketFactory::GetDefaultFactory());

const auto* quic_params = context->quic_context()->params();
EXPECT_FALSE(quic_params->close_sessions_on_ip_change);
EXPECT_FALSE(quic_params->goaway_sessions_on_ip_change);
EXPECT_FALSE(quic_params->migrate_sessions_on_network_change_v2);

const auto* network_session_params = context->GetNetworkSessionParams();
EXPECT_TRUE(network_session_params->ignore_ip_address_changes);
#else // !BUILDFLAG(IS_ANDROID)
GTEST_SKIP() << "BindToNetwork is supported only on Android";
#endif // BUILDFLAG(IS_ANDROID)
}

} // namespace

} // namespace net

0 comments on commit bf5a8a9

Please sign in to comment.