diff --git a/components/fonts/font_context.rs b/components/fonts/font_context.rs index 3604dfbae35..bba34146945 100644 --- a/components/fonts/font_context.rs +++ b/components/fonts/font_context.rs @@ -661,7 +661,6 @@ impl RemoteWebFontDownloader { &core_resource_thread_clone, request, None, - None, Box::new(move |response_message| { match downloader.handle_web_font_fetch_message(response_message) { DownloaderResponseResult::InProcess => {}, diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 17e08b6c4f6..71a6be2d5de 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -3,7 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use std::borrow::Cow; -use std::sync::atomic::Ordering; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use std::{io, mem, str}; @@ -15,7 +15,7 @@ use devtools_traits::DevtoolsControlMsg; use headers::{AccessControlExposeHeaders, ContentType, HeaderMapExt}; use http::header::{self, HeaderMap, HeaderName}; use http::{Method, StatusCode}; -use ipc_channel::ipc::{self, IpcReceiver}; +use ipc_channel::ipc; use log::warn; use mime::{self, Mime}; use net_traits::filemanager_thread::{FileTokenCheck, RelativePos}; @@ -59,37 +59,23 @@ pub struct FetchContext { pub devtools_chan: Option>>>, pub filemanager: Arc>, pub file_token: FileTokenCheck, - pub cancellation_listener: Arc>, + pub cancellation_listener: Arc, pub timing: ServoArc>, pub protocols: Arc, } +#[derive(Default)] pub struct CancellationListener { - cancel_chan: Option>, - cancelled: bool, + cancelled: AtomicBool, } impl CancellationListener { - pub fn new(cancel_chan: Option>) -> Self { - Self { - cancel_chan, - cancelled: false, - } + pub(crate) fn cancelled(&self) -> bool { + self.cancelled.load(Ordering::Relaxed) } - pub fn cancelled(&mut self) -> bool { - if let Some(ref cancel_chan) = self.cancel_chan { - if self.cancelled { - true - } else if cancel_chan.try_recv().is_ok() { - self.cancelled = true; - true - } else { - false - } - } else { - false - } + pub(crate) fn cancel(&self) { + self.cancelled.store(true, Ordering::Relaxed) } } pub type DoneChannel = Option<(TokioSender, TokioReceiver)>; diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index fa67529fe1d..b501d845c9b 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -128,7 +128,7 @@ impl FileManager { pub fn fetch_file( &self, done_sender: &mut TokioSender, - cancellation_listener: Arc>, + cancellation_listener: Arc, id: Uuid, file_token: &FileTokenCheck, origin: FileOrigin, @@ -211,7 +211,7 @@ impl FileManager { done_sender: &mut TokioSender, mut reader: BufReader, res_body: ServoArc>, - cancellation_listener: Arc>, + cancellation_listener: Arc, range: RelativePos, ) { let done_sender = done_sender.clone(); @@ -220,7 +220,7 @@ impl FileManager { .map(|pool| { pool.spawn(move || { loop { - if cancellation_listener.lock().unwrap().cancelled() { + if cancellation_listener.cancelled() { *res_body.lock().unwrap() = ResponseBody::Done(vec![]); let _ = done_sender.send(Data::Cancelled); return; @@ -282,7 +282,7 @@ impl FileManager { fn fetch_blob_buf( &self, done_sender: &mut TokioSender, - cancellation_listener: Arc>, + cancellation_listener: Arc, id: &Uuid, file_token: &FileTokenCheck, origin_in: &FileOrigin, diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 5e72fbe11e4..73f2da34cd4 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -1944,7 +1944,7 @@ async fn http_network_fetch( let meta_status = meta.status; let meta_headers = meta.headers; let cancellation_listener = context.cancellation_listener.clone(); - if cancellation_listener.lock().unwrap().cancelled() { + if cancellation_listener.cancelled() { return Response::network_error(NetworkError::Internal("Fetch aborted".into())); } @@ -1983,7 +1983,7 @@ async fn http_network_fetch( warn!("Error streaming response body: {:?}", e); }) .try_fold(res_body, move |res_body, chunk| { - if cancellation_listener.lock().unwrap().cancelled() { + if cancellation_listener.cancelled() { *res_body.lock().unwrap() = ResponseBody::Done(vec![]); let _ = done_sender.send(Data::Cancelled); return future::ready(Err(())); diff --git a/components/net/resource_thread.rs b/components/net/resource_thread.rs index 5cb56c2399d..38a5112ec70 100644 --- a/components/net/resource_thread.rs +++ b/components/net/resource_thread.rs @@ -10,7 +10,7 @@ use std::fs::File; use std::io::prelude::*; use std::io::{self, BufReader}; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex, RwLock}; +use std::sync::{Arc, Mutex, RwLock, Weak}; use std::thread; use std::time::Duration; @@ -24,7 +24,7 @@ use log::{debug, warn}; use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use net_traits::blob_url_store::parse_blob_url; use net_traits::filemanager_thread::FileTokenCheck; -use net_traits::request::{Destination, RequestBuilder}; +use net_traits::request::{Destination, RequestBuilder, RequestId}; use net_traits::response::{Response, ResponseInit}; use net_traits::storage_thread::StorageThreadMsg; use net_traits::{ @@ -142,6 +142,7 @@ pub fn new_core_resource_thread( config_dir, ca_certificates, ignore_certificate_errors, + cancellation_listeners: Default::default(), }; mem_profiler_chan.run_with_memory_reporting( @@ -168,6 +169,7 @@ struct ResourceChannelManager { config_dir: Option, ca_certificates: CACertificates, ignore_certificate_errors: bool, + cancellation_listeners: HashMap>, } fn create_http_states( @@ -300,6 +302,30 @@ impl ResourceChannelManager { msg.send(vec![public_report, private_report]); } + fn cancellation_listener(&self, request_id: RequestId) -> Option> { + self.cancellation_listeners + .get(&request_id) + .and_then(Weak::upgrade) + } + + fn get_or_create_cancellation_listener( + &mut self, + request_id: RequestId, + ) -> Arc { + if let Some(listener) = self.cancellation_listener(request_id) { + return listener; + } + + // Clear away any cancellation listeners that are no longer valid. + self.cancellation_listeners + .retain(|_, listener| listener.strong_count() > 0); + + let cancellation_listener = Arc::new(Default::default()); + self.cancellation_listeners + .insert(request_id, Arc::downgrade(&cancellation_listener)); + cancellation_listener + } + /// Returns false if the thread should exit. fn process_msg( &mut self, @@ -308,33 +334,45 @@ impl ResourceChannelManager { protocols: Arc, ) -> bool { match msg { - CoreResourceMsg::Fetch(req_init, channels) => match channels { - FetchChannels::ResponseMsg(sender, cancel_chan) => self.resource_manager.fetch( - req_init, - None, - sender, - http_state, - cancel_chan, - protocols, - ), + CoreResourceMsg::Fetch(request_builder, channels) => match channels { + FetchChannels::ResponseMsg(sender) => { + let cancellation_listener = + self.get_or_create_cancellation_listener(request_builder.id); + self.resource_manager.fetch( + request_builder, + None, + sender, + http_state, + cancellation_listener, + protocols, + ); + }, FetchChannels::WebSocket { event_sender, action_receiver, } => self.resource_manager.websocket_connect( - req_init, + request_builder, event_sender, action_receiver, http_state, ), FetchChannels::Prefetch => self.resource_manager.fetch( - req_init, + request_builder, None, DiscardFetch, http_state, - None, + Arc::new(Default::default()), protocols, ), }, + CoreResourceMsg::Cancel(request_ids) => { + for cancellation_listener in request_ids + .into_iter() + .filter_map(|request_id| self.cancellation_listener(request_id)) + { + cancellation_listener.cancel(); + } + }, CoreResourceMsg::DeleteCookies(request) => { http_state .cookie_jar @@ -343,13 +381,15 @@ impl ResourceChannelManager { .clear_storage(&request); return true; }, - CoreResourceMsg::FetchRedirect(req_init, res_init, sender, cancel_chan) => { + CoreResourceMsg::FetchRedirect(request_builder, res_init, sender) => { + let cancellation_listener = + self.get_or_create_cancellation_listener(request_builder.id); self.resource_manager.fetch( - req_init, + request_builder, Some(res_init), sender, http_state, - cancel_chan, + cancellation_listener, protocols, ) }, @@ -698,7 +738,7 @@ impl CoreResourceManager { res_init_: Option, mut sender: Target, http_state: &Arc, - cancel_chan: Option>, + cancellation_listener: Arc, protocols: Arc, ) { let http_state = http_state.clone(); @@ -746,7 +786,7 @@ impl CoreResourceManager { devtools_chan: dc.map(|dc| Arc::new(Mutex::new(dc))), filemanager: Arc::new(Mutex::new(filemanager)), file_token, - cancellation_listener: Arc::new(Mutex::new(CancellationListener::new(cancel_chan))), + cancellation_listener, timing: ServoArc::new(Mutex::new(ResourceFetchTiming::new(request.timing_type()))), protocols, }; diff --git a/components/net/tests/fetch.rs b/components/net/tests/fetch.rs index eef8ba89a66..30460a55d38 100644 --- a/components/net/tests/fetch.rs +++ b/components/net/tests/fetch.rs @@ -26,7 +26,7 @@ use hyper::body::{Bytes, Incoming}; use hyper::{Request as HyperRequest, Response as HyperResponse}; use mime::{self, Mime}; use net::fetch::cors_cache::CorsCache; -use net::fetch::methods::{self, CancellationListener, FetchContext}; +use net::fetch::methods::{self, FetchContext}; use net::filemanager_thread::FileManager; use net::hsts::HstsEntry; use net::protocols::ProtocolRegistry; @@ -702,7 +702,7 @@ fn test_fetch_with_hsts() { Weak::new(), ))), file_token: FileTokenCheck::NotRequired, - cancellation_listener: Arc::new(Mutex::new(CancellationListener::new(None))), + cancellation_listener: Arc::new(Default::default()), timing: ServoArc::new(Mutex::new(ResourceFetchTiming::new( ResourceTimingType::Navigation, ))), @@ -759,7 +759,7 @@ fn test_load_adds_host_to_hsts_list_when_url_is_https() { Weak::new(), ))), file_token: FileTokenCheck::NotRequired, - cancellation_listener: Arc::new(Mutex::new(CancellationListener::new(None))), + cancellation_listener: Arc::new(Default::default()), timing: ServoArc::new(Mutex::new(ResourceFetchTiming::new( ResourceTimingType::Navigation, ))), @@ -818,7 +818,7 @@ fn test_fetch_self_signed() { Weak::new(), ))), file_token: FileTokenCheck::NotRequired, - cancellation_listener: Arc::new(Mutex::new(CancellationListener::new(None))), + cancellation_listener: Arc::new(Default::default()), timing: ServoArc::new(Mutex::new(ResourceFetchTiming::new( ResourceTimingType::Navigation, ))), diff --git a/components/net/tests/main.rs b/components/net/tests/main.rs index 934f0e2102e..e8c576682fe 100644 --- a/components/net/tests/main.rs +++ b/components/net/tests/main.rs @@ -39,7 +39,7 @@ use hyper::{Request as HyperRequest, Response as HyperResponse}; use hyper_util::rt::tokio::TokioIo; use net::connector::{create_http_client, create_tls_config}; use net::fetch::cors_cache::CorsCache; -use net::fetch::methods::{self, CancellationListener, FetchContext}; +use net::fetch::methods::{self, FetchContext}; use net::filemanager_thread::FileManager; use net::protocols::ProtocolRegistry; use net::resource_thread::CoreResourceThreadPool; @@ -183,7 +183,7 @@ fn new_fetch_context( pool_handle.unwrap_or_else(|| Weak::new()), ))), file_token: FileTokenCheck::NotRequired, - cancellation_listener: Arc::new(Mutex::new(CancellationListener::new(None))), + cancellation_listener: Arc::new(Default::default()), timing: ServoArc::new(Mutex::new(ResourceFetchTiming::new( ResourceTimingType::Navigation, ))), diff --git a/components/script/document_loader.rs b/components/script/document_loader.rs index 06e5cfff7ca..f4f6970f6d5 100644 --- a/components/script/document_loader.rs +++ b/components/script/document_loader.rs @@ -6,7 +6,6 @@ //! //! -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>, ) { - 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. diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index fa0eaf7053e..11602547b7e 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -2150,7 +2150,6 @@ impl Document { &self, request: RequestBuilder, listener: Listener, - cancel_override: Option>, ) { 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 diff --git a/components/script/dom/eventsource.rs b/components/script/dom/eventsource.rs index 39adc36332c..1d68817ace5 100644 --- a/components/script/dom/eventsource.rs +++ b/components/script/dom/eventsource.rs @@ -602,12 +602,12 @@ impl EventSourceMethods 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(); } diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index d336843a1f9..e9b1cbcbda5 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -3242,13 +3242,12 @@ impl GlobalScope { request_builder: RequestBuilder, context: Arc>, task_source: SendableTaskSource, - cancellation_sender: Option>, ) { 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, - cancellation_receiver: Option>, ) { fetch_async( &self.core_resource_thread(), request_builder, None, - cancellation_receiver, network_listener.into_callback(), ); } diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index 268cd2985f1..1cbce12ebad 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -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. diff --git a/components/script/dom/htmllinkelement.rs b/components/script/dom/htmllinkelement.rs index 93d38a52aa4..c676c048846 100644 --- a/components/script/dom/htmllinkelement.rs +++ b/components/script/dom/htmllinkelement.rs @@ -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); } /// diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index 87526b4950d..22fb9084189 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -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 { diff --git a/components/script/dom/htmlvideoelement.rs b/components/script/dom/htmlvideoelement.rs index 5291f6c4dbb..d8668dcc859 100644 --- a/components/script/dom/htmlvideoelement.rs +++ b/components/script/dom/htmlvideoelement.rs @@ -54,8 +54,6 @@ pub(crate) struct HTMLVideoElement { video_height: Cell>, /// Incremented whenever tasks associated with this element are cancelled. generation_id: Cell, - /// Poster frame fetch request canceller. - poster_frame_canceller: DomRefCell, /// Load event blocker. Will block the load event while the poster frame /// is being fetched. load_blocker: DomRefCell>, @@ -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 { /// 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); }, _ => (), } } /// - 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), } } } diff --git a/components/script/dom/xmlhttprequest.rs b/components/script/dom/xmlhttprequest.rs index 451176af6d9..ff249653691 100644 --- a/components/script/dom/xmlhttprequest.rs +++ b/components/script/dom/xmlhttprequest.rs @@ -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::() } - - fn initiate_async_xhr( - context: Arc>, - task_source: SendableTaskSource, - global: &GlobalScope, - init: RequestBuilder, - cancellation_chan: ipc::IpcReceiver<()>, - ) { - global.fetch(init, context, task_source, Some(cancellation_chan)); - } } impl XMLHttpRequestMethods 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 { diff --git a/components/script/fetch.rs b/components/script/fetch.rs index 428f4f86e4e..de8c546971b 100644 --- a/components/script/fetch.rs +++ b/components/script/fetch.rs @@ -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>, + request_id: Option, } 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(); diff --git a/components/script/layout_image.rs b/components/script/layout_image.rs index 4a222046462..cbfc68943ab 100644 --- a/components/script/layout_image.rs +++ b/components/script/layout_image.rs @@ -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); } diff --git a/components/script/navigation.rs b/components/script/navigation.rs index 109b079239f..1ec54ad397e 100644 --- a/components/script/navigation.rs +++ b/components/script/navigation.rs @@ -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, - cancellation_receiver: Option>, ) { fetch_async( core_resource_thread, self.request_builder.clone(), response_init, - cancellation_receiver, self.into_callback(), ); } diff --git a/components/script/script_module.rs b/components/script/script_module.rs index 0651983c105..adc805cd86f 100644 --- a/components/script/script_module.rs +++ b/components/script/script_module.rs @@ -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), } } diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index f7ff5c14dad..a4704cb2b42 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -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( diff --git a/components/shared/net/lib.rs b/components/shared/net/lib.rs index 99be7b007f7..bbdad23fa6f 100644 --- a/components/shared/net/lib.rs +++ b/components/shared/net/lib.rs @@ -464,10 +464,7 @@ pub enum WebSocketNetworkEvent { #[derive(Debug, Deserialize, Serialize)] /// IPC channels to communicate with the script thread about network or DOM events. pub enum FetchChannels { - ResponseMsg( - IpcSender, - /* cancel_chan */ Option>, - ), + ResponseMsg(IpcSender), WebSocket { event_sender: IpcSender, action_receiver: IpcReceiver, @@ -480,13 +477,9 @@ pub enum FetchChannels { #[derive(Debug, Deserialize, Serialize)] pub enum CoreResourceMsg { Fetch(RequestBuilder, FetchChannels), + Cancel(Vec), /// Initiate a fetch in response to processing a redirection - FetchRedirect( - RequestBuilder, - ResponseInit, - IpcSender, - /* cancel_chan */ Option>, - ), + FetchRedirect(RequestBuilder, ResponseInit, IpcSender), /// Store a cookie for a given originating URL SetCookieForUrl(ServoUrl, Serde>, CookieSource), /// Store a set of cookies for a given originating URL @@ -522,10 +515,10 @@ pub enum CoreResourceMsg { // FIXME: https://github.com/servo/servo/issues/34591 #[expect(clippy::large_enum_variant)] enum ToFetchThreadMessage { + Cancel(Vec), StartFetch( /* request_builder */ RequestBuilder, /* response_init */ Option, - /* cancel_chan */ Option>, /* callback */ BoxedFetchCallback, ), FetchResponse(FetchResponseMsg), @@ -584,12 +577,7 @@ impl FetchThread { fn run(&mut self) { loop { match self.receiver.recv().unwrap() { - ToFetchThreadMessage::StartFetch( - request_builder, - response_init, - canceller, - callback, - ) => { + ToFetchThreadMessage::StartFetch(request_builder, response_init, callback) => { self.active_fetches.insert(request_builder.id, callback); // Only redirects have a `response_init` field. @@ -598,11 +586,10 @@ impl FetchThread { request_builder, response_init, self.to_fetch_sender.clone(), - canceller, ), None => CoreResourceMsg::Fetch( request_builder, - FetchChannels::ResponseMsg(self.to_fetch_sender.clone(), canceller), + FetchChannels::ResponseMsg(self.to_fetch_sender.clone()), ), }; @@ -623,6 +610,14 @@ impl FetchThread { self.active_fetches.remove(&request_id); } }, + ToFetchThreadMessage::Cancel(request_ids) => { + // Errors are ignored here, because Servo sends many cancellation requests when shutting down. + // At this point the networking task might be shut down completely, so just ignore errors + // during this time. + let _ = self + .core_resource_thread + .send(CoreResourceMsg::Cancel(request_ids)); + }, } } } @@ -635,7 +630,6 @@ pub fn fetch_async( core_resource_thread: &CoreResourceThread, request: RequestBuilder, response_init: Option, - canceller: Option>, callback: BoxedFetchCallback, ) { let _ = FETCH_THREAD @@ -643,11 +637,19 @@ pub fn fetch_async( .send(ToFetchThreadMessage::StartFetch( request, response_init, - canceller, callback, )); } +/// Instruct the resource thread to cancel an existing request. Does nothing if the +/// request has already completed or has not been fetched yet. +pub fn cancel_async_fetch(request_ids: Vec) { + let _ = FETCH_THREAD + .get() + .expect("Tried to cancel request in process that hasn't started one.") + .send(ToFetchThreadMessage::Cancel(request_ids)); +} + #[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)] pub struct ResourceCorsData { /// CORS Preflight flag diff --git a/components/shared/net/request.rs b/components/shared/net/request.rs index 4b697af6be9..fff5dbc0836 100644 --- a/components/shared/net/request.rs +++ b/components/shared/net/request.rs @@ -2,7 +2,6 @@ * 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/. */ -use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use base::id::{PipelineId, TopLevelBrowsingContextId}; @@ -14,6 +13,7 @@ use malloc_size_of_derive::MallocSizeOf; use mime::Mime; use serde::{Deserialize, Serialize}; use servo_url::{ImmutableOrigin, ServoUrl}; +use uuid::Uuid; use crate::policy_container::{PolicyContainer, RequestPolicyContainer}; use crate::response::HttpsState; @@ -21,12 +21,11 @@ use crate::{ReferrerPolicy, ResourceTimingType}; #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)] /// An id to differeniate one network request from another. -pub struct RequestId(usize); +pub struct RequestId(Uuid); -impl RequestId { - pub fn next() -> Self { - static NEXT_REQUEST_ID: AtomicUsize = AtomicUsize::new(0); - Self(NEXT_REQUEST_ID.fetch_add(1, Ordering::Relaxed)) +impl Default for RequestId { + fn default() -> Self { + Self(servo_rand::random_uuid()) } } @@ -283,7 +282,7 @@ pub struct RequestBuilder { impl RequestBuilder { pub fn new(url: ServoUrl, referrer: Referrer) -> RequestBuilder { RequestBuilder { - id: RequestId::next(), + id: RequestId::default(), method: Method::GET, url, headers: HeaderMap::new(), @@ -471,8 +470,9 @@ impl RequestBuilder { /// the Fetch spec. #[derive(Clone, MallocSizeOf)] pub struct Request { - /// The id of this request so that the task that triggered it can route - /// messages to the correct listeners. + /// The unique id of this request so that the task that triggered it can route + /// messages to the correct listeners. This is a UUID that is generated when a request + /// is being built. pub id: RequestId, /// #[ignore_malloc_size_of = "Defined in hyper"]