Enable ForceSynchronousHTMLParsing by default

This feature is enabled at 100% (with 1% stable holdback) on all
channels since M96. No unresolved problems have been reported,
and overall metrics are improved [1]. This CL enables the
ForceSynchronousHTMLParsing feature by default, and removes the
LoaderDataPipeTuning feature entirely, replacing it with newly-
hard-coded values from the launched experiment.

[1] https://docs.google.com/document/d/13GewLNZ50nqs0OI7-rzofOXtjuAlD0R4PMLTUsr73dg/edit#heading=h.ctbltu75kgzp

This part is cool:
Fixed: 901056
Fixed: 461877
Fixed: 761992
Fixed: 789124
Fixed: 995660
Fixed: 1038534
Fixed: 1041006
Fixed: 1128608
Fixed: 1130290
Fixed: 1149988
Fixed: 1231037
-> That's 85 stars worth of bugs, as of 12-13-21.

Not sure this is "fixed" by this CL, but it should at least address
comment #3:
Bug: 1087177

Change-Id: Icbf01ef6665362ae23f28657e5574ca705b82717
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2798812
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#951773}

services/network/public/cpp/features.cc

@@ -195,41 +195,44 @@ const base::Feature kSCTAuditingRetryAndPersistReports{
 // This feature is used for tuning several loading-related data pipe
 // parameters. See crbug.com/1041006.
 const base::Feature kLoaderDataPipeTuningFeature{
-    "LoaderDataPipeTuning", base::FEATURE_DISABLED_BY_DEFAULT};
+    "LoaderDataPipeTuning", base::FEATURE_ENABLED_BY_DEFAULT};
 
 namespace {
-// The default buffer size of DataPipe which is used to send the content body.
-static constexpr uint32_t kDataPipeDefaultAllocationSize = 512 * 1024;
-constexpr base::FeatureParam<int> kDataPipeAllocationSize{
-    &kLoaderDataPipeTuningFeature, "allocation_size_bytes",
-    base::saturated_cast<int>(kDataPipeDefaultAllocationSize)};
+// The default Mojo ring buffer size, used to send the content body.
+static constexpr uint32_t kDefaultDataPipeAllocationSize = 512 * 1024;
+// The larger ring buffer size, used primarily for network::URLLoader loads.
+// This value was optimized via Finch: see crbug.com/1041006.
+static constexpr uint32_t kLargerDataPipeAllocationSize = 2 * 1024 * 1024;
 
 // The maximal number of bytes consumed in a loading task. When there are more
 // bytes in the data pipe, they will be consumed in following tasks. Setting too
 // small of a number will generate many tasks but setting a too large of a
-// number will lead to thread janks.
-static constexpr uint32_t kMaxNumConsumedBytesInTask = 64 * 1024;
-constexpr base::FeatureParam<int> kLoaderChunkSize{
-    &kLoaderDataPipeTuningFeature, "loader_chunk_size",
-    base::saturated_cast<int>(kMaxNumConsumedBytesInTask)};
+// number will lead to thread janks. This value was optimized via Finch:
+// see crbug.com/1041006.
+static constexpr uint32_t kDefaultMaxNumConsumedBytesInTask = 64 * 1024;
+static constexpr uint32_t kLargerMaxNumConsumedBytesInTask = 1024 * 1024;
 }  // namespace
 
 // static
 uint32_t GetDataPipeDefaultAllocationSize(DataPipeAllocationSize option) {
   // For low-memory devices, always use the (smaller) default buffer size.
   if (base::SysInfo::AmountOfPhysicalMemoryMB() <= 512)
-    return kDataPipeDefaultAllocationSize;
+    return kDefaultDataPipeAllocationSize;
+  if (!base::FeatureList::IsEnabled(features::kLoaderDataPipeTuningFeature))
+    return kDefaultDataPipeAllocationSize;
   switch (option) {
     case DataPipeAllocationSize::kDefaultSizeOnly:
-      return kDataPipeDefaultAllocationSize;
+      return kDefaultDataPipeAllocationSize;
     case DataPipeAllocationSize::kLargerSizeIfPossible:
-      return base::saturated_cast<uint32_t>(kDataPipeAllocationSize.Get());
+      return kLargerDataPipeAllocationSize;
   }
 }
 
 // static
 uint32_t GetLoaderChunkSize() {
-  return base::saturated_cast<uint32_t>(kLoaderChunkSize.Get());
+  if (!base::FeatureList::IsEnabled(features::kLoaderDataPipeTuningFeature))
+    return kDefaultMaxNumConsumedBytesInTask;
+  return kLargerMaxNumConsumedBytesInTask;
 }
 
 // Check disk cache to see if the queued requests (especially those don't need

third_party/blink/common/features.cc

@@ -123,7 +123,7 @@ const base::Feature kJSONModules{"JSONModules",
                                  base::FEATURE_ENABLED_BY_DEFAULT};
 
 const base::Feature kForceSynchronousHTMLParsing{
-    "ForceSynchronousHTMLParsing", base::FEATURE_DISABLED_BY_DEFAULT};
+    "ForceSynchronousHTMLParsing", base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Enable EditingNG by default. This feature is for a kill switch.
 const base::Feature kEditingNG{"EditingNG", base::FEATURE_ENABLED_BY_DEFAULT};

third_party/blink/renderer/platform/loader/fetch/response_body_loader_test.cc

@@ -43,15 +43,6 @@ class ResponseBodyLoaderTest : public testing::Test {
   using PublicState = BytesConsumer::PublicState;
   using Result = BytesConsumer::Result;
 
-  static constexpr uint32_t kMaxNumConsumedBytesInTaskForTesting = 512 * 1024;
-  ResponseBodyLoaderTest() {
-    base::FieldTrialParams params;
-    params["loader_chunk_size"] =
-        base::NumberToString(kMaxNumConsumedBytesInTaskForTesting);
-    scoped_feature_list_.InitAndEnableFeatureWithParameters(
-        network::features::kLoaderDataPipeTuningFeature, params);
-  }
-
   class TestClient final : public GarbageCollected<TestClient>,
                            public ResponseBodyLoaderClient {
    public:
@@ -357,7 +348,7 @@ TEST_F(ResponseBodyLoaderTest, Suspend) {
 TEST_F(ResponseBodyLoaderTest, ReadTooBigBuffer) {
   auto task_runner = base::MakeRefCounted<scheduler::FakeTaskRunner>();
   auto* consumer = MakeGarbageCollected<ReplayingBytesConsumer>(task_runner);
-  constexpr auto kMax = kMaxNumConsumedBytesInTaskForTesting;
+  const uint32_t kMax = network::features::GetLoaderChunkSize();
 
   consumer->Add(Command(Command::kData, std::string(kMax - 1, 'a').data()));
   consumer->Add(Command(Command::kData, std::string(2, 'b').data()));