Disallowing sync XHR in pagedimissal.

Disallow sync XHR during page dismissal when the page is being navigated
away or closed by the user. Sync XHR is already on the deprecation path and
its usage hurts end user experience.

Rolling this out behind a base::Feature to help mitigate risk. This feature
is disabled by default.

Bug: 827324
Change-Id: I32cb71c3896281c1311ee32de3e7e434ba80322b
Reviewed-on: https://chromium-review.googlesource.com/c/1336914
Commit-Queue: Katie Dillon <kdillon@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619511}

chrome/test/data/prerender/prerender_loader_with_unload.html

@@ -6,10 +6,8 @@
 <script>
   function UnloadHandler() {
     // Signal to the browser the unload happened by hitting a URL. Must
-    // be synchronous so the request isn't cancelled.
-    var xhr = new XMLHttpRequest();
-    xhr.open('GET', '/unload-url', false);
-    xhr.send();
+    // not be cancelled.
+    fetch('/unload-url', {method: 'GET', keepalive: true});
   }
 
   addEventListener('unload', UnloadHandler, false);

content/child/runtime_features.cc

@@ -477,6 +477,11 @@ void SetRuntimeFeaturesDefaultsAndUpdateFromArgs(
   if (base::FeatureList::IsEnabled(blink::features::kWritableFilesAPI))
     WebRuntimeFeatures::EnableFeatureFromString("WritableFiles", true);
 
+  if (base::FeatureList::IsEnabled(
+          blink::features::kForbidSyncXHRInPageDismissal)) {
+    WebRuntimeFeatures::EnableForbidSyncXHRInPageDismissal(true);
+  }
+
   // End individual features.
   // Do not add individual features below this line.
 

testing/variations/fieldtrial_testing_config.json

@@ -1990,6 +1990,25 @@
             ]
         }
     ],
