Commit 0807dd6f authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

Migrate components/update_client/url_fetcher_downloader.cc

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

Note that UrlFetcherDownloader makes use of URLFetcher's "progress
update" hook, in order to track fetch progresses. It turns out the
progress values were not being used in production at all:

UrlFetcherDownloader::OnURLFetchDownloadProgress used to instantiate
a CrxDownloader::Result object, and pass it all the way to
Component::StateDownloadingDiff::DownloadComplete and
Component::StateDownloading::DownloadComplete. There, |result|
parameter was being ignored in both methods.

Given SimpleURLLoader's lack of ability to hook to a progress update
callback at the time of doing this migration, and that the progress values
were not being used in production code anyways, this CL replaces
URLFetcher's progress upload hook by SimpleURLLoader's "response started"
hook, to indicate that the load has been triggered, and update
Component's status accordingly.

Hence, references to |CrxDownloader::Result::downloaded_bytes| and
|CrxDownloader::Result::total_bytes| were removed from CrxDownloaderTest.*
(components/update_client/crx_downloader_unittest.cc).

On the other hand, references to CrxDownloader::DownloadMetrics::downloaded_bytes
and |CrxDownloader::DownloadMetrics::total_bytes| remain unchanged
(update_client::BuildDownloadCompleteEventElement uses them).

