Bug 1871377 - block FetchParent from cancelling fetch operation during shutdown for keepalive requests. r=edenchuang,necko-reviewers,jesup

Depends on D205720

Differential Revision: https://phabricator.services.mozilla.com/D207862
This commit is contained in:
sunil mayya 2024-06-13 06:27:19 +00:00
parent 46fd85c2da
commit 9e0a01fb20
5 changed files with 33 additions and 51 deletions

View File

@ -416,7 +416,7 @@ void FetchChild::Shutdown() {
mIsShutdown.Flip();
// If mWorkerRef is nullptr here, that means Recv__delete__() must be called
if (!mWorkerRef || !mIsKeepAliveRequest) {
if (!mWorkerRef) {
return;
}
mPromise = nullptr;
@ -424,7 +424,23 @@ void FetchChild::Shutdown() {
Unfollow();
mSignalImpl = nullptr;
mCSPEventListener = nullptr;
Unused << SendAbortFetchOp();
// TODO
// For workers we need to skip aborting the fetch requests if keepalive is set
// This is just a quick fix for Worker.
// Usually, we want FetchChild to get destroyed while FetchParent calls
// Senddelete(). When Worker shutdown, FetchChild must call
// FetchChild::SendAbortFetchOp() to parent, and let FetchParent decide if
// canceling the underlying fetch() or not. But currently, we have no good way
// to distinguish whether the abort is intent by script or by Worker/Window
// shutdown. So, we provide a quick fix here, which makes
// FetchChild/FetchParent live a bit longer, but corresponding resources are
// released in FetchChild::Shutdown(), so this quick fix should not cause any
// leaking.
// And we will fix it in Bug 1901082
if (!mIsKeepAliveRequest) {
Unused << SendAbortFetchOp();
}
mWorkerRef = nullptr;
}

View File

@ -332,9 +332,17 @@ void FetchParent::ActorDestroy(ActorDestroyReason aReason) {
entry.Remove();
FETCH_LOG(("FetchParent::ActorDestroy entry [%p] removed", this));
}
// mRequest can be null when FetchParent has not yet received RecvFetchOp()
if (!mRequest) {
return;
}
// Force to abort the existing fetch.
// Actor can be destoried by shutdown when still fetching.
RecvAbortFetchOp();
if (mRequest->GetKeepalive()) {
FETCH_LOG(("Skip aborting fetch as the request is marked keepalive"));
} else {
RecvAbortFetchOp();
}
// mBackgroundEventTarget = nullptr;
}

View File

@ -2,15 +2,9 @@
[Request interface: new Request('about:blank') must inherit property "isHistoryNavigation" with the proper type]
expected: FAIL
[Request interface: attribute keepalive]
expected: FAIL
[Request interface: attribute isHistoryNavigation]
expected: FAIL
[Request interface: new Request('about:blank') must inherit property "keepalive" with the proper type]
expected: FAIL
[Request interface: attribute isReloadNavigation]
expected: FAIL
@ -34,15 +28,9 @@
[Request interface: new Request('about:blank') must inherit property "isHistoryNavigation" with the proper type]
expected: FAIL
[Request interface: attribute keepalive]
expected: FAIL
[Request interface: attribute isHistoryNavigation]
expected: FAIL
[Request interface: new Request('about:blank') must inherit property "keepalive" with the proper type]
expected: FAIL
[Request interface: attribute isReloadNavigation]
expected: FAIL
@ -66,15 +54,9 @@
[Request interface: new Request('about:blank') must inherit property "isHistoryNavigation" with the proper type]
expected: FAIL
[Request interface: attribute keepalive]
expected: FAIL
[Request interface: attribute isHistoryNavigation]
expected: FAIL
[Request interface: new Request('about:blank') must inherit property "keepalive" with the proper type]
expected: FAIL
[Request interface: attribute isReloadNavigation]
expected: FAIL
@ -98,15 +80,9 @@
[Request interface: new Request('about:blank') must inherit property "isHistoryNavigation" with the proper type]
expected: FAIL
[Request interface: attribute keepalive]
expected: FAIL
[Request interface: attribute isHistoryNavigation]
expected: FAIL
[Request interface: new Request('about:blank') must inherit property "keepalive" with the proper type]
expected: FAIL
[Request interface: attribute isReloadNavigation]
expected: FAIL

View File

@ -1,18 +0,0 @@
[redirect-keepalive.https.any.html]
expected:
if (os == "linux") and fission and not debug and asan: [TIMEOUT, OK]
if (os == "win") and not debug and (processor == "x86"): OK
if (os == "linux") and fission and debug: [TIMEOUT, OK]
if (os == "mac") and debug: TIMEOUT
if (os == "linux") and not fission: OK
if os == "android": OK
[OK, TIMEOUT]
[[keepalive\][iframe\][load\] mixed content redirect; setting up]
expected:
if (os == "linux") and fission and not debug and asan: [TIMEOUT, PASS]
if (os == "win") and not debug and (processor == "x86"): PASS
if (os == "linux") and fission and debug: [TIMEOUT, PASS]
if (os == "mac") and debug: TIMEOUT
if (os == "linux") and not fission: PASS
if os == "android": PASS
[PASS, TIMEOUT]

View File

@ -6,7 +6,7 @@
[A same-origin window[target=''\][features='0'\] can trigger fetchLater.]
expected:
if os == "linux": [TIMEOUT, FAIL]
TIMEOUT
FAIL
[A cross-origin window[target=''\][features='0'\] can trigger fetchLater.]
expected: TIMEOUT
@ -17,7 +17,7 @@
[A same-origin window[target=''\][features='1'\] can trigger fetchLater.]
expected:
if os == "linux": [TIMEOUT, FAIL]
TIMEOUT
FAIL
[A cross-origin window[target=''\][features='1'\] can trigger fetchLater.]
expected: TIMEOUT
@ -28,7 +28,7 @@
[A same-origin window[target=''\][features='2'\] can trigger fetchLater.]
expected:
if os == "linux": [TIMEOUT, FAIL]
TIMEOUT
FAIL
[A cross-origin window[target=''\][features='2'\] can trigger fetchLater.]
expected: TIMEOUT
@ -39,7 +39,7 @@
[A same-origin window[target='_blank'\][features='0'\] can trigger fetchLater.]
expected:
if os == "linux": [TIMEOUT, FAIL]
TIMEOUT
FAIL
[A cross-origin window[target='_blank'\][features='0'\] can trigger fetchLater.]
expected: TIMEOUT
@ -50,7 +50,7 @@
[A same-origin window[target='_blank'\][features='1'\] can trigger fetchLater.]
expected:
if os == "linux": [TIMEOUT, FAIL]
TIMEOUT
FAIL
[A cross-origin window[target='_blank'\][features='1'\] can trigger fetchLater.]
expected: TIMEOUT
@ -61,7 +61,7 @@
[A same-origin window[target='_blank'\][features='2'\] can trigger fetchLater.]
expected:
if os == "linux": [TIMEOUT, FAIL]
TIMEOUT
FAIL
[A cross-origin window[target='_blank'\][features='2'\] can trigger fetchLater.]
expected: TIMEOUT