+    "ForbidSyncXHRInPageDismissal": [
+        {
+            "platforms": [
+                "android",
+                "chromeos",
+                "linux",
+                "mac",
+                "windows"
+            ],
+            "experiments": [
+                {
+                    "name": "Enabled",
+                    "enable_features": [
+                        "ForbidSyncXHRInPageDismissal"
+                    ]
+                }
+            ]
+        }
+    ],
     "FrameTypePriorityExperiment": [
         {
             "platforms": [

third_party/blink/common/features.cc

@@ -137,6 +137,10 @@ const base::Feature kWasmCodeCache = {"WasmCodeCache",
 const base::Feature kWritableFilesAPI{"WritableFilesAPI",
                                       base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Allows for synchronous XHR requests during page dismissal
+const base::Feature kForbidSyncXHRInPageDismissal{
+    "ForbidSyncXHRInPageDismissal", base::FEATURE_DISABLED_BY_DEFAULT};
+
 const char kAutofillPreviewStyleExperimentBgColorParameterName[] = "bg_color";
 
 const char kAutofillPreviewStyleExperimentColorParameterName[] = "color";

third_party/blink/public/common/features.h

@@ -40,6 +40,7 @@ BLINK_COMMON_EXPORT extern const base::Feature kStopInBackground;
 BLINK_COMMON_EXPORT extern const base::Feature kStopNonTimersInBackground;
 BLINK_COMMON_EXPORT extern const base::Feature kWasmCodeCache;
 BLINK_COMMON_EXPORT extern const base::Feature kWritableFilesAPI;
+BLINK_COMMON_EXPORT extern const base::Feature kForbidSyncXHRInPageDismissal;
 
 BLINK_COMMON_EXPORT extern const char
     kAutofillPreviewStyleExperimentBgColorParameterName[];

third_party/blink/public/platform/web_runtime_features.h

@@ -220,6 +220,7 @@ class WebRuntimeFeatures {
   BLINK_PLATFORM_EXPORT static void EnableTranslateService(bool);
   BLINK_PLATFORM_EXPORT static void EnableMergeBlockingNonBlockingPools(bool);
   BLINK_PLATFORM_EXPORT static void EnableGetDisplayMedia(bool);
+  BLINK_PLATFORM_EXPORT static void EnableForbidSyncXHRInPageDismissal(bool);
 
  private:
   WebRuntimeFeatures();

third_party/blink/renderer/core/xmlhttprequest/xml_http_request.cc

@@ -26,7 +26,9 @@
 #include <memory>
 
 #include "base/auto_reset.h"
+#include "base/feature_list.h"
 #include "third_party/blink/public/common/blob/blob_utils.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/mojom/feature_policy/feature_policy.mojom-blink.h"
 #include "third_party/blink/public/platform/web_url_request.h"
 #include "third_party/blink/renderer/bindings/core/v8/array_buffer_or_array_buffer_view_or_blob_or_document_or_string_or_form_data_or_url_search_params.h"
@@ -1099,9 +1101,6 @@ void XMLHttpRequest::CreateRequest(scoped_refptr<EncodedFormData> http_body,
     resource_loader_options.data_buffering_policy = kDoNotBufferData;
   }
 
-  exception_code_ = DOMExceptionCode::kNoError;
-  error_ = false;
-
   if (async_) {
     UseCounter::Count(&execution_context,
                       WebFeature::kXMLHttpRequestAsynchronous);
@@ -1133,11 +1132,22 @@ void XMLHttpRequest::CreateRequest(scoped_refptr<EncodedFormData> http_body,
         DEFINE_STATIC_LOCAL(EnumerationHistogram, syncxhr_pagedismissal_histogram,
                             ("XHR.Sync.PageDismissal", 5));
         syncxhr_pagedismissal_histogram.Count(pagedismissal);
+        // Disallow synchronous requests on page dismissal
+        if (base::FeatureList::IsEnabled(
+                features::kForbidSyncXHRInPageDismissal)) {
+          HandleNetworkError();
+          ThrowForLoadFailureIfNeeded(exception_state,
+                                      "Synchronous XHR in page dismissal.");
+          return;
+        }
       }
     }
     resource_loader_options.synchronous_policy = kRequestSynchronously;
   }
 
+  exception_code_ = DOMExceptionCode::kNoError;
+  error_ = false;
+
   loader_ = MakeGarbageCollected<ThreadableLoader>(execution_context, this,
                                                    resource_loader_options);
   loader_->SetTimeout(timeout_);

third_party/blink/renderer/platform/exported/web_runtime_features.cc

@@ -614,4 +614,8 @@ void WebRuntimeFeatures::EnableGetDisplayMedia(bool enable) {
   RuntimeEnabledFeatures::SetGetDisplayMediaEnabled(enable);
 }
 
+void WebRuntimeFeatures::EnableForbidSyncXHRInPageDismissal(bool enable) {
+  RuntimeEnabledFeatures::SetForbidSyncXHRInPageDismissalEnabled(enable);
+}
+
 }  // namespace blink

third_party/blink/renderer/platform/runtime_enabled_features.json5

@@ -518,6 +518,10 @@
       // a Chromium level feature.
     },
     // For simulating Android's overlay fullscreen video in web tests on Linux.
+    {
+      name: "ForbidSyncXHRInPageDismissal",
+      status: "test",
+    },
     {
       name: "ForceOverlayFullscreenVideo",
     },

tools/metrics/histograms/enums.xml

@@ -31369,6 +31369,7 @@ from previous Chrome versions.
   <int value="105046382" label="ParallelDownloading:disabled"/>
   <int value="106840653" label="mus"/>
   <int value="107900612" label="ChromeHomePersistentIph:disabled"/>
+  <int value="109577361" label="ForbidSyncXHRInPageDismissal:enabled"/>
   <int value="115915570"
       label="OmniboxUIExperimentHideSteadyStateUrlPathQueryAndRef:enabled"/>
   <int value="118991027" label="enable-accelerated-fixed-root-background"/>
@@ -31982,6 +31983,7 @@ from previous Chrome versions.
   <int value="1237297772" label="no-pings"/>
   <int value="1240073971" label="ash-disable-smooth-screen-rotation"/>
   <int value="1242632259" label="ContentSuggestionsCategoryOrder:disabled"/>
+  <int value="1243890754" label="ForbidSyncXHRInPageDismissal:disabled"/>
   <int value="1245889469" label="enable-surface-worker"/>
   <int value="1247293682" label="topchrome-md"/>
   <int value="1250071868" label="disable-timezone-tracking-option"/>