From 9a933efb3e08f73d278712a21ea33daeedc132ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 15 Jul 2017 15:44:45 +0200 Subject: [PATCH] script: Move the layout_wrapper outside of script. This allows us to have ensure_data() and clear_data() functions on the TElement trait, instead of hacking around it adding methods in random traits. This also allows us to do some further cleanup, which I'd rather do in a followup. --- Cargo.lock | 6 +- components/layout/lib.rs | 3 +- components/layout/traversal.rs | 15 +---- components/layout/wrapper.rs | 34 +--------- components/layout_thread/Cargo.toml | 3 + .../dom_wrapper.rs} | 51 +++++++++++---- components/layout_thread/lib.rs | 12 +++- components/script/Cargo.toml | 3 - components/script/lib.rs | 22 +++++-- components/script/test.rs | 13 ---- .../script_layout_interface/wrapper_traits.rs | 1 + components/style/dom.rs | 11 ++++ components/style/gecko/traversal.rs | 14 +--- components/style/gecko/wrapper.rs | 65 ++++++++----------- components/style/traversal.rs | 30 ++------- ports/geckolib/glue.rs | 6 +- 16 files changed, 128 insertions(+), 161 deletions(-) rename components/{script/layout_wrapper.rs => layout_thread/dom_wrapper.rs} (96%) diff --git a/Cargo.lock b/Cargo.lock index 955269456cd1..48a6e4354562 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1485,11 +1485,13 @@ name = "layout_thread" version = "0.0.1" dependencies = [ "app_units 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", + "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", "gfx 0.0.1", "gfx_traits 0.0.1", "heapsize 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "html5ever 0.18.0 (registry+https://github.com/rust-lang/crates.io-index)", "ipc-channel 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "layout 0.0.1", "layout_traits 0.0.1", @@ -1499,6 +1501,7 @@ dependencies = [ "net_traits 0.0.1", "parking_lot 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)", "profile_traits 0.0.1", + "range 0.0.1", "rayon 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)", "script 0.0.1", "script_layout_interface 0.0.1", @@ -2413,7 +2416,6 @@ version = "0.0.1" dependencies = [ "angle 0.2.0 (git+https://github.com/servo/angle?branch=servo)", "app_units 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", - "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "audio-video-metadata 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "base64 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)", "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2431,7 +2433,6 @@ dependencies = [ "encoding 0.2.33 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", - "gfx_traits 0.0.1", "gleam 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)", "half 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "heapsize 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2458,7 +2459,6 @@ dependencies = [ "phf_codegen 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)", "phf_shared 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)", "profile_traits 0.0.1", - "range 0.0.1", "ref_filter_map 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "ref_slice 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "regex 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/components/layout/lib.rs b/components/layout/lib.rs index 868892ea0a59..3527c3590cd7 100644 --- a/components/layout/lib.rs +++ b/components/layout/lib.rs @@ -14,7 +14,6 @@ extern crate atomic_refcell; #[macro_use] extern crate bitflags; extern crate canvas_traits; -extern crate core; extern crate euclid; extern crate fnv; extern crate gfx; @@ -55,7 +54,7 @@ pub mod animation; mod block; pub mod construct; pub mod context; -mod data; +pub mod data; pub mod display_list_builder; mod flex; mod floats; diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index f011925ebb8f..cf75a47bbf82 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -4,7 +4,6 @@ //! Traversals over the DOM and flow trees, running the layout computations. -use atomic_refcell::AtomicRefCell; use construct::FlowConstructor; use context::LayoutContext; use display_list_builder::DisplayListBuildState; @@ -13,13 +12,12 @@ use flow::{CAN_BE_FRAGMENTED, Flow, ImmutableFlowUtils, PostorderFlowTraversal}; use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutNode}; use servo_config::opts; use style::context::{SharedStyleContext, StyleContext}; -use style::data::ElementData; use style::dom::{NodeInfo, TElement, TNode}; use style::selector_parser::RestyleDamage; use style::servo::restyle_damage::{BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, REPAINT}; use style::traversal::{DomTraversal, TraversalDriver, recalc_style_at}; use style::traversal::PerLevelTraversalData; -use wrapper::{GetRawData, LayoutNodeHelpers, LayoutNodeLayoutData}; +use wrapper::{GetRawData, LayoutNodeLayoutData}; use wrapper::ThreadSafeLayoutNodeHelpers; pub struct RecalcStyleAndConstructFlows<'a> { @@ -59,7 +57,7 @@ impl<'a, E> DomTraversal for RecalcStyleAndConstructFlows<'a> context: &mut StyleContext, node: E::ConcreteNode) { // FIXME(pcwalton): Stop allocating here. Ideally this should just be // done by the HTML parser. - node.initialize_data(); + unsafe { node.initialize_data() }; if !node.is_text_node() { let el = node.as_element().unwrap(); @@ -81,15 +79,6 @@ impl<'a, E> DomTraversal for RecalcStyleAndConstructFlows<'a> node.parent_node().unwrap().to_threadsafe().restyle_damage() != RestyleDamage::empty() } - unsafe fn ensure_element_data(element: &E) -> &AtomicRefCell { - element.as_node().initialize_data(); - element.get_data().unwrap() - } - - unsafe fn clear_element_data(element: &E) { - element.as_node().clear_data(); - } - fn shared_context(&self) -> &SharedStyleContext { &self.context.style_context } diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 87fe65bb3b9d..2a31e17cfa13 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -31,21 +31,13 @@ #![allow(unsafe_code)] use atomic_refcell::{AtomicRef, AtomicRefMut}; -use core::nonzero::NonZero; use data::{LayoutData, LayoutDataFlags, StyleAndLayoutData}; -use script_layout_interface::{OpaqueStyleAndLayoutData, StyleData}; -use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode}; +use script_layout_interface::wrapper_traits::{ThreadSafeLayoutElement, ThreadSafeLayoutNode}; use script_layout_interface::wrapper_traits::GetLayoutData; use style::computed_values::content::{self, ContentItem}; use style::dom::{NodeInfo, TNode}; use style::selector_parser::RestyleDamage; -pub unsafe fn drop_style_and_layout_data(data: OpaqueStyleAndLayoutData) { - let ptr: *mut StyleData = data.ptr.get(); - let non_opaque: *mut StyleAndLayoutData = ptr as *mut _; - let _ = Box::from_raw(non_opaque); -} - pub trait LayoutNodeLayoutData { /// Similar to borrow_data*, but returns the full PersistentLayoutData rather /// than only the style::data::ElementData. @@ -81,30 +73,6 @@ impl GetRawData for T { } } -pub trait LayoutNodeHelpers { - fn initialize_data(&self); - fn clear_data(&self); -} - -impl LayoutNodeHelpers for T { - fn initialize_data(&self) { - if self.get_raw_data().is_none() { - let ptr: *mut StyleAndLayoutData = - Box::into_raw(Box::new(StyleAndLayoutData::new())); - let opaque = OpaqueStyleAndLayoutData { - ptr: unsafe { NonZero::new(ptr as *mut StyleData) } - }; - unsafe { self.init_style_and_layout_data(opaque) }; - }; - } - - fn clear_data(&self) { - if self.get_raw_data().is_some() { - unsafe { drop_style_and_layout_data(self.take_style_and_layout_data()) }; - } - } -} - pub trait ThreadSafeLayoutNodeHelpers { /// Returns the layout data flags for this node. fn flags(self) -> LayoutDataFlags; diff --git a/components/layout_thread/Cargo.toml b/components/layout_thread/Cargo.toml index 9ad239eb0071..e1d93d2cbc96 100644 --- a/components/layout_thread/Cargo.toml +++ b/components/layout_thread/Cargo.toml @@ -11,11 +11,13 @@ path = "lib.rs" [dependencies] app_units = "0.5" +atomic_refcell = "0.1" euclid = "0.15" fnv = "1.0" gfx = {path = "../gfx"} gfx_traits = {path = "../gfx_traits"} heapsize = "0.4" +html5ever = "0.18" ipc-channel = "0.8" layout = {path = "../layout"} layout_traits = {path = "../layout_traits"} @@ -25,6 +27,7 @@ msg = {path = "../msg"} net_traits = {path = "../net_traits"} parking_lot = {version = "0.4", features = ["nightly"]} profile_traits = {path = "../profile_traits"} +range = {path = "../range"} rayon = "0.8" script = {path = "../script"} script_layout_interface = {path = "../script_layout_interface"} diff --git a/components/script/layout_wrapper.rs b/components/layout_thread/dom_wrapper.rs similarity index 96% rename from components/script/layout_wrapper.rs rename to components/layout_thread/dom_wrapper.rs index 297ede7a5644..cf4aef91049d 100644 --- a/components/script/layout_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -30,21 +30,22 @@ #![allow(unsafe_code)] -use atomic_refcell::{AtomicRef, AtomicRefCell}; -use dom::bindings::inheritance::{CharacterDataTypeId, ElementTypeId}; -use dom::bindings::inheritance::{HTMLElementTypeId, NodeTypeId}; -use dom::bindings::js::LayoutJS; -use dom::characterdata::LayoutCharacterDataHelpers; -use dom::document::{Document, LayoutDocumentHelpers, PendingRestyle}; -use dom::element::{Element, LayoutElementHelpers, RawLayoutElementHelpers}; -use dom::node::{CAN_BE_FRAGMENTED, DIRTY_ON_VIEWPORT_SIZE_CHANGE, HAS_DIRTY_DESCENDANTS, IS_IN_DOC}; -use dom::node::{HANDLED_SNAPSHOT, HAS_SNAPSHOT}; -use dom::node::{LayoutNodeHelpers, Node}; -use dom::text::Text; +use atomic_refcell::{AtomicRef, AtomicRefMut, AtomicRefCell}; +use core::nonzero::NonZero; use gfx_traits::ByteIndex; use html5ever::{LocalName, Namespace}; +use layout::data::StyleAndLayoutData; +use layout::wrapper::GetRawData; use msg::constellation_msg::{BrowsingContextId, PipelineId}; use range::Range; +use script::layout_exports::{CAN_BE_FRAGMENTED, DIRTY_ON_VIEWPORT_SIZE_CHANGE, HAS_DIRTY_DESCENDANTS, IS_IN_DOC}; +use script::layout_exports::{CharacterDataTypeId, ElementTypeId, HTMLElementTypeId, NodeTypeId}; +use script::layout_exports::{Document, Element, Node, Text}; +use script::layout_exports::{HANDLED_SNAPSHOT, HAS_SNAPSHOT}; +use script::layout_exports::{LayoutCharacterDataHelpers, LayoutDocumentHelpers}; +use script::layout_exports::{LayoutElementHelpers, LayoutNodeHelpers, RawLayoutElementHelpers}; +use script::layout_exports::LayoutJS; +use script::layout_exports::PendingRestyle; use script_layout_interface::{HTMLCanvasData, LayoutNodeType, SVGSVGData, TrustedNodeAddress}; use script_layout_interface::{OpaqueStyleAndLayoutData, StyleData}; use script_layout_interface::wrapper_traits::{DangerousThreadSafeLayoutNode, GetLayoutData, LayoutNode}; @@ -79,6 +80,12 @@ use style::shared_lock::{SharedRwLock as StyleSharedRwLock, Locked as StyleLocke use style::str::is_whitespace; use style::stylearc::Arc; +pub unsafe fn drop_style_and_layout_data(data: OpaqueStyleAndLayoutData) { + let ptr: *mut StyleData = data.ptr.get(); + let non_opaque: *mut StyleAndLayoutData = ptr as *mut _; + let _ = Box::from_raw(non_opaque); +} + #[derive(Copy, Clone)] pub struct ServoLayoutNode<'a> { /// The wrapped node. @@ -244,6 +251,17 @@ impl<'ln> LayoutNode for ServoLayoutNode<'ln> { self.script_type_id().into() } + unsafe fn initialize_data(&self) { + if self.get_raw_data().is_none() { + let ptr: *mut StyleAndLayoutData = + Box::into_raw(Box::new(StyleAndLayoutData::new())); + let opaque = OpaqueStyleAndLayoutData { + ptr: NonZero::new(ptr as *mut StyleData), + }; + self.init_style_and_layout_data(opaque); + }; + } + unsafe fn init_style_and_layout_data(&self, data: OpaqueStyleAndLayoutData) { self.get_jsmanaged().init_style_and_layout_data(data); } @@ -477,6 +495,17 @@ impl<'le> TElement for ServoLayoutElement<'le> { old_value - 1 } + unsafe fn clear_data(&self) { + if self.get_raw_data().is_some() { + drop_style_and_layout_data(self.as_node().take_style_and_layout_data()); + } + } + + unsafe fn ensure_data(&self) -> AtomicRefMut { + self.as_node().initialize_data(); + self.mutate_data().unwrap() + } + fn get_data(&self) -> Option<&AtomicRefCell> { unsafe { self.get_style_and_layout_data().map(|d| { diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index b6f520ff2ee3..fea91c164b79 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -7,13 +7,18 @@ #![feature(box_syntax)] #![feature(mpsc_select)] +#![feature(nonzero)] extern crate app_units; +extern crate atomic_refcell; +extern crate core; extern crate euclid; extern crate fnv; extern crate gfx; extern crate gfx_traits; extern crate heapsize; +#[macro_use] +extern crate html5ever; extern crate ipc_channel; #[macro_use] extern crate layout; @@ -27,6 +32,7 @@ extern crate net_traits; extern crate parking_lot; #[macro_use] extern crate profile_traits; +extern crate range; extern crate rayon; extern crate script; extern crate script_layout_interface; @@ -40,7 +46,11 @@ extern crate servo_url; extern crate style; extern crate webrender_api; +mod dom_wrapper; + use app_units::Au; +use dom_wrapper::{ServoLayoutElement, ServoLayoutDocument, ServoLayoutNode}; +use dom_wrapper::drop_style_and_layout_data; use euclid::{Point2D, Rect, Size2D, ScaleFactor}; use fnv::FnvHashMap; use gfx::display_list::{OpaqueNode, WebRenderImageInfo}; @@ -71,7 +81,6 @@ use layout::sequential; use layout::traversal::{ComputeAbsolutePositions, RecalcStyleAndConstructFlows}; use layout::webrender_helpers::WebRenderDisplayListConverter; use layout::wrapper::LayoutNodeLayoutData; -use layout::wrapper::drop_style_and_layout_data; use layout_traits::LayoutThreadFactory; use msg::constellation_msg::PipelineId; use msg::constellation_msg::TopLevelBrowsingContextId; @@ -80,7 +89,6 @@ use parking_lot::RwLock; use profile_traits::mem::{self, Report, ReportKind, ReportsChan}; use profile_traits::time::{self, TimerMetadata, profile}; use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType}; -use script::layout_wrapper::{ServoLayoutElement, ServoLayoutDocument, ServoLayoutNode}; use script_layout_interface::message::{Msg, NewLayoutThreadInfo, Reflow, ReflowQueryType}; use script_layout_interface::message::{ScriptReflow, ReflowComplete}; use script_layout_interface::rpc::{LayoutRPC, MarginStyleResponse, NodeOverflowResponse, OffsetParentResponse}; diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index cc9ae75842a7..eae31b8fef0e 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -27,7 +27,6 @@ tinyfiledialogs = "2.5.9" angle = {git = "https://github.com/servo/angle", branch = "servo"} app_units = "0.5" audio-video-metadata = "0.1.2" -atomic_refcell = "0.1" base64 = "0.5.2" bitflags = "0.7" bluetooth_traits = {path = "../bluetooth_traits"} @@ -44,7 +43,6 @@ encoding = "0.2" euclid = "0.15" fnv = "1.0" gleam = "0.4" -gfx_traits = {path = "../gfx_traits"} half = "1.0" heapsize = "0.4" heapsize_derive = "0.1" @@ -68,7 +66,6 @@ open = "1.1.1" parking_lot = "0.4" phf = "0.7.18" profile_traits = {path = "../profile_traits"} -range = {path = "../range"} ref_filter_map = "1.0.1" ref_slice = "1.0" regex = "0.2" diff --git a/components/script/lib.rs b/components/script/lib.rs index 82b1f80ab88b..1966ea6df937 100644 --- a/components/script/lib.rs +++ b/components/script/lib.rs @@ -26,7 +26,6 @@ extern crate angle; extern crate app_units; -extern crate atomic_refcell; extern crate audio_video_metadata; extern crate base64; #[macro_use] @@ -46,7 +45,6 @@ extern crate domobject_derive; extern crate encoding; extern crate euclid; extern crate fnv; -extern crate gfx_traits; extern crate gleam; extern crate half; #[macro_use] extern crate heapsize; @@ -78,7 +76,6 @@ extern crate parking_lot; extern crate phf; #[macro_use] extern crate profile_traits; -extern crate range; extern crate ref_filter_map; extern crate ref_slice; extern crate regex; @@ -115,7 +112,6 @@ pub mod document_loader; mod dom; pub mod fetch; mod layout_image; -pub mod layout_wrapper; mod mem; mod microtask; mod network_listener; @@ -132,6 +128,24 @@ mod timers; mod unpremultiplytable; mod webdriver_handlers; +/// A module with everything layout can use from script. +/// +/// Try to keep this small! +/// +/// TODO(emilio): A few of the FooHelpers can go away, presumably... +pub mod layout_exports { + pub use dom::bindings::inheritance::{CharacterDataTypeId, ElementTypeId}; + pub use dom::bindings::inheritance::{HTMLElementTypeId, NodeTypeId}; + pub use dom::bindings::js::LayoutJS; + pub use dom::characterdata::LayoutCharacterDataHelpers; + pub use dom::document::{Document, LayoutDocumentHelpers, PendingRestyle}; + pub use dom::element::{Element, LayoutElementHelpers, RawLayoutElementHelpers}; + pub use dom::node::{CAN_BE_FRAGMENTED, DIRTY_ON_VIEWPORT_SIZE_CHANGE, HAS_DIRTY_DESCENDANTS, IS_IN_DOC}; + pub use dom::node::{HANDLED_SNAPSHOT, HAS_SNAPSHOT}; + pub use dom::node::{LayoutNodeHelpers, Node}; + pub use dom::text::Text; +} + use dom::bindings::codegen::RegisterBindings; use dom::bindings::proxyhandler; use script_traits::SWManagerSenders; diff --git a/components/script/test.rs b/components/script/test.rs index 9aeff2a22a56..bdd84d681b86 100644 --- a/components/script/test.rs +++ b/components/script/test.rs @@ -24,7 +24,6 @@ pub mod size_of { use dom::htmlspanelement::HTMLSpanElement; use dom::node::Node; use dom::text::Text; - use layout_wrapper::{ServoLayoutElement, ServoLayoutNode, ServoThreadSafeLayoutNode}; use std::mem::size_of; pub fn CharacterData() -> usize { @@ -55,18 +54,6 @@ pub mod size_of { size_of::() } - pub fn SendElement() -> usize { - size_of::<::style::dom::SendElement>() - } - - pub fn SendNode() -> usize { - size_of::<::style::dom::SendNode>() - } - - pub fn ServoThreadSafeLayoutNode() -> usize { - size_of::() - } - pub fn Text() -> usize { size_of::() } diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 33116058aa9c..33ffc3b545b7 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -87,6 +87,7 @@ pub trait LayoutNode: Debug + GetLayoutData + TNode { /// Returns the type ID of this node. fn type_id(&self) -> LayoutNodeType; + unsafe fn initialize_data(&self); unsafe fn init_style_and_layout_data(&self, data: OpaqueStyleAndLayoutData); unsafe fn take_style_and_layout_data(&self) -> OpaqueStyleAndLayoutData; diff --git a/components/style/dom.rs b/components/style/dom.rs index ffb874430f6c..fefa193eba96 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -550,6 +550,17 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + /// traversal. Returns the number of children left to process. fn did_process_child(&self) -> isize; + /// Gets a reference to the ElementData container, or creates one. + /// + /// Unsafe because it can race to allocate and leak if not used with + /// exclusive access to the element. + unsafe fn ensure_data(&self) -> AtomicRefMut; + + /// Clears the element data reference, if any. + /// + /// Unsafe following the same reasoning as ensure_data. + unsafe fn clear_data(&self); + /// Gets a reference to the ElementData container. fn get_data(&self) -> Option<&AtomicRefCell>; diff --git a/components/style/gecko/traversal.rs b/components/style/gecko/traversal.rs index 35683ba08e30..9b0770318ce7 100644 --- a/components/style/gecko/traversal.rs +++ b/components/style/gecko/traversal.rs @@ -4,10 +4,8 @@ //! Gecko-specific bits for the styling DOM traversal. -use atomic_refcell::AtomicRefCell; use context::{SharedStyleContext, StyleContext}; -use data::ElementData; -use dom::{NodeInfo, TNode}; +use dom::{NodeInfo, TNode, TElement}; use gecko::wrapper::{GeckoElement, GeckoNode}; use traversal::{DomTraversal, PerLevelTraversalData, TraversalDriver, recalc_style_at}; @@ -36,7 +34,7 @@ impl<'recalc, 'le> DomTraversal> for RecalcStyleOnly<'recalc> { if node.is_element() { let el = node.as_element().unwrap(); - let mut data = unsafe { el.ensure_data() }.borrow_mut(); + let mut data = unsafe { el.ensure_data() }; recalc_style_at(self, traversal_data, context, el, &mut data); } } @@ -48,14 +46,6 @@ impl<'recalc, 'le> DomTraversal> for RecalcStyleOnly<'recalc> /// We don't use the post-order traversal for anything. fn needs_postorder_traversal() -> bool { false } - unsafe fn ensure_element_data<'a>(element: &'a GeckoElement<'le>) -> &'a AtomicRefCell { - element.ensure_data() - } - - unsafe fn clear_element_data<'a>(element: &'a GeckoElement<'le>) { - element.clear_data() - } - fn shared_context(&self) -> &SharedStyleContext { &self.shared } diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index ecee5dbbd6c4..cf28b396500b 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -17,7 +17,7 @@ use CaseSensitivityExt; use app_units::Au; use applicable_declarations::ApplicableDeclarationBlock; -use atomic_refcell::AtomicRefCell; +use atomic_refcell::{AtomicRefCell, AtomicRefMut}; use context::{QuirksMode, SharedStyleContext, UpdateAnimationsTasks}; use data::ElementData; use dom::{self, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode}; @@ -581,42 +581,6 @@ impl<'le> GeckoElement<'le> { self.namespace_id() == (structs::root::kNameSpaceID_XUL as i32) } - /// Clear the element data for a given element. - pub fn clear_data(&self) { - let ptr = self.0.mServoData.get(); - unsafe { - self.unset_flags(ELEMENT_HAS_SNAPSHOT as u32 | - ELEMENT_HANDLED_SNAPSHOT as u32); - } - if !ptr.is_null() { - debug!("Dropping ElementData for {:?}", self); - let data = unsafe { Box::from_raw(self.0.mServoData.get()) }; - self.0.mServoData.set(ptr::null_mut()); - - // Perform a mutable borrow of the data in debug builds. This - // serves as an assertion that there are no outstanding borrows - // when we destroy the data. - debug_assert!({ let _ = data.borrow_mut(); true }); - } - } - - /// Ensures the element has data, returning the existing data or allocating - /// it. - /// - /// Only safe to call with exclusive access to the element, given otherwise - /// it could race to allocate and leak. - pub unsafe fn ensure_data(&self) -> &AtomicRefCell { - match self.get_data() { - Some(x) => x, - None => { - debug!("Creating ElementData for {:?}", self); - let ptr = Box::into_raw(Box::new(AtomicRefCell::new(ElementData::default()))); - self.0.mServoData.set(ptr); - unsafe { &* ptr } - }, - } - } - /// Sets the specified element data, return any existing data. /// /// Like `ensure_data`, only safe to call with exclusive access to the @@ -1067,6 +1031,33 @@ impl<'le> TElement for GeckoElement<'le> { unsafe { self.0.mServoData.get().as_ref() } } + unsafe fn ensure_data(&self) -> AtomicRefMut { + if self.get_data().is_none() { + debug!("Creating ElementData for {:?}", self); + let ptr = Box::into_raw(Box::new(AtomicRefCell::new(ElementData::default()))); + self.0.mServoData.set(ptr); + } + self.mutate_data().unwrap() + } + + unsafe fn clear_data(&self) { + let ptr = self.0.mServoData.get(); + unsafe { + self.unset_flags(ELEMENT_HAS_SNAPSHOT as u32 | + ELEMENT_HANDLED_SNAPSHOT as u32); + } + if !ptr.is_null() { + debug!("Dropping ElementData for {:?}", self); + let data = unsafe { Box::from_raw(self.0.mServoData.get()) }; + self.0.mServoData.set(ptr::null_mut()); + + // Perform a mutable borrow of the data in debug builds. This + // serves as an assertion that there are no outstanding borrows + // when we destroy the data. + debug_assert!({ let _ = data.borrow_mut(); true }); + } + } + fn skip_root_and_item_based_display_fixup(&self) -> bool { // We don't want to fix up display values of native anonymous content. // Additionally, we want to skip root-based display fixup for document diff --git a/components/style/traversal.rs b/components/style/traversal.rs index aeac476b720e..b1a827be521a 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -4,7 +4,6 @@ //! Traversing the DOM tree; the bloom filter. -use atomic_refcell::AtomicRefCell; use context::{ElementCascadeInputs, StyleContext, SharedStyleContext}; use data::{ElementData, ElementStyles}; use dom::{NodeInfo, OpaqueNode, TElement, TNode}; @@ -489,20 +488,6 @@ pub trait DomTraversal : Sync { } } - /// Ensures the existence of the ElementData, and returns it. This can't - /// live on TNode because of the trait-based separation between Servo's - /// script and layout crates. - /// - /// This is only safe to call in top-down traversal before processing the - /// children of |element|. - unsafe fn ensure_element_data(element: &E) -> &AtomicRefCell; - - /// Clears the ElementData attached to this element, if any. - /// - /// This is only safe to call in top-down traversal before processing the - /// children of |element|. - unsafe fn clear_element_data(element: &E); - /// Return the shared style context common to all worker threads. fn shared_context(&self) -> &SharedStyleContext; @@ -647,7 +632,7 @@ where if data.styles.is_display_none() { debug!("{:?} style is display:none - clearing data from descendants.", element); - clear_descendant_data(element, &|e| unsafe { D::clear_element_data(&e) }); + clear_descendant_data(element) } } @@ -855,8 +840,7 @@ where continue; } - let mut child_data = - unsafe { D::ensure_element_data(&child).borrow_mut() }; + let mut child_data = unsafe { child.ensure_data() }; trace!(" > {:?} -> {:?} + {:?}, pseudo: {:?}", child, @@ -880,13 +864,9 @@ where } /// Clear style data for all the subtree under `el`. -pub fn clear_descendant_data( - el: E, - clear_data: &F -) +pub fn clear_descendant_data(el: E) where E: TElement, - F: Fn(E), { for kid in el.as_node().traversal_children() { if let Some(kid) = kid.as_element() { @@ -896,8 +876,8 @@ where // By consequence, any element without data has no descendants with // data. if kid.get_data().is_some() { - clear_data(kid); - clear_descendant_data(kid, clear_data); + unsafe { kid.clear_data() }; + clear_descendant_data(kid); } } } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 0ae6264e12f4..b57a3d07d12d 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -762,7 +762,7 @@ pub extern "C" fn Servo_StyleWorkerThreadCount() -> u32 { #[no_mangle] pub extern "C" fn Servo_Element_ClearData(element: RawGeckoElementBorrowed) { - GeckoElement(element).clear_data(); + unsafe { GeckoElement(element).clear_data() }; } #[no_mangle] @@ -1531,7 +1531,7 @@ pub extern "C" fn Servo_ResolvePseudoStyle(element: RawGeckoElementBorrowed, -> ServoComputedValuesStrong { let element = GeckoElement(element); - let data = unsafe { element.ensure_data() }.borrow_mut(); + let data = unsafe { element.ensure_data() }; let doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow(); debug!("Servo_ResolvePseudoStyle: {:?} {:?}, is_probe: {}", @@ -1582,7 +1582,7 @@ pub extern "C" fn Servo_SetExplicitStyle(element: RawGeckoElementBorrowed, // We only support this API for initial styling. There's no reason it couldn't // work for other things, we just haven't had a reason to do so. debug_assert!(element.get_data().is_none()); - let mut data = unsafe { element.ensure_data() }.borrow_mut(); + let mut data = unsafe { element.ensure_data() }; data.styles.primary = Some(style.clone()); }