From: uazo Date: Thu, 28 Sep 2023 08:11:00 +0000 Subject: Keep disabled FetchLaterAPI Disallow a network request to survive to context destoy --- .../browser/loader/keep_alive_url_loader.cc | 33 +++++++++++++++++-- .../browser/loader/keep_alive_url_loader.h | 3 -- .../Keep-disabled-FetchLaterAPI.inc | 3 ++ .../loader/child_url_loader_factory_bundle.cc | 6 +--- .../platform/loader/fetch/resource_fetcher.cc | 2 +- .../platform/runtime_enabled_features.json5 | 5 ++- 6 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 cromite_flags/third_party/blink/common/features_cc/Keep-disabled-FetchLaterAPI.inc diff --git a/content/browser/loader/keep_alive_url_loader.cc b/content/browser/loader/keep_alive_url_loader.cc --- a/content/browser/loader/keep_alive_url_loader.cc +++ b/content/browser/loader/keep_alive_url_loader.cc @@ -40,6 +40,36 @@ #include "services/network/public/mojom/url_request.mojom.h" #include "third_party/blink/public/common/features.h" +namespace { + constexpr net::NetworkTrafficAnnotationTag kTrafficAnnotation = + net::DefineNetworkTrafficAnnotation( + "keep_alive_url_loader", R"( + semantics { + sender: "Keep alive urls" + description: + "Allow a network request to survive to context destoy" + trigger: + "FetchLater API or keep-alive fetch api." + data: "Mutable" + destination: WEBSITE + last_reviewed: "2024-03-22" + internal { + contacts { + email: "uazo@users.noreply.github.com" + } + } + user_data { + type: NONE + } + } + policy { + cookies_allowed: YES + cookies_store: "user" + setting: "This feature cannot be disabled by settings." + policy_exception_justification: "Not implemented." + })"); +} + namespace content { namespace { @@ -358,7 +388,6 @@ KeepAliveURLLoader::KeepAliveURLLoader( options_(options), resource_request_(resource_request), forwarding_client_(std::move(forwarding_client)), - traffic_annotation_(traffic_annotation), network_loader_factory_(std::move(network_loader_factory)), stored_url_load_(std::make_unique()), policy_container_host_(std::move(policy_container_host)), @@ -410,7 +439,7 @@ void KeepAliveURLLoader::Start() { network_loader_factory_->CreateLoaderAndStart( loader_.BindNewPipeAndPassReceiver(), request_id_, options_, resource_request_, loader_receiver_.BindNewPipeAndPassRemote(), - traffic_annotation_); + net::MutableNetworkTrafficAnnotationTag(kTrafficAnnotation)); loader_receiver_.set_disconnect_handler(base::BindOnce( &KeepAliveURLLoader::OnNetworkConnectionError, base::Unretained(this))); // For FetchLater requests, `forwarding_client_` is not bound to renderer. diff --git a/content/browser/loader/keep_alive_url_loader.h b/content/browser/loader/keep_alive_url_loader.h --- a/content/browser/loader/keep_alive_url_loader.h +++ b/content/browser/loader/keep_alive_url_loader.h @@ -297,9 +297,6 @@ class CONTENT_EXPORT KeepAliveURLLoader // disconnected from the remote of URLLoader in the renderer. base::OneShotTimer disconnected_loader_timer_; - // The NetworkTrafficAnnotationTag for the request being loaded. - net::MutableNetworkTrafficAnnotationTag traffic_annotation_; - // A refptr to the URLLoaderFactory implementation that can actually create a // URLLoader. An extra refptr is required here to support deferred loading. scoped_refptr network_loader_factory_; diff --git a/cromite_flags/third_party/blink/common/features_cc/Keep-disabled-FetchLaterAPI.inc b/cromite_flags/third_party/blink/common/features_cc/Keep-disabled-FetchLaterAPI.inc new file mode 100644 --- /dev/null +++ b/cromite_flags/third_party/blink/common/features_cc/Keep-disabled-FetchLaterAPI.inc @@ -0,0 +1,3 @@ +SET_CROMITE_FEATURE_ENABLED(kKeepAliveInBrowserMigration); +SET_CROMITE_FEATURE_DISABLED(kPendingBeaconAPI); +SET_CROMITE_FEATURE_DISABLED(kAttributionReportingInBrowserMigration); diff --git a/third_party/blink/renderer/platform/loader/child_url_loader_factory_bundle.cc b/third_party/blink/renderer/platform/loader/child_url_loader_factory_bundle.cc --- a/third_party/blink/renderer/platform/loader/child_url_loader_factory_bundle.cc +++ b/third_party/blink/renderer/platform/loader/child_url_loader_factory_bundle.cc @@ -260,11 +260,7 @@ void ChildURLLoaderFactoryBundle::CreateLoaderAndStart( // |keep_alive_loader_factory_| only presents when // features::kKeepAliveInBrowserMigration is true. if (request.keepalive && keep_alive_loader_factory_ && - base::FeatureList::IsEnabled(features::kKeepAliveInBrowserMigration) && - (request.attribution_reporting_eligibility == - network::mojom::AttributionReportingEligibility::kUnset || - base::FeatureList::IsEnabled( - features::kAttributionReportingInBrowserMigration))) { + base::FeatureList::IsEnabled(features::kKeepAliveInBrowserMigration)) { keep_alive_loader_factory_->CreateLoaderAndStart( std::move(loader), request_id, options, request, std::move(client), traffic_annotation); diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc --- a/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc +++ b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc @@ -2619,7 +2619,7 @@ void ResourceFetcher::PrepareForLeakDetection() { void ResourceFetcher::StopFetchingInternal(StopFetchingTarget target) { // TODO(toyoshim): May want to suspend scheduler while canceling loaders so // that the cancellations below do not awake unnecessary scheduling. - + target = StopFetchingTarget::kIncludingKeepaliveLoaders; HeapVector> loaders_to_cancel; for (const auto& loader : non_blocking_loaders_) { if (target == StopFetchingTarget::kIncludingKeepaliveLoaders || diff --git a/third_party/blink/renderer/platform/runtime_enabled_features.json5 b/third_party/blink/renderer/platform/runtime_enabled_features.json5 --- a/third_party/blink/renderer/platform/runtime_enabled_features.json5 +++ b/third_party/blink/renderer/platform/runtime_enabled_features.json5 @@ -1683,10 +1683,9 @@ }, { // The Blink runtime-enabled feature name for the API's IDL. - name: "FetchLaterAPI", - status: "experimental", + name: "FetchLaterAPI", // disable + status: "experimental", // by default origin_trial_feature_name: "FetchLaterAPI", - origin_trial_allows_third_party: true, base_feature: "FetchLaterAPI", // base_feature is meant as kill-switch. This runtime-enabled feature // should follow the Origin Trial unless explicitly overriden by Finch or --