Parallel download: Add a Finch parameter for future experiment.

Currently we have an issue that the Control group always read the
default configuration of parallel download defined in the code.

However, when tweaking the server config, we can use different slice
size, which will result in experiment groups comparing different sample
sets.

This CL adds a Finch parameter to alternatively disable parallel
download in an enable experiment group, and also read configs from
the server.


Bug: 794718
Change-Id: I9cf91233cbaf3dd22917af2e026c736803bd8e19
Reviewed-on: https://chromium-review.googlesource.com/825984
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524109}

content/browser/download/parallel_download_utils.cc

@@ -164,7 +164,13 @@ int64_t GetMaxContiguousDataBlockSizeFromBeginning(
 }
 
 bool IsParallelDownloadEnabled() {
-  return base::FeatureList::IsEnabled(features::kParallelDownloading);
+  bool feature_enabled =
+      base::FeatureList::IsEnabled(features::kParallelDownloading);
+  // Disabled when |kEnableParallelDownloadFinchKey| Finch config is set to
+  // false.
+  bool enabled_parameter = GetFieldTrialParamByFeatureAsBool(
+      features::kParallelDownloading, kEnableParallelDownloadFinchKey, true);
+  return feature_enabled && enabled_parameter;
 }
 
 }  // namespace content

content/browser/download/parallel_download_utils.h

@@ -13,6 +13,11 @@
 
 namespace content {
 
+// Finch parameter key value to enable parallel download. Used in enabled
+// experiment group that needs other parameters, such as min_slice_size, but
+// don't want to actually do parallel download.
+constexpr char kEnableParallelDownloadFinchKey[] = "enable_parallel_download";
+
 // Finch parameter key value for minimum slice size in bytes to use parallel
 // download.
 constexpr char kMinSliceSizeFinchKey[] = "min_slice_size";

content/browser/download/parallel_download_utils_unittest.cc

@@ -4,7 +4,12 @@
 
 #include "content/browser/download/parallel_download_utils.h"
 
+#include <map>
+
+#include "base/strings/string_number_conversions.h"
+#include "base/test/scoped_feature_list.h"
 #include "content/public/browser/download_save_info.h"
+#include "content/public/common/content_features.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
@@ -137,4 +142,70 @@ TEST(ParallelDownloadUtilsTest, GetMaxContiguousDataBlockSizeFromBeginning) {
   EXPECT_EQ(1000, GetMaxContiguousDataBlockSizeFromBeginning(slices));
 }
 
+// Test to verify Finch parameters for enabled experiment group is read
+// correctly.
+TEST(ParallelDownloadUtilsTest, FinchConfigEnabled) {
+  base::test::ScopedFeatureList feature_list;
+  std::map<std::string, std::string> params = {
+      {content::kMinSliceSizeFinchKey, "1234"},
+      {content::kParallelRequestCountFinchKey, "6"},
+      {content::kParallelRequestDelayFinchKey, "2000"},
+      {content::kParallelRequestRemainingTimeFinchKey, "3"}};
+  feature_list.InitAndEnableFeatureWithParameters(
+      features::kParallelDownloading, params);
+  EXPECT_TRUE(IsParallelDownloadEnabled());
+  EXPECT_EQ(GetMinSliceSizeConfig(), 1234);
+  EXPECT_EQ(GetParallelRequestCountConfig(), 6);
+  EXPECT_EQ(GetParallelRequestDelayConfig(), base::TimeDelta::FromSeconds(2));
+  EXPECT_EQ(GetParallelRequestRemainingTimeConfig(),
+            base::TimeDelta::FromSeconds(3));
+}
+
+// Test to verify the disable experiment group will actually disable the
+// feature.
+TEST(ParallelDownloadUtilsTest, FinchConfigDisabled) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndDisableFeature(features::kParallelDownloading);
+  EXPECT_FALSE(IsParallelDownloadEnabled());
+}
+
+// Test to verify that the Finch parameter |enable_parallel_download| works
+// correctly.
+TEST(ParallelDownloadUtilsTest, FinchConfigDisabledWithParameter) {
+  {
+    base::test::ScopedFeatureList feature_list;
+    std::map<std::string, std::string> params = {
+        {content::kMinSliceSizeFinchKey, "4321"},
+        {content::kEnableParallelDownloadFinchKey, "false"}};
+    feature_list.InitAndEnableFeatureWithParameters(
+        features::kParallelDownloading, params);
+    // Use |enable_parallel_download| to disable parallel download in enabled
+    // experiment group.
+    EXPECT_FALSE(IsParallelDownloadEnabled());
+    EXPECT_EQ(GetMinSliceSizeConfig(), 4321);
+  }
+  {
+    base::test::ScopedFeatureList feature_list;
+    std::map<std::string, std::string> params = {
+        {content::kMinSliceSizeFinchKey, "4321"},
+        {content::kEnableParallelDownloadFinchKey, "true"}};
+    feature_list.InitAndEnableFeatureWithParameters(
+        features::kParallelDownloading, params);
+    // Disable only if |enable_parallel_download| sets to false.
+    EXPECT_TRUE(IsParallelDownloadEnabled());
+    EXPECT_EQ(GetMinSliceSizeConfig(), 4321);
+  }
+  {
+    base::test::ScopedFeatureList feature_list;
+    std::map<std::string, std::string> params = {
+        {content::kMinSliceSizeFinchKey, "4321"}};
+    feature_list.InitAndEnableFeatureWithParameters(
+        features::kParallelDownloading, params);
+    // Empty |enable_parallel_download| in an enabled experiment group will have
+    // no impact.
+    EXPECT_TRUE(IsParallelDownloadEnabled());
+    EXPECT_EQ(GetMinSliceSizeConfig(), 4321);
+  }
+}
+
 }  // namespace content