Auto merge of #14925 - servo:callback, r=jdm

Some cleanup around JS callbacks.

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14925)
<!-- Reviewable:end -->

components/script/dom/bindings/callback.rs

@@ -30,6 +30,48 @@ pub enum ExceptionHandling {
     Rethrow,
 }
 
+
+/// A common base class for representing IDL callback function and
+/// callback interface types.
+#[derive(Default, JSTraceable)]
+pub struct CallbackObject {
+    /// The underlying `JSObject`.
+    callback: Heap<*mut JSObject>,
+}
+
+impl CallbackObject {
+    fn new() -> CallbackObject {
+        CallbackObject {
+            callback: Heap::default(),
+        }
+    }
+
+    pub fn get(&self) -> *mut JSObject {
+        self.callback.get()
+    }
+}
+
+impl PartialEq for CallbackObject {
+    fn eq(&self, other: &CallbackObject) -> bool {
+        self.callback.get() == other.callback.get()
+    }
+}
+
+
+/// A trait to be implemented by concrete IDL callback function and
+/// callback interface types.
+pub trait CallbackContainer {
+    /// Create a new CallbackContainer object for the given `JSObject`.
+    fn new(callback: *mut JSObject) -> Rc<Self>;
+    /// Returns the underlying `CallbackObject`.
+    fn callback_holder(&self) -> &CallbackObject;
+    /// Returns the underlying `JSObject`.
+    fn callback(&self) -> *mut JSObject {
+        self.callback_holder().get()
+    }
+}
+
+
 /// A common base class for representing IDL callback function types.
 #[derive(JSTraceable, PartialEq)]
 pub struct CallbackFunction {
@@ -40,12 +82,15 @@ impl CallbackFunction {
     /// Create a new `CallbackFunction` for this object.
     pub fn new() -> CallbackFunction {
         CallbackFunction {
-            object: CallbackObject {
+            object: CallbackObject::new(),
-                callback: Heap::default(),
-            },
         }
     }
 
+    /// Returns the underlying `CallbackObject`.
+    pub fn callback_holder(&self) -> &CallbackObject {
+        &self.object
+    }
+
     /// Initialize the callback function with a value.
     /// Should be called once this object is done moving.
     pub fn init(&mut self, callback: *mut JSObject) {
@@ -53,59 +98,26 @@ impl CallbackFunction {
     }
 }
 
+
 /// A common base class for representing IDL callback interface types.
 #[derive(JSTraceable, PartialEq)]
 pub struct CallbackInterface {
     object: CallbackObject,
 }
 
-/// A common base class for representing IDL callback function and
-/// callback interface types.
-#[derive(JSTraceable)]
-struct CallbackObject {
-    /// The underlying `JSObject`.
-    callback: Heap<*mut JSObject>,
-}
-
-impl PartialEq for CallbackObject {
-    fn eq(&self, other: &CallbackObject) -> bool {
-        self.callback.get() == other.callback.get()
-    }
-}
-
-/// A trait to be implemented by concrete IDL callback function and
-/// callback interface types.
-pub trait CallbackContainer {
-    /// Create a new CallbackContainer object for the given `JSObject`.
-    fn new(callback: *mut JSObject) -> Rc<Self>;
-    /// Returns the underlying `JSObject`.
-    fn callback(&self) -> *mut JSObject;
-}
-
-impl CallbackInterface {
-    /// Returns the underlying `JSObject`.
-    pub fn callback(&self) -> *mut JSObject {
-        self.object.callback.get()
-    }
-}
-
-impl CallbackFunction {
-    /// Returns the underlying `JSObject`.
-    pub fn callback(&self) -> *mut JSObject {
-        self.object.callback.get()
-    }
-}
-
 impl CallbackInterface {
     /// Create a new CallbackInterface object for the given `JSObject`.
     pub fn new() -> CallbackInterface {
         CallbackInterface {
-            object: CallbackObject {
+            object: CallbackObject::new(),
-                callback: Heap::default(),
-            },
         }
     }
 
+    /// Returns the underlying `CallbackObject`.
+    pub fn callback_holder(&self) -> &CallbackObject {
+        &self.object
+    }
+
     /// Initialize the callback function with a value.
     /// Should be called once this object is done moving.
     pub fn init(&mut self, callback: *mut JSObject) {
@@ -116,7 +128,7 @@ impl CallbackInterface {
     /// or an error otherwise.
     pub fn get_callable_property(&self, cx: *mut JSContext, name: &str) -> Fallible<JSVal> {
         rooted!(in(cx) let mut callable = UndefinedValue());
-        rooted!(in(cx) let obj = self.callback());
+        rooted!(in(cx) let obj = self.callback_holder().get());
         unsafe {
             let c_name = CString::new(name).unwrap();
             if !JS_GetProperty(cx, obj.handle(), c_name.as_ptr(), callable.handle_mut()) {
@@ -132,6 +144,7 @@ impl CallbackInterface {
     }
 }
 
+
 /// Wraps the reflector for `p` into the compartment of `cx`.
 pub fn wrap_call_this_object<T: DomObject>(cx: *mut JSContext,
                                            p: &T,
@@ -146,6 +159,7 @@ pub fn wrap_call_this_object<T: DomObject>(cx: *mut JSContext,
     }
 }
 
+
 /// A class that performs whatever setup we need to safely make a call while
 /// this class is on the stack. After `new` returns, the call is safe to make.
 pub struct CallSetup {

components/script/dom/bindings/codegen/CodegenRust.py

@@ -5565,6 +5565,7 @@ def generate_imports(config, cgthings, descriptors, callbacks=None, dictionaries
         'dom::bindings::callback::CallbackContainer',
         'dom::bindings::callback::CallbackInterface',
         'dom::bindings::callback::CallbackFunction',
+        'dom::bindings::callback::CallbackObject',
         'dom::bindings::callback::ExceptionHandling',
         'dom::bindings::callback::wrap_call_this_object',
         'dom::bindings::conversions::ConversionBehavior',
@@ -6339,8 +6340,8 @@ impl CallbackContainer for ${type} {
         ${type}::new(callback)
     }
 
-    fn callback(&self) -> *mut JSObject {
+    fn callback_holder(&self) -> &CallbackObject {
-        self.parent.callback()
+        self.parent.callback_holder()
     }
 }
 
@@ -6619,11 +6620,11 @@ class CallCallback(CallbackMethod):
         return "aThisObj.get()"
 
     def getCallableDecl(self):
-        return "rooted!(in(cx) let callable = ObjectValue(self.parent.callback()));\n"
+        return "rooted!(in(cx) let callable = ObjectValue(self.callback()));\n"
 
     def getCallGuard(self):
         if self.callback._treatNonObjectAsNull:
-            return "!IsCallable(self.parent.callback()) || "
+            return "!IsCallable(self.callback()) || "
         return ""
 
 
@@ -6638,11 +6639,11 @@ class CallbackOperationBase(CallbackMethod):
 
     def getThisObj(self):
         if not self.singleOperation:
-            return "self.parent.callback()"
+            return "self.callback()"
         # This relies on getCallableDecl declaring a boolean
         # isCallable in the case when we're a single-operation
         # interface.
-        return "if isCallable { aThisObj.get() } else { self.parent.callback() }"
+        return "if isCallable { aThisObj.get() } else { self.callback() }"
 
     def getCallableDecl(self):
         replacements = {
@@ -6654,11 +6655,11 @@ class CallbackOperationBase(CallbackMethod):
         if not self.singleOperation:
             return 'rooted!(in(cx) let callable =\n' + getCallableFromProp + ');\n'
         return (
-            'let isCallable = IsCallable(self.parent.callback());\n'
+            'let isCallable = IsCallable(self.callback());\n'
             'rooted!(in(cx) let callable =\n' +
             CGIndenter(
                 CGIfElseWrapper('isCallable',
-                                CGGeneric('ObjectValue(self.parent.callback())'),
+                                CGGeneric('ObjectValue(self.callback())'),
                                 CGGeneric(getCallableFromProp))).define() + ');\n')
 
     def getCallGuard(self):

components/script/dom/eventtarget.rs

@@ -69,7 +69,7 @@ pub enum ListenerPhase {
 
 /// https://html.spec.whatwg.org/multipage/#internal-raw-uncompiled-handler
 #[derive(JSTraceable, Clone, PartialEq)]
-pub struct InternalRawUncompiledHandler {
+struct InternalRawUncompiledHandler {
     source: DOMString,
     url: ServoUrl,
     line: usize,
@@ -77,7 +77,7 @@ pub struct InternalRawUncompiledHandler {
 
 /// A representation of an event handler, either compiled or uncompiled raw source, or null.
 #[derive(JSTraceable, PartialEq, Clone)]
-pub enum InlineEventListener {
+enum InlineEventListener {
     Uncompiled(InternalRawUncompiledHandler),
     Compiled(CommonEventHandler),
     Null,
@@ -308,7 +308,7 @@ impl EventTarget {
     }
 
     /// https://html.spec.whatwg.org/multipage/#event-handler-attributes:event-handlers-11
-    pub fn set_inline_event_listener(&self,
+    fn set_inline_event_listener(&self,
                                  ty: Atom,
                                  listener: Option<InlineEventListener>) {
         let mut handlers = self.handlers.borrow_mut();
@@ -363,7 +363,7 @@ impl EventTarget {
 
     // https://html.spec.whatwg.org/multipage/#getting-the-current-value-of-the-event-handler
     #[allow(unsafe_code)]
-    pub fn get_compiled_event_handler(&self,
+    fn get_compiled_event_handler(&self,
                                   handler: InternalRawUncompiledHandler,
                                   ty: &Atom)
                                   -> Option<CommonEventHandler> {
@@ -484,7 +484,7 @@ impl EventTarget {
 
     pub fn get_event_handler_common<T: CallbackContainer>(&self, ty: &str) -> Option<Rc<T>> {
         let listener = self.get_inline_event_listener(&Atom::from(ty));
-        listener.map(|listener| CallbackContainer::new(listener.parent().callback()))
+        listener.map(|listener| CallbackContainer::new(listener.parent().callback_holder().get()))
     }
 
     pub fn has_handlers(&self) -> bool {