net: Use RequestId to cancel fetches instead of creating an IPC channel (#34883)

Instead of creating an IPC channel for every fetch, allow cancelling
fetches based on the `RequestId` of the original request. This requires
that `RequestId`s be UUIDs so that they are unique between processes
that might communicating with the resource process.

In addition, the resource process loop now keeps a `HashMap` or `Weak`
handles to cancellers and cleans them up.

This allows for creating mutiple `FetchCanceller`s in `script` for a
single fetch request, allowing integration of the media and video
elements to integrate with the `Document` canceller list -- meaning
these fetches also get cancelled when the `Document` unloads.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This commit is contained in:
Martin Robinson 2025-01-11 12:49:22 +01:00 committed by GitHub
parent e2be55b873
commit 748954d610
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
23 changed files with 179 additions and 226 deletions

View file

@ -6,7 +6,6 @@
//!
//! <https://html.spec.whatwg.org/multipage/#the-end>
use ipc_channel::ipc;
use net_traits::request::RequestBuilder;
use net_traits::{fetch_async, BoxedFetchCallback, ResourceThreads};
use servo_url::ServoUrl;
@ -121,7 +120,7 @@ impl DocumentLoader {
callback: BoxedFetchCallback,
) {
self.add_blocking_load(load);
self.fetch_async_background(request, callback, None);
self.fetch_async_background(request, callback);
}
/// Initiate a new fetch that does not block the document load event.
@ -129,22 +128,9 @@ impl DocumentLoader {
&mut self,
request: RequestBuilder,
callback: BoxedFetchCallback,
cancel_override: Option<ipc::IpcReceiver<()>>,
) {
let canceller = cancel_override.unwrap_or_else(|| {
let mut canceller = FetchCanceller::new();
let cancel_receiver = canceller.initialize();
self.cancellers.push(canceller);
cancel_receiver
});
fetch_async(
&self.resource_threads.core_thread,
request,
None, /* response_init */
Some(canceller),
callback,
);
self.cancellers.push(FetchCanceller::new(request.id));
fetch_async(&self.resource_threads.core_thread, request, None, callback);
}
/// Mark an in-progress network request complete.

View file

@ -2150,7 +2150,6 @@ impl Document {
&self,
request: RequestBuilder,
listener: Listener,
cancel_override: Option<ipc::IpcReceiver<()>>,
) {
let callback = NetworkListener {
context: std::sync::Arc::new(Mutex::new(listener)),
@ -2161,8 +2160,7 @@ impl Document {
.into(),
}
.into_callback();
self.loader_mut()
.fetch_async_background(request, callback, cancel_override);
self.loader_mut().fetch_async_background(request, callback);
}
// https://html.spec.whatwg.org/multipage/#the-end

View file

@ -602,12 +602,12 @@ impl EventSourceMethods<crate::DomTypeHolder> for EventSource {
listener.notify_fetch(message.unwrap());
}),
);
let cancel_receiver = ev.canceller.borrow_mut().initialize();
*ev.canceller.borrow_mut() = FetchCanceller::new(request.id);
global
.core_resource_thread()
.send(CoreResourceMsg::Fetch(
request,
FetchChannels::ResponseMsg(action_sender, Some(cancel_receiver)),
FetchChannels::ResponseMsg(action_sender),
))
.unwrap();
// Step 13
@ -681,7 +681,7 @@ impl EventSourceTimeoutCallback {
.core_resource_thread()
.send(CoreResourceMsg::Fetch(
request,
FetchChannels::ResponseMsg(self.action_sender, None),
FetchChannels::ResponseMsg(self.action_sender),
))
.unwrap();
}

View file

