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

[ios] Migrate URLDownloader to SimpleURLLoader

URLFetcher will stop working with advent of Network Service, and
SimpleURLLoader is the replacement API for most clients.
This CL migrates URLDownloader and the respective unittests away
from URLFetcher.

BUG=773295

Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I8abfd9c9d9e66757c555479fe6a153e3dc08b9c5
Reviewed-on: https://chromium-review.googlesource.com/1234635Reviewed-by: default avatarOlivier Robin <olivierrobin@chromium.org>
Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#592808}
parent b365ac65
......@@ -18,6 +18,7 @@
#include "components/reading_list/core/reading_list_entry.h"
#include "components/reading_list/core/reading_list_model.h"
#include "ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
namespace {
// Status of the download when it ends, for UMA report.
......@@ -61,7 +62,7 @@ ReadingListDownloadService::ReadingListDownloadService(
ReadingListModel* reading_list_model,
PrefService* prefs,
base::FilePath chrome_profile_path,
net::URLRequestContextGetter* url_request_context_getter,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
std::unique_ptr<dom_distiller::DistillerFactory> distiller_factory,
std::unique_ptr<reading_list::ReadingListDistillerPageFactory>
distiller_page_factory)
......@@ -75,7 +76,7 @@ ReadingListDownloadService::ReadingListDownloadService(
url_downloader_ = std::make_unique<URLDownloader>(
distiller_factory_.get(), distiller_page_factory_.get(), prefs,
chrome_profile_path, url_request_context_getter,
chrome_profile_path, url_loader_factory,
base::Bind(&ReadingListDownloadService::OnDownloadEnd,
base::Unretained(this)),
base::Bind(&ReadingListDownloadService::OnDeleteEnd,
......
......@@ -35,7 +35,7 @@ class ReadingListDownloadService
ReadingListModel* reading_list_model,
PrefService* prefs,
base::FilePath chrome_profile_path,
net::URLRequestContextGetter* url_request_context_getter,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
std::unique_ptr<dom_distiller::DistillerFactory> distiller_factory,
std::unique_ptr<reading_list::ReadingListDistillerPageFactory>
distiller_page_factory);
......
......@@ -66,8 +66,8 @@ ReadingListDownloadServiceFactory::BuildServiceInstanceFor(
return std::make_unique<ReadingListDownloadService>(
ReadingListModelFactory::GetForBrowserState(chrome_browser_state),
chrome_browser_state->GetPrefs(), chrome_browser_state->GetStatePath(),
chrome_browser_state->GetRequestContext(), std::move(distiller_factory),
std::move(distiller_page_factory));
chrome_browser_state->GetSharedURLLoaderFactory(),
std::move(distiller_factory), std::move(distiller_page_factory));
}
web::BrowserState* ReadingListDownloadServiceFactory::GetBrowserStateToUse(
......
......@@ -21,9 +21,8 @@
#include "net/base/escape.h"
#include "net/base/load_flags.h"
#include "net/http/http_response_headers.h"
#include "net/url_request/url_fetcher.h"
#include "net/url_request/url_request_context_getter.h"
#include "net/url_request/url_request_status.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "url/gurl.h"
namespace {
......@@ -47,7 +46,7 @@ URLDownloader::URLDownloader(
reading_list::ReadingListDistillerPageFactory* distiller_page_factory,
PrefService* prefs,
base::FilePath chrome_profile_path,
net::URLRequestContextGetter* url_request_context_getter,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const DownloadCompletion& download_completion,
const SuccessCompletion& delete_completion)
: distiller_page_factory_(distiller_page_factory),
......@@ -58,7 +57,7 @@ URLDownloader::URLDownloader(
working_(false),
base_directory_(chrome_profile_path),
mime_type_(),
url_request_context_getter_(url_request_context_getter),
url_loader_factory_(std::move(url_loader_factory)),
task_runner_(base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})),
......@@ -195,30 +194,28 @@ void URLDownloader::DistilledPageHasMimeType(const GURL& original_url,
mime_type_ = mime_type;
}
void URLDownloader::OnURLFetchComplete(const net::URLFetcher* source) {
DCHECK(source == fetcher_.get());
void URLDownloader::OnURLLoadComplete(const GURL& original_url,
base::FilePath response_path) {
// At the moment, only pdf files are downloaded using URLFetcher.
DCHECK(mime_type_ == "application/pdf");
base::FilePath path = reading_list::OfflinePagePath(
original_url_, reading_list::OFFLINE_TYPE_PDF);
std::string mime_type;
if (fetcher_->GetResponseHeaders()) {
fetcher_->GetResponseHeaders()->GetMimeType(&mime_type);
if (url_loader_->ResponseInfo()) {
mime_type = url_loader_->ResponseInfo()->mime_type;
}
if (!fetcher_->GetStatus().is_success() || mime_type != mime_type_) {
if (response_path.empty() || mime_type != mime_type_) {
return DownloadCompletionHandler(original_url_, "", path, ERROR);
}
base::FilePath temporary_path;
// Do not take ownership of the file until the file is moved. This ensures
// that the file is cleaned if there a problem before file is moved.
fetcher_->GetResponseAsFilePath(false, &temporary_path);
task_tracker_.PostTaskAndReplyWithResult(
task_runner_.get(), FROM_HERE,
base::Bind(&URLDownloader::SavePDFFile, base::Unretained(this),
temporary_path),
response_path),
base::Bind(&URLDownloader::DownloadCompletionHandler,
base::Unretained(this), source->GetOriginalURL(), "", path));
base::Unretained(this), original_url, "", path));
url_loader_.reset();
}
void URLDownloader::CancelTask() {
......@@ -229,11 +226,16 @@ void URLDownloader::CancelTask() {
void URLDownloader::FetchPDFFile() {
const GURL& pdf_url =
distilled_url_.is_valid() ? distilled_url_ : original_url_;
fetcher_ = net::URLFetcher::Create(0, pdf_url, net::URLFetcher::GET, this);
fetcher_->SetRequestContext(url_request_context_getter_.get());
fetcher_->SetLoadFlags(net::LOAD_SKIP_CACHE_VALIDATION);
fetcher_->SaveResponseToTemporaryFile(task_runner_.get());
fetcher_->Start();
auto resource_request = std::make_unique<network::ResourceRequest>();
resource_request->url = pdf_url;
resource_request->load_flags = net::LOAD_SKIP_CACHE_VALIDATION;
url_loader_ = network::SimpleURLLoader::Create(std::move(resource_request),
NO_TRAFFIC_ANNOTATION_YET);
url_loader_->DownloadToTempFile(
url_loader_factory_.get(),
base::BindOnce(&URLDownloader::OnURLLoadComplete, base::Unretained(this),
pdf_url));
}
URLDownloader::SuccessState URLDownloader::SavePDFFile(
......
......@@ -11,7 +11,6 @@
#include "base/task/cancelable_task_tracker.h"
#include "ios/chrome/browser/dom_distiller/distiller_viewer.h"
#include "ios/chrome/browser/reading_list/reading_list_distiller_page.h"
#include "net/url_request/url_fetcher_delegate.h"
class PrefService;
class GURL;
......@@ -20,9 +19,9 @@ class FilePath;
class SequencedTaskRunner;
}
namespace net {
class URLFetcher;
class URLRequestContextGetter;
namespace network {
class SharedURLLoaderFactory;
class SimpleURLLoader;
}
namespace reading_list {
......@@ -39,8 +38,7 @@ class ReadingListDistillerPageFactory;
// folders within an offline folder, using md5 hashing to create unique file
// names. When a deletion is requested, all previous downloads for that URL are
// cancelled as they would be deleted.
class URLDownloader : public net::URLFetcherDelegate,
reading_list::ReadingListDistillerPageDelegate {
class URLDownloader : reading_list::ReadingListDistillerPageDelegate {
friend class MockURLDownloader;
public:
......@@ -82,7 +80,7 @@ class URLDownloader : public net::URLFetcherDelegate,
reading_list::ReadingListDistillerPageFactory* distiller_page_factory,
PrefService* prefs,
base::FilePath chrome_profile_path,
net::URLRequestContextGetter* url_request_context_getter,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const DownloadCompletion& download_completion,
const SuccessCompletion& delete_completion);
~URLDownloader() override;
......@@ -96,8 +94,9 @@ class URLDownloader : public net::URLFetcherDelegate,
// Asynchronously remove the offline version of the URL if it exists.
void RemoveOfflineURL(const GURL& url);
// URLFetcherDelegate delegate method.
void OnURLFetchComplete(const net::URLFetcher* source) override;
// URL loader completion callback.
void OnURLLoadComplete(const GURL& original_url,
base::FilePath response_path);
// Cancels the current download task.
void CancelTask();
......@@ -173,7 +172,8 @@ class URLDownloader : public net::URLFetcherDelegate,
// Starts fetching the PDF file. If |original_url_| triggered a redirection,
// directly save |distilled_url_|.
virtual void FetchPDFFile();
// Saves the file downloaded by |fetcher_|. Creates the directory if needed.
// Saves the file downloaded by |url_loader_|. Creates the directory if
// needed.
SuccessState SavePDFFile(const base::FilePath& temporary_path);
reading_list::ReadingListDistillerPageFactory* distiller_page_factory_;
......@@ -189,10 +189,10 @@ class URLDownloader : public net::URLFetcherDelegate,
GURL distilled_url_;
int64_t saved_size_;
std::string mime_type_;
// Fetcher used to redownload the document and save it in the sandbox.
std::unique_ptr<net::URLFetcher> fetcher_;
// URLRequestContextGetter needed for the URLFetcher.
scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
// URL loader used to redownload the document and save it in the sandbox.
std::unique_ptr<network::SimpleURLLoader> url_loader_;
// URLLoaderFactory needed for the URLLoader.
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
std::unique_ptr<dom_distiller::DistillerViewerInterface> distiller_;
scoped_refptr<base::SequencedTaskRunner> task_runner_;
base::CancelableTaskTracker task_tracker_;
......
......@@ -16,6 +16,7 @@
#include "ios/chrome/browser/dom_distiller/distiller_viewer.h"
#include "ios/chrome/browser/reading_list/offline_url_utils.h"
#include "ios/chrome/browser/reading_list/reading_list_distiller_page.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/platform_test.h"
......
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