Auto merge of #7628 - Ms2ger:events-markers, r=jdm

Emit markers for all events, not just UI events.



<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7628)
<!-- Reviewable:end -->

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<Page>,
-                                   script_task: &ScriptTask,
                                    marker_types: Vec<TimelineMarkerType>,
                                    reply: IpcSender<TimelineMarker>) {
-    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());
+    window.set_devtools_timeline_markers(marker_types, reply);
-            }
-            TimelineMarkerType::DOMEvent => {
-                script_task.set_devtools_timeline_marker(TimelineMarkerType::DOMEvent, reply.clone());
-            }
-        }
-    }
 }
 
 pub fn handle_drop_timeline_markers(page: &Rc<Page>,
-                                    script_task: &ScriptTask,
                                     marker_types: Vec<TimelineMarkerType>) {
     let window = page.window();
-    for marker_type in &marker_types {
+    window.drop_devtools_timeline_markers(marker_types);
-        match *marker_type {
-            TimelineMarkerType::Reflow => {
-                window.r().drop_devtools_timeline_markers();
-            }
-            TimelineMarkerType::DOMEvent => {
-                script_task.drop_devtools_timeline_markers();
-            }
-        }
-    }
 }
 
 pub fn handle_request_animation_frame(page: &Rc<Page>, id: PipelineId, actor_name: String) {

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<window::Window>),
@@ -230,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<T: Reflectable>(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 {

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)

components/script/dom/eventdispatcher.rs

@@ -5,28 +5,149 @@
 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<Window>,
+    marker: Option<StartedTimelineMarker>,
+}
+
+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 */
+    event.set_phase(EventPhase::Capturing);
+    for cur_target in chain.iter().rev() {
+        if let Some(listeners) = cur_target.get_listeners_for(&type_, ListenerPhase::Capturing) {
+            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() {
+                return;
+            }
+        }
+    }
+
+    assert!(!event.stop_propagation());
+    assert!(!event.stop_immediate());
+
+    /* at target */
+    event.set_phase(EventPhase::AtTarget);
+    event.set_current_target(target);
+
+    if let Some(listeners) = target.get_listeners(&type_) {
+        for listener in listeners {
+            handle_event(window.r(), &listener, target, event);
+
+            if event.stop_immediate() {
+                return;
+            }
+        }
+        if event.stop_propagation() {
+            return;
+        }
+    }
+
+    assert!(!event.stop_propagation());
+    assert!(!event.stop_immediate());
+
+    /* bubbling */
+    if !event.bubbles() {
+        return;
+    }
+
+    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() {
+                return;
+            }
+        }
+    }
+}
 
 // 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());
+    assert_eq!(event.phase(), EventPhase::None);
+    assert!(event.GetCurrentTarget().is_none());
 
     event.set_target(match pseudo_target {
         Some(pseudo_target) => pseudo_target,
         None => target.clone(),
     });
+
+    if event.stop_propagation() {
+        return !event.DefaultPrevented();
+    }
+
     event.set_dispatching(true);
 
-    let type_ = event.Type();
-
-    //TODO: no chain if not participating in a tree
     let mut chain: RootedVec<JS<EventTarget>> = RootedVec::new();
     if let Some(target_node) = NodeCast::to_ref(target) {
         for ancestor in target_node.ancestors() {
@@ -35,78 +156,7 @@ pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>,
         }
     }
 
-    event.set_phase(EventPhase::Capturing);
+    dispatch_to_listeners(event, target, chain.r());
-
-    //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) {
-            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 event.stop_immediate() {
-                        break;
-                    }
-                }
-
-                event.stop_propagation()
-            }
-            None => false
-        };
-
-        if stopped {
-            break;
-        }
-    }
-
-    /* at target */
-    if !event.stop_propagation() {
-        event.set_phase(EventPhase::AtTarget);
-        event.set_current_target(target.clone());
-
-        let opt_listeners = target.get_listeners(&type_);
-        for listeners in opt_listeners {
-            for listener in listeners {
-                // Explicitly drop any exception on the floor.
-                listener.call_or_handle_event(target, event, Report);
-
-                if event.stop_immediate() {
-                    break;
-                }
-            }
-        }
-    }
-
-    /* bubbling */
-    if event.bubbles() && !event.stop_propagation() {
-        event.set_phase(EventPhase::Bubbling);
-
-        for cur_target in chain.r() {
-            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 event.stop_immediate() {
-                            break;
-                        }
-                    }
-
-                    event.stop_propagation()
-                }
-                None => false
-            };
-            if stopped {
-                break;
-            }
-        }
-    }
 
     /* default action */
     let target = event.GetTarget();

components/script/dom/window.rs

@@ -1217,17 +1217,22 @@ impl Window {
         sender.send(marker).unwrap();
     }
 
-    pub fn set_devtools_timeline_marker(&self,
+    pub fn set_devtools_timeline_markers(&self,
-                                    marker: TimelineMarkerType,
+                                         markers: Vec<TimelineMarkerType>,
                                          reply: IpcSender<TimelineMarker>) {
         *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) {
+    pub fn drop_devtools_timeline_markers(&self, markers: Vec<TimelineMarkerType>) {
-        self.devtools_markers.borrow_mut().clear();
+        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<IpcSender<WebDriverJSResult>>) {
         *self.webdriver_script_chan.borrow_mut() = chan;

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<DevtoolScriptControlMsg>,
     devtools_sender: IpcSender<DevtoolScriptControlMsg>,
-    /// For sending timeline markers. Will be ignored if
-    /// no devtools server
-    devtools_markers: RefCell<HashSet<TimelineMarkerType>>,
-    devtools_marker_sender: RefCell<Option<IpcSender<TimelineMarker>>>,
 
     /// The JavaScript runtime.
     js_runtime: Rc<Runtime>,
@@ -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<f32>) {
-        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<TimelineMarker>) {
-        *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<StartedTimelineMarker>,
-}
-
-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<Page>, exit_type: PipelineExitType) {
     let mut channels = vec!();

tests/wpt/metadata/DOMEvents/tests/approved/stopPropagation.dispatchEvent.html.ini

@@ -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
-

tests/wpt/metadata/DOMEvents/tests/submissions/Microsoft/converted/stopPropagation.dispatchEvent.html.ini

@@ -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
-