Add a flag to allow parallel downloading without accept-ranges header in response.
If a download response doesn't have accept-ranges header, we don't fork
parallel requests today. A majority server doesn't send accept-ranges
header even though they support it.
This CL adds a flag to enable parallel downloading even accept-ranges
header are not present.
BUG=971366
Change-Id: I11fc2e7c131a7407d7ee5f269ee5e6b3ff2ce789
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1648797
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#668062}
diff --git a/components/download/internal/common/download_job_factory.cc b/components/download/internal/common/download_job_factory.cc
index 7d62075..87da385 100644
--- a/components/download/internal/common/download_job_factory.cc
+++ b/components/download/internal/common/download_job_factory.cc
@@ -10,6 +10,7 @@
#include "components/download/internal/common/parallel_download_job.h"
#include "components/download/internal/common/parallel_download_utils.h"
#include "components/download/internal/common/save_package_download_job.h"
+#include "components/download/public/common/download_features.h"
#include "components/download/public/common/download_item.h"
#include "components/download/public/common/download_stats.h"
#include "components/download/public/common/download_url_loader_factory_getter.h"
@@ -47,11 +48,15 @@
create_info.method == "GET" && create_info.url().SchemeIsHTTPOrHTTPS();
bool partial_response_success =
download_item->GetReceivedSlices().empty() || create_info.offset != 0;
- bool is_parallelizable =
- has_strong_validator &&
- create_info.accept_range == RangeRequestSupportType::kSupport &&
- has_content_length && satisfy_min_file_size && satisfy_connection_type &&
- http_get_method && partial_response_success;
+ bool range_support_allowed =
+ create_info.accept_range == RangeRequestSupportType::kSupport ||
+ (base::FeatureList::IsEnabled(
+ features::kUseParallelRequestsForUnknwonRangeSupport) &&
+ create_info.accept_range == RangeRequestSupportType::kUnknown);
+ bool is_parallelizable = has_strong_validator && range_support_allowed &&
+ has_content_length && satisfy_min_file_size &&
+ satisfy_connection_type && http_get_method &&
+ partial_response_success;
if (!IsParallelDownloadEnabled())
return is_parallelizable;
@@ -65,9 +70,13 @@
RecordParallelDownloadCreationEvent(
ParallelDownloadCreationEvent::FALLBACK_REASON_STRONG_VALIDATORS);
}
- if (create_info.accept_range != RangeRequestSupportType::kSupport) {
+ if (!range_support_allowed) {
RecordParallelDownloadCreationEvent(
ParallelDownloadCreationEvent::FALLBACK_REASON_ACCEPT_RANGE_HEADER);
+ if (create_info.accept_range == RangeRequestSupportType::kUnknown) {
+ RecordParallelDownloadCreationEvent(
+ ParallelDownloadCreationEvent::FALLBACK_REASON_UNKNOWN_RANGE_SUPPORT);
+ }
}
if (!has_content_length) {
RecordParallelDownloadCreationEvent(
diff --git a/components/download/internal/common/download_stats.cc b/components/download/internal/common/download_stats.cc
index 1a91d9e..1fe8b19 100644
--- a/components/download/internal/common/download_stats.cc
+++ b/components/download/internal/common/download_stats.cc
@@ -1054,8 +1054,16 @@
request_count, 1, 10, 11);
}
-void RecordParallelDownloadAddStreamSuccess(bool success) {
- UMA_HISTOGRAM_BOOLEAN("Download.ParallelDownloadAddStreamSuccess", success);
+void RecordParallelDownloadAddStreamSuccess(bool success,
+ bool support_range_request) {
+ if (support_range_request) {
+ base::UmaHistogramBoolean("Download.ParallelDownloadAddStreamSuccess",
+ success);
+ } else {
+ base::UmaHistogramBoolean(
+ "Download.ParallelDownloadAddStreamSuccess.NoAcceptRangesHeader",
+ success);
+ }
}
void RecordParallelizableContentLength(int64_t content_length) {
diff --git a/components/download/internal/common/parallel_download_job.cc b/components/download/internal/common/parallel_download_job.cc
index 1ac7dbbb..763116b 100644
--- a/components/download/internal/common/parallel_download_job.cc
+++ b/components/download/internal/common/parallel_download_job.cc
@@ -35,6 +35,7 @@
content_length_(create_info.total_bytes),
requests_sent_(false),
is_canceled_(false),
+ range_support_(create_info.accept_range),
url_loader_factory_getter_(std::move(url_loader_factory_getter)),
url_request_context_getter_(url_request_context_getter),
connector_(connector) {}
@@ -134,7 +135,9 @@
success = DownloadJob::AddInputStream(std::move(input_stream),
worker->offset(), worker->length());
}
- RecordParallelDownloadAddStreamSuccess(success);
+
+ RecordParallelDownloadAddStreamSuccess(
+ success, range_support_ == RangeRequestSupportType::kSupport);
// Destroy the request if the sink is gone.
if (!success) {
diff --git a/components/download/internal/common/parallel_download_job.h b/components/download/internal/common/parallel_download_job.h
index 71c45c86..ca41083 100644
--- a/components/download/internal/common/parallel_download_job.h
+++ b/components/download/internal/common/parallel_download_job.h
@@ -14,6 +14,7 @@
#include "base/timer/timer.h"
#include "components/download/internal/common/download_job_impl.h"
#include "components/download/internal/common/download_worker.h"
+#include "components/download/public/common/download_create_info.h"
#include "components/download/public/common/download_export.h"
#include "components/download/public/common/parallel_download_configs.h"
@@ -119,6 +120,9 @@
// If the download progress is canceled.
bool is_canceled_;
+ // Whether the server accepts range requests.
+ RangeRequestSupportType range_support_;
+
// URLLoaderFactory getter to issue network requests with network service
scoped_refptr<download::DownloadURLLoaderFactoryGetter>
url_loader_factory_getter_;
diff --git a/components/download/public/common/download_features.cc b/components/download/public/common/download_features.cc
index 7e945dd..4692f756 100644
--- a/components/download/public/common/download_features.cc
+++ b/components/download/public/common/download_features.cc
@@ -48,6 +48,11 @@
const base::Feature kAllowDownloadResumptionWithoutStrongValidators{
"AllowDownloadResumptionWithoutStrongValidators",
base::FEATURE_DISABLED_BY_DEFAULT};
+
+const base::Feature kUseParallelRequestsForUnknwonRangeSupport{
+ "UseParallelRequestForUnknwonRangeSupport",
+ base::FEATURE_DISABLED_BY_DEFAULT};
+
} // namespace features
} // namespace download
diff --git a/components/download/public/common/download_features.h b/components/download/public/common/download_features.h
index a3d7fda7..5dcfc4f 100644
--- a/components/download/public/common/download_features.h
+++ b/components/download/public/common/download_features.h
@@ -45,7 +45,12 @@
// Whether download resumption is allowed when there are no strong validators.
COMPONENTS_DOWNLOAD_EXPORT extern const base::Feature
kAllowDownloadResumptionWithoutStrongValidators;
+
+// Whether download resumption is allowed when there are no strong validators.
+COMPONENTS_DOWNLOAD_EXPORT extern const base::Feature
+ kUseParallelRequestsForUnknwonRangeSupport;
} // namespace features
+
} // namespace download
#endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_FEATURES_H_
diff --git a/components/download/public/common/download_stats.h b/components/download/public/common/download_stats.h
index b1b3947412..447e8fd 100644
--- a/components/download/public/common/download_stats.h
+++ b/components/download/public/common/download_stats.h
@@ -216,6 +216,9 @@
// The http method or url scheme does not meet the requirement.
FALLBACK_REASON_HTTP_METHOD,
+ // Range support is unknown from the response.
+ FALLBACK_REASON_UNKNOWN_RANGE_SUPPORT,
+
// Last entry of the enum.
COUNT,
};
@@ -323,8 +326,11 @@
int request_count);
// Records if each byte stream is successfully added to download sink.
+// |support_range_request| indicates whether the server strongly supports range
+// requests.
COMPONENTS_DOWNLOAD_EXPORT void RecordParallelDownloadAddStreamSuccess(
- bool success);
+ bool success,
+ bool support_range_request);
// Records the bandwidth for parallelizable download and estimates the saved
// time at the file end. Does not count in any hash computation or file
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index c7d93e5..acaf08b 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -44358,6 +44358,7 @@
label="Remaining download time does not meet the requirement."/>
<int value="8"
label="HTTP method or url scheme does not meet the requirement."/>
+ <int value="9" label="Unknown range support from the response header."/>
</enum>
<enum name="ParentFrameKnown">
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index d476eeb..a15089e 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -155523,6 +155523,12 @@
<affected-histogram name="Tabs.StateTransfer.Time_Inactive"/>
</histogram_suffixes>
+<histogram_suffixes name="NoAcceptRangesHeader" separator=".">
+ <suffix name="NoAcceptRangesHeader"
+ label="For parallel requests created without accept-ranges header."/>
+ <affected-histogram name="Download.ParallelDownloadAddStreamSuccess"/>
+</histogram_suffixes>
+
<histogram_suffixes name="NotificationDisplayExperiment" separator="_">
<obsolete>
Deprecated October 2017 (feature enabled by default).
