Commit 75ef3e36 authored by Matt Menke's avatar Matt Menke Committed by Commit Bot

Refactor SimpleURLLoader API.

Make it take in a unique_ptr<content::ResourceRequest>, to avoid having
to copy it on retry, or when making a delayed request.

Also make the constructor take the ResourceRequest and NetworkAnnotation
as parameters, to simplify things a little.

TBR=jdufault@chromium.org

Bug: 746977
Change-Id: Ic0e2a573c8b8601e2b1ab9a4b58430006b8679dc
Reviewed-on: https://chromium-review.googlesource.com/777587
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarRandy Smith <rdsmith@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518451}
parent 3d4e31d5
......@@ -71,16 +71,17 @@ class DriveAppConverter::IconFetcher : public ImageDecoder::ImageRequest {
policy_exception_justification:
"Not implemented, considered not useful."
})");
simple_loader_ = content::SimpleURLLoader::Create();
content::ResourceRequest resource_request = content::ResourceRequest();
resource_request.url = icon_url_;
resource_request.load_flags = net::LOAD_DO_NOT_SAVE_COOKIES;
auto resource_request = std::make_unique<content::ResourceRequest>();
resource_request->url = icon_url_;
resource_request->load_flags = net::LOAD_DO_NOT_SAVE_COOKIES;
simple_loader_ = content::SimpleURLLoader::Create(
std::move(resource_request), traffic_annotation);
content::mojom::URLLoaderFactory* loader_factory =
content::BrowserContext::GetDefaultStoragePartition(
converter_->profile_)
->GetURLLoaderFactoryForBrowserProcess();
simple_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
resource_request, loader_factory, traffic_annotation,
loader_factory,
base::BindOnce(&DriveAppConverter::IconFetcher::OnCompleteCallback,
base::Unretained(this)));
}
......
......@@ -112,23 +112,24 @@ void TermsOfServiceScreen::StartDownload() {
"Not implemented, considered not useful."
})");
// Start downloading the Terms of Service.
terms_of_service_loader_ = content::SimpleURLLoader::Create();
auto resource_request = std::make_unique<content::ResourceRequest>();
resource_request->url = GURL(terms_of_service_url);
// Request a text/plain MIME type as only plain-text Terms of Service are
// accepted.
resource_request->headers.SetHeader("Accept", "text/plain");
terms_of_service_loader_ = content::SimpleURLLoader::Create(
std::move(resource_request), traffic_annotation);
// Retry up to three times if network changes are detected during the
// download.
terms_of_service_loader_->SetRetryOptions(
3, content::SimpleURLLoader::RETRY_ON_NETWORK_CHANGE);
content::ResourceRequest resource_request;
resource_request.url = GURL(terms_of_service_url);
// Request a text/plain MIME type as only plain-text Terms of Service are
// accepted.
resource_request.headers.SetHeader("Accept", "text/plain");
content::mojom::URLLoaderFactory* loader_factory =
g_browser_process->system_network_context_manager()
->GetURLLoaderFactory();
terms_of_service_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
resource_request, loader_factory, traffic_annotation,
base::BindOnce(&TermsOfServiceScreen::OnDownloaded,
base::Unretained(this)));
loader_factory, base::BindOnce(&TermsOfServiceScreen::OnDownloaded,
base::Unretained(this)));
// Abort the download attempt if it takes longer than one minute.
download_timer_.Start(FROM_HERE, base::TimeDelta::FromMinutes(1), this,
......
......@@ -141,15 +141,16 @@ class NetworkContextConfigurationBrowserTest
};
IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, BasicRequest) {
std::unique_ptr<content::ResourceRequest> request =
std::make_unique<content::ResourceRequest>();
request->url = embedded_test_server()->GetURL("/echo");
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create();
content::SimpleURLLoader::Create(std::move(request),
TRAFFIC_ANNOTATION_FOR_TESTS);
content::ResourceRequest request;
request.url = embedded_test_server()->GetURL("/echo");
simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper.GetCallback());
loader_factory(), simple_loader_helper.GetCallback());
simple_loader_helper.WaitForCallback();
ASSERT_TRUE(simple_loader->ResponseInfo());
......@@ -160,15 +161,16 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, BasicRequest) {
}
IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, DataURL) {
std::unique_ptr<content::ResourceRequest> request =
std::make_unique<content::ResourceRequest>();
request->url = GURL("data:text/plain,foo");
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create();
content::SimpleURLLoader::Create(std::move(request),
TRAFFIC_ANNOTATION_FOR_TESTS);
content::ResourceRequest request;
request.url = GURL("data:text/plain,foo");
simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper.GetCallback());
loader_factory(), simple_loader_helper.GetCallback());
simple_loader_helper.WaitForCallback();
ASSERT_TRUE(simple_loader->ResponseInfo());
......@@ -189,15 +191,16 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, FileURL) {
ASSERT_EQ(static_cast<int>(strlen(kFileContents)),
base::WriteFile(file_path, kFileContents, strlen(kFileContents)));
std::unique_ptr<content::ResourceRequest> request =
std::make_unique<content::ResourceRequest>();
request->url = net::FilePathToFileURL(file_path);
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create();
content::SimpleURLLoader::Create(std::move(request),
TRAFFIC_ANNOTATION_FOR_TESTS);
content::ResourceRequest request;
request.url = net::FilePathToFileURL(file_path);
simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper.GetCallback());
loader_factory(), simple_loader_helper.GetCallback());
simple_loader_helper.WaitForCallback();
ASSERT_TRUE(simple_loader->ResponseInfo());
......@@ -209,16 +212,18 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, FileURL) {
// Make sure a cache is used when expected.
IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, Cache) {
// Make a request whose response should be cached.
GURL request_url = embedded_test_server()->GetURL("/cachetime");
std::unique_ptr<content::ResourceRequest> request =
std::make_unique<content::ResourceRequest>();
request->url = request_url;
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create();
content::SimpleURLLoader::Create(std::move(request),
TRAFFIC_ANNOTATION_FOR_TESTS);
// Make a request whose response should be cached.
content::ResourceRequest request;
request.url = embedded_test_server()->GetURL("/cachetime");
simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper.GetCallback());
loader_factory(), simple_loader_helper.GetCallback());
simple_loader_helper.WaitForCallback();
ASSERT_TRUE(simple_loader_helper.response_body());
......@@ -229,12 +234,15 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, Cache) {
// Make the request again, and make sure it's cached or not, according to
// expectations. Reuse the content::ResourceRequest, but nothing else.
std::unique_ptr<content::ResourceRequest> request2 =
std::make_unique<content::ResourceRequest>();
request2->url = request_url;
content::SimpleURLLoaderTestHelper simple_loader_helper2;
std::unique_ptr<content::SimpleURLLoader> simple_loader2 =
content::SimpleURLLoader::Create();
content::SimpleURLLoader::Create(std::move(request2),
TRAFFIC_ANNOTATION_FOR_TESTS);
simple_loader2->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper2.GetCallback());
loader_factory(), simple_loader_helper2.GetCallback());
simple_loader_helper2.WaitForCallback();
if (GetHttpCacheType() == StorageType::kNone) {
// If there's no cache, and not server running, the request should have
......@@ -266,17 +274,17 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, PRE_DiskCache) {
base::WriteFile(save_url_file_path, test_url.spec().c_str(),
test_url.spec().length()));
// Make a request whose response should be cached.
std::unique_ptr<content::ResourceRequest> request =
std::make_unique<content::ResourceRequest>();
request->url = test_url;
request->headers.SetHeader("foo", "foopity foo");
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create();
// Make a request whose response should be cached.
content::ResourceRequest request;
request.url = test_url;
request.headers.SetHeader("foo", "foopity foo");
content::SimpleURLLoader::Create(std::move(request),
TRAFFIC_ANNOTATION_FOR_TESTS);
simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper.GetCallback());
loader_factory(), simple_loader_helper.GetCallback());
simple_loader_helper.WaitForCallback();
EXPECT_EQ(net::OK, simple_loader->NetError());
......@@ -342,18 +350,19 @@ class NetworkContextConfigurationFixedPortBrowserTest
// respected.
IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationFixedPortBrowserTest,
TestingFixedPort) {
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create();
content::ResourceRequest request;
std::unique_ptr<content::ResourceRequest> request =
std::make_unique<content::ResourceRequest>();
// This URL does not use the port the embedded test server is using. The
// command line switch should make it result in the request being directed to
// the test server anyways.
request.url = GURL("http://127.0.0.1/echo");
request->url = GURL("http://127.0.0.1/echo");
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create(std::move(request),
TRAFFIC_ANNOTATION_FOR_TESTS);
simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper.GetCallback());
loader_factory(), simple_loader_helper.GetCallback());
simple_loader_helper.WaitForCallback();
EXPECT_EQ(net::OK, simple_loader->NetError());
......
......@@ -77,14 +77,16 @@ IN_PROC_BROWSER_TEST_P(ProfileNetworkContextServiceBrowsertest,
DiskCacheLocation) {
// Run a request that caches the response, to give the network service time to
// create a cache directory.
std::unique_ptr<content::ResourceRequest> request =
std::make_unique<content::ResourceRequest>();
request->url = embedded_test_server()->GetURL("/cachetime");
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create();
content::ResourceRequest request;
request.url = embedded_test_server()->GetURL("/cachetime");
content::SimpleURLLoader::Create(std::move(request),
TRAFFIC_ANNOTATION_FOR_TESTS);
simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper.GetCallback());
loader_factory(), simple_loader_helper.GetCallback());
simple_loader_helper.WaitForCallback();
ASSERT_TRUE(simple_loader_helper.response_body());
......@@ -103,21 +105,21 @@ IN_PROC_BROWSER_TEST_P(ProfileNetworkContextServiceBrowsertest, BrotliEnabled) {
base::FilePath(FILE_PATH_LITERAL("content/test/data")));
ASSERT_TRUE(https_server.Start());
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create();
content::ResourceRequest request;
request.url = https_server.GetURL("/echoheader?accept-encoding");
std::unique_ptr<content::ResourceRequest> request =
std::make_unique<content::ResourceRequest>();
request->url = https_server.GetURL("/echoheader?accept-encoding");
// On OSX, test certs aren't currently hooked up correctly when using the
// network service.
// TODO(mmenke): Remove this line once that's fixed.
#if defined(OS_MACOSX)
request.load_flags |= net::LOAD_IGNORE_ALL_CERT_ERRORS;
request->load_flags |= net::LOAD_IGNORE_ALL_CERT_ERRORS;
#endif // !defined(OS_MACOSX)
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create(std::move(request),
TRAFFIC_ANNOTATION_FOR_TESTS);
simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper.GetCallback());
loader_factory(), simple_loader_helper.GetCallback());
simple_loader_helper.WaitForCallback();
ASSERT_TRUE(simple_loader_helper.response_body());
std::vector<std::string> encodings =
......@@ -158,14 +160,16 @@ IN_PROC_BROWSER_TEST_P(ProfileNetworkContextServiceDiskCacheDirBrowsertest,
// Run a request that caches the response, to give the network service time to
// create a cache directory.
std::unique_ptr<content::ResourceRequest> request =
std::make_unique<content::ResourceRequest>();
request->url = embedded_test_server()->GetURL("/cachetime");
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create();
content::ResourceRequest request;
request.url = embedded_test_server()->GetURL("/cachetime");
content::SimpleURLLoader::Create(std::move(request),
TRAFFIC_ANNOTATION_FOR_TESTS);
simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper.GetCallback());
loader_factory(), simple_loader_helper.GetCallback());
simple_loader_helper.WaitForCallback();
ASSERT_TRUE(simple_loader_helper.response_body());
......
......@@ -79,13 +79,15 @@ class BrowsingDataRemoverImplBrowserTest : public ContentBrowserTest {
// Issues a request for kHstsPath on localhost, and expects it to enable HSTS
// for the domain.
void IssueRequestThatSetsHsts() {
std::unique_ptr<ResourceRequest> request =
std::make_unique<ResourceRequest>();
request->url = ssl_server_.GetURL("localhost", kHstsPath);
SimpleURLLoaderTestHelper loader_helper;
std::unique_ptr<SimpleURLLoader> loader = SimpleURLLoader::Create();
ResourceRequest request;
request.url = ssl_server_.GetURL("localhost", kHstsPath);
std::unique_ptr<SimpleURLLoader> loader = SimpleURLLoader::Create(
std::move(request), TRAFFIC_ANNOTATION_FOR_TESTS);
loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, url_loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
loader_helper.GetCallback());
url_loader_factory(), loader_helper.GetCallback());
loader_helper.WaitForCallback();
ASSERT_TRUE(loader_helper.response_body());
EXPECT_EQ(kHstsResponseBody, *loader_helper.response_body());
......@@ -99,19 +101,19 @@ class BrowsingDataRemoverImplBrowserTest : public ContentBrowserTest {
// over HTTPS, so HSTS is enabled. If it fails, the request was send using
// HTTP instead, so HSTS is not enabled for the domain.
bool IsHstsSet() {
SimpleURLLoaderTestHelper loader_helper;
std::unique_ptr<SimpleURLLoader> loader = SimpleURLLoader::Create();
ResourceRequest request;
GURL url = ssl_server_.GetURL("localhost", "/echo");
GURL::Replacements replacements;
replacements.SetSchemeStr("http");
url = url.ReplaceComponents(replacements);
request.url = url;
std::unique_ptr<ResourceRequest> request =
std::make_unique<ResourceRequest>();
request->url = url;
std::unique_ptr<SimpleURLLoader> loader = SimpleURLLoader::Create(
std::move(request), TRAFFIC_ANNOTATION_FOR_TESTS);
SimpleURLLoaderTestHelper loader_helper;
loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, url_loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
loader_helper.GetCallback());
url_loader_factory(), loader_helper.GetCallback());
loader_helper.WaitForCallback();
// On success, HSTS was enabled for the domain.
......
......@@ -53,16 +53,16 @@ class NetworkServiceRestartBrowserTest : public ContentBrowserTest {
network_context->CreateURLLoaderFactory(MakeRequest(&url_loader_factory),
0);
std::unique_ptr<ResourceRequest> request =
std::make_unique<ResourceRequest>();
request->url = embedded_test_server()->GetURL("/echo");
content::SimpleURLLoaderTestHelper simple_loader_helper;
std::unique_ptr<content::SimpleURLLoader> simple_loader =
content::SimpleURLLoader::Create();
ResourceRequest request;
request.url = embedded_test_server()->GetURL("/echo");
content::SimpleURLLoader::Create(std::move(request),
TRAFFIC_ANNOTATION_FOR_TESTS);
simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
request, url_loader_factory.get(), TRAFFIC_ANNOTATION_FOR_TESTS,
simple_loader_helper.GetCallback());
url_loader_factory.get(), simple_loader_helper.GetCallback());
simple_loader_helper.WaitForCallback();
return simple_loader->NetError();
......
This diff is collapsed.
......@@ -81,13 +81,18 @@ class CONTENT_EXPORT SimpleURLLoader {
base::RepeatingCallback<void(const net::RedirectInfo& redirect_info,
const ResourceResponseHead& response_head)>;
static std::unique_ptr<SimpleURLLoader> Create();
// Creates a SimpleURLLoader for |resource_request|. The request can be
// started by calling any one of the Download methods once. The loader may not
// be reused.
static std::unique_ptr<SimpleURLLoader> Create(
std::unique_ptr<ResourceRequest> resource_request,
const net::NetworkTrafficAnnotationTag& annotation_tag);
virtual ~SimpleURLLoader();
// Starts a request for |resource_request| using |network_context|. The
// SimpleURLLoader will accumulate all downloaded data in an in-memory string
// of bounded size. If |max_body_size| is exceeded, the request will fail with
// Starts the request using |network_context|. The SimpleURLLoader will
// accumulate all downloaded data in an in-memory string of bounded size. If
// |max_body_size| is exceeded, the request will fail with
// net::ERR_INSUFFICIENT_RESOURCES. |max_body_size| must be no greater than 1
// MiB. For anything larger, it's recommended to either save to a temp file,
// or consume the data as it is received.
......@@ -97,9 +102,7 @@ class CONTENT_EXPORT SimpleURLLoader {
// SimpleURLLoader before the callback is invoked will return in cancelling
// the request, and the callback will not be called.
virtual void DownloadToString(
const ResourceRequest& resource_request,
mojom::URLLoaderFactory* url_loader_factory,
const net::NetworkTrafficAnnotationTag& annotation_tag,
BodyAsStringCallback body_as_string_callback,
size_t max_body_size) = 0;
......@@ -109,9 +112,7 @@ class CONTENT_EXPORT SimpleURLLoader {
// instead (DownloadToString if the body is expected to be of reasonable
// length, or DownloadToFile otherwise).
virtual void DownloadToStringOfUnboundedSizeUntilCrashAndDie(
const ResourceRequest& resource_request,
mojom::URLLoaderFactory* url_loader_factory,
const net::NetworkTrafficAnnotationTag& annotation_tag,
BodyAsStringCallback body_as_string_callback) = 0;
// SimpleURLLoader will download the entire response to a file at the
......@@ -128,9 +129,7 @@ class CONTENT_EXPORT SimpleURLLoader {
// downloaded file will be deleted asynchronously and the callback will not be
// invoked, regardless of other settings.
virtual void DownloadToFile(
const ResourceRequest& resource_request,
mojom::URLLoaderFactory* url_loader_factory,
const net::NetworkTrafficAnnotationTag& annotation_tag,
DownloadToFileCompleteCallback download_to_file_complete_callback,
const base::FilePath& file_path,
int64_t max_body_size = std::numeric_limits<int64_t>::max()) = 0;
......@@ -138,9 +137,7 @@ class CONTENT_EXPORT SimpleURLLoader {
// Same as DownloadToFile, but creates a temporary file instead of taking a
// FilePath.
virtual void DownloadToTempFile(
const ResourceRequest& resource_request,
mojom::URLLoaderFactory* url_loader_factory,
const net::NetworkTrafficAnnotationTag& annotation_tag,
DownloadToFileCompleteCallback download_to_file_complete_callback,
int64_t max_body_size = std::numeric_limits<int64_t>::max()) = 0;
......
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