@ -3242,13 +3242,12 @@ impl GlobalScope {
request_builder: RequestBuilder,
context: Arc<Mutex<Listener>>,
task_source: SendableTaskSource,
cancellation_sender: Option<ipc::IpcReceiver<()>>,
) {
let network_listener = NetworkListener {
context,
task_source,
};
self.fetch_with_network_listener(request_builder, network_listener, cancellation_sender);
self.fetch_with_network_listener(request_builder, network_listener);
}
pub(crate) fn fetch_with_network_listener<
@ -3257,13 +3256,11 @@ impl GlobalScope {
&self,
request_builder: RequestBuilder,
network_listener: NetworkListener<Listener>,
cancellation_receiver: Option<ipc::IpcReceiver<()>>,
) {
fetch_async(
&self.core_resource_thread(),
request_builder,
None,
cancellation_receiver,
network_listener.into_callback(),
);
}

View file

@ -419,7 +419,7 @@ impl HTMLImageElement {
// This is a background load because the load blocker already fulfills the
// purpose of delaying the document's load event.
document.fetch_background(request, context, None);
document.fetch_background(request, context);
}
// Steps common to when an image has been loaded.

View file

@ -384,7 +384,7 @@ impl HTMLLinkElement {
resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource),
};
document.fetch_background(request, fetch_context, None);
document.fetch_background(request, fetch_context);
}
/// <https://html.spec.whatwg.org/multipage/#concept-link-obtain>

View file