Bug:844972,871211

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I50410b46576263aec9a9a4b648a8a627f4832702
Reviewed-on: https://chromium-review.googlesource.com/1160725Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Reviewed-by: default avatarJoshua Pawlicki <waffles@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#582150}
parent 55ec60bc
...@@ -177,6 +177,8 @@ source_set("unit_tests") { ...@@ -177,6 +177,8 @@ source_set("unit_tests") {
"//components/version_info:version_info", "//components/version_info:version_info",
"//courgette:courgette_lib", "//courgette:courgette_lib",
"//net:test_support", "//net:test_support",
"//services/network:test_support",
"//services/network/public/cpp:cpp",
"//services/network/public/cpp:cpp_base", "//services/network/public/cpp:cpp_base",
"//services/service_manager/public/cpp", "//services/service_manager/public/cpp",
"//services/service_manager/public/cpp/test:test_support", "//services/service_manager/public/cpp/test:test_support",
......
...@@ -507,7 +507,7 @@ void Component::StateDownloadingDiff::DoHandle() { ...@@ -507,7 +507,7 @@ void Component::StateDownloadingDiff::DoHandle() {
crx_downloader_ = update_context.crx_downloader_factory( crx_downloader_ = update_context.crx_downloader_factory(
component.CanDoBackgroundDownload(), component.CanDoBackgroundDownload(),
update_context.config->RequestContext()); update_context.config->URLLoaderFactory());
const auto& id = component.id_; const auto& id = component.id_;
crx_downloader_->set_progress_callback( crx_downloader_->set_progress_callback(
...@@ -576,7 +576,7 @@ void Component::StateDownloading::DoHandle() { ...@@ -576,7 +576,7 @@ void Component::StateDownloading::DoHandle() {
crx_downloader_ = update_context.crx_downloader_factory( crx_downloader_ = update_context.crx_downloader_factory(
component.CanDoBackgroundDownload(), component.CanDoBackgroundDownload(),
update_context.config->RequestContext()); update_context.config->URLLoaderFactory());
const auto& id = component.id_; const auto& id = component.id_;
crx_downloader_->set_progress_callback( crx_downloader_->set_progress_callback(
......
...@@ -21,7 +21,7 @@ ...@@ -21,7 +21,7 @@
#include "components/update_client/update_client_errors.h" #include "components/update_client/update_client_errors.h"
#include "components/update_client/url_fetcher_downloader.h" #include "components/update_client/url_fetcher_downloader.h"
#include "components/update_client/utils.h" #include "components/update_client/utils.h"
#include "net/url_request/url_request_context_getter.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
namespace update_client { namespace update_client {
...@@ -41,9 +41,10 @@ CrxDownloader::DownloadMetrics::DownloadMetrics() ...@@ -41,9 +41,10 @@ CrxDownloader::DownloadMetrics::DownloadMetrics()
// which uses the BITS service. // which uses the BITS service.
std::unique_ptr<CrxDownloader> CrxDownloader::Create( std::unique_ptr<CrxDownloader> CrxDownloader::Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
std::unique_ptr<CrxDownloader> url_fetcher_downloader = std::unique_ptr<CrxDownloader> url_fetcher_downloader =
std::make_unique<UrlFetcherDownloader>(nullptr, context_getter); std::make_unique<UrlFetcherDownloader>(nullptr,
std::move(url_loader_factory));
#if defined(OS_WIN) #if defined(OS_WIN)
if (is_background_download) { if (is_background_download) {
......
...@@ -19,9 +19,8 @@ ...@@ -19,9 +19,8 @@
#include "base/threading/thread_checker.h" #include "base/threading/thread_checker.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace network {
namespace net { class SharedURLLoaderFactory;
class URLRequestContextGetter;
} }
namespace update_client { namespace update_client {
...@@ -65,6 +64,8 @@ class CrxDownloader { ...@@ -65,6 +64,8 @@ class CrxDownloader {
// Path of the downloaded file if the download was successful. // Path of the downloaded file if the download was successful.
base::FilePath response; base::FilePath response;
// TODO(crbug.com/871211): These values are not being used in production
// code. Clean it up.
// Number of bytes actually downloaded, not including the bytes downloaded // Number of bytes actually downloaded, not including the bytes downloaded
// as a result of falling back on urls. // as a result of falling back on urls.
int64_t downloaded_bytes; int64_t downloaded_bytes;
...@@ -88,7 +89,7 @@ class CrxDownloader { ...@@ -88,7 +89,7 @@ class CrxDownloader {
using Factory = std::unique_ptr<CrxDownloader> (*)( using Factory = std::unique_ptr<CrxDownloader> (*)(
bool, bool,
scoped_refptr<net::URLRequestContextGetter>); scoped_refptr<network::SharedURLLoaderFactory>);
// Factory method to create an instance of this class and build the // Factory method to create an instance of this class and build the
// chain of responsibility. |is_background_download| specifies that a // chain of responsibility. |is_background_download| specifies that a
...@@ -97,7 +98,7 @@ class CrxDownloader { ...@@ -97,7 +98,7 @@ class CrxDownloader {
// code such as file IO operations. // code such as file IO operations.
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter); scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
virtual ~CrxDownloader(); virtual ~CrxDownloader();
void set_progress_callback(const ProgressCallback& progress_callback); void set_progress_callback(const ProgressCallback& progress_callback);
......
...@@ -278,7 +278,7 @@ TEST_F(UpdateClientTest, OneCrxNoUpdate) { ...@@ -278,7 +278,7 @@ TEST_F(UpdateClientTest, OneCrxNoUpdate) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -443,7 +443,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { ...@@ -443,7 +443,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -646,7 +646,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateFirstServerIgnoresSecond) { ...@@ -646,7 +646,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateFirstServerIgnoresSecond) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -858,7 +858,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoCrxComponentData) { ...@@ -858,7 +858,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoCrxComponentData) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -1001,7 +1001,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoCrxComponentDataAtAll) { ...@@ -1001,7 +1001,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoCrxComponentDataAtAll) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -1191,7 +1191,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { ...@@ -1191,7 +1191,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -1486,7 +1486,7 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) { ...@@ -1486,7 +1486,7 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -1764,7 +1764,7 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { ...@@ -1764,7 +1764,7 @@ TEST_F(UpdateClientTest, OneCrxInstallError) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -2015,7 +2015,7 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { ...@@ -2015,7 +2015,7 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -2240,7 +2240,7 @@ TEST_F(UpdateClientTest, OneCrxNoUpdateQueuedCall) { ...@@ -2240,7 +2240,7 @@ TEST_F(UpdateClientTest, OneCrxNoUpdateQueuedCall) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -2388,7 +2388,7 @@ TEST_F(UpdateClientTest, OneCrxInstall) { ...@@ -2388,7 +2388,7 @@ TEST_F(UpdateClientTest, OneCrxInstall) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -2520,7 +2520,7 @@ TEST_F(UpdateClientTest, OneCrxInstallNoCrxComponentData) { ...@@ -2520,7 +2520,7 @@ TEST_F(UpdateClientTest, OneCrxInstallNoCrxComponentData) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -2641,7 +2641,7 @@ TEST_F(UpdateClientTest, ConcurrentInstallSameCRX) { ...@@ -2641,7 +2641,7 @@ TEST_F(UpdateClientTest, ConcurrentInstallSameCRX) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -2731,7 +2731,7 @@ TEST_F(UpdateClientTest, EmptyIdList) { ...@@ -2731,7 +2731,7 @@ TEST_F(UpdateClientTest, EmptyIdList) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -2792,7 +2792,7 @@ TEST_F(UpdateClientTest, SendUninstallPing) { ...@@ -2792,7 +2792,7 @@ TEST_F(UpdateClientTest, SendUninstallPing) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return nullptr; return nullptr;
} }
...@@ -2924,7 +2924,7 @@ TEST_F(UpdateClientTest, RetryAfter) { ...@@ -2924,7 +2924,7 @@ TEST_F(UpdateClientTest, RetryAfter) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -3167,7 +3167,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { ...@@ -3167,7 +3167,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -3338,7 +3338,7 @@ TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) { ...@@ -3338,7 +3338,7 @@ TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -3485,7 +3485,7 @@ TEST_F(UpdateClientTest, OneCrxErrorUnknownApp) { ...@@ -3485,7 +3485,7 @@ TEST_F(UpdateClientTest, OneCrxErrorUnknownApp) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -3669,7 +3669,7 @@ TEST_F(UpdateClientTest, ActionRun_Install) { ...@@ -3669,7 +3669,7 @@ TEST_F(UpdateClientTest, ActionRun_Install) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
...@@ -3816,7 +3816,7 @@ TEST_F(UpdateClientTest, ActionRun_NoUpdate) { ...@@ -3816,7 +3816,7 @@ TEST_F(UpdateClientTest, ActionRun_NoUpdate) {
public: public:
static std::unique_ptr<CrxDownloader> Create( static std::unique_ptr<CrxDownloader> Create(
bool is_background_download, bool is_background_download,
scoped_refptr<net::URLRequestContextGetter> context_getter) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return std::make_unique<MockCrxDownloader>(); return std::make_unique<MockCrxDownloader>();
} }
......
...@@ -18,8 +18,9 @@ ...@@ -18,8 +18,9 @@
#include "components/update_client/utils.h" #include "components/update_client/utils.h"
#include "net/base/load_flags.h" #include "net/base/load_flags.h"
#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/traffic_annotation/network_traffic_annotation.h"
#include "net/url_request/url_fetcher.h" #include "services/network/public/cpp/resource_response.h"
#include "net/url_request/url_request_context_getter.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace { namespace {
...@@ -32,32 +33,11 @@ constexpr base::TaskTraits kTaskTraits = { ...@@ -32,32 +33,11 @@ constexpr base::TaskTraits kTaskTraits = {
namespace update_client { namespace update_client {
UrlFetcherDownloader::URLFetcherDelegate::URLFetcherDelegate(
UrlFetcherDownloader* downloader)
: downloader_(downloader) {}
UrlFetcherDownloader::URLFetcherDelegate::~URLFetcherDelegate() = default;
void UrlFetcherDownloader::URLFetcherDelegate::OnURLFetchComplete(
const net::URLFetcher* source) {
downloader_->OnURLFetchComplete(source);
}
void UrlFetcherDownloader::URLFetcherDelegate::OnURLFetchDownloadProgress(
const net::URLFetcher* source,
int64_t current,
int64_t total,
int64_t current_network_bytes) {
downloader_->OnURLFetchDownloadProgress(source, current, total,
current_network_bytes);
}
UrlFetcherDownloader::UrlFetcherDownloader( UrlFetcherDownloader::UrlFetcherDownloader(
std::unique_ptr<CrxDownloader> successor, std::unique_ptr<CrxDownloader> successor,
scoped_refptr<net::URLRequestContextGetter> context_getter) scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
: CrxDownloader(std::move(successor)), : CrxDownloader(std::move(successor)),
delegate_(std::make_unique<URLFetcherDelegate>(this)), url_loader_factory_(std::move(url_loader_factory)) {}
context_getter_(context_getter) {}
UrlFetcherDownloader::~UrlFetcherDownloader() { UrlFetcherDownloader::~UrlFetcherDownloader() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
...@@ -114,15 +94,13 @@ void UrlFetcherDownloader::StartURLFetch(const GURL& url) { ...@@ -114,15 +94,13 @@ void UrlFetcherDownloader::StartURLFetch(const GURL& url) {
if (download_dir_.empty()) { if (download_dir_.empty()) {
Result result; Result result;
result.error = -1; result.error = -1;
result.downloaded_bytes = downloaded_bytes_;
result.total_bytes = total_bytes_;
DownloadMetrics download_metrics; DownloadMetrics download_metrics;
download_metrics.url = url; download_metrics.url = url;
download_metrics.downloader = DownloadMetrics::kUrlFetcher; download_metrics.downloader = DownloadMetrics::kUrlFetcher;
download_metrics.error = -1; download_metrics.error = -1;
download_metrics.downloaded_bytes = downloaded_bytes_; download_metrics.downloaded_bytes = -1;
download_metrics.total_bytes = total_bytes_; download_metrics.total_bytes = -1;
download_metrics.download_time_ms = 0; download_metrics.download_time_ms = 0;
main_task_runner()->PostTask( main_task_runner()->PostTask(
...@@ -134,27 +112,37 @@ void UrlFetcherDownloader::StartURLFetch(const GURL& url) { ...@@ -134,27 +112,37 @@ void UrlFetcherDownloader::StartURLFetch(const GURL& url) {
const base::FilePath response = const base::FilePath response =
download_dir_.AppendASCII(url.ExtractFileName()); download_dir_.AppendASCII(url.ExtractFileName());
auto resource_request = std::make_unique<network::ResourceRequest>();
url_fetcher_ = net::URLFetcher::Create(0, url, net::URLFetcher::GET, resource_request->url = url;
delegate_.get(), traffic_annotation); resource_request->load_flags = net::LOAD_DO_NOT_SEND_COOKIES |
url_fetcher_->SetRequestContext(context_getter_.get()); net::LOAD_DO_NOT_SAVE_COOKIES |
url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DISABLE_CACHE;
net::LOAD_DO_NOT_SAVE_COOKIES | url_loader_ = network::SimpleURLLoader::Create(std::move(resource_request),
net::LOAD_DISABLE_CACHE); traffic_annotation);
url_fetcher_->SetAutomaticallyRetryOn5xx(false); const int kMaxRetries = 3;
url_fetcher_->SetAutomaticallyRetryOnNetworkChanges(3); url_loader_->SetRetryOptions(
url_fetcher_->SaveResponseToFileAtPath( kMaxRetries,
response, base::CreateSequencedTaskRunnerWithTraits(kTaskTraits)); network::SimpleURLLoader::RetryMode::RETRY_ON_NETWORK_CHANGE);
data_use_measurement::DataUseUserData::AttachToFetcher(
url_fetcher_.get(), data_use_measurement::DataUseUserData::UPDATE_CLIENT); url_loader_->SetOnResponseStartedCallback(base::BindOnce(
&UrlFetcherDownloader::OnResponseStarted, base::Unretained(this)));
// For the end-to-end system it is important that the client reports the
// number of bytes it loaded from the server even in the case that the
// overall network transaction failed.
url_loader_->SetAllowPartialResults(true);
VLOG(1) << "Starting background download: " << url.spec(); VLOG(1) << "Starting background download: " << url.spec();
url_fetcher_->Start(); url_loader_->DownloadToFile(
url_loader_factory_.get(),
base::BindOnce(&UrlFetcherDownloader::OnURLLoadComplete,
base::Unretained(this)),
response);
download_start_time_ = base::TimeTicks::Now(); download_start_time_ = base::TimeTicks::Now();
} }
void UrlFetcherDownloader::OnURLFetchComplete(const net::URLFetcher* source) { void UrlFetcherDownloader::OnURLLoadComplete(base::FilePath file_path) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
const base::TimeTicks download_end_time(base::TimeTicks::Now()); const base::TimeTicks download_end_time(base::TimeTicks::Now());
...@@ -166,28 +154,44 @@ void UrlFetcherDownloader::OnURLFetchComplete(const net::URLFetcher* source) { ...@@ -166,28 +154,44 @@ void UrlFetcherDownloader::OnURLFetchComplete(const net::URLFetcher* source) {
// Consider a 5xx response from the server as an indication to terminate // Consider a 5xx response from the server as an indication to terminate
// the request and avoid overloading the server in this case. // the request and avoid overloading the server in this case.
// is not accepting requests for the moment. // is not accepting requests for the moment.
const int fetch_error(GetFetchError(*url_fetcher_)); int response_code = -1;
if (url_loader_->ResponseInfo() && url_loader_->ResponseInfo()->headers) {
response_code = url_loader_->ResponseInfo()->headers->response_code();
}
int fetch_error = -1;
if (!file_path.empty() && response_code == 200) {
fetch_error = 0;
} else if (response_code != -1) {
fetch_error = response_code;
} else {
fetch_error = url_loader_->NetError();
}
const bool is_handled = fetch_error == 0 || IsHttpServerError(fetch_error); const bool is_handled = fetch_error == 0 || IsHttpServerError(fetch_error);
Result result; Result result;
result.error = fetch_error; result.error = fetch_error;
if (!fetch_error) { if (!fetch_error) {
source->GetResponseAsFilePath(true, &result.response); result.response = file_path;
} }
result.downloaded_bytes = downloaded_bytes_;
result.total_bytes = total_bytes_;
DownloadMetrics download_metrics; DownloadMetrics download_metrics;
download_metrics.url = url(); download_metrics.url = url();
download_metrics.downloader = DownloadMetrics::kUrlFetcher; download_metrics.downloader = DownloadMetrics::kUrlFetcher;
download_metrics.error = fetch_error; download_metrics.error = fetch_error;
download_metrics.downloaded_bytes = downloaded_bytes_; // Tests expected -1, in case of failures and no content is available.
download_metrics.downloaded_bytes =
fetch_error && !url_loader_->GetContentSize()
? -1
: url_loader_->GetContentSize();
download_metrics.total_bytes = total_bytes_; download_metrics.total_bytes = total_bytes_;
download_metrics.download_time_ms = download_time.InMilliseconds(); download_metrics.download_time_ms = download_time.InMilliseconds();
VLOG(1) << "Downloaded " << downloaded_bytes_ << " bytes in " VLOG(1) << "Downloaded " << url_loader_->GetContentSize() << " bytes in "
<< download_time.InMilliseconds() << "ms from " << download_time.InMilliseconds() << "ms from "
<< source->GetURL().spec() << " to " << result.response.value(); << url_loader_->GetFinalURL().spec() << " to "
<< result.response.value();
// Delete the download directory in the error cases. // Delete the download directory in the error cases.
if (fetch_error && !download_dir_.empty()) if (fetch_error && !download_dir_.empty())
...@@ -201,21 +205,18 @@ void UrlFetcherDownloader::OnURLFetchComplete(const net::URLFetcher* source) { ...@@ -201,21 +205,18 @@ void UrlFetcherDownloader::OnURLFetchComplete(const net::URLFetcher* source) {
download_metrics)); download_metrics));
} }
void UrlFetcherDownloader::OnURLFetchDownloadProgress( // This callback is used to indicate that a download has been started.
const net::URLFetcher* source, void UrlFetcherDownloader::OnResponseStarted(
int64_t current, const GURL& final_url,
int64_t total, const network::ResourceResponseHead& response_head) {
int64_t current_network_bytes) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
downloaded_bytes_ = current; if (response_head.content_length != -1)
total_bytes_ = total; total_bytes_ = response_head.content_length;
Result result;
result.downloaded_bytes = downloaded_bytes_;
result.total_bytes = total_bytes_;
OnDownloadProgress(result); // TODO(crbug.com/871211): |Result| is not being used on production.
// Clean it up.
OnDownloadProgress(Result());
} }
} // namespace update_client } // namespace update_client
...@@ -15,65 +15,42 @@ ...@@ -15,65 +15,42 @@
#include "base/threading/thread_checker.h" #include "base/threading/thread_checker.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "components/update_client/crx_downloader.h" #include "components/update_client/crx_downloader.h"
#include "net/url_request/url_fetcher_delegate.h"
namespace net { namespace network {
class URLFetcher; class SharedURLLoaderFactory;
class URLRequestContextGetter; class SimpleURLLoader;
} struct ResourceResponseHead;
} // namespace network
namespace update_client { namespace update_client {
// Implements a CRX downloader in top of the URLFetcher. // Implements a CRX downloader in top of the SimpleURLLoader.
class UrlFetcherDownloader : public CrxDownloader { class UrlFetcherDownloader : public CrxDownloader {
public: public:
UrlFetcherDownloader( UrlFetcherDownloader(
std::unique_ptr<CrxDownloader> successor, std::unique_ptr<CrxDownloader> successor,
scoped_refptr<net::URLRequestContextGetter> context_getter); scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
~UrlFetcherDownloader() override; ~UrlFetcherDownloader() override;
private: private:
class URLFetcherDelegate : public net::URLFetcherDelegate {
public:
explicit URLFetcherDelegate(UrlFetcherDownloader* downloader);
~URLFetcherDelegate() override;
private:
// Overrides for URLFetcherDelegate.
void OnURLFetchComplete(const net::URLFetcher* source) override;
void OnURLFetchDownloadProgress(const net::URLFetcher* source,
int64_t current,
int64_t total,
int64_t current_network_bytes) override;
// Not owned by this class.
UrlFetcherDownloader* downloader_ = nullptr;
DISALLOW_COPY_AND_ASSIGN(URLFetcherDelegate);
};
// Overrides for CrxDownloader. // Overrides for CrxDownloader.
void DoStartDownload(const GURL& url) override; void DoStartDownload(const GURL& url) override;
void CreateDownloadDir(); void CreateDownloadDir();
void StartURLFetch(const GURL& url); void StartURLFetch(const GURL& url);
void OnURLFetchComplete(const net::URLFetcher* source); void OnURLLoadComplete(base::FilePath file_path);
void OnURLFetchDownloadProgress(const net::URLFetcher* source, void OnResponseStarted(const GURL& final_url,
int64_t current, const network::ResourceResponseHead& response_head);
int64_t total,
int64_t current_network_bytes);
THREAD_CHECKER(thread_checker_); THREAD_CHECKER(thread_checker_);
std::unique_ptr<URLFetcherDelegate> delegate_; scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
std::unique_ptr<network::SimpleURLLoader> url_loader_;
std::unique_ptr<net::URLFetcher> url_fetcher_;
scoped_refptr<net::URLRequestContextGetter> context_getter_;
// Contains a temporary download directory for the downloaded file. // Contains a temporary download directory for the downloaded file.
base::FilePath download_dir_; base::FilePath download_dir_;
base::TimeTicks download_start_time_; base::TimeTicks download_start_time_;
int64_t downloaded_bytes_ = -1;
int64_t total_bytes_ = -1; int64_t total_bytes_ = -1;
DISALLOW_COPY_AND_ASSIGN(UrlFetcherDownloader); DISALLOW_COPY_AND_ASSIGN(UrlFetcherDownloader);
......
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