Skip to content

Commit

Permalink
Remove TestURLRequestContext
Browse files Browse the repository at this point in the history
Remove TestURLRequestContext and its subclasses, and use
URLRequestContextBuilder instead.

Currently many tests set context's members after it is built, which is
a violation of a contract. This change is a step to remove setters from
URLRequestContext to make the violation impossible.

Bug: 1282359, 1282361
Change-Id: I344156a3ef04745aa451193eea6966603ebf0776
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3527944
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/main@{#984336}
  • Loading branch information
yutakahirano authored and Chromium LUCI CQ committed Mar 23, 2022
1 parent d854c1f commit d81ac3f
Show file tree
Hide file tree
Showing 21 changed files with 147 additions and 292 deletions.
Expand Up @@ -95,9 +95,7 @@ class MockPersonalDataManager : public TestPersonalDataManager {
class FullCardRequestTest : public testing::Test {
public:
FullCardRequestTest()
: request_context_(new net::TestURLRequestContextGetter(
base::ThreadTaskRunnerHandle::Get())),
test_shared_loader_factory_(
: test_shared_loader_factory_(
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&test_url_loader_factory_)) {
payments_client_ = std::make_unique<PaymentsClient>(
Expand Down Expand Up @@ -164,7 +162,6 @@ class FullCardRequestTest : public testing::Test {
MockResultDelegate result_delegate_;
MockUIDelegate ui_delegate_;
TestAutofillClient autofill_client_;
scoped_refptr<net::TestURLRequestContextGetter> request_context_;
network::TestURLLoaderFactory test_url_loader_factory_;
scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
std::unique_ptr<PaymentsClient> payments_client_;
Expand Down
2 changes: 0 additions & 2 deletions components/gcm_driver/gcm_driver_desktop_unittest.cc
Expand Up @@ -240,8 +240,6 @@ FakeGCMClient* GCMDriverTest::GetGCMClient() {
}

void GCMDriverTest::CreateDriver() {
scoped_refptr<net::URLRequestContextGetter> request_context =
new net::TestURLRequestContextGetter(io_thread_.task_runner());
GCMClient::ChromeBuildInfo chrome_build_info;
chrome_build_info.product_category_for_subtypes = "com.chrome.macosx";
driver_ = std::make_unique<GCMDriverDesktop>(
Expand Down
3 changes: 0 additions & 3 deletions components/gcm_driver/gcm_driver_unittest.cc
Expand Up @@ -23,7 +23,6 @@
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h"
#include "crypto/ec_private_key.h"
#include "net/url_request/url_request_context_getter.h"
#include "net/url_request/url_request_test_util.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
Expand Down Expand Up @@ -148,8 +147,6 @@ void GCMDriverBaseTest::PumpIOLoop() {
}

void GCMDriverBaseTest::CreateDriver() {
scoped_refptr<net::URLRequestContextGetter> request_context =
new net::TestURLRequestContextGetter(io_thread_.task_runner());
GCMClient::ChromeBuildInfo chrome_build_info;
chrome_build_info.product_category_for_subtypes = "com.chrome.macosx";
driver_ = std::make_unique<GCMDriverDesktop>(
Expand Down
Expand Up @@ -15,6 +15,8 @@
#include "components/password_manager/core/browser/password_manager_test_utils.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "net/http/transport_security_state.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_builder.h"
#include "net/url_request/url_request_test_util.h"
#include "services/network/network_context.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -63,12 +65,11 @@ HSTSStateManager::~HSTSStateManager() {
class HSTSQueryTest : public testing::Test {
public:
HSTSQueryTest()
: request_context_(new net::TestURLRequestContextGetter(
base::ThreadTaskRunnerHandle::Get())),
: request_context_(net::CreateTestURLRequestContextBuilder()->Build()),
network_context_(std::make_unique<network::NetworkContext>(
nullptr,
network_context_remote_.BindNewPipeAndPassReceiver(),
request_context_->GetURLRequestContext(),
request_context_.get(),
/*cors_exempt_header_list=*/std::vector<std::string>())) {}

HSTSQueryTest(const HSTSQueryTest&) = delete;
Expand All @@ -79,7 +80,7 @@ class HSTSQueryTest : public testing::Test {
private:
// Used by request_context_.
base::test::SingleThreadTaskEnvironment task_environment_;
scoped_refptr<net::TestURLRequestContextGetter> request_context_;
std::unique_ptr<net::URLRequestContext> request_context_;
mojo::Remote<network::mojom::NetworkContext> network_context_remote_;
std::unique_ptr<network::NetworkContext> network_context_;
};
Expand Down
Expand Up @@ -17,6 +17,8 @@
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_builder.h"
#include "net/url_request/url_request_test_util.h"
#include "services/network/network_context.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -175,12 +177,11 @@ TEST_P(HttpCredentialCleanerTest, ReportHttpMigrationMetrics) {
}
store_->AddLogin(https_form);

auto request_context = base::MakeRefCounted<net::TestURLRequestContextGetter>(
base::ThreadTaskRunnerHandle::Get());
auto request_context = net::CreateTestURLRequestContextBuilder()->Build();
mojo::Remote<network::mojom::NetworkContext> network_context_remote;
auto network_context = std::make_unique<network::NetworkContext>(
nullptr, network_context_remote.BindNewPipeAndPassReceiver(),
request_context->GetURLRequestContext(),
request_context.get(),
/*cors_exempt_header_list=*/std::vector<std::string>());

if (test.is_hsts_enabled) {
Expand Down Expand Up @@ -280,13 +281,11 @@ TEST(HttpCredentialCleaner, StartCleanUpTest) {
continue;
}

auto request_context =
base::MakeRefCounted<net::TestURLRequestContextGetter>(
base::ThreadTaskRunnerHandle::Get());
auto request_context = net::CreateTestURLRequestContextBuilder()->Build();
mojo::Remote<network::mojom::NetworkContext> network_context_remote;
auto network_context = std::make_unique<network::NetworkContext>(
nullptr, network_context_remote.BindNewPipeAndPassReceiver(),
request_context->GetURLRequestContext(),
request_context.get(),
/*cors_exempt_header_list=*/std::vector<std::string>());

MockCredentialsCleanerObserver observer;
Expand Down
6 changes: 4 additions & 2 deletions fuchsia/base/test/test_devtools_list_fetcher.cc
Expand Up @@ -14,16 +14,18 @@
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "net/url_request/url_fetcher.h"
#include "net/url_request/url_fetcher_delegate.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_builder.h"
#include "net/url_request/url_request_test_util.h"

namespace cr_fuchsia {

base::Value GetDevToolsListFromPort(uint16_t port) {
GURL url(base::StringPrintf("http://127.0.0.1:%d/json/list", port));
net::TestURLRequestContext request_context;
auto request_context = net::CreateTestURLRequestContextBuilder()->Build();
net::TestDelegate delegate;

std::unique_ptr<net::URLRequest> request(request_context.CreateRequest(
std::unique_ptr<net::URLRequest> request(request_context->CreateRequest(
url, net::DEFAULT_PRIORITY, &delegate, TRAFFIC_ANNOTATION_FOR_TESTS));
request->Start();
delegate.RunUntilComplete();
Expand Down
14 changes: 8 additions & 6 deletions google_apis/gcm/base/socket_stream_unittest.cc
Expand Up @@ -23,6 +23,8 @@
#include "net/log/net_log_source.h"
#include "net/socket/socket_test_util.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_builder.h"
#include "net/url_request/url_request_test_util.h"
#include "services/network/network_context.h"
#include "services/network/network_service.h"
Expand Down Expand Up @@ -97,7 +99,7 @@ class GCMSocketStreamTest : public testing::Test {
std::unique_ptr<network::NetworkService> network_service_;
mojo::Remote<network::mojom::NetworkContext> network_context_remote_;
net::MockClientSocketFactory socket_factory_;
net::TestURLRequestContext url_request_context_;
std::unique_ptr<net::URLRequestContext> url_request_context_;
std::unique_ptr<network::NetworkContext> network_context_;
mojo::Remote<network::mojom::ProxyResolvingSocketFactory>
mojo_socket_factory_remote_;
Expand All @@ -109,18 +111,18 @@ GCMSocketStreamTest::GCMSocketStreamTest()
: task_environment_(base::test::TaskEnvironment::MainThreadType::IO),
network_change_notifier_(
net::NetworkChangeNotifier::CreateMockIfNeeded()),
network_service_(network::NetworkService::CreateForTesting()),
url_request_context_(true /* delay_initialization */) {
network_service_(network::NetworkService::CreateForTesting()) {
address_list_ = net::AddressList::CreateFromIPAddress(
net::IPAddress::IPv4Localhost(), 5228);
socket_factory_.set_enable_read_if_ready(true);
url_request_context_.set_client_socket_factory(&socket_factory_);
url_request_context_.Init();
auto context_builder = net::CreateTestURLRequestContextBuilder();
context_builder->set_client_socket_factory_for_testing(&socket_factory_);
url_request_context_ = context_builder->Build();

network_context_ = std::make_unique<network::NetworkContext>(
network_service_.get(),
network_context_remote_.BindNewPipeAndPassReceiver(),
&url_request_context_,
url_request_context_.get(),
/*cors_exempt_header_list=*/std::vector<std::string>());
}

Expand Down
14 changes: 8 additions & 6 deletions google_apis/gcm/engine/connection_handler_impl_unittest.cc
Expand Up @@ -34,6 +34,8 @@
#include "net/socket/stream_socket.h"
#include "net/test/gtest_util.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_builder.h"
#include "net/url_request/url_request_test_util.h"
#include "services/network/network_context.h"
#include "services/network/network_service.h"
Expand Down Expand Up @@ -199,7 +201,7 @@ class GCMConnectionHandlerImplTest : public testing::Test {
std::unique_ptr<network::NetworkService> network_service_;
mojo::Remote<network::mojom::NetworkContext> network_context_remote_;
net::MockClientSocketFactory socket_factory_;
net::TestURLRequestContext url_request_context_;
std::unique_ptr<net::URLRequestContext> url_request_context_;
std::unique_ptr<network::NetworkContext> network_context_;
mojo::Remote<network::mojom::ProxyResolvingSocketFactory>
mojo_socket_factory_remote_;
Expand All @@ -212,18 +214,18 @@ GCMConnectionHandlerImplTest::GCMConnectionHandlerImplTest()
task_environment_(base::test::TaskEnvironment::MainThreadType::IO),
network_change_notifier_(
net::NetworkChangeNotifier::CreateMockIfNeeded()),
network_service_(network::NetworkService::CreateForTesting()),
url_request_context_(true /* delay_initialization */) {
network_service_(network::NetworkService::CreateForTesting()) {
address_list_ = net::AddressList::CreateFromIPAddress(
net::IPAddress::IPv4Localhost(), kMCSPort);
socket_factory_.set_enable_read_if_ready(true);
url_request_context_.set_client_socket_factory(&socket_factory_);
url_request_context_.Init();
auto context_builder = net::CreateTestURLRequestContextBuilder();
context_builder->set_client_socket_factory_for_testing(&socket_factory_);
url_request_context_ = context_builder->Build();

network_context_ = std::make_unique<network::NetworkContext>(
network_service_.get(),
network_context_remote_.BindNewPipeAndPassReceiver(),
&url_request_context_,
url_request_context_.get(),
/*cors_exempt_header_list=*/std::vector<std::string>());
}

Expand Down
4 changes: 3 additions & 1 deletion ios/net/protocol_handler_util_unittest.mm
Expand Up @@ -17,6 +17,8 @@
#include "net/http/http_request_headers.h"
#include "net/http/http_response_headers.h"
#include "net/url_request/url_request.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_builder.h"
#include "net/url_request/url_request_filter.h"
#include "net/url_request/url_request_interceptor.h"
#include "net/url_request/url_request_job.h"
Expand Down Expand Up @@ -109,7 +111,7 @@ void GetResponseInfo(HttpResponseInfo* info) override {
public:
ProtocolHandlerUtilTest()
: task_environment_(base::test::TaskEnvironment::MainThreadType::IO),
request_context_(std::make_unique<TestURLRequestContext>()) {
request_context_(net::CreateTestURLRequestContextBuilder()->Build()) {
URLRequestFilter::GetInstance()->AddHostnameInterceptor(
"http", "foo.test", std::make_unique<NetURLRequestInterceptor>());
}
Expand Down
14 changes: 8 additions & 6 deletions ios/web/public/test/fakes/fake_browser_state.cc
Expand Up @@ -11,8 +11,8 @@
#include "ios/web/test/test_url_constants.h"
#include "ios/web/webui/url_data_manager_ios_backend.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_builder.h"
#include "net/url_request/url_request_context_getter.h"
#include "net/url_request/url_request_job_factory.h"
#include "net/url_request/url_request_test_util.h"

namespace web {
Expand All @@ -22,13 +22,16 @@ namespace {
class TestContextURLRequestContextGetter : public net::URLRequestContextGetter {
public:
TestContextURLRequestContextGetter(web::BrowserState* browser_state) {
job_factory_.SetProtocolHandler(
auto context_builder = net::CreateTestURLRequestContextBuilder();
context_builder->SetProtocolHandler(
kTestWebUIScheme,
web::URLDataManagerIOSBackend::CreateProtocolHandler(browser_state));
context_.set_job_factory(&job_factory_);
context_ = context_builder->Build();
}

net::URLRequestContext* GetURLRequestContext() override { return &context_; }
net::URLRequestContext* GetURLRequestContext() override {
return context_.get();
}

scoped_refptr<base::SingleThreadTaskRunner> GetNetworkTaskRunner()
const override {
Expand All @@ -38,8 +41,7 @@ class TestContextURLRequestContextGetter : public net::URLRequestContextGetter {
private:
~TestContextURLRequestContextGetter() override {}

net::TestURLRequestContext context_;
net::URLRequestJobFactory job_factory_;
std::unique_ptr<net::URLRequestContext> context_;
};

} // namespace
Expand Down
14 changes: 8 additions & 6 deletions net/android/traffic_stats_unittest.cc
Expand Up @@ -7,6 +7,8 @@
#include "base/run_loop.h"
#include "base/test/task_environment.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_builder.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -34,11 +36,11 @@ TEST(TrafficStatsAndroidTest, BasicsTest) {
EXPECT_GE(rx_bytes_before_request, 0);

TestDelegate test_delegate;
TestURLRequestContext context(false);
auto context = CreateTestURLRequestContextBuilder()->Build();

std::unique_ptr<URLRequest> request(
context.CreateRequest(embedded_test_server.GetURL("/echo.html"),
DEFAULT_PRIORITY, &test_delegate));
context->CreateRequest(embedded_test_server.GetURL("/echo.html"),
DEFAULT_PRIORITY, &test_delegate));
request->Start();
base::RunLoop().Run();

Expand Down Expand Up @@ -70,11 +72,11 @@ TEST(TrafficStatsAndroidTest, UIDBasicsTest) {
EXPECT_GE(rx_bytes_before_request, 0);

TestDelegate test_delegate;
TestURLRequestContext context(false);
auto context = CreateTestURLRequestContextBuilder()->Build();

std::unique_ptr<URLRequest> request(
context.CreateRequest(embedded_test_server.GetURL("/echo.html"),
DEFAULT_PRIORITY, &test_delegate));
context->CreateRequest(embedded_test_server.GetURL("/echo.html"),
DEFAULT_PRIORITY, &test_delegate));
request->Start();
base::RunLoop().Run();

Expand Down
6 changes: 4 additions & 2 deletions net/cert/cert_verify_proc_builtin_unittest.cc
Expand Up @@ -29,6 +29,8 @@
#include "net/test/embedded_test_server/http_response.h"
#include "net/test/embedded_test_server/request_handler_util.h"
#include "net/test/gtest_util.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_builder.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand Down Expand Up @@ -130,7 +132,7 @@ class CertVerifyProcBuiltinTest : public ::testing::Test {
verify_proc_ = CreateCertVerifyProcBuiltin(
cert_net_fetcher_, std::move(mock_system_trust_store));

context_ = std::make_unique<net::TestURLRequestContext>();
context_ = CreateTestURLRequestContextBuilder()->Build();

cert_net_fetcher_->SetURLRequestContext(context_.get());
}
Expand Down Expand Up @@ -181,7 +183,7 @@ class CertVerifyProcBuiltinTest : public ::testing::Test {
};

CertVerifier::Config config_;
std::unique_ptr<net::TestURLRequestContext> context_;
std::unique_ptr<net::URLRequestContext> context_;
MockSystemTrustStore* mock_system_trust_store_;
scoped_refptr<CertVerifyProc> verify_proc_;
scoped_refptr<CertNetFetcherURLRequest> cert_net_fetcher_;
Expand Down
14 changes: 8 additions & 6 deletions net/dns/dns_transaction_unittest.cc
Expand Up @@ -56,6 +56,8 @@
#include "net/test/test_with_task_environment.h"
#include "net/test/url_request/url_request_failed_job.h"
#include "net/third_party/uri_template/uri_template.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_builder.h"
#include "net/url_request/url_request_filter.h"
#include "net/url_request/url_request_interceptor.h"
#include "net/url_request/url_request_test_util.h"
Expand Down Expand Up @@ -910,7 +912,7 @@ class DnsTransactionTestBase : public testing::Test {
// and an arbitrary fallback period.
config_.fallback_period = kFallbackPeriod;

request_context_ = std::make_unique<TestURLRequestContext>();
request_context_ = CreateTestURLRequestContextBuilder()->Build();
resolve_context_ = std::make_unique<ResolveContext>(
request_context_.get(), false /* enable_caching */);

Expand Down Expand Up @@ -948,7 +950,7 @@ class DnsTransactionTestBase : public testing::Test {

base::circular_deque<int> transaction_ids_;
std::unique_ptr<TestSocketFactory> socket_factory_;
std::unique_ptr<TestURLRequestContext> request_context_;
std::unique_ptr<URLRequestContext> request_context_;
std::unique_ptr<ResolveContext> resolve_context_;
scoped_refptr<DnsSession> session_;
std::unique_ptr<DnsTransactionFactory> transaction_factory_;
Expand Down Expand Up @@ -2597,8 +2599,8 @@ TEST_F(DnsTransactionTest, MAYBE_HttpsPostLookupWithLog) {
true /* secure */, resolve_context_.get());
helper0.RunUntilComplete();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(observer.count(), 5);
EXPECT_EQ(observer.dict_count(), 3);
EXPECT_EQ(observer.count(), 14);
EXPECT_EQ(observer.dict_count(), 6);
}

// Test for when a slow DoH response is delayed until after the initial fallback
Expand Down Expand Up @@ -3569,8 +3571,8 @@ TEST_F(DnsTransactionTestWithMockTime, MultipleProbeRunners_SeparateContexts) {
DnsQuery::PaddingStrategy::BLOCK_LENGTH_128,
false /* enqueue_transaction_id */);

TestURLRequestContext request_context2;
ResolveContext context2(&request_context2, false /* enable_caching */);
auto request_context2 = CreateTestURLRequestContextBuilder()->Build();
ResolveContext context2(request_context2.get(), false /* enable_caching */);
context2.InvalidateCachesAndPerSessionData(session_.get(),
false /* network_change */);

Expand Down

0 comments on commit d81ac3f

Please sign in to comment.