Commit 36193c58 authored by Chong Zhang's avatar Chong Zhang Committed by Commit Bot

Fix 2 unittests that don't have appropriate URLRequestContext setup

This CL:
1. Fixes |AudioOutputAuthorizationHandlerTest| by using
   |net::TestURLRequestContextGetter| (The original code has an
   use-after-release issue, see bug for logs).
2. Fixes |RenderProcessHostUnitTest| by returning a valid pointer in
   |TestBrowserContext::CreateRequestContextForStoragePartition()|.

This patch doesn't have behavior change.

Bug: 826869
Change-Id: I05982ce49a9d42029cc3278fa248587820790d74
Reviewed-on: https://chromium-review.googlesource.com/1026986
Commit-Queue: Chong Zhang <chongz@chromium.org>
Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556127}
parent a244e759
...@@ -48,20 +48,22 @@ url::Origin SecurityOrigin() { ...@@ -48,20 +48,22 @@ url::Origin SecurityOrigin() {
// TestBrowserContext has a URLRequestContextGetter which uses a NullTaskRunner. // TestBrowserContext has a URLRequestContextGetter which uses a NullTaskRunner.
// This causes it to be destroyed on the wrong thread. This BrowserContext // This causes it to be destroyed on the wrong thread. This BrowserContext
// instead returns nullptr since it's not required by the test. // instead uses the IO thread task runner for the URLRequestContextGetter.
class TestBrowserContextWithoutURLRequestContextGetter class TestBrowserContextWithRealURLRequestContextGetter
: public TestBrowserContext { : public TestBrowserContext {
public: public:
TestBrowserContextWithoutURLRequestContextGetter() { TestBrowserContextWithRealURLRequestContextGetter() {
request_context_ = base::MakeRefCounted<net::TestURLRequestContextGetter>(
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));
salt_ = TestBrowserContext::GetMediaDeviceIDSalt(); salt_ = TestBrowserContext::GetMediaDeviceIDSalt();
} }
~TestBrowserContextWithoutURLRequestContextGetter() override {} ~TestBrowserContextWithRealURLRequestContextGetter() override {}
net::URLRequestContextGetter* CreateRequestContext( net::URLRequestContextGetter* CreateRequestContext(
ProtocolHandlerMap* protocol_handlers, ProtocolHandlerMap* protocol_handlers,
URLRequestInterceptorScopedVector request_interceptors) override { URLRequestInterceptorScopedVector request_interceptors) override {
return nullptr; return request_context_.get();
} }
std::string GetMediaDeviceIDSalt() override { return salt_; } std::string GetMediaDeviceIDSalt() override { return salt_; }
...@@ -69,6 +71,7 @@ class TestBrowserContextWithoutURLRequestContextGetter ...@@ -69,6 +71,7 @@ class TestBrowserContextWithoutURLRequestContextGetter
void set_media_device_id_salt(std::string salt) { salt_ = std::move(salt); } void set_media_device_id_salt(std::string salt) { salt_ = std::move(salt); }
private: private:
scoped_refptr<net::TestURLRequestContextGetter> request_context_;
std::string salt_; std::string salt_;
}; };
...@@ -90,7 +93,7 @@ class AudioOutputAuthorizationHandlerTest : public RenderViewHostTestHarness { ...@@ -90,7 +93,7 @@ class AudioOutputAuthorizationHandlerTest : public RenderViewHostTestHarness {
BrowserContext* CreateBrowserContext() override { BrowserContext* CreateBrowserContext() override {
// Caller takes ownership. // Caller takes ownership.
return new TestBrowserContextWithoutURLRequestContextGetter(); return new TestBrowserContextWithRealURLRequestContextGetter();
} }
void SetUp() override { void SetUp() override {
...@@ -405,7 +408,7 @@ TEST_F(AudioOutputAuthorizationHandlerTest, ...@@ -405,7 +408,7 @@ TEST_F(AudioOutputAuthorizationHandlerTest,
// Reset the salt and expect authorization of the device ID hashed with // Reset the salt and expect authorization of the device ID hashed with
// the old salt to fail. // the old salt to fail.
auto* context = auto* context =
static_cast<TestBrowserContextWithoutURLRequestContextGetter*>( static_cast<TestBrowserContextWithRealURLRequestContextGetter*>(
browser_context()); browser_context());
context->set_media_device_id_salt("new salt"); context->set_media_device_id_salt("new salt");
EXPECT_CALL(listener, Run(media::OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND, _, _, EXPECT_CALL(listener, Run(media::OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND, _, _,
......
...@@ -442,10 +442,7 @@ StoragePartitionImpl* StoragePartitionImplMap::Get( ...@@ -442,10 +442,7 @@ StoragePartitionImpl* StoragePartitionImplMap::Get(
// be initialized using |url_request_context_|, which is initialized by // be initialized using |url_request_context_|, which is initialized by
// SetURLRequestContext(). // SetURLRequestContext().
DCHECK(partition->url_loader_factory_getter()); DCHECK(partition->url_loader_factory_getter());
// TODO(crbug.com/826869): |url_request_context_| is not configured DCHECK(partition->url_request_context_);
// correctly in some unittests. We should fix those tests and turn this 'if'
// into a DCHECK.
if (partition->url_request_context_)
partition->url_loader_factory_getter()->HandleFactoryRequests(); partition->url_loader_factory_getter()->HandleFactoryRequests();
} }
......
...@@ -173,7 +173,9 @@ TestBrowserContext::CreateRequestContextForStoragePartition( ...@@ -173,7 +173,9 @@ TestBrowserContext::CreateRequestContextForStoragePartition(
ProtocolHandlerMap* protocol_handlers, ProtocolHandlerMap* protocol_handlers,
URLRequestInterceptorScopedVector request_interceptors) { URLRequestInterceptorScopedVector request_interceptors) {
request_interceptors_ = std::move(request_interceptors); request_interceptors_ = std::move(request_interceptors);
return nullptr; // Simply returns the same RequestContext since no tests is relying on the
// expected behavior.
return GetRequestContext();
} }
net::URLRequestContextGetter* TestBrowserContext::CreateMediaRequestContext() { net::URLRequestContextGetter* TestBrowserContext::CreateMediaRequestContext() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment