From f2ee941da2f56b22d2258057a83998306e862350 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Sun, 1 Jul 2018 20:30:05 +0800 Subject: [PATCH] Introduce DOMTracker, cancel eventsource fetch when aborting document load --- .../script/dom/bindings/codegen/Bindings.conf | 4 ++ components/script/dom/bindings/weakref.rs | 32 +++++++++++ components/script/dom/document.rs | 5 +- components/script/dom/eventsource.rs | 54 ++++++++++++++----- components/script/dom/globalscope.rs | 25 +++++++++ .../eventsource-request-cancellation.htm.ini | 5 -- 6 files changed, 105 insertions(+), 20 deletions(-) delete mode 100644 tests/wpt/metadata/eventsource/eventsource-request-cancellation.htm.ini diff --git a/components/script/dom/bindings/codegen/Bindings.conf b/components/script/dom/bindings/codegen/Bindings.conf index 2f0b8e7b66b1..b97d199147b4 100644 --- a/components/script/dom/bindings/codegen/Bindings.conf +++ b/components/script/dom/bindings/codegen/Bindings.conf @@ -26,6 +26,10 @@ DOMInterfaces = { 'weakReferenceable': True, }, +'EventSource': { + 'weakReferenceable': True, +}, + #FIXME(jdm): This should be 'register': False, but then we don't generate enum types 'TestBinding': {}, diff --git a/components/script/dom/bindings/weakref.rs b/components/script/dom/bindings/weakref.rs index e4e5f00103f9..abb1e07bc08c 100644 --- a/components/script/dom/bindings/weakref.rs +++ b/components/script/dom/bindings/weakref.rs @@ -11,6 +11,7 @@ //! slot. When all associated `WeakRef` values are dropped, the //! `WeakBox` itself is dropped too. +use dom::bindings::cell::DomRefCell; use dom::bindings::reflector::DomObject; use dom::bindings::root::DomRoot; use dom::bindings::trace::JSTraceable; @@ -286,3 +287,34 @@ impl<'a, T: WeakReferenceable + 'a> Drop for WeakRefEntry<'a, T> { *self.index += 1; } } + +#[derive(MallocSizeOf)] +pub struct DOMTracker { + dom_objects: DomRefCell> +} + +impl DOMTracker { + pub fn new() -> Self { + Self { + dom_objects: DomRefCell::new(WeakRefVec::new()) + } + } + + pub fn track(&self, dom_object: &T) { + self.dom_objects.borrow_mut().push(WeakRef::new(dom_object)); + } + + pub fn for_each)>(&self, mut f: F) { + self.dom_objects.borrow_mut().update(|weak_ref| { + let root = weak_ref.root().unwrap(); + f(root); + }); + } +} + +#[allow(unsafe_code)] +unsafe impl JSTraceable for DOMTracker { + unsafe fn trace(&self, _: *mut JSTracer) { + self.dom_objects.borrow_mut().retain_alive(); + } +} diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 1ff29786491a..4c9fddbeb5ee 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -2029,7 +2029,10 @@ impl Document { *self.asap_scripts_set.borrow_mut() = vec![]; self.asap_in_order_scripts_list.clear(); self.deferred_scripts.clear(); - if self.loader.borrow_mut().cancel_all_loads() { + let global_scope = self.window.upcast::(); + let loads_cancelled = self.loader.borrow_mut().cancel_all_loads(); + let event_sources_canceled = global_scope.close_event_sources(); + if loads_cancelled || event_sources_canceled { // If any loads were canceled. self.salvageable.set(false); }; diff --git a/components/script/dom/eventsource.rs b/components/script/dom/eventsource.rs index 48b762978ca0..072a3dd00803 100644 --- a/components/script/dom/eventsource.rs +++ b/components/script/dom/eventsource.rs @@ -16,6 +16,7 @@ use dom::globalscope::GlobalScope; use dom::messageevent::MessageEvent; use dom_struct::dom_struct; use euclid::Length; +use fetch::FetchCanceller; use hyper::header::{Accept, qitem}; use ipc_channel::ipc; use ipc_channel::router::ROUTER; @@ -64,6 +65,7 @@ pub struct EventSource { ready_state: Cell, with_credentials: bool, + canceller: DomRefCell, } enum ParserState { @@ -118,19 +120,7 @@ impl EventSourceContext { if self.gen_id != event_source.generation_id.get() { return; } - let global = event_source.global(); - let event_source = self.event_source.clone(); - // FIXME(nox): Why are errors silenced here? - let _ = global.remote_event_task_source().queue( - task!(fail_the_event_source_connection: move || { - let event_source = event_source.root(); - if event_source.ready_state.get() != ReadyState::Closed { - event_source.ready_state.set(ReadyState::Closed); - event_source.upcast::().fire_event(atom!("error")); - } - }), - &global, - ); + event_source.fail_the_connection(); } // https://html.spec.whatwg.org/multipage/#reestablish-the-connection @@ -410,6 +400,7 @@ impl EventSource { ready_state: Cell::new(ReadyState::Connecting), with_credentials: with_credentials, + canceller: DomRefCell::new(Default::default()) } } @@ -419,6 +410,29 @@ impl EventSource { Wrap) } + // https://html.spec.whatwg.org/multipage/#sse-processing-model:fail-the-connection-3 + pub fn cancel(&self) { + self.canceller.borrow_mut().cancel(); + self.fail_the_connection(); + } + + /// + pub fn fail_the_connection(&self) { + let global = self.global(); + let event_source = Trusted::new(self); + // FIXME(nox): Why are errors silenced here? + let _ = global.remote_event_task_source().queue( + task!(fail_the_event_source_connection: move || { + let event_source = event_source.root(); + if event_source.ready_state.get() != ReadyState::Closed { + event_source.ready_state.set(ReadyState::Closed); + event_source.upcast::().fire_event(atom!("error")); + } + }), + &global, + ); + } + pub fn request(&self) -> RequestInit { self.request.borrow().clone().unwrap() } @@ -437,6 +451,7 @@ impl EventSource { }; // Step 1, 5 let ev = EventSource::new(global, url_record.clone(), event_source_init.withCredentials); + global.track_event_source(&ev); // Steps 6-7 let cors_attribute_state = if event_source_init.withCredentials { CorsSettings::UseCredentials @@ -491,13 +506,23 @@ impl EventSource { ROUTER.add_route(action_receiver.to_opaque(), Box::new(move |message| { listener.notify_fetch(message.to().unwrap()); })); + let cancel_receiver = ev.canceller.borrow_mut().initialize(); global.core_resource_thread().send( - CoreResourceMsg::Fetch(request, FetchChannels::ResponseMsg(action_sender, None))).unwrap(); + CoreResourceMsg::Fetch(request, FetchChannels::ResponseMsg(action_sender, Some(cancel_receiver)))).unwrap(); // Step 13 Ok(ev) } } +// https://html.spec.whatwg.org/multipage/#garbage-collection-2 +impl Drop for EventSource { + fn drop(&mut self) { + // If an EventSource object is garbage collected while its connection is still open, + // the user agent must abort any instance of the fetch algorithm opened by this EventSource. + self.canceller.borrow_mut().cancel(); + } +} + impl EventSourceMethods for EventSource { // https://html.spec.whatwg.org/multipage/#handler-eventsource-onopen event_handler!(open, GetOnopen, SetOnopen); @@ -527,6 +552,7 @@ impl EventSourceMethods for EventSource { fn Close(&self) { let GenerationId(prev_id) = self.generation_id.get(); self.generation_id.set(GenerationId(prev_id + 1)); + self.canceller.borrow_mut().cancel(); self.ready_state.set(ReadyState::Closed); } } diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index 08e7b2d3d628..325e05de8822 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -4,6 +4,7 @@ use devtools_traits::{ScriptToDevtoolsControlMsg, WorkerId}; use dom::bindings::cell::DomRefCell; +use dom::bindings::codegen::Bindings::EventSourceBinding::EventSourceBinding::EventSourceMethods; use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods; use dom::bindings::codegen::Bindings::WorkerGlobalScopeBinding::WorkerGlobalScopeMethods; use dom::bindings::conversions::root_from_object; @@ -13,10 +14,12 @@ use dom::bindings::reflector::DomObject; use dom::bindings::root::{DomRoot, MutNullableDom}; use dom::bindings::settings_stack::{AutoEntryScript, entry_global, incumbent_global}; use dom::bindings::str::DOMString; +use dom::bindings::weakref::DOMTracker; use dom::crypto::Crypto; use dom::dedicatedworkerglobalscope::DedicatedWorkerGlobalScope; use dom::errorevent::ErrorEvent; use dom::event::{Event, EventBubbles, EventCancelable, EventStatus}; +use dom::eventsource::EventSource; use dom::eventtarget::EventTarget; use dom::performance::Performance; use dom::window::Window; @@ -131,6 +134,9 @@ pub struct GlobalScope { /// Vector storing closing references of all workers #[ignore_malloc_size_of = "Arc"] list_auto_close_worker: DomRefCell>, + + /// Vector storing references of all eventsources. + event_source_tracker: DOMTracker, } impl GlobalScope { @@ -164,6 +170,7 @@ impl GlobalScope { origin, microtask_queue, list_auto_close_worker: Default::default(), + event_source_tracker: DOMTracker::new(), } } @@ -171,6 +178,24 @@ impl GlobalScope { self.list_auto_close_worker.borrow_mut().push(AutoCloseWorker(closing_worker)); } + pub fn track_event_source(&self, event_source: &EventSource) { + self.event_source_tracker.track(event_source); + } + + pub fn close_event_sources(&self) -> bool { + let mut canceled_any_fetch = false; + self.event_source_tracker.for_each(|event_source: DomRoot| { + match event_source.ReadyState() { + 2 => {}, + _ => { + event_source.cancel(); + canceled_any_fetch = true; + } + } + }); + canceled_any_fetch + } + /// Returns the global scope of the realm that the given DOM object's reflector /// was created in. #[allow(unsafe_code)] diff --git a/tests/wpt/metadata/eventsource/eventsource-request-cancellation.htm.ini b/tests/wpt/metadata/eventsource/eventsource-request-cancellation.htm.ini deleted file mode 100644 index 94f8dc31936a..000000000000 --- a/tests/wpt/metadata/eventsource/eventsource-request-cancellation.htm.ini +++ /dev/null @@ -1,5 +0,0 @@ -[eventsource-request-cancellation.htm] - type: testharness - [EventSource: request cancellation] - expected: FAIL -