From: uazo Date: Thu, 28 Sep 2023 08:11:00 +0000 Subject: Keep disabled FetchLaterAPI Disallow a network request to survive to context destoy. For Request with active keepalive, if the request does not start from the same origin. It also inhibits the blocked request exception to javascript. License: GPL-2.0-or-later - https://spdx.org/licenses/GPL-2.0-or-later.html --- .../browser/loader/keep_alive_url_loader.cc | 33 +++++++++++++++++-- .../browser/loader/keep_alive_url_loader.h | 3 -- .../Keep-disabled-FetchLaterAPI.inc | 2 ++ .../renderer/core/fetch/fetch_manager.cc | 12 +++++-- .../loader/child_url_loader_factory_bundle.cc | 14 ++++---- .../platform/loader/fetch/resource_fetcher.cc | 2 +- .../platform/runtime_enabled_features.json5 | 5 ++- 7 files changed, 54 insertions(+), 17 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 { @@ -301,7 +331,6 @@ KeepAliveURLLoader::KeepAliveURLLoader( forwarding_client_( std::make_unique(this, 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)), @@ -348,7 +377,7 @@ void KeepAliveURLLoader::Start() { url_loader_ = blink::ThrottlingURLLoader::CreateLoaderAndStart( network_loader_factory_, throttles_getter_.Run(), request_id_, options_, &resource_request_, forwarding_client_.get(), - net::NetworkTrafficAnnotationTag(traffic_annotation_), + net::NetworkTrafficAnnotationTag(kTrafficAnnotation), base::SingleThreadTaskRunner::GetCurrentDefault(), /*cors_exempt_header_list=*/std::nullopt, // `this`'s lifetime is at least the same as `url_loader_`. 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 @@ -312,9 +312,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,2 @@ +SET_CROMITE_FEATURE_ENABLED(kKeepAliveInBrowserMigration); +SET_CROMITE_FEATURE_DISABLED(kAttributionReportingInBrowserMigration); diff --git a/third_party/blink/renderer/core/fetch/fetch_manager.cc b/third_party/blink/renderer/core/fetch/fetch_manager.cc --- a/third_party/blink/renderer/core/fetch/fetch_manager.cc +++ b/third_party/blink/renderer/core/fetch/fetch_manager.cc @@ -1168,7 +1168,11 @@ void FetchManager::Loader::Failed( if (resolver_) { ScriptState::Scope scope(GetScriptState()); if (dom_exception) { - resolver_->Reject(dom_exception); + if (!GetFetchRequestData()->Keepalive()) { + resolver_->Reject(dom_exception); + } else { + resolver_->Detach(); + } } else { v8::Local value = exception_.Get(GetScriptState()->GetIsolate()); @@ -1188,7 +1192,11 @@ void FetchManager::Loader::Failed( V8String(GetScriptState()->GetIsolate(), IdentifiersFactory::IdFromToken(*issue_id))); } - resolver_->Reject(value); + if (!GetFetchRequestData()->Keepalive()) { + resolver_->Reject(value);; + } else { + resolver_->Detach(); + } LogIfKeepalive("Failed"); } } 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 @@ -255,6 +255,12 @@ void ChildURLLoaderFactoryBundle::CreateLoaderAndStart( return; } + bool keepalive = request.keepalive; + if (keepalive && !request.is_fetch_later_api && + url::IsSameOriginWith(request.url, request.referrer)) { + keepalive = false; + } + // Use |keep_alive_loader_factory_| to send the keepalive requests to the // KeepAliveURLLoaderService in the browser process and trigger the special // keepalive request handling. @@ -263,12 +269,8 @@ void ChildURLLoaderFactoryBundle::CreateLoaderAndStart( if (request.keepalive) { FetchUtils::LogFetchKeepAliveRequestSentToServiceMetric(request); } - 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))) { + if (keepalive && keep_alive_loader_factory_ && + 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 @@ -2912,7 +2912,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 @@ -1768,10 +1768,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 --