From e8c9c12b6ef39e66173bd5c5f4b88e7e22b97edb Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 6 Dec 2016 10:17:42 -1000 Subject: [PATCH] Remove usage of FnBox for animation frame callbacks (fixes #14416) --- components/script/devtools.rs | 9 ++---- components/script/dom/bindings/trace.rs | 8 ------ components/script/dom/document.rs | 38 +++++++++++++++++++++---- components/script/dom/window.rs | 14 ++------- components/script/lib.rs | 2 +- 5 files changed, 39 insertions(+), 32 deletions(-) diff --git a/components/script/devtools.rs b/components/script/devtools.rs index 9e6a0f1e528..1b1ef4596d6 100644 --- a/components/script/devtools.rs +++ b/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::().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 }); } } diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 4c1ca5cb4ea..cb5deabd3f4 100644 --- a/components/script/dom/bindings/trace.rs +++ b/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 { - #[inline] - unsafe fn trace(&self, _trc: *mut JSTracer) { - // Do nothing - } -} - unsafe impl<'a> JSTraceable for &'a str { #[inline] unsafe fn trace(&self, _: *mut JSTracer) { diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 8b66d2db75d..2590b88d23e 100644 --- a/components/script/dom/document.rs +++ b/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, /// 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>)>>, + animation_frame_list: DOMRefCell)>>, /// 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) -> 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 + }, +} + +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::().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); + } + } + } +} diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index e5e85051c7c..4b4b74b846e 100644 --- a/components/script/dom/window.rs +++ b/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) -> 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 diff --git a/components/script/lib.rs b/components/script/lib.rs index cf3aadf1658..e80e9553956 100644 --- a/components/script/lib.rs +++ b/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)]