From 3ef3ba937894cf94965b28b882f4abc1f5b86b1f Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Sat, 13 Sep 2025 20:34:14 +0200 Subject: [PATCH] Add spec steps and comments for fetch abort steps (#39283) While trying to figure out what the status of this implementation was, I added steps and comments to see what we are missing. Also updated some links, since I couldn't find an implementation of `window.fetch`, since the spec URL was pointing to the chapter instead of the algorithm. Part of #34866 Signed-off-by: Tim van der Lippe --- components/script/dom/abortcontroller.rs | 9 ++-- components/script/dom/abortsignal.rs | 45 ++++++++++--------- components/script/dom/window.rs | 2 +- components/script/dom/workerglobalscope.rs | 2 +- components/script/fetch.rs | 45 +++++++++++++++++-- .../script_bindings/webidls/Fetch.webidl | 11 ----- .../webidls/WindowOrWorkerGlobalScope.webidl | 5 +++ 7 files changed, 78 insertions(+), 41 deletions(-) delete mode 100644 components/script_bindings/webidls/Fetch.webidl diff --git a/components/script/dom/abortcontroller.rs b/components/script/dom/abortcontroller.rs index 2df26511be4..aecb4aa2b41 100644 --- a/components/script/dom/abortcontroller.rs +++ b/components/script/dom/abortcontroller.rs @@ -18,7 +18,7 @@ use crate::script_runtime::{CanGc, JSContext}; pub(crate) struct AbortController { reflector_: Reflector, - /// An AbortController object has an associated signal (an AbortSignal object). + /// signal: Dom, } @@ -27,7 +27,7 @@ impl AbortController { fn new_inherited(signal: &AbortSignal) -> AbortController { // Note: continuation of the constructor steps. - // Set this’s signal to signal. + // Step 2. Set this’s signal to signal. AbortController { reflector_: Reflector::new(), signal: Dom::from_ref(signal), @@ -40,9 +40,9 @@ impl AbortController { proto: Option, can_gc: CanGc, ) -> DomRoot { - // The new AbortController() constructor steps are: - // Let signal be a new AbortSignal object. + // Step 1. Let signal be a new AbortSignal object. let signal = AbortSignal::new_with_proto(global, None, can_gc); + // Step 2. Set this’s signal to signal. reflect_dom_object_with_proto( Box::new(AbortController::new_inherited(&signal)), global, @@ -59,6 +59,7 @@ impl AbortController { realm: InRealm, can_gc: CanGc, ) { + // To signal abort on an AbortController controller with an optional reason, // signal abort on controller’s signal with reason if it is given. self.signal.signal_abort(cx, reason, realm, can_gc); } diff --git a/components/script/dom/abortsignal.rs b/components/script/dom/abortsignal.rs index 9f84eeed110..e3c4e1b0f0f 100644 --- a/components/script/dom/abortsignal.rs +++ b/components/script/dom/abortsignal.rs @@ -82,42 +82,48 @@ impl AbortSignal { ) { let global = self.global(); - // If signal is aborted, then return. + // Step 1. If signal is aborted, then return. if self.Aborted() { return; } + // Step 2. Set signal’s abort reason to reason if it is given; + // otherwise to a new "AbortError" DOMException. let abort_reason = reason.get(); - - // Set signal’s abort reason to reason if it is given; if !abort_reason.is_undefined() { self.abort_reason.set(abort_reason); } else { - // otherwise to a new "AbortError" DOMException. rooted!(in(*cx) let mut rooted_error = UndefinedValue()); Error::Abort.to_jsval(cx, &global, rooted_error.handle_mut(), can_gc); self.abort_reason.set(rooted_error.get()) } - // Let dependentSignalsToAbort be a new list. - // For each dependentSignal of signal’s dependent signals: - // TODO: #36936 + // Step 3. Let dependentSignalsToAbort be a new list. + // TODO + // Step 4. For each dependentSignal of signal’s dependent signals: + // TODO + // Step 4.1. If dependentSignal is not aborted: + // TODO + // Step 4.1.1. Set dependentSignal’s abort reason to signal’s abort reason. + // TODO + // Step 4.1.2. Append dependentSignal to dependentSignalsToAbort. + // TODO - // Run the abort steps for signal. + // Step 5. Run the abort steps for signal. self.run_the_abort_steps(cx, &global, realm, can_gc); - // For each dependentSignal of dependentSignalsToAbort, run the abort steps for dependentSignal. - // TODO: #36936 + // Step 6. For each dependentSignal of dependentSignalsToAbort, run the abort steps for dependentSignal. + // TODO } /// pub(crate) fn add(&self, algorithm: &AbortAlgorithm) { - // If signal is aborted, then return. + // Step 1. If signal is aborted, then return. if self.aborted() { return; } - // Append algorithm to signal’s abort algorithms. + // Step 2. Append algorithm to signal’s abort algorithms. self.abort_algorithms.borrow_mut().push(algorithm.clone()); } @@ -151,15 +157,14 @@ impl AbortSignal { realm: InRealm, can_gc: CanGc, ) { - // For each algorithm of signal’s abort algorithms: run algorithm. + // Step 1. For each algorithm of signal’s abort algorithms: run algorithm. for algo in self.abort_algorithms.borrow().iter() { self.run_abort_algorithm(cx, global, algo, realm, can_gc); } - - // Empty signal’s abort algorithms. + // Step 2. Empty signal’s abort algorithms. self.abort_algorithms.borrow_mut().clear(); - // Fire an event named abort at signal. + // Step 3. Fire an event named abort at signal. self.upcast::() .fire_event(atom!("abort"), can_gc); } @@ -185,21 +190,21 @@ impl AbortSignalMethods for AbortSignal { reason: HandleValue, can_gc: CanGc, ) -> DomRoot { - // Let signal be a new AbortSignal object. + // Step 1. Let signal be a new AbortSignal object. let signal = AbortSignal::new_with_proto(global, None, can_gc); + // Step 2. Set signal’s abort reason to reason if it is given; + // otherwise to a new "AbortError" DOMException. let abort_reason = reason.get(); - // Set signal’s abort reason to reason if it is given; if !abort_reason.is_undefined() { signal.abort_reason.set(abort_reason); } else { - // otherwise to a new "AbortError" DOMException. rooted!(in(*cx) let mut rooted_error = UndefinedValue()); Error::Abort.to_jsval(cx, global, rooted_error.handle_mut(), can_gc); signal.abort_reason.set(rooted_error.get()) } - // Return signal. + // Step 3. Return signal. signal } diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 6745f19a88a..d8b2d9a60b9 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1798,7 +1798,7 @@ impl WindowMethods for Window { mql } - // https://fetch.spec.whatwg.org/#fetch-method + /// fn Fetch( &self, input: RequestOrUSVString, diff --git a/components/script/dom/workerglobalscope.rs b/components/script/dom/workerglobalscope.rs index 82115b1dce3..595a7d62e44 100644 --- a/components/script/dom/workerglobalscope.rs +++ b/components/script/dom/workerglobalscope.rs @@ -590,7 +590,7 @@ impl WorkerGlobalScopeMethods for WorkerGlobalScope { } #[cfg_attr(crown, allow(crown::unrooted_must_root))] - // https://fetch.spec.whatwg.org/#fetch-method + /// fn Fetch( &self, input: RequestOrUSVString, diff --git a/components/script/fetch.rs b/components/script/fetch.rs index 15de5d4293a..2f9b14bb4fd 100644 --- a/components/script/fetch.rs +++ b/components/script/fetch.rs @@ -139,7 +139,7 @@ fn request_init_from_request(request: NetTraitsRequest) -> RequestBuilder { } } -/// +/// #[allow(non_snake_case)] #[cfg_attr(crown, allow(crown::unrooted_must_root))] pub(crate) fn Fetch( @@ -176,7 +176,12 @@ pub(crate) fn Fetch( request_init.policy_container = RequestPolicyContainer::PolicyContainer(global.policy_container()); - // TODO: Step 4. If requestObject’s signal is aborted, then: [..] + // Step 4. If requestObject’s signal is aborted, then: + // TODO + // Step 4.1. Abort the fetch() call with p, request, null, and requestObject’s signal’s abort reason. + // TODO + // Step 4.2. Return p. + // TODO // Step 5. Let globalObject be request’s client’s global object. // NOTE: We already get the global object as an argument @@ -187,16 +192,48 @@ pub(crate) fn Fetch( request_init.service_workers_mode = ServiceWorkersMode::None; } - // TODO: Steps 8-11, abortcontroller stuff + // Step 8. Let relevantRealm be this’s relevant realm. + // + // Is `comp` as argument + + // Step 9. Let locallyAborted be false. + // TODO + // Step 10. Let controller be null. + // TODO + // Step 11. Add the following abort steps to requestObject’s signal: + // TODO + // Step 11.1. Set locallyAborted to true. + // TODO + // Step 11.2. Assert: controller is non-null. + // TODO + // Step 11.3. Abort controller with requestObject’s signal’s abort reason. + // TODO + // Step 11.4. Abort the fetch() call with p, request, responseObject, and requestObject’s signal’s abort reason. + // TODO // Step 12. Set controller to the result of calling fetch given request and - // processResponse given response being these steps: [..] + // processResponse given response being these steps: let fetch_context = Arc::new(Mutex::new(FetchContext { fetch_promise: Some(TrustedPromise::new(promise.clone())), response_object: Trusted::new(&*response), resource_timing: ResourceFetchTiming::new(timing_type), })); + // Step 12.1. If locallyAborted is true, then abort these steps. + // TODO + // Step 12.2. If response’s aborted flag is set, then: + // TODO + // Step 12.2.1. Let deserializedError be the result of deserialize a serialized + // abort reason given controller’s serialized abort reason and relevantRealm. + // TODO + // Step 12.2.2. Abort the fetch() call with p, request, responseObject, and deserializedError. + // TODO + // Step 12.2.3. Abort these steps. + // TODO + + // Step 12.3. If response is a network error, then reject p with a TypeError and abort these steps. + // Step 12.4. Set responseObject to the result of creating a Response object, given response, "immutable", and relevantRealm. + // Step 12.5. Resolve p with responseObject. global.fetch( request_init, fetch_context, diff --git a/components/script_bindings/webidls/Fetch.webidl b/components/script_bindings/webidls/Fetch.webidl deleted file mode 100644 index abedcf620ea..00000000000 --- a/components/script_bindings/webidls/Fetch.webidl +++ /dev/null @@ -1,11 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ - -// https://fetch.spec.whatwg.org/#fetch-method - -[Exposed=(Window,Worker)] - -partial interface mixin WindowOrWorkerGlobalScope { - [NewObject] Promise fetch(RequestInfo input, optional RequestInit init = {}); -}; diff --git a/components/script_bindings/webidls/WindowOrWorkerGlobalScope.webidl b/components/script_bindings/webidls/WindowOrWorkerGlobalScope.webidl index a4acdb5192e..58b38d3ed05 100644 --- a/components/script_bindings/webidls/WindowOrWorkerGlobalScope.webidl +++ b/components/script_bindings/webidls/WindowOrWorkerGlobalScope.webidl @@ -49,5 +49,10 @@ partial interface mixin WindowOrWorkerGlobalScope { readonly attribute TrustedTypePolicyFactory trustedTypes; }; +// https://fetch.spec.whatwg.org/#fetch-method +partial interface mixin WindowOrWorkerGlobalScope { + [NewObject] Promise fetch(RequestInfo input, optional RequestInit init = {}); +}; + Window includes WindowOrWorkerGlobalScope; WorkerGlobalScope includes WindowOrWorkerGlobalScope;