@ -898,15 +898,12 @@ impl HTMLMediaElement {
if let Some(ref mut current_fetch_context) = *current_fetch_context {
current_fetch_context.cancel(CancelReason::Overridden);
}
let (fetch_context, cancel_receiver) = HTMLMediaElementFetchContext::new();
*current_fetch_context = Some(fetch_context);
*current_fetch_context = Some(HTMLMediaElementFetchContext::new(request.id));
let listener =
HTMLMediaElementFetchListener::new(self, url.clone(), offset.unwrap_or(0), seek_lock);
// TODO: If this is supposed to to be a "fetch" as defined in the specification
// this should probably be integrated into the Document's list of cancellable fetches.
self.owner_document()
.fetch_background(request, listener, Some(cancel_receiver));
self.owner_document().fetch_background(request, listener);
}
// https://html.spec.whatwg.org/multipage/#concept-media-load-resource
@ -2598,17 +2595,12 @@ pub(crate) struct HTMLMediaElementFetchContext {
}
impl HTMLMediaElementFetchContext {
fn new() -> (HTMLMediaElementFetchContext, ipc::IpcReceiver<()>) {
let mut fetch_canceller = FetchCanceller::new();
let cancel_receiver = fetch_canceller.initialize();
(
HTMLMediaElementFetchContext {
cancel_reason: None,
is_seekable: false,
fetch_canceller,
},
cancel_receiver,
)
fn new(request_id: RequestId) -> HTMLMediaElementFetchContext {
HTMLMediaElementFetchContext {
cancel_reason: None,
is_seekable: false,
fetch_canceller: FetchCanceller::new(request_id),
}
}
fn is_seekable(&self) -> bool {

View file

@ -54,8 +54,6 @@ pub(crate) struct HTMLVideoElement {
video_height: Cell<Option<u32>>,
/// Incremented whenever tasks associated with this element are cancelled.
generation_id: Cell<u32>,
/// Poster frame fetch request canceller.
poster_frame_canceller: DomRefCell<FetchCanceller>,
/// Load event blocker. Will block the load event while the poster frame
/// is being fetched.
load_blocker: DomRefCell<Option<LoadBlocker>>,
@ -78,7 +76,6 @@ impl HTMLVideoElement {
video_width: Cell::new(None),
video_height: Cell::new(None),
generation_id: Cell::new(0),
poster_frame_canceller: DomRefCell::new(Default::default()),
load_blocker: Default::default(),
last_frame: Default::default(),
sent_resize: Cell::new(None),
@ -162,7 +159,6 @@ impl HTMLVideoElement {
/// <https://html.spec.whatwg.org/multipage/#poster-frame>
fn fetch_poster_frame(&self, poster_url: &str, can_gc: CanGc) {
// Step 1.
let cancel_receiver = self.poster_frame_canceller.borrow_mut().initialize();
self.generation_id.set(self.generation_id.get() + 1);
// Step 2.
@ -199,20 +195,14 @@ impl HTMLVideoElement {
self.process_image_response(ImageResponse::Loaded(image, url), can_gc);
},
ImageCacheResult::ReadyForRequest(id) => {
self.do_fetch_poster_frame(poster_url, id, cancel_receiver, can_gc)
self.do_fetch_poster_frame(poster_url, id, can_gc);
},
_ => (),
}
}
/// <https://html.spec.whatwg.org/multipage/#poster-frame>
fn do_fetch_poster_frame(
&self,
poster_url: ServoUrl,
id: PendingImageId,
cancel_receiver: ipc::IpcReceiver<()>,
can_gc: CanGc,
) {
fn do_fetch_poster_frame(&self, poster_url: ServoUrl, id: PendingImageId, can_gc: CanGc) {
// Continuation of step 4.
let document = self.owner_document();
let request = RequestBuilder::new(poster_url.clone(), document.global().get_referrer())
@ -235,12 +225,8 @@ impl HTMLVideoElement {
LoadType::Image(poster_url.clone()),
));
let context = PosterFrameFetchContext::new(self, poster_url, id);
// TODO: If this is supposed to to be a "fetch" as defined in the specification
// this should probably be integrated into the Document's list of cancellable fetches.
self.owner_document()
.fetch_background(request, context, Some(cancel_receiver));
let context = PosterFrameFetchContext::new(self, poster_url, id, request.id);
self.owner_document().fetch_background(request, context);
}
}
@ -338,11 +324,16 @@ struct PosterFrameFetchContext {
resource_timing: ResourceFetchTiming,
/// Url for the resource
url: ServoUrl,
/// A [`FetchCanceller`] for this request.
fetch_canceller: FetchCanceller,
}
impl FetchResponseListener for PosterFrameFetchContext {
fn process_request_body(&mut self, _: RequestId) {}
fn process_request_eof(&mut self, _: RequestId) {}
fn process_request_eof(&mut self, _: RequestId) {
self.fetch_canceller.ignore()
}
fn process_response(
&mut self,
@ -365,11 +356,7 @@ impl FetchResponseListener for PosterFrameFetchContext {
if !status_is_ok {
self.cancelled = true;
self.elem
.root()
.poster_frame_canceller
.borrow_mut()
.cancel();
self.fetch_canceller.cancel();
}
}
@ -433,7 +420,12 @@ impl PreInvoke for PosterFrameFetchContext {
}
impl PosterFrameFetchContext {
fn new(elem: &HTMLVideoElement, url: ServoUrl, id: PendingImageId) -> PosterFrameFetchContext {
fn new(
elem: &HTMLVideoElement,
url: ServoUrl,
id: PendingImageId,
request_id: RequestId,
) -> PosterFrameFetchContext {
let window = elem.owner_window();
PosterFrameFetchContext {
image_cache: window.image_cache(),
@ -442,6 +434,7 @@ impl PosterFrameFetchContext {
cancelled: false,
resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource),
url,
fetch_canceller: FetchCanceller::new(request_id),
}
}
}

View file

@ -18,7 +18,6 @@ use html5ever::serialize::SerializeOpts;
use http::header::{self, HeaderMap, HeaderName, HeaderValue};
use http::Method;
use hyper_serde::Serde;
use ipc_channel::ipc;
use js::jsapi::{Heap, JS_ClearPendingException};
use js::jsval::{JSVal, NullValue};
use js::rust::wrappers::JS_ParseJSON;
@ -291,16 +290,6 @@ impl XMLHttpRequest {
fn sync_in_window(&self) -> bool {
self.sync.get() && self.global().is::<Window>()
}
fn initiate_async_xhr(
context: Arc<Mutex<XHRContext>>,
task_source: SendableTaskSource,
global: &GlobalScope,
init: RequestBuilder,
cancellation_chan: ipc::IpcReceiver<()>,
) {
global.fetch(init, context, task_source, Some(cancellation_chan));
}
}
impl XMLHttpRequestMethods<crate::DomTypeHolder> for XMLHttpRequest {
@ -1548,7 +1537,7 @@ impl XMLHttpRequest {
self.response_status.set(Err(()));
}
fn fetch(&self, init: RequestBuilder, global: &GlobalScope) -> ErrorResult {
fn fetch(&self, request_builder: RequestBuilder, global: &GlobalScope) -> ErrorResult {
let xhr = Trusted::new(self);
let context = Arc::new(Mutex::new(XHRContext {
@ -1556,7 +1545,7 @@ impl XMLHttpRequest {
gen_id: self.generation_id.get(),
sync_status: DomRefCell::new(None),
resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource),
url: init.url.clone(),
url: request_builder.url.clone(),
}));
let (task_source, script_port) = if self.sync.get() {
@ -1577,15 +1566,8 @@ impl XMLHttpRequest {
)
};
let cancel_receiver = self.canceller.borrow_mut().initialize();
XMLHttpRequest::initiate_async_xhr(
context.clone(),
task_source,
global,
init,
cancel_receiver,
);
*self.canceller.borrow_mut() = FetchCanceller::new(request_builder.id);
global.fetch(request_builder, context.clone(), task_source);
if let Some(script_port) = script_port {
loop {

View file

@ -12,9 +12,9 @@ use net_traits::request::{
RequestBuilder, RequestId, RequestMode, ServiceWorkersMode,
};
use net_traits::{
CoreResourceMsg, CoreResourceThread, FetchChannels, FetchMetadata, FetchResponseListener,
FetchResponseMsg, FilteredMetadata, Metadata, NetworkError, ResourceFetchTiming,
ResourceTimingType,
cancel_async_fetch, CoreResourceMsg, CoreResourceThread, FetchChannels, FetchMetadata,
FetchResponseListener, FetchResponseMsg, FilteredMetadata, Metadata, NetworkError,
ResourceFetchTiming, ResourceTimingType,
};
use servo_url::ServoUrl;
@ -52,44 +52,34 @@ struct FetchContext {
/// or let it cancel on Drop in that case.
#[derive(Default, JSTraceable, MallocSizeOf)]
pub(crate) struct FetchCanceller {
#[ignore_malloc_size_of = "channels are hard"]
#[no_trace]
cancel_chan: Option<ipc::IpcSender<()>>,
request_id: Option<RequestId>,
}
impl FetchCanceller {
/// Create an empty FetchCanceller
pub(crate) fn new() -> Self {
Default::default()
}
/// Obtain an IpcReceiver to send over to Fetch, and initialize
/// the internal sender
pub(crate) fn initialize(&mut self) -> ipc::IpcReceiver<()> {
// cancel previous fetch
self.cancel();
let (rx, tx) = ipc::channel().unwrap();
self.cancel_chan = Some(rx);
tx
pub(crate) fn new(request_id: RequestId) -> Self {
Self {
request_id: Some(request_id),
}
}
/// Cancel a fetch if it is ongoing
pub(crate) fn cancel(&mut self) {
if let Some(chan) = self.cancel_chan.take() {
if let Some(request_id) = self.request_id.take() {
// stop trying to make fetch happen
// it's not going to happen
// The receiver will be destroyed if the request has already completed;
// so we throw away the error. Cancellation is a courtesy call,
// No error handling here. Cancellation is a courtesy call,
// we don't actually care if the other side heard.
let _ = chan.send(());
cancel_async_fetch(vec![request_id]);
}
}
/// Use this if you don't want it to send a cancellation request
/// on drop (e.g. if the fetch completes)
pub(crate) fn ignore(&mut self) {
let _ = self.cancel_chan.take();
let _ = self.request_id.take();
}
}
@ -197,7 +187,6 @@ pub(crate) fn Fetch(
request_init,
fetch_context,
global.task_manager().networking_task_source().to_sendable(),
None,
);
// Step 13. Return p.
@ -344,7 +333,7 @@ pub(crate) fn load_whole_resource(
core_resource_thread
.send(CoreResourceMsg::Fetch(
request,
FetchChannels::ResponseMsg(action_sender, None),
FetchChannels::ResponseMsg(action_sender),
))
.unwrap();

View file

@ -112,5 +112,5 @@ pub(crate) fn fetch_image_for_layout(
.pipeline_id(Some(document.global().pipeline_id()));
// Layout image loads do not delay the document load event.
document.fetch_background(request, context, None);
document.fetch_background(request, context);
}

View file

@ -13,7 +13,6 @@ use base::id::{BrowsingContextId, PipelineId, TopLevelBrowsingContextId};
use content_security_policy::Destination;
use crossbeam_channel::Sender;
use http::header;
use ipc_channel::ipc;
use net_traits::request::{CredentialsMode, RedirectMode, RequestBuilder, RequestMode};
use net_traits::response::ResponseInit;
use net_traits::{
@ -55,13 +54,11 @@ impl NavigationListener {
self,
core_resource_thread: &CoreResourceThread,
response_init: Option<ResponseInit>,
cancellation_receiver: Option<ipc::IpcReceiver<()>>,
) {
fetch_async(
core_resource_thread,
self.request_builder.clone(),
response_init,
cancellation_receiver,
self.into_callback(),
);
}

View file

@ -1780,7 +1780,7 @@ fn fetch_single_module_script(
network_listener.into_callback(),
);
},
None => global.fetch_with_network_listener(request, network_listener, None),
None => global.fetch_with_network_listener(request, network_listener),
}
}

View file

@ -139,6 +139,7 @@ use crate::dom::window::Window;
use crate::dom::windowproxy::{CreatorBrowsingContextInfo, WindowProxy};
use crate::dom::worklet::WorkletThreadPool;
use crate::dom::workletglobalscope::WorkletGlobalScopeInit;
use crate::fetch::FetchCanceller;
use crate::messaging::{
CommonScriptMsg, MainThreadScriptMsg, MixedMessage, ScriptEventLoopSender,
ScriptThreadReceivers, ScriptThreadSenders,
@ -3426,17 +3427,10 @@ impl ScriptThread {
.borrow_mut()
.push((incomplete.pipeline_id, context));
let cancellation_receiver = incomplete.canceller.initialize();
NavigationListener::new(
incomplete.request_builder(),
self.senders.self_sender.clone(),
)
.initiate_fetch(
&self.resource_threads.core_thread,
None,
Some(cancellation_receiver),
);
let request_builder = incomplete.request_builder();
incomplete.canceller = FetchCanceller::new(request_builder.id);
NavigationListener::new(request_builder, self.senders.self_sender.clone())
.initiate_fetch(&self.resource_threads.core_thread, None);
self.incomplete_loads.borrow_mut().push(incomplete);
}
@ -3557,12 +3551,9 @@ impl ScriptThread {
.unwrap_or(200),
});
let cancellation_receiver = incomplete_load.canceller.initialize();
NavigationListener::new(request_builder, self.senders.self_sender.clone()).initiate_fetch(
&self.resource_threads.core_thread,
response_init,
Some(cancellation_receiver),
);
incomplete_load.canceller = FetchCanceller::new(request_builder.id);
NavigationListener::new(request_builder, self.senders.self_sender.clone())
.initiate_fetch(&self.resource_threads.core_thread, response_init);
}
/// Synchronously fetch `about:blank`. Stores the `InProgressLoad`
@ -3590,7 +3581,7 @@ impl ScriptThread {
self.incomplete_loads.borrow_mut().push(incomplete);
let dummy_request_id = RequestId::next();
let dummy_request_id = RequestId::default();
context.process_response(dummy_request_id, Ok(FetchMetadata::Unfiltered(meta)));
context.process_response_chunk(dummy_request_id, chunk);
context.process_response_eof(
@ -3614,7 +3605,8 @@ impl ScriptThread {
self.incomplete_loads.borrow_mut().push(incomplete);
let mut context = ParserContext::new(id, url);
let dummy_request_id = RequestId::next();
let dummy_request_id = RequestId::default();
context.process_response(dummy_request_id, Ok(FetchMetadata::Unfiltered(meta)));
context.process_response_chunk(dummy_request_id, chunk);
context.process_response_eof(