Commit e7b0b560 authored by Sorin Jianu's avatar Sorin Jianu Committed by Commit Bot

Fix error handling in the request sender and url fetcher downloader.

That means handling the network errors by primarily looking at net_error.

Bug: 1028369
Change-Id: I8181bced25f8b56144ea336a03883d0dceea5108
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1935428Reviewed-by: default avatarJoshua Pawlicki <waffles@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719199}
parent 897faea0
......@@ -157,14 +157,12 @@ void RequestSender::OnNetworkFetcherComplete(
VLOG(1) << "request completed from url: " << original_url.spec();
int error = -1;
if (response_body && response_code_ == 200) {
DCHECK_EQ(0, net_error);
if (!net_error && response_code_ == 200)
error = 0;
} else if (response_code_ != -1) {
else if (response_code_ != -1)
error = response_code_;
} else {
else
error = net_error;
}
int retry_after_sec = -1;
if (original_url.SchemeIsCryptographic() && error > 0)
......
......@@ -75,22 +75,21 @@ void UrlFetcherDownloader::StartURLFetch(const GURL& url) {
return;
}
const auto file_path = download_dir_.AppendASCII(url.ExtractFileName());
file_path_ = download_dir_.AppendASCII(url.ExtractFileName());
network_fetcher_ = network_fetcher_factory_->Create();
network_fetcher_->DownloadToFile(
url, file_path,
url, file_path_,
base::BindOnce(&UrlFetcherDownloader::OnResponseStarted,
base::Unretained(this)),
base::BindRepeating(&UrlFetcherDownloader::OnDownloadProgress,
base::Unretained(this)),
base::BindOnce(&UrlFetcherDownloader::OnNetworkFetcherComplete,
base::Unretained(this), file_path));
base::Unretained(this)));
download_start_time_ = base::TimeTicks::Now();
}
void UrlFetcherDownloader::OnNetworkFetcherComplete(base::FilePath file_path,
int net_error,
void UrlFetcherDownloader::OnNetworkFetcherComplete(int net_error,
int64_t content_size) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
......@@ -104,22 +103,19 @@ void UrlFetcherDownloader::OnNetworkFetcherComplete(base::FilePath file_path,
// the request and avoid overloading the server in this case.
// is not accepting requests for the moment.
int error = -1;
if (!file_path.empty() && response_code_ == 200) {
DCHECK_EQ(0, net_error);
if (!net_error && response_code_ == 200)
error = 0;
} else if (response_code_ != -1) {
else if (response_code_ != -1)
error = response_code_;
} else {
else
error = net_error;
}
const bool is_handled = error == 0 || IsHttpServerError(error);
Result result;
result.error = error;
if (!error) {
result.response = file_path;
}
if (!error)
result.response = file_path_;
DownloadMetrics download_metrics;
download_metrics.url = url();
......
......@@ -35,9 +35,7 @@ class UrlFetcherDownloader : public CrxDownloader {
void CreateDownloadDir();
void StartURLFetch(const GURL& url);
void OnNetworkFetcherComplete(base::FilePath file_path,
int net_error,
int64_t content_size);
void OnNetworkFetcherComplete(int net_error, int64_t content_size);
void OnResponseStarted(int response_code, int64_t content_length);
void OnDownloadProgress(int64_t content_length);
......@@ -49,6 +47,9 @@ class UrlFetcherDownloader : public CrxDownloader {
// Contains a temporary download directory for the downloaded file.
base::FilePath download_dir_;
// Contains the file path to the downloaded file.
base::FilePath file_path_;
base::TimeTicks download_start_time_;
int response_code_ = -1;
......
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