Compile raw inline event handlers lazily. Resolves #8489.

components/script/dom/bindings/trace.rs

@@ -255,6 +255,15 @@ impl<A: JSTraceable, B: JSTraceable> JSTraceable for (A, B) {
     }
 }
 
+impl<A: JSTraceable, B: JSTraceable, C: JSTraceable> JSTraceable for (A, B, C) {
+    #[inline]
+    fn trace(&self, trc: *mut JSTracer) {
+        let (ref a, ref b, ref c) = *self;
+        a.trace(trc);
+        b.trace(trc);
+        c.trace(trc);
+    }
+}
 
 no_jsmanaged_fields!(bool, f32, f64, String, Url, AtomicBool, Uuid);
 no_jsmanaged_fields!(usize, u8, u16, u32, u64);

components/script/dom/eventdispatcher.rs

@@ -11,7 +11,7 @@ use dom::bindings::js::{JS, Root, RootedReference};
 use dom::bindings::reflector::Reflectable;
 use dom::bindings::trace::RootedVec;
 use dom::event::{Event, EventPhase};
-use dom::eventtarget::{EventListenerType, EventTarget, ListenerPhase};
+use dom::eventtarget::{CompiledEventListener, EventTarget, ListenerPhase};
 use dom::node::Node;
 use dom::virtualmethods::vtable_for;
 use dom::window::Window;
@@ -36,7 +36,7 @@ impl Drop for AutoDOMEventMarker {
     }
 }
 
