From 9ddeec09c39c1931843e61bd2f5d552131dfa67f Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 14 Sep 2015 17:04:40 +0200 Subject: [PATCH 01/14] Remove outdated FIXME comment. --- components/script/dom/eventdispatcher.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 7ea4a706fea..05133f35304 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -37,8 +37,6 @@ pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, event.set_phase(EventPhase::Capturing); - //FIXME: The "callback this value" should be currentTarget - /* capturing */ for cur_target in chain.r().iter().rev() { let stopped = match cur_target.get_listeners_for(&type_, ListenerPhase::Capturing) { From 15717173eed3e3a2eba05cac2b927c984b1b1505 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 14 Sep 2015 17:07:09 +0200 Subject: [PATCH 02/14] Reorder the code in dispatch_event a bit. --- components/script/dom/eventdispatcher.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 05133f35304..33c1a5c8314 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -24,8 +24,6 @@ pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, }); event.set_dispatching(true); - let type_ = event.Type(); - //TODO: no chain if not participating in a tree let mut chain: RootedVec> = RootedVec::new(); if let Some(target_node) = NodeCast::to_ref(target) { @@ -35,9 +33,10 @@ pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, } } - event.set_phase(EventPhase::Capturing); + let type_ = event.Type(); /* capturing */ + event.set_phase(EventPhase::Capturing); for cur_target in chain.r().iter().rev() { let stopped = match cur_target.get_listeners_for(&type_, ListenerPhase::Capturing) { Some(listeners) => { From 76eea43c83be8269631595edecccf7427d447dbe Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 14 Sep 2015 17:08:00 +0200 Subject: [PATCH 03/14] Factor out a dispatch_to_listeners function from dispatch_event. --- components/script/dom/eventdispatcher.rs | 50 +++++++++++++----------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 33c1a5c8314..3e1b9d83d7b 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -12,32 +12,12 @@ use dom::eventtarget::{EventTarget, ListenerPhase}; use dom::node::Node; use dom::virtualmethods::vtable_for; -// See https://dom.spec.whatwg.org/#concept-event-dispatch for the full dispatch algorithm -pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, - event: &Event) -> bool { - assert!(!event.dispatching()); - assert!(event.initialized()); - - event.set_target(match pseudo_target { - Some(pseudo_target) => pseudo_target, - None => target.clone(), - }); - event.set_dispatching(true); - - //TODO: no chain if not participating in a tree - let mut chain: RootedVec> = RootedVec::new(); - if let Some(target_node) = NodeCast::to_ref(target) { - for ancestor in target_node.ancestors() { - let ancestor_target = EventTargetCast::from_ref(ancestor.r()); - chain.push(JS::from_ref(ancestor_target)) - } - } - +fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTarget]) { let type_ = event.Type(); /* capturing */ event.set_phase(EventPhase::Capturing); - for cur_target in chain.r().iter().rev() { + for cur_target in chain.iter().rev() { let stopped = match cur_target.get_listeners_for(&type_, ListenerPhase::Capturing) { Some(listeners) => { event.set_current_target(cur_target); @@ -82,7 +62,7 @@ pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, if event.bubbles() && !event.stop_propagation() { event.set_phase(EventPhase::Bubbling); - for cur_target in chain.r() { + for cur_target in chain { let stopped = match cur_target.get_listeners_for(&type_, ListenerPhase::Bubbling) { Some(listeners) => { event.set_current_target(cur_target); @@ -104,6 +84,30 @@ pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, } } } +} + +// See https://dom.spec.whatwg.org/#concept-event-dispatch for the full dispatch algorithm +pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, + event: &Event) -> bool { + assert!(!event.dispatching()); + assert!(event.initialized()); + + event.set_target(match pseudo_target { + Some(pseudo_target) => pseudo_target, + None => target.clone(), + }); + event.set_dispatching(true); + + //TODO: no chain if not participating in a tree + let mut chain: RootedVec> = RootedVec::new(); + if let Some(target_node) = NodeCast::to_ref(target) { + for ancestor in target_node.ancestors() { + let ancestor_target = EventTargetCast::from_ref(ancestor.r()); + chain.push(JS::from_ref(ancestor_target)) + } + } + + dispatch_to_listeners(event, target, chain.r()); /* default action */ let target = event.GetTarget(); From b342dff07dbbfc4a12fdf1178076d7a77b989d2d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 16 Sep 2015 14:48:36 +0200 Subject: [PATCH 04/14] Add more assertions to dispatch_event. --- components/script/dom/event.rs | 7 ++++++- components/script/dom/eventdispatcher.rs | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/components/script/dom/event.rs b/components/script/dom/event.rs index d45407a51fe..2cec78b48cc 100644 --- a/components/script/dom/event.rs +++ b/components/script/dom/event.rs @@ -19,7 +19,7 @@ use std::default::Default; use time; -#[derive(JSTraceable, Copy, Clone)] +#[derive(JSTraceable, Copy, Clone, Debug, PartialEq, Eq)] #[repr(u16)] #[derive(HeapSizeOf)] pub enum EventPhase { @@ -136,6 +136,11 @@ impl Event { self.target.set(Some(JS::from_ref(val))); } + #[inline] + pub fn phase(&self) -> EventPhase { + self.phase.get() + } + #[inline] pub fn set_phase(&self, val: EventPhase) { self.phase.set(val) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 3e1b9d83d7b..33c07c69bb0 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -91,6 +91,8 @@ pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, event: &Event) -> bool { assert!(!event.dispatching()); assert!(event.initialized()); + assert_eq!(event.phase(), EventPhase::None); + assert!(event.GetCurrentTarget().is_none()); event.set_target(match pseudo_target { Some(pseudo_target) => pseudo_target, From a5925020ce2752192ff45a401e86205dbbcb65f1 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 16 Sep 2015 14:54:26 +0200 Subject: [PATCH 05/14] Avoid calling any listeners for events whose stop propagation flag is set prior to dispatching. --- components/script/dom/eventdispatcher.rs | 8 ++++++++ .../tests/approved/stopPropagation.dispatchEvent.html.ini | 5 ----- .../converted/stopPropagation.dispatchEvent.html.ini | 5 ----- 3 files changed, 8 insertions(+), 10 deletions(-) delete mode 100644 tests/wpt/metadata/DOMEvents/tests/approved/stopPropagation.dispatchEvent.html.ini delete mode 100644 tests/wpt/metadata/DOMEvents/tests/submissions/Microsoft/converted/stopPropagation.dispatchEvent.html.ini diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 33c07c69bb0..13ccf5be4b9 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -13,6 +13,9 @@ use dom::node::Node; use dom::virtualmethods::vtable_for; fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTarget]) { + assert!(!event.stop_propagation()); + assert!(!event.stop_immediate()); + let type_ = event.Type(); /* capturing */ @@ -98,6 +101,11 @@ pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, Some(pseudo_target) => pseudo_target, None => target.clone(), }); + + if event.stop_propagation() { + return !event.DefaultPrevented(); + } + event.set_dispatching(true); //TODO: no chain if not participating in a tree diff --git a/tests/wpt/metadata/DOMEvents/tests/approved/stopPropagation.dispatchEvent.html.ini b/tests/wpt/metadata/DOMEvents/tests/approved/stopPropagation.dispatchEvent.html.ini deleted file mode 100644 index ad9ce52a5f2..00000000000 --- a/tests/wpt/metadata/DOMEvents/tests/approved/stopPropagation.dispatchEvent.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[stopPropagation.dispatchEvent.html] - type: testharness - [Test Description: If Event.stopPropagation() has been called prior to the dispatch, all phases must be skipped.] - expected: FAIL - diff --git a/tests/wpt/metadata/DOMEvents/tests/submissions/Microsoft/converted/stopPropagation.dispatchEvent.html.ini b/tests/wpt/metadata/DOMEvents/tests/submissions/Microsoft/converted/stopPropagation.dispatchEvent.html.ini deleted file mode 100644 index ad9ce52a5f2..00000000000 --- a/tests/wpt/metadata/DOMEvents/tests/submissions/Microsoft/converted/stopPropagation.dispatchEvent.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[stopPropagation.dispatchEvent.html] - type: testharness - [Test Description: If Event.stopPropagation() has been called prior to the dispatch, all phases must be skipped.] - expected: FAIL - From 995cb21b481366f21aeb599236e68083d5c28781 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 16 Sep 2015 13:41:33 +0200 Subject: [PATCH 06/14] Use if-let in dispatch_to_listeners, and simplify break conditions. --- components/script/dom/eventdispatcher.rs | 54 ++++++++++-------------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 13ccf5be4b9..51617a49a2d 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -21,25 +21,20 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar /* capturing */ event.set_phase(EventPhase::Capturing); for cur_target in chain.iter().rev() { - let stopped = match cur_target.get_listeners_for(&type_, ListenerPhase::Capturing) { - Some(listeners) => { - event.set_current_target(cur_target); - for listener in &listeners { - // Explicitly drop any exception on the floor. - listener.call_or_handle_event(*cur_target, event, Report); + if let Some(listeners) = cur_target.get_listeners_for(&type_, ListenerPhase::Capturing) { + event.set_current_target(cur_target); + for listener in &listeners { + // Explicitly drop any exception on the floor. + listener.call_or_handle_event(*cur_target, event, Report); - if event.stop_immediate() { - break; - } + if event.stop_immediate() { + break; } - - event.stop_propagation() } - None => false - }; - if stopped { - break; + if event.stop_propagation() { + break; + } } } @@ -48,8 +43,7 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar event.set_phase(EventPhase::AtTarget); event.set_current_target(target.clone()); - let opt_listeners = target.get_listeners(&type_); - for listeners in opt_listeners { + if let Some(listeners) = target.get_listeners(&type_) { for listener in listeners { // Explicitly drop any exception on the floor. listener.call_or_handle_event(target, event, Report); @@ -66,24 +60,20 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar event.set_phase(EventPhase::Bubbling); for cur_target in chain { - let stopped = match cur_target.get_listeners_for(&type_, ListenerPhase::Bubbling) { - Some(listeners) => { - event.set_current_target(cur_target); - for listener in &listeners { - // Explicitly drop any exception on the floor. - listener.call_or_handle_event(*cur_target, event, Report); + if let Some(listeners) = cur_target.get_listeners_for(&type_, ListenerPhase::Bubbling) { + event.set_current_target(cur_target); + for listener in &listeners { + // Explicitly drop any exception on the floor. + listener.call_or_handle_event(*cur_target, event, Report); - if event.stop_immediate() { - break; - } + if event.stop_immediate() { + break; } - - event.stop_propagation() } - None => false - }; - if stopped { - break; + + if event.stop_propagation() { + break; + } } } } From af6bc108f345fceb1da02864cb7554f7abe78c02 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 16 Sep 2015 13:47:13 +0200 Subject: [PATCH 07/14] Use early returns in dispatch_to_listeners. --- components/script/dom/eventdispatcher.rs | 26 ++++++++++++++---------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 51617a49a2d..6ac2d38e976 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -37,26 +37,30 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar } } } + if event.stop_propagation() { + return; + } /* at target */ - if !event.stop_propagation() { - event.set_phase(EventPhase::AtTarget); - event.set_current_target(target.clone()); + event.set_phase(EventPhase::AtTarget); + event.set_current_target(target.clone()); - if let Some(listeners) = target.get_listeners(&type_) { - for listener in listeners { - // Explicitly drop any exception on the floor. - listener.call_or_handle_event(target, event, Report); + if let Some(listeners) = target.get_listeners(&type_) { + for listener in listeners { + // Explicitly drop any exception on the floor. + listener.call_or_handle_event(target, event, Report); - if event.stop_immediate() { - break; - } + if event.stop_immediate() { + break; } } } + if event.stop_propagation() { + return; + } /* bubbling */ - if event.bubbles() && !event.stop_propagation() { + if event.bubbles() { event.set_phase(EventPhase::Bubbling); for cur_target in chain { From a85196398da7d6328163f0b5b7d35f6fef5f5499 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 16 Sep 2015 13:47:31 +0200 Subject: [PATCH 08/14] Return immediately from dispatch_to_listeners if propagation is stopped. --- components/script/dom/eventdispatcher.rs | 25 +++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 6ac2d38e976..3b54aaccf28 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -28,18 +28,18 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar listener.call_or_handle_event(*cur_target, event, Report); if event.stop_immediate() { - break; + return; } } if event.stop_propagation() { - break; + return; } } } - if event.stop_propagation() { - return; - } + + assert!(!event.stop_propagation()); + assert!(!event.stop_immediate()); /* at target */ event.set_phase(EventPhase::AtTarget); @@ -51,13 +51,16 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar listener.call_or_handle_event(target, event, Report); if event.stop_immediate() { - break; + return; } } + if event.stop_propagation() { + return; + } } - if event.stop_propagation() { - return; - } + + assert!(!event.stop_propagation()); + assert!(!event.stop_immediate()); /* bubbling */ if event.bubbles() { @@ -71,12 +74,12 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar listener.call_or_handle_event(*cur_target, event, Report); if event.stop_immediate() { - break; + return; } } if event.stop_propagation() { - break; + return; } } } From 69ed59d78fb50b2d5a7bdf9f13b42874888f1ce5 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 14 Sep 2015 17:28:02 +0200 Subject: [PATCH 09/14] Remove the outdated no_move annotation from GlobalRoot. --- components/script/dom/bindings/global.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/script/dom/bindings/global.rs b/components/script/dom/bindings/global.rs index f968305e08b..ae68290166c 100644 --- a/components/script/dom/bindings/global.rs +++ b/components/script/dom/bindings/global.rs @@ -38,7 +38,6 @@ pub enum GlobalRef<'a> { } /// A stack-based rooted reference to a global object. -#[no_move] pub enum GlobalRoot { /// A root for a `Window` object. Window(Root), From 6dab37c88c2b5aa05ba2cec7710bbfa29d23bba6 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 14 Sep 2015 17:28:23 +0200 Subject: [PATCH 10/14] Implement a global_object_for_reflector method. --- components/script/dom/bindings/global.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/script/dom/bindings/global.rs b/components/script/dom/bindings/global.rs index ae68290166c..3ff081521cc 100644 --- a/components/script/dom/bindings/global.rs +++ b/components/script/dom/bindings/global.rs @@ -229,6 +229,11 @@ impl GlobalField { } } +/// Returns the global object of the realm that the given DOM object's reflector was created in. +pub fn global_object_for_reflector(reflector: &T) -> GlobalRoot { + global_object_for_js_object(*reflector.reflector().get_jsobject()) +} + /// Returns the global object of the realm that the given JS object was created in. #[allow(unrooted_must_root)] pub fn global_object_for_js_object(obj: *mut JSObject) -> GlobalRoot { From b7a0440f91eb0db6816768a03668eb01530f754e Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 14 Sep 2015 17:55:47 +0200 Subject: [PATCH 11/14] Emit markers for all events, not just UI events. --- components/script/devtools.rs | 28 ++------- components/script/dom/eventdispatcher.rs | 58 ++++++++++++++++--- components/script/dom/window.rs | 19 ++++--- components/script/script_task.rs | 72 +----------------------- 4 files changed, 69 insertions(+), 108 deletions(-) diff --git a/components/script/devtools.rs b/components/script/devtools.rs index 6c843e9e9eb..f1ccb2a069e 100644 --- a/components/script/devtools.rs +++ b/components/script/devtools.rs @@ -19,7 +19,7 @@ use js::jsapi::{ObjectClassName, RootedObject, RootedValue}; use js::jsval::UndefinedValue; use msg::constellation_msg::PipelineId; use page::{IterablePage, Page}; -use script_task::{get_page, ScriptTask}; +use script_task::get_page; use std::ffi::CStr; use std::rc::Rc; use std::str; @@ -171,36 +171,16 @@ pub fn handle_wants_live_notifications(global: &GlobalRef, send_notifications: b } pub fn handle_set_timeline_markers(page: &Rc, - script_task: &ScriptTask, marker_types: Vec, reply: IpcSender) { - for marker_type in &marker_types { - match *marker_type { - TimelineMarkerType::Reflow => { - let window = page.window(); - window.r().set_devtools_timeline_marker(TimelineMarkerType::Reflow, reply.clone()); - } - TimelineMarkerType::DOMEvent => { - script_task.set_devtools_timeline_marker(TimelineMarkerType::DOMEvent, reply.clone()); - } - } - } + let window = page.window(); + window.set_devtools_timeline_markers(marker_types, reply); } pub fn handle_drop_timeline_markers(page: &Rc, - script_task: &ScriptTask, marker_types: Vec) { let window = page.window(); - for marker_type in &marker_types { - match *marker_type { - TimelineMarkerType::Reflow => { - window.r().drop_devtools_timeline_markers(); - } - TimelineMarkerType::DOMEvent => { - script_task.drop_devtools_timeline_markers(); - } - } - } + window.drop_devtools_timeline_markers(marker_types); } pub fn handle_request_animation_frame(page: &Rc, id: PipelineId, actor_name: String) { diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 3b54aaccf28..50249e405f5 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -5,17 +5,62 @@ use dom::bindings::callback::ExceptionHandling::Report; use dom::bindings::codegen::Bindings::EventBinding::EventMethods; use dom::bindings::codegen::InheritTypes::{EventTargetCast, NodeCast}; -use dom::bindings::js::JS; +use dom::bindings::global::{GlobalRoot, global_object_for_reflector}; +use dom::bindings::js::{JS, Root, RootedReference}; use dom::bindings::trace::RootedVec; use dom::event::{Event, EventPhase}; -use dom::eventtarget::{EventTarget, ListenerPhase}; +use dom::eventtarget::{EventTarget, ListenerPhase, EventListenerType}; use dom::node::Node; use dom::virtualmethods::vtable_for; +use dom::window::Window; + +use devtools_traits::{StartedTimelineMarker, TimelineMarker, TimelineMarkerType}; + +struct AutoDOMEventMarker { + window: Root, + marker: Option, +} + +impl AutoDOMEventMarker { + fn new(window: &Window) -> AutoDOMEventMarker { + AutoDOMEventMarker { + window: Root::from_ref(window), + marker: Some(TimelineMarker::start("DOMEvent".to_owned())), + } + } +} + +impl Drop for AutoDOMEventMarker { + fn drop(&mut self) { + self.window.emit_timeline_marker(self.marker.take().unwrap().end()); + } +} + +fn handle_event(window: Option<&Window>, listener: &EventListenerType, + current_target: &EventTarget, event: &Event) { + let _marker; + if let Some(window) = window { + _marker = AutoDOMEventMarker::new(window); + } + + listener.call_or_handle_event(current_target, event, Report); +} fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTarget]) { assert!(!event.stop_propagation()); assert!(!event.stop_immediate()); + let window = match global_object_for_reflector(target) { + GlobalRoot::Window(window) => { + if window.need_emit_timeline_marker(TimelineMarkerType::DOMEvent) { + Some(window) + } else { + None + } + }, + _ => None, + }; + let type_ = event.Type(); /* capturing */ @@ -24,8 +69,7 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar if let Some(listeners) = cur_target.get_listeners_for(&type_, ListenerPhase::Capturing) { event.set_current_target(cur_target); for listener in &listeners { - // Explicitly drop any exception on the floor. - listener.call_or_handle_event(*cur_target, event, Report); + handle_event(window.r(), listener, *cur_target, event); if event.stop_immediate() { return; @@ -47,8 +91,7 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar if let Some(listeners) = target.get_listeners(&type_) { for listener in listeners { - // Explicitly drop any exception on the floor. - listener.call_or_handle_event(target, event, Report); + handle_event(window.r(), &listener, target, event); if event.stop_immediate() { return; @@ -70,8 +113,7 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar if let Some(listeners) = cur_target.get_listeners_for(&type_, ListenerPhase::Bubbling) { event.set_current_target(cur_target); for listener in &listeners { - // Explicitly drop any exception on the floor. - listener.call_or_handle_event(*cur_target, event, Report); + handle_event(window.r(), listener, *cur_target, event); if event.stop_immediate() { return; diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 1e0e90fb80e..16c146f91fe 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1217,16 +1217,21 @@ impl Window { sender.send(marker).unwrap(); } - pub fn set_devtools_timeline_marker(&self, - marker: TimelineMarkerType, - reply: IpcSender) { + pub fn set_devtools_timeline_markers(&self, + markers: Vec, + reply: IpcSender) { *self.devtools_marker_sender.borrow_mut() = Some(reply); - self.devtools_markers.borrow_mut().insert(marker); + self.devtools_markers.borrow_mut().extend(markers.into_iter()); } - pub fn drop_devtools_timeline_markers(&self) { - self.devtools_markers.borrow_mut().clear(); - *self.devtools_marker_sender.borrow_mut() = None; + pub fn drop_devtools_timeline_markers(&self, markers: Vec) { + let mut devtools_markers = self.devtools_markers.borrow_mut(); + for marker in markers { + devtools_markers.remove(&marker); + } + if devtools_markers.is_empty() { + *self.devtools_marker_sender.borrow_mut() = None; + } } pub fn set_webdriver_script_chan(&self, chan: Option>) { diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 207ebbb648a..3d0c4e7c65e 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -50,9 +50,8 @@ use parse::html::{ParseContext, parse_html}; use timers::TimerId; use webdriver_handlers; +use devtools_traits::ScriptToDevtoolsControlMsg; use devtools_traits::{DevtoolsPageInfo, DevtoolScriptControlMsg}; -use devtools_traits::{ScriptToDevtoolsControlMsg, TimelineMarker}; -use devtools_traits::{StartedTimelineMarker, TimelineMarkerType}; use msg::compositor_msg::{LayerId, ScriptToCompositorMsg}; use msg::constellation_msg::Msg as ConstellationMsg; use msg::constellation_msg::{ConstellationChan, FocusType}; @@ -408,10 +407,6 @@ pub struct ScriptTask { /// no such server exists. devtools_port: Receiver, devtools_sender: IpcSender, - /// For sending timeline markers. Will be ignored if - /// no devtools server - devtools_markers: RefCell>, - devtools_marker_sender: RefCell>>, /// The JavaScript runtime. js_runtime: Rc, @@ -663,8 +658,6 @@ impl ScriptTask { devtools_chan: devtools_chan, devtools_port: devtools_port, devtools_sender: ipc_devtools_sender, - devtools_markers: RefCell::new(HashSet::new()), - devtools_marker_sender: RefCell::new(None), js_runtime: Rc::new(runtime), mouse_over_targets: DOMRefCell::new(vec!()), @@ -1043,9 +1036,9 @@ impl ScriptTask { devtools::handle_wants_live_notifications(&global_ref, to_send) }, DevtoolScriptControlMsg::SetTimelineMarkers(_pipeline_id, marker_types, reply) => - devtools::handle_set_timeline_markers(&page, self, marker_types, reply), + devtools::handle_set_timeline_markers(&page, marker_types, reply), DevtoolScriptControlMsg::DropTimelineMarkers(_pipeline_id, marker_types) => - devtools::handle_drop_timeline_markers(&page, self, marker_types), + devtools::handle_drop_timeline_markers(&page, marker_types), DevtoolScriptControlMsg::RequestAnimationFrame(pipeline_id, name) => devtools::handle_request_animation_frame(&page, pipeline_id, name), } @@ -1717,11 +1710,6 @@ impl ScriptTask { match event { ResizeEvent(new_size) => { - let _marker; - if self.need_emit_timeline_marker(TimelineMarkerType::DOMEvent) { - _marker = AutoDOMEventMarker::new(self); - } - self.handle_resize_event(pipeline_id, new_size); } @@ -1738,10 +1726,6 @@ impl ScriptTask { } MouseMoveEvent(point) => { - let _marker; - if self.need_emit_timeline_marker(TimelineMarkerType::DOMEvent) { - _marker = AutoDOMEventMarker::new(self); - } let page = get_page(&self.root_page(), pipeline_id); let document = page.document(); @@ -1790,10 +1774,6 @@ impl ScriptTask { } KeyEvent(key, state, modifiers) => { - let _marker; - if self.need_emit_timeline_marker(TimelineMarkerType::DOMEvent) { - _marker = AutoDOMEventMarker::new(self); - } let page = get_page(&self.root_page(), pipeline_id); let document = page.document(); document.r().dispatch_key_event( @@ -1807,10 +1787,6 @@ impl ScriptTask { mouse_event_type: MouseEventType, button: MouseButton, point: Point2D) { - let _marker; - if self.need_emit_timeline_marker(TimelineMarkerType::DOMEvent) { - _marker = AutoDOMEventMarker::new(self); - } let page = get_page(&self.root_page(), pipeline_id); let document = page.document(); document.r().handle_mouse_event(self.js_runtime.rt(), button, point, mouse_event_type); @@ -1927,28 +1903,6 @@ impl ScriptTask { self.incomplete_loads.borrow_mut().push(incomplete); } - fn need_emit_timeline_marker(&self, timeline_type: TimelineMarkerType) -> bool { - self.devtools_markers.borrow().contains(&timeline_type) - } - - fn emit_timeline_marker(&self, marker: TimelineMarker) { - let sender = self.devtools_marker_sender.borrow(); - let sender = sender.as_ref().expect("There is no marker sender"); - sender.send(marker).unwrap(); - } - - pub fn set_devtools_timeline_marker(&self, - marker: TimelineMarkerType, - reply: IpcSender) { - *self.devtools_marker_sender.borrow_mut() = Some(reply); - self.devtools_markers.borrow_mut().insert(marker); - } - - pub fn drop_devtools_timeline_markers(&self) { - self.devtools_markers.borrow_mut().clear(); - *self.devtools_marker_sender.borrow_mut() = None; - } - fn handle_parsing_complete(&self, id: PipelineId) { let parent_page = self.root_page(); let page = match parent_page.find(id) { @@ -1992,26 +1946,6 @@ impl Drop for ScriptTask { } } -struct AutoDOMEventMarker<'a> { - script_task: &'a ScriptTask, - marker: Option, -} - -impl<'a> AutoDOMEventMarker<'a> { - fn new(script_task: &'a ScriptTask) -> AutoDOMEventMarker<'a> { - AutoDOMEventMarker { - script_task: script_task, - marker: Some(TimelineMarker::start("DOMEvent".to_owned())), - } - } -} - -impl<'a> Drop for AutoDOMEventMarker<'a> { - fn drop(&mut self) { - self.script_task.emit_timeline_marker(self.marker.take().unwrap().end()); - } -} - /// Shuts down layout for the given page tree. fn shut_down_layout(page_tree: &Rc, exit_type: PipelineExitType) { let mut channels = vec!(); From e85b1e4e89ab0d9165d08e462dc60fc8da8ddb5d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 15 Sep 2015 14:28:35 +0200 Subject: [PATCH 12/14] Use an early return in dispatch_to_listeners. --- components/script/dom/eventdispatcher.rs | 27 ++++++++++++------------ 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 50249e405f5..918763f884e 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -106,24 +106,25 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar assert!(!event.stop_immediate()); /* bubbling */ - if event.bubbles() { - event.set_phase(EventPhase::Bubbling); + if !event.bubbles() { + return; + } - for cur_target in chain { - if let Some(listeners) = cur_target.get_listeners_for(&type_, ListenerPhase::Bubbling) { - event.set_current_target(cur_target); - for listener in &listeners { - handle_event(window.r(), listener, *cur_target, event); + event.set_phase(EventPhase::Bubbling); + for cur_target in chain { + if let Some(listeners) = cur_target.get_listeners_for(&type_, ListenerPhase::Bubbling) { + event.set_current_target(cur_target); + for listener in &listeners { + handle_event(window.r(), listener, *cur_target, event); - if event.stop_immediate() { - return; - } - } - - if event.stop_propagation() { + if event.stop_immediate() { return; } } + + if event.stop_propagation() { + return; + } } } } From f4cf90f127a289b0acfed746102520dd42551a61 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 15 Sep 2015 14:28:57 +0200 Subject: [PATCH 13/14] Remove an obsolete TODO comment. --- components/script/dom/eventdispatcher.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 918763f884e..63759dccb04 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -148,7 +148,6 @@ pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, event.set_dispatching(true); - //TODO: no chain if not participating in a tree let mut chain: RootedVec> = RootedVec::new(); if let Some(target_node) = NodeCast::to_ref(target) { for ancestor in target_node.ancestors() { From 73b9925cc1236790f233530eb3ef4b84a2c13e0c Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 16 Sep 2015 15:01:03 +0200 Subject: [PATCH 14/14] Remove a pointless clone() call. --- components/script/dom/eventdispatcher.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 63759dccb04..f8cb7652529 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -87,7 +87,7 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar /* at target */ event.set_phase(EventPhase::AtTarget); - event.set_current_target(target.clone()); + event.set_current_target(target); if let Some(listeners) = target.get_listeners(&type_) { for listener in listeners {