0

Add killswitch for the race network request fix

This CL adds the killswitch for the fix to the SWResourceLoader lifetime
issue, which was recently added.

We'd like to cherry-pick this fix on M123 so we need a killswitch,
because a killswitch is one of the criteria of sending a merge request.

Bug: 340949948
Change-Id: I8d68f63ec523e59b67b959ef2670826ed8f4b958
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5552694
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Minoru Chikamune <chikamune@chromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Commit-Queue: Shunya Shishido <sisidovski@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1304201}
This commit is contained in:
Shunya Shishido
2024-05-22 01:50:10 +00:00
committed by Chromium LUCI CQ
parent 3e21af1f44
commit 8b51c00895
4 changed files with 49 additions and 17 deletions

@@ -99,6 +99,11 @@ std::string GetContainerHostClientId(int frame_tree_node_id) {
return client_uuid; return client_uuid;
} }
bool IsStaticRouterRaceRequestFixEnabled() {
return base::FeatureList::IsEnabled(
features::kServiceWorkerStaticRouterRaceRequestFix);
}
} // namespace } // namespace
// This class waits for completion of a stream response from the service worker. // This class waits for completion of a stream response from the service worker.
@@ -717,11 +722,14 @@ void ServiceWorkerMainResourceLoader::DidDispatchFetchEvent(
// When kRaceNetworkRequest preload is triggered, it's possible that the // When kRaceNetworkRequest preload is triggered, it's possible that the
// response is already committed without waiting for the fetch event result. // response is already committed without waiting for the fetch event result.
// Invalidate and destruct if the class already detached from the request. // Invalidate and destruct if the class already detached from the request.
has_fetch_event_finished_ = true; if (IsStaticRouterRaceRequestFixEnabled()) {
if (dispatched_preload_type() == DispatchedPreloadType::kRaceNetworkRequest && has_fetch_event_finished_ = true;
is_detached_ && status_ == Status::kCompleted) { if (dispatched_preload_type() ==
InvalidateAndDeleteIfNeeded(); DispatchedPreloadType::kRaceNetworkRequest &&
return; is_detached_ && status_ == Status::kCompleted) {
InvalidateAndDeleteIfNeeded();
return;
}
} }
bool is_fallback = bool is_fallback =
@@ -1119,11 +1127,14 @@ void ServiceWorkerMainResourceLoader::InvalidateAndDeleteIfNeeded() {
// 1) RaceNetworkRequest is dispatched and the network wins the race. // 1) RaceNetworkRequest is dispatched and the network wins the race.
// 2) The fetch event result is not received yet. // 2) The fetch event result is not received yet.
// The postponed things will be done in DidDispatchFetchEvent(). // The postponed things will be done in DidDispatchFetchEvent().
if (dispatched_preload_type() == DispatchedPreloadType::kRaceNetworkRequest && if (IsStaticRouterRaceRequestFixEnabled()) {
race_network_request_url_loader_client_.has_value() && if (dispatched_preload_type() ==
!has_fetch_event_finished_) { DispatchedPreloadType::kRaceNetworkRequest &&
CHECK(fetch_dispatcher_); race_network_request_url_loader_client_.has_value() &&
return; !has_fetch_event_finished_) {
CHECK(fetch_dispatcher_);
return;
}
} }
// The fetch dispatcher or stream waiter may still be running. Don't let them // The fetch dispatcher or stream waiter may still be running. Don't let them

@@ -473,6 +473,13 @@ BASE_FEATURE(kServiceWorkerStaticRouterStartServiceWorker,
"ServiceWorkerStaticRouterStartServiceWorker", "ServiceWorkerStaticRouterStartServiceWorker",
base::FEATURE_ENABLED_BY_DEFAULT); base::FEATURE_ENABLED_BY_DEFAULT);
// (crbug.com/340949948): Killswitch for the fix to address the ServiceWorker
// main and subreosurce loader lifetime issue, which introduces fetch() failure
// in the sw fetch handler.
BASE_FEATURE(kServiceWorkerStaticRouterRaceRequestFix,
"kServiceWorkerStaticRouterRaceRequestFix",
base::FEATURE_ENABLED_BY_DEFAULT);
// The set of ServiceWorker to bypass while making navigation request. // The set of ServiceWorker to bypass while making navigation request.
// They are represented by a comma separated list of HEX encoded SHA256 hash of // They are represented by a comma separated list of HEX encoded SHA256 hash of
// the ServiceWorker's scripts. // the ServiceWorker's scripts.

@@ -111,6 +111,7 @@ CONTENT_EXPORT BASE_DECLARE_FEATURE(kSendBeaconThrowForBlobWithNonSimpleType);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kServiceWorkerAutoPreload); CONTENT_EXPORT BASE_DECLARE_FEATURE(kServiceWorkerAutoPreload);
CONTENT_EXPORT BASE_DECLARE_FEATURE( CONTENT_EXPORT BASE_DECLARE_FEATURE(
kServiceWorkerStaticRouterStartServiceWorker); kServiceWorkerStaticRouterStartServiceWorker);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kServiceWorkerStaticRouterRaceRequestFix);
CONTENT_EXPORT BASE_DECLARE_FEATURE( CONTENT_EXPORT BASE_DECLARE_FEATURE(
kServiceWorkerBypassFetchHandlerHashStrings); kServiceWorkerBypassFetchHandlerHashStrings);
CONTENT_EXPORT extern const base::FeatureParam<std::string> CONTENT_EXPORT extern const base::FeatureParam<std::string>

