Commit d6913fa0 authored by Yutaka Hirano's avatar Yutaka Hirano Committed by Commit Bot

Remove unused test-only code from WebURLLoaderImpl

data URLs are now handled in blink, so we don't need to use
WebURLLoaderImpl for data URL handling in tests. Remove
|default_loader| param from WebURLLoaderMockFactory::CreateURLLoader.

That allows us to expect non-null |resource_dispatcher_| and
|loader_factory_| in WebURLLoaderImpl. Remove the redundant code.

Bug: 751425
Change-Id: Iddcc5178b641c37aa563dce65b0626472d710d02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1704717Reviewed-by: default avatarTakashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684273}
parent 424571cb
...@@ -348,29 +348,16 @@ WebURLLoaderFactoryImpl::WebURLLoaderFactoryImpl( ...@@ -348,29 +348,16 @@ WebURLLoaderFactoryImpl::WebURLLoaderFactoryImpl(
base::WeakPtr<ResourceDispatcher> resource_dispatcher, base::WeakPtr<ResourceDispatcher> resource_dispatcher,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory) scoped_refptr<network::SharedURLLoaderFactory> loader_factory)
: resource_dispatcher_(std::move(resource_dispatcher)), : resource_dispatcher_(std::move(resource_dispatcher)),
loader_factory_(std::move(loader_factory)) {} loader_factory_(std::move(loader_factory)) {
DCHECK(resource_dispatcher_);
DCHECK(loader_factory_);
}
WebURLLoaderFactoryImpl::~WebURLLoaderFactoryImpl() = default; WebURLLoaderFactoryImpl::~WebURLLoaderFactoryImpl() = default;
std::unique_ptr<blink::WebURLLoader> WebURLLoaderFactoryImpl::CreateURLLoader( std::unique_ptr<blink::WebURLLoader> WebURLLoaderFactoryImpl::CreateURLLoader(
const blink::WebURLRequest& request, const blink::WebURLRequest& request,
std::unique_ptr<WebResourceLoadingTaskRunnerHandle> task_runner_handle) { std::unique_ptr<WebResourceLoadingTaskRunnerHandle> task_runner_handle) {
if (!loader_factory_) {
// In some tests like RenderViewTests loader_factory_ is not available.
// These tests can still use data URLs to bypass the ResourceDispatcher.
if (!task_runner_handle) {
// TODO(altimin): base::ThreadTaskRunnerHandle::Get is deprecated in
// the renderer. Fix this for frame and non-frame clients.
task_runner_handle =
WebResourceLoadingTaskRunnerHandle::CreateUnprioritized(
base::ThreadTaskRunnerHandle::Get());
}
return std::make_unique<WebURLLoaderImpl>(resource_dispatcher_.get(),
std::move(task_runner_handle),
nullptr /* factory */);
}
DCHECK(task_runner_handle); DCHECK(task_runner_handle);
DCHECK(resource_dispatcher_); DCHECK(resource_dispatcher_);
return std::make_unique<WebURLLoaderImpl>(resource_dispatcher_.get(), return std::make_unique<WebURLLoaderImpl>(resource_dispatcher_.get(),
...@@ -583,14 +570,14 @@ WebURLLoaderImpl::Context::Context( ...@@ -583,14 +570,14 @@ WebURLLoaderImpl::Context::Context(
defers_loading_(NOT_DEFERRING), defers_loading_(NOT_DEFERRING),
request_id_(-1), request_id_(-1),
url_loader_factory_(std::move(url_loader_factory)) { url_loader_factory_(std::move(url_loader_factory)) {
DCHECK(url_loader_factory_ || !resource_dispatcher); DCHECK(resource_dispatcher_);
DCHECK(url_loader_factory_);
} }
void WebURLLoaderImpl::Context::Cancel() { void WebURLLoaderImpl::Context::Cancel() {
TRACE_EVENT_WITH_FLOW0("loading", "WebURLLoaderImpl::Context::Cancel", this, TRACE_EVENT_WITH_FLOW0("loading", "WebURLLoaderImpl::Context::Cancel", this,
TRACE_EVENT_FLAG_FLOW_IN); TRACE_EVENT_FLAG_FLOW_IN);
if (resource_dispatcher_ && // NULL in unittest. if (request_id_ != -1) {
request_id_ != -1) {
resource_dispatcher_->Cancel(request_id_, task_runner_); resource_dispatcher_->Cancel(request_id_, task_runner_);
request_id_ = -1; request_id_ = -1;
} }
......
...@@ -24,7 +24,6 @@ ...@@ -24,7 +24,6 @@
#include "content/app/mojo/mojo_init.h" #include "content/app/mojo/mojo_init.h"
#include "content/child/child_process.h" #include "content/child/child_process.h"
#include "content/public/common/service_names.mojom.h" #include "content/public/common/service_names.mojom.h"
#include "content/renderer/loader/web_url_loader_impl.h"
#include "content/renderer/mojo/blink_interface_provider_impl.h" #include "content/renderer/mojo/blink_interface_provider_impl.h"
#include "content/test/mock_clipboard_host.h" #include "content/test/mock_clipboard_host.h"
#include "media/base/media.h" #include "media/base/media.h"
...@@ -103,12 +102,7 @@ class WebURLLoaderFactoryWithMock : public blink::WebURLLoaderFactory { ...@@ -103,12 +102,7 @@ class WebURLLoaderFactoryWithMock : public blink::WebURLLoaderFactory {
std::unique_ptr<blink::scheduler::WebResourceLoadingTaskRunnerHandle> std::unique_ptr<blink::scheduler::WebResourceLoadingTaskRunnerHandle>
task_runner_handle) override { task_runner_handle) override {
DCHECK(platform_); DCHECK(platform_);
// This loader should be used only for process-local resources such as return platform_->GetURLLoaderMockFactory()->CreateURLLoader();
// data URLs.
auto default_loader = std::make_unique<content::WebURLLoaderImpl>(
nullptr, std::move(task_runner_handle), nullptr);
return platform_->GetURLLoaderMockFactory()->CreateURLLoader(
std::move(default_loader));
} }
private: private:
......
...@@ -27,10 +27,7 @@ class WebURLLoaderMockFactory { ...@@ -27,10 +27,7 @@ class WebURLLoaderMockFactory {
virtual ~WebURLLoaderMockFactory() = default; virtual ~WebURLLoaderMockFactory() = default;
// Create a WebURLLoader that takes care of mocked requests. // Create a WebURLLoader that takes care of mocked requests.
// Non-mocked request are forwarded to given loader which should not virtual std::unique_ptr<WebURLLoader> CreateURLLoader() = 0;
// be nullptr.
virtual std::unique_ptr<WebURLLoader> CreateURLLoader(
std::unique_ptr<WebURLLoader>) = 0;
// Registers a response and the file to be served when the specified URL // Registers a response and the file to be served when the specified URL
// is loaded. If no file is specified then the response content will be empty. // is loaded. If no file is specified then the response content will be empty.
......
...@@ -55,8 +55,7 @@ class FakeServiceWorkerNetworkProvider final ...@@ -55,8 +55,7 @@ class FakeServiceWorkerNetworkProvider final
std::unique_ptr<WebURLLoader> CreateURLLoader( std::unique_ptr<WebURLLoader> CreateURLLoader(
const WebURLRequest& request, const WebURLRequest& request,
std::unique_ptr<scheduler::WebResourceLoadingTaskRunnerHandle>) override { std::unique_ptr<scheduler::WebResourceLoadingTaskRunnerHandle>) override {
return Platform::Current()->GetURLLoaderMockFactory()->CreateURLLoader( return Platform::Current()->GetURLLoaderMockFactory()->CreateURLLoader();
nullptr);
} }
blink::mojom::ControllerServiceWorkerMode GetControllerServiceWorkerMode() blink::mojom::ControllerServiceWorkerMode GetControllerServiceWorkerMode()
......
...@@ -18,7 +18,7 @@ WebURLLoaderFactoryWithMock::~WebURLLoaderFactoryWithMock() = default; ...@@ -18,7 +18,7 @@ WebURLLoaderFactoryWithMock::~WebURLLoaderFactoryWithMock() = default;
std::unique_ptr<WebURLLoader> WebURLLoaderFactoryWithMock::CreateURLLoader( std::unique_ptr<WebURLLoader> WebURLLoaderFactoryWithMock::CreateURLLoader(
const WebURLRequest& request, const WebURLRequest& request,
std::unique_ptr<blink::scheduler::WebResourceLoadingTaskRunnerHandle>) { std::unique_ptr<blink::scheduler::WebResourceLoadingTaskRunnerHandle>) {
return mock_factory_->CreateURLLoader(nullptr); return mock_factory_->CreateURLLoader();
} }
} // namespace blink } // namespace blink
...@@ -17,21 +17,8 @@ ...@@ -17,21 +17,8 @@
namespace blink { namespace blink {
namespace { WebURLLoaderMock::WebURLLoaderMock(WebURLLoaderMockFactoryImpl* factory)
: factory_(factory) {}
void AssertFallbackLoaderAvailability(const WebURL& url,
const WebURLLoader* default_loader) {
DCHECK(KURL(url).ProtocolIsData())
<< "shouldn't be falling back: " << url.GetString().Utf8();
DCHECK(default_loader) << "default_loader wasn't set: "
<< url.GetString().Utf8();
}
} // namespace
WebURLLoaderMock::WebURLLoaderMock(WebURLLoaderMockFactoryImpl* factory,
std::unique_ptr<WebURLLoader> default_loader)
: factory_(factory), default_loader_(std::move(default_loader)) {}
WebURLLoaderMock::~WebURLLoaderMock() { WebURLLoaderMock::~WebURLLoaderMock() {
Cancel(); Cancel();
...@@ -42,7 +29,6 @@ void WebURLLoaderMock::ServeAsynchronousRequest( ...@@ -42,7 +29,6 @@ void WebURLLoaderMock::ServeAsynchronousRequest(
const WebURLResponse& response, const WebURLResponse& response,
const WebData& data, const WebData& data,
const base::Optional<WebURLError>& error) { const base::Optional<WebURLError>& error) {
DCHECK(!using_default_loader_);
if (!client_) if (!client_)
return; return;
...@@ -115,47 +101,26 @@ void WebURLLoaderMock::LoadSynchronously( ...@@ -115,47 +101,26 @@ void WebURLLoaderMock::LoadSynchronously(
int64_t& encoded_data_length, int64_t& encoded_data_length,
int64_t& encoded_body_length, int64_t& encoded_body_length,
blink::WebBlobInfo& downloaded_blob) { blink::WebBlobInfo& downloaded_blob) {
if (factory_->IsMockedURL(request.Url())) { DCHECK(factory_->IsMockedURL(request.Url()));
factory_->LoadSynchronously(request, &response, &error, &data, factory_->LoadSynchronously(request, &response, &error, &data,
&encoded_data_length); &encoded_data_length);
return;
}
AssertFallbackLoaderAvailability(request.Url(), default_loader_.get());
using_default_loader_ = true;
default_loader_->LoadSynchronously(request, client, response, error, data,
encoded_data_length, encoded_body_length,
downloaded_blob);
} }
void WebURLLoaderMock::LoadAsynchronously(const WebURLRequest& request, void WebURLLoaderMock::LoadAsynchronously(const WebURLRequest& request,
WebURLLoaderClient* client) { WebURLLoaderClient* client) {
DCHECK(client); DCHECK(client);
if (factory_->IsMockedURL(request.Url())) { DCHECK(factory_->IsMockedURL(request.Url()));
client_ = client; client_ = client;
factory_->LoadAsynchronouly(request, this); factory_->LoadAsynchronouly(request, this);
return;
}
AssertFallbackLoaderAvailability(request.Url(), default_loader_.get());
using_default_loader_ = true;
default_loader_->LoadAsynchronously(request, client);
} }
void WebURLLoaderMock::Cancel() { void WebURLLoaderMock::Cancel() {
if (using_default_loader_ && default_loader_) {
default_loader_.reset();
return;
}
client_ = nullptr; client_ = nullptr;
factory_->CancelLoad(this); factory_->CancelLoad(this);
} }
void WebURLLoaderMock::SetDefersLoading(bool deferred) { void WebURLLoaderMock::SetDefersLoading(bool deferred) {
is_deferred_ = deferred; is_deferred_ = deferred;
if (using_default_loader_) {
default_loader_->SetDefersLoading(deferred);
return;
}
// Ignores setDefersLoading(false) safely. // Ignores setDefersLoading(false) safely.
if (!deferred) if (!deferred)
return; return;
......
...@@ -29,9 +29,7 @@ const uint32_t kRedirectResponseOverheadBytes = 300; ...@@ -29,9 +29,7 @@ const uint32_t kRedirectResponseOverheadBytes = 300;
// forwards it to the default loader. // forwards it to the default loader.
class WebURLLoaderMock : public WebURLLoader { class WebURLLoaderMock : public WebURLLoader {
public: public:
// This object becomes the owner of |default_loader|. explicit WebURLLoaderMock(WebURLLoaderMockFactoryImpl* factory);
WebURLLoaderMock(WebURLLoaderMockFactoryImpl* factory,
std::unique_ptr<WebURLLoader> default_loader);
~WebURLLoaderMock() override; ~WebURLLoaderMock() override;
// Simulates the asynchronous request being served. // Simulates the asynchronous request being served.
...@@ -70,8 +68,6 @@ class WebURLLoaderMock : public WebURLLoader { ...@@ -70,8 +68,6 @@ class WebURLLoaderMock : public WebURLLoader {
WebURLLoaderMockFactoryImpl* factory_ = nullptr; WebURLLoaderMockFactoryImpl* factory_ = nullptr;
WebURLLoaderClient* client_ = nullptr; WebURLLoaderClient* client_ = nullptr;
std::unique_ptr<WebURLLoader> default_loader_;
bool using_default_loader_ = false;
bool is_deferred_ = false; bool is_deferred_ = false;
base::WeakPtrFactory<WebURLLoaderMock> weak_factory_{this}; base::WeakPtrFactory<WebURLLoaderMock> weak_factory_{this};
......
...@@ -39,9 +39,8 @@ WebURLLoaderMockFactoryImpl::WebURLLoaderMockFactoryImpl( ...@@ -39,9 +39,8 @@ WebURLLoaderMockFactoryImpl::WebURLLoaderMockFactoryImpl(
WebURLLoaderMockFactoryImpl::~WebURLLoaderMockFactoryImpl() = default; WebURLLoaderMockFactoryImpl::~WebURLLoaderMockFactoryImpl() = default;
std::unique_ptr<WebURLLoader> WebURLLoaderMockFactoryImpl::CreateURLLoader( std::unique_ptr<WebURLLoader> WebURLLoaderMockFactoryImpl::CreateURLLoader() {
std::unique_ptr<WebURLLoader> default_loader) { return std::make_unique<WebURLLoaderMock>(this);
return std::make_unique<WebURLLoaderMock>(this, std::move(default_loader));
} }
void WebURLLoaderMockFactoryImpl::RegisterURL(const WebURL& url, void WebURLLoaderMockFactoryImpl::RegisterURL(const WebURL& url,
......
...@@ -35,8 +35,7 @@ class WebURLLoaderMockFactoryImpl : public WebURLLoaderMockFactory { ...@@ -35,8 +35,7 @@ class WebURLLoaderMockFactoryImpl : public WebURLLoaderMockFactory {
~WebURLLoaderMockFactoryImpl() override; ~WebURLLoaderMockFactoryImpl() override;
// WebURLLoaderMockFactory: // WebURLLoaderMockFactory:
std::unique_ptr<WebURLLoader> CreateURLLoader( std::unique_ptr<WebURLLoader> CreateURLLoader() override;
std::unique_ptr<WebURLLoader> default_loader) override;
void RegisterURL(const WebURL& url, void RegisterURL(const WebURL& url,
const WebURLResponse& response, const WebURLResponse& response,
const WebString& file_path = WebString()) override; const WebString& file_path = WebString()) override;
......
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