Remove usage of FnBox for animation frame callbacks (fixes #14416)

components/script/devtools.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use devtools_traits::{AutoMargins, CONSOLE_API, CachedConsoleMessage, CachedConsoleMessageTypes};
-use devtools_traits::{ComputedNodeLayout, ConsoleAPI, PageError, ScriptToDevtoolsControlMsg};
+use devtools_traits::{ComputedNodeLayout, ConsoleAPI, PageError};
 use devtools_traits::{EvaluateJSReply, Modification, NodeInfo, PAGE_ERROR, TimelineMarker};
 use devtools_traits::TimelineMarkerType;
 use dom::bindings::codegen::Bindings::CSSStyleDeclarationBinding::CSSStyleDeclarationMethods;
@@ -17,6 +17,7 @@ use dom::bindings::inheritance::Castable;
 use dom::bindings::js::Root;
 use dom::bindings::reflector::Reflectable;
 use dom::bindings::str::DOMString;
+use dom::document::AnimationFrameCallback;
 use dom::element::Element;
 use dom::globalscope::GlobalScope;
 use dom::node::{Node, window_from_node};
@@ -253,11 +254,7 @@ pub fn handle_request_animation_frame(documents: &Documents,
                                       id: PipelineId,
                                       actor_name: String) {
     if let Some(doc) = documents.find_document(id) {
-        let devtools_sender = doc.window().upcast::<GlobalScope>().devtools_chan().unwrap().clone();
-        doc.request_animation_frame(box move |time| {
-            let msg = ScriptToDevtoolsControlMsg::FramerateTick(actor_name, time);
-            devtools_sender.send(msg).unwrap();
-        });
+        doc.request_animation_frame(AnimationFrameCallback::DevtoolsFramerateTick { actor_name });
     }
 }
 

components/script/dom/bindings/trace.rs

@@ -81,7 +81,6 @@ use serde::{Deserialize, Serialize};
 use servo_atoms::Atom;
 use servo_url::ServoUrl;
 use smallvec::SmallVec;
-use std::boxed::FnBox;
 use std::cell::{Cell, UnsafeCell};
 use std::collections::{BTreeMap, HashMap, HashSet, VecDeque};
 use std::hash::{BuildHasher, Hash};
@@ -371,13 +370,6 @@ unsafe_no_jsmanaged_fields!(WebGLShaderId);
 unsafe_no_jsmanaged_fields!(WebGLTextureId);
 unsafe_no_jsmanaged_fields!(MediaList);
 
-unsafe impl JSTraceable for Box<FnBox(f64, )> {
-    #[inline]
-    unsafe fn trace(&self, _trc: *mut JSTracer) {
-        // Do nothing
-    }
-}
-
 unsafe impl<'a> JSTraceable for &'a str {
     #[inline]
     unsafe fn trace(&self, _: *mut JSTracer) {

components/script/dom/document.rs

@@ -3,9 +3,11 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use core::nonzero::NonZero;
+use devtools_traits::ScriptToDevtoolsControlMsg;
 use document_loader::{DocumentLoader, LoadType};
 use dom::activation::{ActivationSource, synthetic_click_activation};
 use dom::attr::Attr;
+use dom::bindings::callback::ExceptionHandling;
 use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::DOMRectBinding::DOMRectMethods;
 use dom::bindings::codegen::Bindings::DocumentBinding;
@@ -18,7 +20,7 @@ use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
 use dom::bindings::codegen::Bindings::NodeFilterBinding::NodeFilter;
 use dom::bindings::codegen::Bindings::PerformanceBinding::PerformanceMethods;
 use dom::bindings::codegen::Bindings::TouchBinding::TouchMethods;
-use dom::bindings::codegen::Bindings::WindowBinding::{ScrollBehavior, WindowMethods};
+use dom::bindings::codegen::Bindings::WindowBinding::{FrameRequestCallback, ScrollBehavior, WindowMethods};
 use dom::bindings::codegen::UnionTypes::NodeOrString;
 use dom::bindings::error::{Error, ErrorResult, Fallible};
 use dom::bindings::inheritance::{Castable, ElementTypeId, HTMLElementTypeId, NodeTypeId};
@@ -111,7 +113,6 @@ use servo_atoms::Atom;
 use servo_url::ServoUrl;
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
-use std::boxed::FnBox;
 use std::cell::{Cell, Ref, RefMut};
 use std::collections::HashMap;
 use std::collections::hash_map::Entry::{Occupied, Vacant};
@@ -232,8 +233,7 @@ pub struct Document {
     animation_frame_ident: Cell<u32>,
     /// https://html.spec.whatwg.org/multipage/#list-of-animation-frame-callbacks
     /// List of animation frame callbacks
-    #[ignore_heap_size_of = "closures are hard"]
-    animation_frame_list: DOMRefCell<Vec<(u32, Option<Box<FnBox(f64)>>)>>,
+    animation_frame_list: DOMRefCell<Vec<(u32, Option<AnimationFrameCallback>)>>,
     /// Whether we're in the process of running animation callbacks.
     ///
     /// Tracking this is not necessary for correctness. Instead, it is an optimization to avoid
@@ -1457,7 +1457,7 @@ impl Document {
     }
 
     /// https://html.spec.whatwg.org/multipage/#dom-window-requestanimationframe
-    pub fn request_animation_frame(&self, callback: Box<FnBox(f64)>) -> u32 {
+    pub fn request_animation_frame(&self, callback: AnimationFrameCallback) -> u32 {
         let ident = self.animation_frame_ident.get() + 1;
 
         self.animation_frame_ident.set(ident);
@@ -1498,7 +1498,7 @@ impl Document {
 
         for (_, callback) in animation_frame_list.drain(..) {
             if let Some(callback) = callback {
-                callback(*timing);
+                callback.call(self, *timing);
             }
         }
 
@@ -3178,3 +3178,29 @@ pub enum FocusEventType {
     Focus,      // Element gained focus. Doesn't bubble.
     Blur,       // Element lost focus. Doesn't bubble.
 }
+
+#[derive(HeapSizeOf, JSTraceable)]
+pub enum AnimationFrameCallback {
+    DevtoolsFramerateTick { actor_name: String },
+    FrameRequestCallback {
+        #[ignore_heap_size_of = "Rc is hard"]
+        callback: Rc<FrameRequestCallback>
+    },
+}
+
+impl AnimationFrameCallback {
+    fn call(&self, document: &Document, now: f64) {
+        match *self {
+            AnimationFrameCallback::DevtoolsFramerateTick { ref actor_name } => {
+                let msg = ScriptToDevtoolsControlMsg::FramerateTick(actor_name.clone(), now);
+                let devtools_sender = document.window().upcast::<GlobalScope>().devtools_chan().unwrap();
+                devtools_sender.send(msg).unwrap();
+            }
+            AnimationFrameCallback::FrameRequestCallback { ref callback } => {
+                // TODO(jdm): The spec says that any exceptions should be suppressed:
+                // https://github.com/servo/servo/issues/6928
+                let _ = callback.Call__(Finite::wrap(now), ExceptionHandling::Report);
+            }
+        }
+    }
+}

components/script/dom/window.rs

@@ -6,7 +6,6 @@ use app_units::Au;
 use bluetooth_traits::BluetoothRequest;
 use cssparser::Parser;
 use devtools_traits::{ScriptToDevtoolsControlMsg, TimelineMarker, TimelineMarkerType};
-use dom::bindings::callback::ExceptionHandling;
 use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::DocumentBinding::{DocumentMethods, DocumentReadyState};
 use dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull;
@@ -30,7 +29,7 @@ use dom::bindings::utils::{GlobalStaticData, WindowProxyHandler};
 use dom::browsingcontext::BrowsingContext;
 use dom::crypto::Crypto;
 use dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration};
-use dom::document::Document;
+use dom::document::{AnimationFrameCallback, Document};
 use dom::element::Element;
 use dom::event::Event;
 use dom::globalscope::GlobalScope;
@@ -599,15 +598,8 @@ impl WindowMethods for Window {
 
     /// https://html.spec.whatwg.org/multipage/#dom-window-requestanimationframe
     fn RequestAnimationFrame(&self, callback: Rc<FrameRequestCallback>) -> u32 {
-        let doc = self.Document();
-
-        let callback = move |now: f64| {
-            // TODO: @jdm The spec says that any exceptions should be suppressed;
-            // https://github.com/servo/servo/issues/6928
-            let _ = callback.Call__(Finite::wrap(now), ExceptionHandling::Report);
-        };
-
-        doc.request_animation_frame(Box::new(callback))
+        self.Document()
+            .request_animation_frame(AnimationFrameCallback::FrameRequestCallback { callback })
     }
 
     /// https://html.spec.whatwg.org/multipage/#dom-window-cancelanimationframe

components/script/lib.rs

@@ -6,7 +6,7 @@
 #![feature(conservative_impl_trait)]
 #![feature(const_fn)]
 #![feature(core_intrinsics)]
-#![feature(fnbox)]
+#![feature(field_init_shorthand)]
 #![feature(mpsc_select)]
 #![feature(nonzero)]
 #![feature(on_unimplemented)]