@@ -16,6 +16,7 @@
#include "base/strings/strcat.h" #include "base/strings/strcat.h"
#include "base/task/sequenced_task_runner.h" #include "base/task/sequenced_task_runner.h"
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
#include "content/common/features.h"
#include "content/common/fetch/fetch_request_type_converters.h" #include "content/common/fetch/fetch_request_type_converters.h"
#include "content/common/service_worker/race_network_request_url_loader_client.h" #include "content/common/service_worker/race_network_request_url_loader_client.h"
#include "content/common/service_worker/service_worker_router_evaluator.h" #include "content/common/service_worker/service_worker_router_evaluator.h"
@@ -172,6 +173,12 @@ blink::mojom::ServiceWorkerFetchEventTimingPtr AdjustTimingIfNeededCrBug1342408(
base::UmaHistogramBoolean(kMetricsName, true); base::UmaHistogramBoolean(kMetricsName, true);
return timing; return timing;
} }
bool IsStaticRouterRaceRequestFixEnabled() {
return base::FeatureList::IsEnabled(
features::kServiceWorkerStaticRouterRaceRequestFix);
}
} // namespace } // namespace
// A ServiceWorkerStreamCallback implementation which waits for completion of // A ServiceWorkerStreamCallback implementation which waits for completion of
@@ -346,10 +353,13 @@ void ServiceWorkerSubresourceLoader::MaybeDeleteThis() {
// 2) The fetch event handler has not been finished yet. // 2) The fetch event handler has not been finished yet.
// The postponed destruction will be done in // The postponed destruction will be done in
// ServiceWorkerFetchResponseCallback methods. // ServiceWorkerFetchResponseCallback methods.
if (dispatched_preload_type() == DispatchedPreloadType::kRaceNetworkRequest && if (IsStaticRouterRaceRequestFixEnabled()) {
race_network_request_loader_client_.has_value() && if (dispatched_preload_type() ==
controller_connector_observation_.IsObserving()) { DispatchedPreloadType::kRaceNetworkRequest &&
return; race_network_request_loader_client_.has_value() &&
controller_connector_observation_.IsObserving()) {
return;
}
} }
delete this; delete this;
} }
@@ -627,7 +637,8 @@ void ServiceWorkerSubresourceLoader::OnResponse(
TRACE_ID_LOCAL(request_id_)), TRACE_ID_LOCAL(request_id_)),
TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT); TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
SettleFetchEventDispatch(blink::ServiceWorkerStatusCode::kOk); SettleFetchEventDispatch(blink::ServiceWorkerStatusCode::kOk);
if (IsResponseAlreadyCommittedByRaceNetworkRequest()) { if (IsStaticRouterRaceRequestFixEnabled() &&
IsResponseAlreadyCommittedByRaceNetworkRequest()) {
MaybeDeleteThis(); MaybeDeleteThis();
return; return;
} }
@@ -648,7 +659,8 @@ void ServiceWorkerSubresourceLoader::OnResponseStream(
TRACE_ID_LOCAL(request_id_)), TRACE_ID_LOCAL(request_id_)),
TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT); TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
SettleFetchEventDispatch(blink::ServiceWorkerStatusCode::kOk); SettleFetchEventDispatch(blink::ServiceWorkerStatusCode::kOk);
if (IsResponseAlreadyCommittedByRaceNetworkRequest()) { if (IsStaticRouterRaceRequestFixEnabled() &&
IsResponseAlreadyCommittedByRaceNetworkRequest()) {
MaybeDeleteThis(); MaybeDeleteThis();
return; return;
} }
@@ -660,7 +672,8 @@ void ServiceWorkerSubresourceLoader::OnFallback(
std::optional<network::DataElementChunkedDataPipe> request_body, std::optional<network::DataElementChunkedDataPipe> request_body,
blink::mojom::ServiceWorkerFetchEventTimingPtr timing) { blink::mojom::ServiceWorkerFetchEventTimingPtr timing) {
SettleFetchEventDispatch(blink::ServiceWorkerStatusCode::kOk); SettleFetchEventDispatch(blink::ServiceWorkerStatusCode::kOk);
if (IsResponseAlreadyCommittedByRaceNetworkRequest()) { if (IsStaticRouterRaceRequestFixEnabled() &&
IsResponseAlreadyCommittedByRaceNetworkRequest()) {
MaybeDeleteThis(); MaybeDeleteThis();
return; return;
} }