-fn handle_event(window: Option<&Window>, listener: &EventListenerType,
+fn handle_event(window: Option<&Window>, listener: &CompiledEventListener,
                 current_target: &EventTarget, event: &Event) {
     let _marker;
     if let Some(window) = window {

components/script/dom/eventtarget.rs

@@ -9,20 +9,22 @@ use dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull;
 use dom::bindings::codegen::Bindings::EventHandlerBinding::OnErrorEventHandlerNonNull;
 use dom::bindings::codegen::Bindings::EventListenerBinding::EventListener;
 use dom::bindings::codegen::Bindings::EventTargetBinding::EventTargetMethods;
+use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
 use dom::bindings::codegen::UnionTypes::EventOrString;
 use dom::bindings::error::{Error, Fallible, report_pending_exception};
 use dom::bindings::inheritance::{Castable, EventTargetTypeId};
 use dom::bindings::js::Root;
 use dom::bindings::reflector::{Reflectable, Reflector};
+use dom::element::Element;
 use dom::errorevent::ErrorEvent;
 use dom::event::{Event, EventBubbles, EventCancelable};
 use dom::eventdispatcher::dispatch_event;
+use dom::node::document_from_node;
 use dom::virtualmethods::VirtualMethods;
 use dom::window::Window;
 use fnv::FnvHasher;
 use heapsize::HeapSizeOf;
-use js::jsapi::{CompileFunction, JS_GetFunctionObject, RootedValue};
-use js::jsapi::{HandleObject, JSContext, RootedFunction};
+use js::jsapi::{CompileFunction, JS_GetFunctionObject, RootedValue, RootedFunction};
 use js::jsapi::{JSAutoCompartment, JSAutoRequest};
 use js::rust::{AutoObjectVectorWrapper, CompileOptionsWrapper};
 use libc::{c_char, size_t};
@@ -31,6 +33,8 @@ use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::default::Default;
 use std::ffi::CString;
 use std::hash::BuildHasherDefault;
+use std::mem;
+use std::ops::{Deref, DerefMut};
 use std::rc::Rc;
 use std::{intrinsics, ptr};
 use string_cache::Atom;
@@ -94,10 +98,49 @@ impl EventTargetTypeId {
     }
 }
 
+/// https://html.spec.whatwg.org/multipage/#internal-raw-uncompiled-handler
 #[derive(JSTraceable, Clone, PartialEq)]
-pub enum EventListenerType {
+pub struct InternalRawUncompiledHandler {
+    source: DOMString,
+    url: Url,
+    line: usize,
+}
+
+/// A representation of an event handler, either compiled or uncompiled raw source, or null.
+#[derive(JSTraceable, PartialEq, Clone)]
+pub enum InlineEventListener {
+    Uncompiled(InternalRawUncompiledHandler),
+    Compiled(CommonEventHandler),
+    Null,
+}
+
+impl InlineEventListener {
+    /// Get a compiled representation of this event handler, compiling it from its
+    /// raw source if necessary.
+    /// https://html.spec.whatwg.org/multipage/#getting-the-current-value-of-the-event-handler
+    fn get_compiled_handler(&mut self, owner: &EventTarget, ty: &Atom)
+                            -> Option<CommonEventHandler> {
+        match mem::replace(self, InlineEventListener::Null) {
+            InlineEventListener::Null => None,
+            InlineEventListener::Uncompiled(handler) => {
+                let result = owner.get_compiled_event_handler(handler, ty);
+                if let Some(ref compiled) = result {
+                    *self = InlineEventListener::Compiled(compiled.clone());
+                }
+                result
+            }
+            InlineEventListener::Compiled(handler) => {
+                *self = InlineEventListener::Compiled(handler.clone());
+                Some(handler)
+            }
+        }
+    }
+}
+
+#[derive(JSTraceable, Clone, PartialEq)]
+enum EventListenerType {
     Additive(Rc<EventListener>),
-    Inline(CommonEventHandler),
+    Inline(InlineEventListener),
 }
 
 impl HeapSizeOf for EventListenerType {
@@ -108,6 +151,26 @@ impl HeapSizeOf for EventListenerType {
 }
 
 impl EventListenerType {
+    fn get_compiled_listener(&mut self, owner: &EventTarget, ty: &Atom)
+                             -> Option<CompiledEventListener> {
+        match self {
+            &mut EventListenerType::Inline(ref mut inline) =>
+                inline.get_compiled_handler(owner, ty)
+                      .map(CompiledEventListener::Handler),
+            &mut EventListenerType::Additive(ref listener) =>
+                Some(CompiledEventListener::Listener(listener.clone())),
+        }
+    }
+}
+
+/// A representation of an EventListener/EventHandler object that has previously
+/// been compiled successfully, if applicable.
+pub enum CompiledEventListener {
+    Listener(Rc<EventListener>),
+    Handler(CommonEventHandler),
+}
+
+impl CompiledEventListener {
     // https://html.spec.whatwg.org/multipage/#the-event-handler-processing-algorithm
     pub fn call_or_handle_event<T: Reflectable>(&self,
                                                 object: &T,
@@ -115,10 +178,10 @@ impl EventListenerType {
                                                 exception_handle: ExceptionHandling) {
         // Step 3
         match *self {
-            EventListenerType::Additive(ref listener) => {
+            CompiledEventListener::Listener(ref listener) => {
                 let _ = listener.HandleEvent_(object, event, exception_handle);
             },
-            EventListenerType::Inline(ref handler) => {
+            CompiledEventListener::Handler(ref handler) => {
                 match *handler {
                     CommonEventHandler::ErrorEventHandler(ref handler) => {
                         if let Some(event) = event.downcast::<ErrorEvent>() {
@@ -152,15 +215,61 @@ impl EventListenerType {
 
 #[derive(JSTraceable, Clone, PartialEq, HeapSizeOf)]
 #[privatize]
-pub struct EventListenerEntry {
+/// A listener in a collection of event listeners.
+struct EventListenerEntry {
     phase: ListenerPhase,
     listener: EventListenerType
 }
 
+#[derive(JSTraceable, HeapSizeOf)]
+/// A mix of potentially uncompiled and compiled event listeners of the same type.
+struct EventListeners(Vec<EventListenerEntry>);
+
+impl Deref for EventListeners {
+    type Target = Vec<EventListenerEntry>;
+    fn deref(&self) -> &Vec<EventListenerEntry> {
+        &self.0
+    }
+}
+
+impl DerefMut for EventListeners {
+    fn deref_mut(&mut self) -> &mut Vec<EventListenerEntry> {
+        &mut self.0
+    }
+}
+
+impl EventListeners {
+    // https://html.spec.whatwg.org/multipage/#getting-the-current-value-of-the-event-handler
+    fn get_inline_listener(&mut self, owner: &EventTarget, ty: &Atom) -> Option<CommonEventHandler> {
+        for entry in &mut self.0 {
+            if let EventListenerType::Inline(ref mut inline) = entry.listener {
+                // Step 1.1-1.8 and Step 2
+                return inline.get_compiled_handler(owner, ty);
+            }
+        }
+
+        // Step 2
+        None
+    }
+
+    // https://html.spec.whatwg.org/multipage/#getting-the-current-value-of-the-event-handler
+    fn get_listeners(&mut self, phase: Option<ListenerPhase>, owner: &EventTarget, ty: &Atom)
+                     -> Vec<CompiledEventListener> {
+        self.0.iter_mut().filter_map(|entry| {
+            if phase.is_none() || Some(entry.phase) == phase {
+                // Step 1.1-1.8, 2
+                entry.listener.get_compiled_listener(owner, ty)
+            } else {
+                None
+            }
+        }).collect()
+    }
+}
+
 #[dom_struct]
 pub struct EventTarget {
     reflector_: Reflector,
-    handlers: DOMRefCell<HashMap<Atom, Vec<EventListenerEntry>, BuildHasherDefault<FnvHasher>>>,
+    handlers: DOMRefCell<HashMap<Atom, EventListeners, BuildHasherDefault<FnvHasher>>>,
 }
 
 impl EventTarget {
@@ -171,17 +280,16 @@ impl EventTarget {
         }
     }
 
-    pub fn get_listeners(&self, type_: &Atom) -> Option<Vec<EventListenerType>> {
-        self.handlers.borrow().get(type_).map(|listeners| {
-            listeners.iter().map(|entry| entry.listener.clone()).collect()
+    pub fn get_listeners(&self, type_: &Atom) -> Option<Vec<CompiledEventListener>> {
+        self.handlers.borrow_mut().get_mut(type_).map(|listeners| {
+            listeners.get_listeners(None, self, type_)
         })
     }
 
     pub fn get_listeners_for(&self, type_: &Atom, desired_phase: ListenerPhase)
-        -> Option<Vec<EventListenerType>> {
-        self.handlers.borrow().get(type_).map(|listeners| {
-            let filtered = listeners.iter().filter(|entry| entry.phase == desired_phase);
-            filtered.map(|entry| entry.listener.clone()).collect()
+                             -> Option<Vec<CompiledEventListener>> {
+        self.handlers.borrow_mut().get_mut(type_).map(|listeners| {
+            listeners.get_listeners(Some(desired_phase), self, type_)
         })
     }
 
@@ -195,13 +303,14 @@ impl EventTarget {
         dispatch_event(self, None, event)
     }
 
+    /// https://html.spec.whatwg.org/multipage/#event-handler-attributes:event-handlers-11
     pub fn set_inline_event_listener(&self,
                                      ty: Atom,
-                                     listener: Option<CommonEventHandler>) {
+                                     listener: Option<InlineEventListener>) {
         let mut handlers = self.handlers.borrow_mut();
         let entries = match handlers.entry(ty) {
             Occupied(entry) => entry.into_mut(),
-            Vacant(entry) => entry.insert(vec!()),
+            Vacant(entry) => entry.insert(EventListeners(vec!())),
         };
 
         let idx = entries.iter().position(|ref entry| {
@@ -213,12 +322,8 @@ impl EventTarget {
 
         match idx {
             Some(idx) => {
-                match listener {
-                    Some(listener) => entries[idx].listener = EventListenerType::Inline(listener),
-                    None => {
-                        entries.remove(idx);
-                    }
-                }
+                entries[idx].listener =
+                    EventListenerType::Inline(listener.unwrap_or(InlineEventListener::Null));
             }
             None => {
                 if listener.is_some() {
@@ -231,28 +336,52 @@ impl EventTarget {
         }
     }
 
-    pub fn get_inline_event_listener(&self, ty: &Atom) -> Option<CommonEventHandler> {
-        let handlers = self.handlers.borrow();
-        let entries = handlers.get(ty);
-        entries.and_then(|entries| entries.iter().filter_map(|entry| {
-            match entry.listener {
-                EventListenerType::Inline(ref handler) => Some(handler.clone()),
-                _ => None,
-            }
-        }).next())
+    fn get_inline_event_listener(&self, ty: &Atom) -> Option<CommonEventHandler> {
+        let mut handlers = self.handlers.borrow_mut();
+        handlers.get_mut(ty).and_then(|entry| entry.get_inline_listener(self, ty))
     }
 
-    #[allow(unsafe_code)]
-    // https://html.spec.whatwg.org/multipage/#getting-the-current-value-of-the-event-handler
+    /// Store the raw uncompiled event handler for on-demand compilation later.
+    /// https://html.spec.whatwg.org/multipage/#event-handler-attributes:event-handler-content-attributes-3
     pub fn set_event_handler_uncompiled(&self,
-                                    cx: *mut JSContext,
-                                    url: Url,
-                                    scope: HandleObject,
-                                    ty: &str,
-                                    source: DOMString) {
-        let url = CString::new(url.serialize()).unwrap();
-        let name = CString::new(ty).unwrap();
-        let lineno = 0; //XXXjdm need to get a real number here
+                                        url: Url,
+                                        line: usize,
+                                        ty: &str,
+                                        source: DOMString) {
+        let handler = InternalRawUncompiledHandler {
+            source: source,
+            line: line,
+            url: url,
+        };
+        self.set_inline_event_listener(Atom::from(ty),
+                                       Some(InlineEventListener::Uncompiled(handler)));
+    }
+
+    // https://html.spec.whatwg.org/multipage/#getting-the-current-value-of-the-event-handler
+    #[allow(unsafe_code)]
+    pub fn get_compiled_event_handler(&self,
+                                      handler: InternalRawUncompiledHandler,
+                                      ty: &Atom)
+                                      -> Option<CommonEventHandler> {
+        // Step 1.1
+        let element = self.downcast::<Element>();
+        let document = match element {
+            Some(element) => document_from_node(element),
+            None => self.downcast::<Window>().unwrap().Document(),
+        };
+
+        // TODO step 1.2 (browsing context/scripting enabled)
+
+        // Step 1.3
+        let body: Vec<u16> = handler.source.utf16_units().collect();
+
+        // TODO step 1.5 (form owner)
+
+        // Step 1.6
+        let window = document.window();
+
+        let url_serialized = CString::new(handler.url.serialize()).unwrap();
+        let name = CString::new(&**ty).unwrap();
 
         static mut ARG_NAMES: [*const c_char; 1] = [b"event\0" as *const u8 as *const c_char];
         static mut ERROR_ARG_NAMES: [*const c_char; 5] = [b"event\0" as *const u8 as *const c_char,
@@ -261,7 +390,7 @@ impl EventTarget {
                                                           b"colno\0" as *const u8 as *const c_char,
                                                           b"error\0" as *const u8 as *const c_char];
         // step 10
-        let is_error = ty == "error" && self.is::<Window>();
+        let is_error = ty == &Atom::from("error") && self.is::<Window>();
         let args = unsafe {
             if is_error {
                 &ERROR_ARG_NAMES[..]
@@ -270,12 +399,14 @@ impl EventTarget {
             }
         };
 
-        let source: Vec<u16> = source.utf16_units().collect();
-        let options = CompileOptionsWrapper::new(cx, url.as_ptr(), lineno);
+        let cx = window.get_cx();
+        let options = CompileOptionsWrapper::new(cx, url_serialized.as_ptr(), handler.line as u32);
+        // TODO step 1.10.1-3 (document, form owner, element in scope chain)
+
         let scopechain = AutoObjectVectorWrapper::new(cx);
 
         let _ar = JSAutoRequest::new(cx);
-        let _ac = JSAutoCompartment::new(cx, scope.get());
+        let _ac = JSAutoCompartment::new(cx, window.reflector().get_jsobject().get());
         let mut handler = RootedFunction::new(cx, ptr::null_mut());
         let rv = unsafe {
             CompileFunction(cx,
@@ -284,21 +415,25 @@ impl EventTarget {
                             name.as_ptr(),
                             args.len() as u32,
                             args.as_ptr(),
-                            source.as_ptr(),
-                            source.len() as size_t,
+                            body.as_ptr(),
+                            body.len() as size_t,
                             handler.handle_mut())
         };
         if !rv || handler.ptr.is_null() {
+            // Step 1.8.2
             report_pending_exception(cx, self.reflector().get_jsobject().get());
-            return;
+            // Step 1.8.1 / 1.8.3
+            return None;
         }
 
+        // TODO step 1.11-13
         let funobj = unsafe { JS_GetFunctionObject(handler.ptr) };
         assert!(!funobj.is_null());
+        // Step 1.14
         if is_error {
-            self.set_error_event_handler(ty, Some(OnErrorEventHandlerNonNull::new(funobj)));
+            Some(CommonEventHandler::ErrorEventHandler(OnErrorEventHandlerNonNull::new(funobj)))
         } else {
-            self.set_event_handler_common(ty, Some(EventHandlerNonNull::new(funobj)));
+            Some(CommonEventHandler::EventHandler(EventHandlerNonNull::new(funobj)))
         }
     }
 
@@ -306,8 +441,9 @@ impl EventTarget {
         &self, ty: &str, listener: Option<Rc<T>>)
     {
         let event_listener = listener.map(|listener|
-                                          CommonEventHandler::EventHandler(
-                                              EventHandlerNonNull::new(listener.callback())));
+                                          InlineEventListener::Compiled(
+                                              CommonEventHandler::EventHandler(
+                                                  EventHandlerNonNull::new(listener.callback()))));
         self.set_inline_event_listener(Atom::from(ty), event_listener);
     }
 
@@ -315,8 +451,9 @@ impl EventTarget {
         &self, ty: &str, listener: Option<Rc<T>>)
     {
         let event_listener = listener.map(|listener|
-                                          CommonEventHandler::ErrorEventHandler(
-                                              OnErrorEventHandlerNonNull::new(listener.callback())));
+                                          InlineEventListener::Compiled(
+                                              CommonEventHandler::ErrorEventHandler(
+                                                  OnErrorEventHandlerNonNull::new(listener.callback()))));
         self.set_inline_event_listener(Atom::from(ty), event_listener);
     }
 
@@ -359,7 +496,7 @@ impl EventTargetMethods for EventTarget {
             let mut handlers = self.handlers.borrow_mut();
             let entry = match handlers.entry(Atom::from(&*ty)) {
                 Occupied(entry) => entry.into_mut(),
-                Vacant(entry) => entry.insert(vec!()),
+                Vacant(entry) => entry.insert(EventListeners(vec!())),
             };
 
             let phase = if capture { ListenerPhase::Capturing } else { ListenerPhase::Bubbling };

components/script/dom/htmlbodyelement.rs

@@ -162,9 +162,6 @@ impl VirtualMethods for HTMLBodyElement {
         let do_super_mutate = match (attr.local_name(), mutation) {
             (name, AttributeMutation::Set(_)) if name.starts_with("on") => {
                 let window = window_from_node(self);
-                let (cx, url, reflector) = (window.get_cx(),
-                                            window.get_url(),
-                                            window.reflector().get_jsobject());
                 // https://html.spec.whatwg.org/multipage/
                 // #event-handlers-on-elements,-document-objects,-and-window-objects:event-handlers-3
                 match name {
@@ -175,7 +172,9 @@ impl VirtualMethods for HTMLBodyElement {
                     &atom!("onresize") | &atom!("onunload") | &atom!("onerror")
                       => {
                           let evtarget = window.upcast::<EventTarget>(); // forwarded event
-                          evtarget.set_event_handler_uncompiled(cx, url, reflector,
+                          let source_line = 1; //TODO(#9604) obtain current JS execution line
+                          evtarget.set_event_handler_uncompiled(window.get_url(),
+                                                                source_line,
                                                                 &name[2..],
                                                                 DOMString::from((**attr.value()).to_owned()));
                           false

components/script/dom/htmlelement.rs

@@ -453,12 +453,10 @@ impl VirtualMethods for HTMLElement {
         self.super_type().unwrap().attribute_mutated(attr, mutation);
         match (attr.local_name(), mutation) {
             (name, AttributeMutation::Set(_)) if name.starts_with("on") => {
-                let window = window_from_node(self);
-                let (cx, url, reflector) = (window.get_cx(),
-                                            window.get_url(),
-                                            window.reflector().get_jsobject());
                 let evtarget = self.upcast::<EventTarget>();
-                evtarget.set_event_handler_uncompiled(cx, url, reflector,
+                let source_line = 1; //TODO(#9604) get current JS execution line
+                evtarget.set_event_handler_uncompiled(window_from_node(self).get_url(),
+                                                      source_line,
                                                       &name[2..],
                                                       // FIXME(ajeffrey): Convert directly from AttrValue to DOMString
                                                       DOMString::from(&**attr.value()));