Commit ba0104a1 authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

ChromeDriver FetchUrlTest unit test failures in debug builds

network::mojom::URLLoaderFactory has stricter threading restrictions
than its predecessor (URLRequestContextGetter) - see [1].

This CL adapts an existing subset of chromedriver unittests (namely
FetchUrlTest), that deals with url fetching, to this threading
restrictions.

Basically, the CL allows the test to inject the TaskRunner that
the URLLoaderFactory handle was created on, and use it afterwards.

A similar approach to this one (of injecting a test-specific
TaskRunner instance) was also used in [2].

[1] https://crrev.com/c/1292933
[2] https://crrev.com/c/1174655/14/components/sync/engine/net/http_bridge.cc#148

BUG=902618

Change-Id: If37bfee8cc891c1a95491cfeb3febf5fe673f52e
Reviewed-on: https://chromium-review.googlesource.com/c/1327443Reviewed-by: default avatarJohn Chen <johnchen@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#606726}
parent 7a664a92
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/bind_helpers.h" #include "base/bind_helpers.h"
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/lazy_instance.h"
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/synchronization/waitable_event.h" #include "base/synchronization/waitable_event.h"
...@@ -20,6 +21,9 @@ ...@@ -20,6 +21,9 @@
namespace { namespace {
base::LazyInstance<scoped_refptr<base::SequencedTaskRunner>>::Leaky
g_io_capable_task_runner_for_tests = LAZY_INSTANCE_INITIALIZER;
class SyncUrlFetcher { class SyncUrlFetcher {
public: public:
SyncUrlFetcher(const GURL& url, SyncUrlFetcher(const GURL& url,
...@@ -27,8 +31,10 @@ class SyncUrlFetcher { ...@@ -27,8 +31,10 @@ class SyncUrlFetcher {
std::string* response) std::string* response)
: url_(url), : url_(url),
url_loader_factory_(url_loader_factory), url_loader_factory_(url_loader_factory),
network_task_runner_( network_task_runner_(g_io_capable_task_runner_for_tests.Get()
base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()})), ? g_io_capable_task_runner_for_tests.Get()
: base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock()})),
response_(response), response_(response),
event_(base::WaitableEvent::ResetPolicy::AUTOMATIC, event_(base::WaitableEvent::ResetPolicy::AUTOMATIC,
base::WaitableEvent::InitialState::NOT_SIGNALED) {} base::WaitableEvent::InitialState::NOT_SIGNALED) {}
...@@ -104,6 +110,11 @@ int NetAddress::port() const { ...@@ -104,6 +110,11 @@ int NetAddress::port() const {
return port_; return port_;
} }
void SetIOCapableTaskRunnerForTest(
scoped_refptr<base::SequencedTaskRunner> task_runner) {
g_io_capable_task_runner_for_tests.Get() = task_runner;
}
bool FetchUrl(const std::string& url, bool FetchUrl(const std::string& url,
network::mojom::URLLoaderFactory* factory, network::mojom::URLLoaderFactory* factory,
std::string* response) { std::string* response) {
......
...@@ -7,6 +7,12 @@ ...@@ -7,6 +7,12 @@
#include <string> #include <string>
#include "base/memory/scoped_refptr.h"
namespace base {
class SequencedTaskRunner;
}
namespace network { namespace network {
namespace mojom { namespace mojom {
class URLLoaderFactory; class URLLoaderFactory;
...@@ -33,6 +39,9 @@ class NetAddress { ...@@ -33,6 +39,9 @@ class NetAddress {
int port_; int port_;
}; };
void SetIOCapableTaskRunnerForTest(
scoped_refptr<base::SequencedTaskRunner> task_runner);
// Synchronously fetches data from a GET HTTP request to the given URL. // Synchronously fetches data from a GET HTTP request to the given URL.
// Returns true if response is 200 OK and sets response body to |response|. // Returns true if response is 200 OK and sets response body to |response|.
bool FetchUrl(const std::string& url, bool FetchUrl(const std::string& url,
......
...@@ -43,11 +43,7 @@ class FetchUrlTest : public testing::Test, ...@@ -43,11 +43,7 @@ class FetchUrlTest : public testing::Test,
base::test::ScopedTaskEnvironment::MainThreadType::IO) { base::test::ScopedTaskEnvironment::MainThreadType::IO) {
base::Thread::Options options(base::MessageLoop::TYPE_IO, 0); base::Thread::Options options(base::MessageLoop::TYPE_IO, 0);
CHECK(io_thread_.StartWithOptions(options)); CHECK(io_thread_.StartWithOptions(options));
scoped_refptr<URLRequestContextGetter> context_getter =
new URLRequestContextGetter(io_thread_.task_runner());
url_loader_factory_owner_ =
std::make_unique<network::TransitionalURLLoaderFactoryOwner>(
context_getter);
base::WaitableEvent event(base::WaitableEvent::ResetPolicy::AUTOMATIC, base::WaitableEvent event(base::WaitableEvent::ResetPolicy::AUTOMATIC,
base::WaitableEvent::InitialState::NOT_SIGNALED); base::WaitableEvent::InitialState::NOT_SIGNALED);
io_thread_.task_runner()->PostTask( io_thread_.task_runner()->PostTask(
...@@ -69,6 +65,14 @@ class FetchUrlTest : public testing::Test, ...@@ -69,6 +65,14 @@ class FetchUrlTest : public testing::Test,
} }
void InitOnIO(base::WaitableEvent* event) { void InitOnIO(base::WaitableEvent* event) {
scoped_refptr<URLRequestContextGetter> context_getter =
new URLRequestContextGetter(io_thread_.task_runner());
url_loader_factory_owner_ =
std::make_unique<network::TransitionalURLLoaderFactoryOwner>(
context_getter);
url_loader_factory_ =
url_loader_factory_owner_->GetURLLoaderFactory().get();
std::unique_ptr<net::ServerSocket> server_socket( std::unique_ptr<net::ServerSocket> server_socket(
new net::TCPServerSocket(NULL, net::NetLogSource())); new net::TCPServerSocket(NULL, net::NetLogSource()));
server_socket->ListenWithAddressAndPort("127.0.0.1", 0, 1); server_socket->ListenWithAddressAndPort("127.0.0.1", 0, 1);
...@@ -80,6 +84,7 @@ class FetchUrlTest : public testing::Test, ...@@ -80,6 +84,7 @@ class FetchUrlTest : public testing::Test,
} }
void DestroyServerOnIO(base::WaitableEvent* event) { void DestroyServerOnIO(base::WaitableEvent* event) {
url_loader_factory_owner_.reset();
server_.reset(NULL); server_.reset(NULL);
event->Signal(); event->Signal();
} }
...@@ -106,9 +111,8 @@ class FetchUrlTest : public testing::Test, ...@@ -106,9 +111,8 @@ class FetchUrlTest : public testing::Test,
} }
bool DoFetchURL(const std::string& server_url, std::string* response) { bool DoFetchURL(const std::string& server_url, std::string* response) {
return FetchUrl(server_url, SetIOCapableTaskRunnerForTest(io_thread_.task_runner());
url_loader_factory_owner_->GetURLLoaderFactory().get(), return FetchUrl(server_url, url_loader_factory_, response);
response);
} }
void OnWebSocketRequest(int connection_id, void OnWebSocketRequest(int connection_id,
...@@ -129,6 +133,7 @@ class FetchUrlTest : public testing::Test, ...@@ -129,6 +133,7 @@ class FetchUrlTest : public testing::Test,
std::unique_ptr<net::HttpServer> server_; std::unique_ptr<net::HttpServer> server_;
std::unique_ptr<network::TransitionalURLLoaderFactoryOwner> std::unique_ptr<network::TransitionalURLLoaderFactoryOwner>
url_loader_factory_owner_; url_loader_factory_owner_;
network::mojom::URLLoaderFactory* url_loader_factory_;
std::string server_url_; std::string server_url_;
base::test::ScopedTaskEnvironment scoped_task_environment_; base::test::ScopedTaskEnvironment scoped_task_environment_;
}; };
......
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