diff options
29 files changed, 488 insertions, 247 deletions
diff --git a/components/plugins/lints/unrooted_must_root.rs b/components/plugins/lints/unrooted_must_root.rs index 4e4d2328324..3170bb190e7 100644 --- a/components/plugins/lints/unrooted_must_root.rs +++ b/components/plugins/lints/unrooted_must_root.rs @@ -53,6 +53,8 @@ fn is_unrooted_ty(cx: &LateContext, ty: &ty::TyS, in_new_function: bool) -> bool false } else if match_def_path(cx, did.did, &["core", "cell", "Ref"]) || match_def_path(cx, did.did, &["core", "cell", "RefMut"]) + || match_def_path(cx, did.did, &["style", "refcell", "Ref"]) + || match_def_path(cx, did.did, &["style", "refcell", "RefMut"]) || match_def_path(cx, did.did, &["core", "slice", "Iter"]) || match_def_path(cx, did.did, &["std", "collections", "hash", "map", "OccupiedEntry"]) || match_def_path(cx, did.did, &["std", "collections", "hash", "map", "VacantEntry"]) { diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index 30aab73f112..b573f182547 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -56,7 +56,6 @@ plugins = {path = "../plugins"} profile_traits = {path = "../profile_traits"} rand = "0.3" range = {path = "../range"} -ref_filter_map = "1.0" ref_slice = "1.0" regex = "0.1.43" rustc-serialize = "0.3" diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs index d5a00e6c1fa..36de48a1f9b 100644 --- a/components/script/dom/attr.rs +++ b/components/script/dom/attr.rs @@ -15,10 +15,10 @@ use dom::element::{AttributeMutation, Element}; use dom::virtualmethods::vtable_for; use dom::window::Window; use std::borrow::ToOwned; -use std::cell::Ref; use std::mem; use string_cache::{Atom, Namespace}; use style::attr::{AttrIdentifier, AttrValue}; +use style::refcell::Ref; // https://dom.spec.whatwg.org/#interface-attr #[dom_struct] diff --git a/components/script/dom/bindings/mod.rs b/components/script/dom/bindings/mod.rs index 439016ee15b..02d5c9c8bef 100644 --- a/components/script/dom/bindings/mod.rs +++ b/components/script/dom/bindings/mod.rs @@ -128,8 +128,9 @@ //! return `Err()` from the method with the appropriate [error value] //! (error/enum.Error.html). +pub use style::domrefcell as cell; + pub mod callback; -pub mod cell; pub mod conversions; pub mod error; pub mod global; diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index d6969f38d6c..507e19e4bcb 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -88,6 +88,7 @@ use std::sync::mpsc::{Receiver, Sender}; use std::time::SystemTime; use string_cache::{Atom, Namespace, QualName}; use style::attr::{AttrIdentifier, AttrValue, LengthOrPercentageOrAuto}; +use style::domrefcell::DOMRefCell; use style::element_state::*; use style::properties::PropertyDeclarationBlock; use style::selector_impl::{PseudoElement, ElementSnapshot}; @@ -172,6 +173,13 @@ impl<T: JSTraceable> JSTraceable for UnsafeCell<T> { } } +impl<T: JSTraceable> JSTraceable for DOMRefCell<T> { + fn trace(&self, trc: *mut JSTracer) { + unsafe { + (*self).borrow_for_gc_trace().trace(trc) + } + } +} impl JSTraceable for Heap<*mut JSObject> { fn trace(&self, trc: *mut JSTracer) { diff --git a/components/script/dom/characterdata.rs b/components/script/dom/characterdata.rs index 01f11e3ac50..979e9a70434 100644 --- a/components/script/dom/characterdata.rs +++ b/components/script/dom/characterdata.rs @@ -19,7 +19,7 @@ use dom::element::Element; use dom::node::{Node, NodeDamage}; use dom::processinginstruction::ProcessingInstruction; use dom::text::Text; -use std::cell::Ref; +use style::refcell::Ref; use util::opts; // https://dom.spec.whatwg.org/#characterdata diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index 0335a7498c0..c048446d813 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -14,12 +14,13 @@ use dom::element::Element; use dom::node::{Node, NodeDamage, window_from_node}; use dom::window::Window; use std::ascii::AsciiExt; -use std::cell::Ref; use std::slice; +use std::sync::Arc; use string_cache::Atom; use style::parser::ParserContextExtraData; use style::properties::{PropertyDeclaration, Shorthand, Importance}; use style::properties::{is_supported_property, parse_one_declaration, parse_style_attribute}; +use style::refcell::Ref; use style::selector_impl::PseudoElement; // http://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface @@ -365,7 +366,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { *element.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() { None // Step 2 } else { - Some(decl_block) + Some(Arc::new(decl_block)) }; element.sync_property_with_attrs_style(); let node = element.upcast::<Node>(); diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index f6f06e7d851..4322e348c92 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -111,7 +111,7 @@ use script_traits::{TouchEventType, TouchId}; use std::ascii::AsciiExt; use std::borrow::ToOwned; use std::boxed::FnBox; -use std::cell::{Cell, Ref, RefMut}; +use std::cell::Cell; use std::collections::HashMap; use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::default::Default; @@ -122,6 +122,7 @@ use std::sync::Arc; use string_cache::{Atom, QualName}; use style::attr::AttrValue; use style::context::ReflowGoal; +use style::refcell::{Ref, RefMut}; use style::selector_impl::ElementSnapshot; use style::str::{split_html_space_chars, str_join}; use style::stylesheets::Stylesheet; diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 2d57ddab4d6..e0342ebc3a6 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -70,13 +70,12 @@ use html5ever::serialize::SerializeOpts; use html5ever::serialize::TraversalScope; use html5ever::serialize::TraversalScope::{ChildrenOnly, IncludeNode}; use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks}; -use ref_filter_map::ref_filter_map; use selectors::matching::{ElementFlags, matches}; use selectors::matching::{HAS_SLOW_SELECTOR, HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS}; use selectors::parser::{AttrSelector, NamespaceConstraint, parse_author_origin_selector_list_from_str}; use std::ascii::AsciiExt; use std::borrow::Cow; -use std::cell::{Cell, Ref}; +use std::cell::Cell; use std::convert::TryFrom; use std::default::Default; use std::fmt; @@ -90,6 +89,7 @@ use style::parser::ParserContextExtraData; use style::properties::longhands::{self, background_image, border_spacing, font_family, overflow_x, font_size}; use style::properties::{DeclaredValue, Importance}; use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, parse_style_attribute}; +use style::refcell::Ref; use style::selector_impl::{NonTSPseudoClass, ServoSelectorImpl}; use style::selector_matching::DeclarationBlock; use style::sink::Push; @@ -109,7 +109,7 @@ pub struct Element { prefix: Option<DOMString>, attrs: DOMRefCell<Vec<JS<Attr>>>, id_attribute: DOMRefCell<Option<Atom>>, - style_attribute: DOMRefCell<Option<PropertyDeclarationBlock>>, + style_attribute: DOMRefCell<Option<Arc<PropertyDeclarationBlock>>>, attr_list: MutNullableHeap<JS<NamedNodeMap>>, class_list: MutNullableHeap<JS<DOMTokenList>>, state: Cell<ElementState>, @@ -297,7 +297,7 @@ pub trait LayoutElementHelpers { #[allow(unsafe_code)] unsafe fn html_element_in_html_document_for_layout(&self) -> bool; fn id_attribute(&self) -> *const Option<Atom>; - fn style_attribute(&self) -> *const Option<PropertyDeclarationBlock>; + fn style_attribute(&self) -> *const Option<Arc<PropertyDeclarationBlock>>; fn local_name(&self) -> &Atom; fn namespace(&self) -> &Namespace; fn get_checked_state_for_layout(&self) -> bool; @@ -329,7 +329,10 @@ impl LayoutElementHelpers for LayoutJS<Element> { #[inline] fn from_declaration(rule: PropertyDeclaration) -> DeclarationBlock { DeclarationBlock::from_declarations( - Arc::new(vec![(rule, Importance::Normal)]), + Arc::new(PropertyDeclarationBlock { + declarations: vec![(rule, Importance::Normal)], + important_count: 0, + }), Importance::Normal) } @@ -615,7 +618,7 @@ impl LayoutElementHelpers for LayoutJS<Element> { } #[allow(unsafe_code)] - fn style_attribute(&self) -> *const Option<PropertyDeclarationBlock> { + fn style_attribute(&self) -> *const Option<Arc<PropertyDeclarationBlock>> { unsafe { (*self.unsafe_get()).style_attribute.borrow_for_layout() } @@ -704,7 +707,7 @@ impl Element { self.attrs.borrow() } - pub fn style_attribute(&self) -> &DOMRefCell<Option<PropertyDeclarationBlock>> { + pub fn style_attribute(&self) -> &DOMRefCell<Option<Arc<PropertyDeclarationBlock>>> { &self.style_attribute } @@ -774,7 +777,8 @@ impl Element { matching }); if let Some(index) = index { - Arc::make_mut(&mut declarations.declarations).remove(index); + let declarations = Arc::make_mut(declarations); + declarations.declarations.remove(index); if importance.unwrap().important() { declarations.important_count -= 1; } @@ -796,7 +800,8 @@ impl Element { { // Usually, the reference count will be 1 here. But transitions could make it greater // than that. - let existing_declarations = Arc::make_mut(&mut declaration_block.declarations); + let declaration_block = Arc::make_mut(declaration_block); + let existing_declarations = &mut declaration_block.declarations; 'outer: for incoming_declaration in declarations { for existing_declaration in &mut *existing_declarations { @@ -829,10 +834,10 @@ impl Element { 0 }; - *inline_declarations = Some(PropertyDeclarationBlock { - declarations: Arc::new(declarations.into_iter().map(|d| (d, importance)).collect()), + *inline_declarations = Some(Arc::new(PropertyDeclarationBlock { + declarations: declarations.into_iter().map(|d| (d, importance)).collect(), important_count: important_count, - }); + })); } update(self, declarations, importance); @@ -847,7 +852,8 @@ impl Element { if let &mut Some(ref mut block) = &mut *inline_declarations { // Usually, the reference counts of `from` and `to` will be 1 here. But transitions // could make them greater than that. - let declarations = Arc::make_mut(&mut block.declarations); + let block = Arc::make_mut(block); + let declarations = &mut block.declarations; for &mut (ref declaration, ref mut importance) in declarations { if properties.iter().any(|p| declaration.name() == **p) { match (*importance, new_importance) { @@ -871,7 +877,7 @@ impl Element { pub fn get_inline_style_declaration(&self, property: &Atom) -> Option<Ref<(PropertyDeclaration, Importance)>> { - ref_filter_map(self.style_attribute.borrow(), |inline_declarations| { + Ref::filter_map(self.style_attribute.borrow(), |inline_declarations| { inline_declarations.as_ref().and_then(|declarations| { declarations.declarations .iter() @@ -2102,8 +2108,11 @@ impl VirtualMethods for Element { *self.style_attribute.borrow_mut() = mutation.new_value(attr).map(|value| { let win = window_from_node(self); - parse_style_attribute(&value, &doc.base_url(), win.css_error_reporter(), - ParserContextExtraData::default()) + Arc::new(parse_style_attribute( + &value, + &doc.base_url(), + win.css_error_reporter(), + ParserContextExtraData::default())) }); if node.is_in_doc() { node.dirty(NodeDamage::NodeStyleDamaged); diff --git a/components/script/dom/htmlmetaelement.rs b/components/script/dom/htmlmetaelement.rs index b8c2cd423fa..e6f86f41d86 100644 --- a/components/script/dom/htmlmetaelement.rs +++ b/components/script/dom/htmlmetaelement.rs @@ -79,7 +79,7 @@ impl HTMLMetaElement { if !content.is_empty() { if let Some(translated_rule) = ViewportRule::from_meta(&**content) { *self.stylesheet.borrow_mut() = Some(Arc::new(Stylesheet { - rules: vec![CSSRule::Viewport(translated_rule)], + rules: vec![CSSRule::Viewport(Arc::new(translated_rule))], origin: Origin::Author, media: None, // Viewport constraints are always recomputed on resize; they don't need to diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 4edd821ad0b..a7f0a5019f3 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -459,9 +459,9 @@ impl<'le> TElement for ServoLayoutElement<'le> { ServoLayoutNode::from_layout_js(self.element.upcast()) } - fn style_attribute(&self) -> &Option<PropertyDeclarationBlock> { + fn style_attribute(&self) -> Option<&Arc<PropertyDeclarationBlock>> { unsafe { - &*self.element.style_attribute() + (*self.element.style_attribute()).as_ref() } } diff --git a/components/script/lib.rs b/components/script/lib.rs index ff3715de24c..910706f4a1a 100644 --- a/components/script/lib.rs +++ b/components/script/lib.rs @@ -17,7 +17,6 @@ #![feature(slice_patterns)] #![feature(stmt_expr_attributes)] #![feature(question_mark)] -#![feature(try_borrow)] #![feature(try_from)] #![deny(unsafe_code)] @@ -69,7 +68,6 @@ extern crate phf; extern crate profile_traits; extern crate rand; extern crate range; -extern crate ref_filter_map; extern crate ref_slice; extern crate regex; extern crate rustc_serialize; diff --git a/components/servo/Cargo.lock b/components/servo/Cargo.lock index ca979b2ca99..6b549b51221 100644 --- a/components/servo/Cargo.lock +++ b/components/servo/Cargo.lock @@ -1840,11 +1840,6 @@ dependencies = [ ] [[package]] -name = "ref_filter_map" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" - -[[package]] name = "ref_slice" version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -1929,7 +1924,6 @@ dependencies = [ "profile_traits 0.0.1", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "range 0.0.1", - "ref_filter_map 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "ref_slice 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "regex 0.1.73 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2866,7 +2860,6 @@ dependencies = [ "checksum quickersort 2.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e952ea7699262481636004bc4ab8afaccf2bc13f91b79d1aee6617bd8fc39651" "checksum rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)" = "2791d88c6defac799c3f20d74f094ca33b9332612d9aef9078519c82e4fe04a5" "checksum rayon 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8e501871917624668fe601ad12a730450414f9b0b64722a898b040ce3ae1b0fa" -"checksum ref_filter_map 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "2b5ceb840e4009da4841ed22a15eb49f64fdd00a2138945c5beacf506b2fb5ed" "checksum ref_slice 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "24c91f8f8903c37f0525112df98ef53b1985abca5702972e5e00854cd874baf2" "checksum regex 0.1.73 (registry+https://github.com/rust-lang/crates.io-index)" = "56b7ee9f764ecf412c6e2fff779bca4b22980517ae335a21aeaf4e32625a5df2" "checksum regex-syntax 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "31040aad7470ad9d8c46302dcffba337bb4289ca5da2e3cd6e37b64109a85199" diff --git a/components/style/dom.rs b/components/style/dom.rs index f503917e866..85b2d12c356 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -195,7 +195,7 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre fn as_node(&self) -> Self::ConcreteNode; - fn style_attribute(&self) -> &Option<PropertyDeclarationBlock>; + fn style_attribute(&self) -> Option<&Arc<PropertyDeclarationBlock>>; fn get_state(&self) -> ElementState; diff --git a/components/script/dom/bindings/cell.rs b/components/style/domrefcell.rs index d1b40af5b1a..a61a2a0d7fe 100644 --- a/components/script/dom/bindings/cell.rs +++ b/components/style/domrefcell.rs @@ -4,17 +4,15 @@ //! A shareable mutable container for the DOM. -use dom::bindings::trace::JSTraceable; -use js::jsapi::JSTracer; -use std::cell::{BorrowError, BorrowMutError, Ref, RefCell, RefMut}; -use style::thread_state; -use style::thread_state::SCRIPT; +use refcell::{BorrowError, BorrowMutError, Ref, RefCell, RefMut}; +use thread_state; /// A mutable field in the DOM. /// /// This extends the API of `core::cell::RefCell` to allow unsafe access in /// certain situations, with dynamic checking in debug builds. -#[derive(Clone, HeapSizeOf)] +#[derive(Clone)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct DOMRefCell<T> { value: RefCell<T>, } @@ -48,7 +46,7 @@ impl<T> DOMRefCell<T> { /// #[allow(unsafe_code)] pub unsafe fn borrow_for_script_deallocation(&self) -> &mut T { - debug_assert!(thread_state::get().contains(SCRIPT)); + debug_assert!(thread_state::get().contains(thread_state::SCRIPT)); &mut *self.value.as_ptr() } @@ -60,14 +58,6 @@ impl<T> DOMRefCell<T> { } } -impl<T: JSTraceable> JSTraceable for DOMRefCell<T> { - fn trace(&self, trc: *mut JSTracer) { - unsafe { - (*self).borrow_for_gc_trace().trace(trc) - } - } -} - // Functionality duplicated with `core::cell::RefCell` // =================================================== impl<T> DOMRefCell<T> { diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index 56bf4607d4d..d5ee58157b4 100644 --- a/components/style/keyframes.rs +++ b/components/style/keyframes.rs @@ -7,7 +7,7 @@ use cssparser::{DeclarationListParser, DeclarationParser}; use parser::{ParserContext, log_css_error}; use properties::PropertyDeclarationParseResult; use properties::animated_properties::TransitionProperty; -use properties::{PropertyDeclaration, Importance}; +use properties::{PropertyDeclaration, PropertyDeclarationBlock, Importance}; use std::sync::Arc; /// A number from 1 to 100, indicating the percentage of the animation where @@ -77,7 +77,7 @@ pub struct Keyframe { /// so the second value of these tuples is always `Importance::Normal`. /// But including them enables `compute_style_for_animation_step` to create a `DeclarationBlock` /// by cloning an `Arc<_>` (incrementing a reference count) rather than re-creating a `Vec<_>`. - pub declarations: Arc<Vec<(PropertyDeclaration, Importance)>>, + pub block: Arc<PropertyDeclarationBlock>, } /// A keyframes step value. This can be a synthetised keyframes animation, that @@ -88,7 +88,7 @@ pub struct Keyframe { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum KeyframesStepValue { /// See `Keyframe::declarations`’s docs about the presence of `Importance`. - Declarations(Arc<Vec<(PropertyDeclaration, Importance)>>), + Declarations(Arc<PropertyDeclarationBlock>), ComputedValues, } @@ -113,8 +113,8 @@ impl KeyframesStep { fn new(percentage: KeyframePercentage, value: KeyframesStepValue) -> Self { let declared_timing_function = match value { - KeyframesStepValue::Declarations(ref declarations) => { - declarations.iter().any(|&(ref prop_decl, _)| { + KeyframesStepValue::Declarations(ref block) => { + block.declarations.iter().any(|&(ref prop_decl, _)| { match *prop_decl { PropertyDeclaration::AnimationTimingFunction(..) => true, _ => false, @@ -154,7 +154,7 @@ fn get_animated_properties(keyframe: &Keyframe) -> Vec<TransitionProperty> { let mut ret = vec![]; // NB: declarations are already deduplicated, so we don't have to check for // it here. - for &(ref declaration, _) in keyframe.declarations.iter() { + for &(ref declaration, _) in keyframe.block.declarations.iter() { if let Some(property) = TransitionProperty::from_declaration(declaration) { ret.push(property); } @@ -164,7 +164,7 @@ fn get_animated_properties(keyframe: &Keyframe) -> Vec<TransitionProperty> { } impl KeyframesAnimation { - pub fn from_keyframes(keyframes: &[Keyframe]) -> Option<Self> { + pub fn from_keyframes(keyframes: &[Arc<Keyframe>]) -> Option<Self> { if keyframes.is_empty() { return None; } @@ -179,7 +179,7 @@ impl KeyframesAnimation { for keyframe in keyframes { for percentage in keyframe.selector.0.iter() { steps.push(KeyframesStep::new(*percentage, - KeyframesStepValue::Declarations(keyframe.declarations.clone()))); + KeyframesStepValue::Declarations(keyframe.block.clone()))); } } @@ -216,7 +216,7 @@ struct KeyframeListParser<'a> { context: &'a ParserContext<'a>, } -pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser) -> Vec<Keyframe> { +pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser) -> Vec<Arc<Keyframe>> { RuleListParser::new_for_nested_rule(input, KeyframeListParser { context: context }) .filter_map(Result::ok) .collect() @@ -225,12 +225,12 @@ pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser) -> Vec<K enum Void {} impl<'a> AtRuleParser for KeyframeListParser<'a> { type Prelude = Void; - type AtRule = Keyframe; + type AtRule = Arc<Keyframe>; } impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { type Prelude = KeyframeSelector; - type QualifiedRule = Keyframe; + type QualifiedRule = Arc<Keyframe>; fn parse_prelude(&self, input: &mut Parser) -> Result<Self::Prelude, ()> { let start = input.position(); @@ -263,10 +263,13 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { } // `parse_important` is not called here, `!important` is not allowed in keyframe blocks. } - Ok(Keyframe { + Ok(Arc::new(Keyframe { selector: prelude, - declarations: Arc::new(declarations), - }) + block: Arc::new(PropertyDeclarationBlock { + declarations: declarations, + important_count: 0, + }), + })) } } diff --git a/components/style/lib.rs b/components/style/lib.rs index fe1957d7c4e..9c1387b79c5 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -81,6 +81,7 @@ pub mod context; pub mod custom_properties; pub mod data; pub mod dom; +pub mod domrefcell; pub mod element_state; pub mod error_reporting; pub mod font_face; diff --git a/components/style/matching.rs b/components/style/matching.rs index ca040c9670b..749187fa58d 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -14,7 +14,7 @@ use context::{StyleContext, SharedStyleContext}; use data::PrivateStyleData; use dom::{TElement, TNode, TRestyleDamage, UnsafeNode}; use properties::longhands::display::computed_value as display; -use properties::{ComputedValues, cascade}; +use properties::{ComputedValues, cascade, PropertyDeclarationBlock}; use selector_impl::{TheSelectorImpl, PseudoElement}; use selector_matching::{DeclarationBlock, Stylist}; use selectors::bloom::BloomFilter; @@ -139,7 +139,7 @@ impl<'a> Hash for ApplicableDeclarationsCacheQuery<'a> { for declaration in self.declarations { // Each declaration contians an Arc, which is a stable // pointer; we use that for hashing and equality. - let ptr: *const Vec<_> = &*declaration.mixed_declarations; + let ptr: *const PropertyDeclarationBlock = &*declaration.mixed_declarations; ptr.hash(state); declaration.importance.hash(state); } @@ -651,7 +651,7 @@ pub trait ElementMatchMethods : TElement { applicable_declarations: &mut ApplicableDeclarations) -> StyleRelations { use traversal::relations_are_shareable; - let style_attribute = self.style_attribute().as_ref(); + let style_attribute = self.style_attribute(); let mut relations = stylist.push_applicable_declarations(self, diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 1c3c87501e3..09809cdc113 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -282,16 +282,36 @@ impl Importance { /// Overridden declarations are skipped. // FIXME (https://github.com/servo/servo/issues/3426) -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Clone)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct PropertyDeclarationBlock { #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")] - pub declarations: Arc<Vec<(PropertyDeclaration, Importance)>>, + pub declarations: Vec<(PropertyDeclaration, Importance)>, /// The number of entries in `self.declaration` with `Importance::Important` pub important_count: u32, } +impl PropertyDeclarationBlock { + /// Returns wheather this block contains any declaration with `!important`. + /// + /// This is based on the `important_count` counter, + /// which should be maintained whenever `declarations` is changed. + // FIXME: make fields private and maintain it here in methods? + pub fn any_important(&self) -> bool { + self.important_count > 0 + } + + /// Returns wheather this block contains any declaration without `!important`. + /// + /// This is based on the `important_count` counter, + /// which should be maintained whenever `declarations` is changed. + // FIXME: make fields private and maintain it here in methods? + pub fn any_normal(&self) -> bool { + self.declarations.len() > self.important_count as usize + } +} + impl ToCss for PropertyDeclarationBlock { // https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { @@ -567,7 +587,7 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars } } let mut block = PropertyDeclarationBlock { - declarations: Arc::new(declarations), + declarations: declarations, important_count: important_count, }; deduplicate_property_declarations(&mut block); @@ -583,8 +603,7 @@ fn deduplicate_property_declarations(block: &mut PropertyDeclarationBlock) { let mut seen_custom_normal = Vec::new(); let mut seen_custom_important = Vec::new(); - let declarations = Arc::get_mut(&mut block.declarations).unwrap(); - for (declaration, importance) in declarations.drain(..).rev() { + for (declaration, importance) in block.declarations.drain(..).rev() { match declaration { % for property in data.longhands: PropertyDeclaration::${property.camel_case}(..) => { @@ -636,7 +655,7 @@ fn deduplicate_property_declarations(block: &mut PropertyDeclarationBlock) { deduplicated.push((declaration, importance)) } deduplicated.reverse(); - *declarations = deduplicated; + block.declarations = deduplicated; } #[inline] diff --git a/components/style/refcell.rs b/components/style/refcell.rs index a2413b94ab2..f9a2b4a0672 100644 --- a/components/style/refcell.rs +++ b/components/style/refcell.rs @@ -15,8 +15,11 @@ #![allow(unsafe_code)] +#[cfg(feature = "servo")] use heapsize::HeapSizeOf; use std::cell::{UnsafeCell, Cell}; use std::cmp::Ordering; +use std::fmt::{self, Debug, Display}; +use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; /// A fork of std::cell::RefCell that makes `as_unsafe_cell` usable on stable Rust. @@ -28,7 +31,13 @@ pub struct RefCell<T: ?Sized> { borrow: Cell<BorrowFlag>, value: UnsafeCell<T>, } -type BorrowFlag = usize; + +#[cfg(feature = "servo")] +impl<T: HeapSizeOf> HeapSizeOf for RefCell<T> { + fn heap_size_of_children(&self) -> usize { + self.borrow().heap_size_of_children() + } +} /// An enumeration of values returned from the `state` method on a `RefCell<T>`. #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -41,8 +50,43 @@ pub enum BorrowState { Unused, } +/// An error returned by [`RefCell::try_borrow`](struct.RefCell.html#method.try_borrow). +pub struct BorrowError<'a, T: 'a + ?Sized> { + marker: PhantomData<&'a RefCell<T>>, +} + +impl<'a, T: ?Sized> Debug for BorrowError<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("BorrowError").finish() + } +} + +impl<'a, T: ?Sized> Display for BorrowError<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + Display::fmt("already mutably borrowed", f) + } +} + +/// An error returned by [`RefCell::try_borrow_mut`](struct.RefCell.html#method.try_borrow_mut). +pub struct BorrowMutError<'a, T: 'a + ?Sized> { + marker: PhantomData<&'a RefCell<T>>, +} + +impl<'a, T: ?Sized> Debug for BorrowMutError<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("BorrowMutError").finish() + } +} + +impl<'a, T: ?Sized> Display for BorrowMutError<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + Display::fmt("already borrowed", f) + } +} + // Values [1, MAX-1] represent the number of `Ref` active // (will not outgrow its range since `usize` is the size of the address space) +type BorrowFlag = usize; const UNUSED: BorrowFlag = 0; const WRITING: BorrowFlag = !0; @@ -90,6 +134,22 @@ impl<T: ?Sized> RefCell<T> { /// /// The returned value can be dispatched on to determine if a call to /// `borrow` or `borrow_mut` would succeed. + /// + /// # Examples + /// + /// ``` + /// #![feature(borrow_state)] + /// + /// use std::cell::{BorrowState, RefCell}; + /// + /// let c = RefCell::new(5); + /// + /// match c.borrow_state() { + /// BorrowState::Writing => println!("Cannot be borrowed"), + /// BorrowState::Reading => println!("Cannot be borrowed mutably"), + /// BorrowState::Unused => println!("Can be borrowed (mutably as well)"), + /// } + /// ``` #[inline] pub fn borrow_state(&self) -> BorrowState { match self.borrow.get() { @@ -106,7 +166,8 @@ impl<T: ?Sized> RefCell<T> { /// /// # Panics /// - /// Panics if the value is currently mutably borrowed. + /// Panics if the value is currently mutably borrowed. For a non-panicking variant, use + /// [`try_borrow`](#method.try_borrow). /// /// # Examples /// @@ -136,12 +197,44 @@ impl<T: ?Sized> RefCell<T> { /// ``` #[inline] pub fn borrow(&self) -> Ref<T> { + self.try_borrow().expect("already mutably borrowed") + } + + /// Immutably borrows the wrapped value, returning an error if the value is currently mutably + /// borrowed. + /// + /// The borrow lasts until the returned `Ref` exits scope. Multiple immutable borrows can be + /// taken out at the same time. + /// + /// This is the non-panicking variant of [`borrow`](#method.borrow). + /// + /// # Examples + /// + /// ``` + /// #![feature(try_borrow)] + /// + /// use std::cell::RefCell; + /// + /// let c = RefCell::new(5); + /// + /// { + /// let m = c.borrow_mut(); + /// assert!(c.try_borrow().is_err()); + /// } + /// + /// { + /// let m = c.borrow(); + /// assert!(c.try_borrow().is_ok()); + /// } + /// ``` + #[inline] + pub fn try_borrow(&self) -> Result<Ref<T>, BorrowError<T>> { match BorrowRef::new(&self.borrow) { - Some(b) => Ref { + Some(b) => Ok(Ref { value: unsafe { &*self.value.get() }, borrow: b, - }, - None => panic!("RefCell<T> already mutably borrowed"), + }), + None => Err(BorrowError { marker: PhantomData }), } } @@ -152,7 +245,8 @@ impl<T: ?Sized> RefCell<T> { /// /// # Panics /// - /// Panics if the value is currently borrowed. + /// Panics if the value is currently borrowed. For a non-panicking variant, use + /// [`try_borrow_mut`](#method.try_borrow_mut). /// /// # Examples /// @@ -183,12 +277,40 @@ impl<T: ?Sized> RefCell<T> { /// ``` #[inline] pub fn borrow_mut(&self) -> RefMut<T> { + self.try_borrow_mut().expect("already borrowed") + } + + /// Mutably borrows the wrapped value, returning an error if the value is currently borrowed. + /// + /// The borrow lasts until the returned `RefMut` exits scope. The value cannot be borrowed + /// while this borrow is active. + /// + /// This is the non-panicking variant of [`borrow_mut`](#method.borrow_mut). + /// + /// # Examples + /// + /// ``` + /// #![feature(try_borrow)] + /// + /// use std::cell::RefCell; + /// + /// let c = RefCell::new(5); + /// + /// { + /// let m = c.borrow(); + /// assert!(c.try_borrow_mut().is_err()); + /// } + /// + /// assert!(c.try_borrow_mut().is_ok()); + /// ``` + #[inline] + pub fn try_borrow_mut(&self) -> Result<RefMut<T>, BorrowMutError<T>> { match BorrowRefMut::new(&self.borrow) { - Some(b) => RefMut { + Some(b) => Ok(RefMut { value: unsafe { &mut *self.value.get() }, borrow: b, - }, - None => panic!("RefCell<T> already borrowed"), + }), + None => Err(BorrowMutError { marker: PhantomData }), } } @@ -197,15 +319,53 @@ impl<T: ?Sized> RefCell<T> { /// This can be used to circumvent `RefCell`'s safety checks. /// /// This function is `unsafe` because `UnsafeCell`'s field is public. + /// + /// # Examples + /// + /// ``` + /// #![feature(as_unsafe_cell)] + /// + /// use std::cell::RefCell; + /// + /// let c = RefCell::new(5); + /// let c = unsafe { c.as_unsafe_cell() }; + /// ``` #[inline] pub unsafe fn as_unsafe_cell(&self) -> &UnsafeCell<T> { &self.value } + /// Returns a raw pointer to the underlying data in this cell. + /// + /// # Examples + /// + /// ``` + /// use std::cell::RefCell; + /// + /// let c = RefCell::new(5); + /// + /// let ptr = c.as_ptr(); + /// ``` + #[inline] + pub fn as_ptr(&self) -> *mut T { + self.value.get() + } + /// Returns a mutable reference to the underlying data. /// /// This call borrows `RefCell` mutably (at compile-time) so there is no /// need for dynamic checks. + /// + /// # Examples + /// + /// ``` + /// use std::cell::RefCell; + /// + /// let mut c = RefCell::new(5); + /// *c.get_mut() += 1; + /// + /// assert_eq!(c, RefCell::new(6)); + /// ``` #[inline] pub fn get_mut(&mut self) -> &mut T { unsafe { @@ -375,6 +535,18 @@ impl<'b, T: ?Sized> Ref<'b, T> { borrow: orig.borrow, } } + + #[inline] + pub fn filter_map<U: ?Sized, F>(orig: Ref<'b, T>, f: F) -> Option<Ref<'b, U>> + where F: FnOnce(&T) -> Option<&U> + { + f(orig.value).map(move |new_value| { + Ref { + value: new_value, + borrow: orig.borrow, + } + }) + } } impl<'b, T: ?Sized> RefMut<'b, T> { @@ -461,3 +633,35 @@ impl<'b, T: ?Sized> DerefMut for RefMut<'b, T> { self.value } } + + +// Imported from src/libcore/fmt/mod.rs + +impl<T: ?Sized + Debug> Debug for RefCell<T> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self.borrow_state() { + BorrowState::Unused | BorrowState::Reading => { + f.debug_struct("RefCell") + .field("value", &self.borrow()) + .finish() + } + BorrowState::Writing => { + f.debug_struct("RefCell") + .field("value", &"<borrowed>") + .finish() + } + } + } +} + +impl<'b, T: ?Sized + Debug> Debug for Ref<'b, T> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + Debug::fmt(&**self, f) + } +} + +impl<'b, T: ?Sized + Debug> Debug for RefMut<'b, T> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + Debug::fmt(&*(self.deref()), f) + } +} diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs index 66f533d174b..0fd1d01453e 100644 --- a/components/style/selector_matching.rs +++ b/components/style/selector_matching.rs @@ -165,28 +165,26 @@ impl Stylist { // Take apart the StyleRule into individual Rules and insert // them into the SelectorMap of that priority. macro_rules! append( - ($style_rule: ident, $priority: ident, $importance: expr, $count: expr) => { - if $count > 0 { - for selector in &$style_rule.selectors { - let map = if let Some(ref pseudo) = selector.pseudo_element { - self.pseudos_map - .entry(pseudo.clone()) - .or_insert_with(PerPseudoElementSelectorMap::new) - .borrow_for_origin(&stylesheet.origin) - } else { - self.element_map.borrow_for_origin(&stylesheet.origin) - }; - - map.$priority.insert(Rule { - selector: selector.complex_selector.clone(), - declarations: DeclarationBlock { - specificity: selector.specificity, - mixed_declarations: $style_rule.declarations.declarations.clone(), - importance: $importance, - source_order: rules_source_order, - }, - }); - } + ($style_rule: ident, $priority: ident, $importance: expr) => { + for selector in &$style_rule.selectors { + let map = if let Some(ref pseudo) = selector.pseudo_element { + self.pseudos_map + .entry(pseudo.clone()) + .or_insert_with(PerPseudoElementSelectorMap::new) + .borrow_for_origin(&stylesheet.origin) + } else { + self.element_map.borrow_for_origin(&stylesheet.origin) + }; + + map.$priority.insert(Rule { + selector: selector.complex_selector.clone(), + declarations: DeclarationBlock { + specificity: selector.specificity, + mixed_declarations: $style_rule.declarations.clone(), + importance: $importance, + source_order: rules_source_order, + }, + }); } }; ); @@ -194,10 +192,8 @@ impl Stylist { for rule in stylesheet.effective_rules(&self.device) { match *rule { CSSRule::Style(ref style_rule) => { - let important_count = style_rule.declarations.important_count; - let normal_count = style_rule.declarations.declarations.len() as u32 - important_count; - append!(style_rule, normal, Importance::Normal, normal_count); - append!(style_rule, important, Importance::Important, important_count); + append!(style_rule, normal, Importance::Normal); + append!(style_rule, important, Importance::Important); rules_source_order += 1; for selector in &style_rule.selectors { @@ -346,7 +342,7 @@ impl Stylist { &self, element: &E, parent_bf: Option<&BloomFilter>, - style_attribute: Option<&PropertyDeclarationBlock>, + style_attribute: Option<&Arc<PropertyDeclarationBlock>>, pseudo_element: Option<&PseudoElement>, applicable_declarations: &mut V) -> StyleRelations where E: Element<Impl=TheSelectorImpl> + @@ -373,7 +369,8 @@ impl Stylist { map.user_agent.normal.get_all_matching_rules(element, parent_bf, applicable_declarations, - &mut relations); + &mut relations, + Importance::Normal); debug!("UA normal: {:?}", relations); // Step 2: Presentational hints. @@ -389,23 +386,23 @@ impl Stylist { map.user.normal.get_all_matching_rules(element, parent_bf, applicable_declarations, - &mut relations); + &mut relations, + Importance::Normal); debug!("user normal: {:?}", relations); map.author.normal.get_all_matching_rules(element, parent_bf, applicable_declarations, - &mut relations); + &mut relations, + Importance::Normal); debug!("author normal: {:?}", relations); // Step 4: Normal style attributes. - if let Some(ref sa) = style_attribute { - if sa.declarations.len() as u32 - sa.important_count > 0 { + if let Some(sa) = style_attribute { + if sa.any_normal() { relations |= AFFECTED_BY_STYLE_ATTRIBUTE; Push::push( applicable_declarations, - DeclarationBlock::from_declarations( - sa.declarations.clone(), - Importance::Normal)); + DeclarationBlock::from_declarations(sa.clone(), Importance::Normal)); } } @@ -415,19 +412,18 @@ impl Stylist { map.author.important.get_all_matching_rules(element, parent_bf, applicable_declarations, - &mut relations); + &mut relations, + Importance::Important); debug!("author important: {:?}", relations); // Step 6: `!important` style attributes. - if let Some(ref sa) = style_attribute { - if sa.important_count > 0 { + if let Some(sa) = style_attribute { + if sa.any_important() { relations |= AFFECTED_BY_STYLE_ATTRIBUTE; Push::push( applicable_declarations, - DeclarationBlock::from_declarations( - sa.declarations.clone(), - Importance::Important)); + DeclarationBlock::from_declarations(sa.clone(), Importance::Important)); } } @@ -437,14 +433,16 @@ impl Stylist { map.user.important.get_all_matching_rules(element, parent_bf, applicable_declarations, - &mut relations); + &mut relations, + Importance::Important); debug!("user important: {:?}", relations); map.user_agent.important.get_all_matching_rules(element, parent_bf, applicable_declarations, - &mut relations); + &mut relations, + Importance::Important); debug!("UA important: {:?}", relations); @@ -651,7 +649,8 @@ impl SelectorMap { element: &E, parent_bf: Option<&BloomFilter>, matching_rules_list: &mut V, - relations: &mut StyleRelations) + relations: &mut StyleRelations, + importance: Importance) where E: Element<Impl=TheSelectorImpl>, V: VecLike<DeclarationBlock> { @@ -667,7 +666,8 @@ impl SelectorMap { &self.id_hash, &id, matching_rules_list, - relations) + relations, + importance) } element.each_class(|class| { @@ -676,7 +676,8 @@ impl SelectorMap { &self.class_hash, class, matching_rules_list, - relations); + relations, + importance); }); let local_name_hash = if element.is_html_element_in_html_document() { @@ -689,13 +690,15 @@ impl SelectorMap { local_name_hash, element.get_local_name(), matching_rules_list, - relations); + relations, + importance); SelectorMap::get_matching_rules(element, parent_bf, &self.other_rules, matching_rules_list, - relations); + relations, + importance); // Sort only the rules we just added. sort_by_key(&mut matching_rules_list[init_len..], @@ -731,7 +734,8 @@ impl SelectorMap { hash: &FnvHashMap<Str, Vec<Rule>>, key: &BorrowedStr, matching_rules: &mut Vector, - relations: &mut StyleRelations) + relations: &mut StyleRelations, + importance: Importance) where E: Element<Impl=TheSelectorImpl>, Str: Borrow<BorrowedStr> + Eq + Hash, BorrowedStr: Eq + Hash, @@ -742,7 +746,8 @@ impl SelectorMap { parent_bf, rules, matching_rules, - relations) + relations, + importance) } } @@ -751,12 +756,20 @@ impl SelectorMap { parent_bf: Option<&BloomFilter>, rules: &[Rule], matching_rules: &mut V, - relations: &mut StyleRelations) + relations: &mut StyleRelations, + importance: Importance) where E: Element<Impl=TheSelectorImpl>, V: VecLike<DeclarationBlock> { for rule in rules.iter() { - if matches_complex_selector(&*rule.selector, + let block = &rule.declarations.mixed_declarations; + let any_declaration_for_importance = if importance.important() { + block.any_important() + } else { + block.any_normal() + }; + if any_declaration_for_importance && + matches_complex_selector(&*rule.selector, element, parent_bf, relations) { matching_rules.push(rule.declarations.clone()); } @@ -845,7 +858,7 @@ pub struct Rule { pub struct DeclarationBlock { /// Contains declarations of either importance, but only those of self.importance are relevant. /// Use DeclarationBlock::iter - pub mixed_declarations: Arc<Vec<(PropertyDeclaration, Importance)>>, + pub mixed_declarations: Arc<PropertyDeclarationBlock>, pub importance: Importance, pub source_order: usize, pub specificity: u32, @@ -853,7 +866,7 @@ pub struct DeclarationBlock { impl DeclarationBlock { #[inline] - pub fn from_declarations(declarations: Arc<Vec<(PropertyDeclaration, Importance)>>, + pub fn from_declarations(declarations: Arc<PropertyDeclarationBlock>, importance: Importance) -> Self { DeclarationBlock { @@ -866,7 +879,7 @@ impl DeclarationBlock { pub fn iter(&self) -> DeclarationBlockIter { DeclarationBlockIter { - iter: self.mixed_declarations.iter(), + iter: self.mixed_declarations.declarations.iter(), importance: self.importance, } } diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index e622679fee5..6c4f452f7d9 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -19,6 +19,7 @@ use smallvec::SmallVec; use std::cell::Cell; use std::iter::Iterator; use std::slice; +use std::sync::Arc; use string_cache::{Atom, Namespace}; use url::Url; use viewport::ViewportRule; @@ -64,31 +65,37 @@ pub struct UserAgentStylesheets { #[derive(Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum CSSRule { - Charset(String), - Namespace { - /// `None` for the default Namespace - prefix: Option<Atom>, - url: Namespace, - }, - Style(StyleRule), - Media(MediaRule), - FontFace(FontFaceRule), - Viewport(ViewportRule), - Keyframes(KeyframesRule), + // No Charset here, CSSCharsetRule has been removed from CSSOM + // https://drafts.csswg.org/cssom/#changes-from-5-december-2013 + + Namespace(Arc<NamespaceRule>), + Style(Arc<StyleRule>), + Media(Arc<MediaRule>), + FontFace(Arc<FontFaceRule>), + Viewport(Arc<ViewportRule>), + Keyframes(Arc<KeyframesRule>), } #[derive(Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub struct NamespaceRule { + /// `None` for the default Namespace + pub prefix: Option<Atom>, + pub url: Namespace, +} + +#[derive(Debug, PartialEq)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct KeyframesRule { pub name: Atom, - pub keyframes: Vec<Keyframe>, + pub keyframes: Vec<Arc<Keyframe>>, } #[derive(Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct MediaRule { - pub media_queries: MediaQueryList, + pub media_queries: Arc<MediaQueryList>, pub rules: Vec<CSSRule>, } @@ -104,7 +111,7 @@ impl MediaRule { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct StyleRule { pub selectors: Vec<Selector<TheSelectorImpl>>, - pub declarations: PropertyDeclarationBlock, + pub declarations: Arc<PropertyDeclarationBlock>, } @@ -154,13 +161,13 @@ impl Stylesheet { while let Some(result) = iter.next() { match result { Ok(rule) => { - if let CSSRule::Namespace { ref prefix, ref url } = rule { - if let Some(prefix) = prefix.as_ref() { + if let CSSRule::Namespace(ref rule) = rule { + if let Some(ref prefix) = rule.prefix { iter.parser.context.selector_context.namespace_prefixes.insert( - prefix.clone(), url.clone()); + prefix.clone(), rule.url.clone()); } else { iter.parser.context.selector_context.default_namespace = - Some(url.clone()); + Some(rule.url.clone()); } } @@ -408,7 +415,7 @@ enum AtRulePrelude { /// A @font-face rule prelude. FontFace, /// A @media rule prelude, with its media queries. - Media(MediaQueryList), + Media(Arc<MediaQueryList>), /// A @viewport rule prelude. Viewport, /// A @keyframes rule, with its animation name. @@ -423,16 +430,6 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { fn parse_prelude(&self, name: &str, input: &mut Parser) -> Result<AtRuleType<AtRulePrelude, CSSRule>, ()> { match_ignore_ascii_case! { name, - "charset" => { - if self.state.get() <= State::Start { - // Valid @charset rules are just ignored - self.state.set(State::Imports); - let charset = try!(input.expect_string()).into_owned(); - return Ok(AtRuleType::WithoutBlock(CSSRule::Charset(charset))) - } else { - return Err(()) // "@charset must be the first rule" - } - }, "import" => { if self.state.get() <= State::Imports { self.state.set(State::Imports); @@ -448,14 +445,17 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { let prefix = input.try(|input| input.expect_ident()).ok().map(|p| p.into()); let url = Namespace(Atom::from(try!(input.expect_url_or_string()))); - return Ok(AtRuleType::WithoutBlock(CSSRule::Namespace { + return Ok(AtRuleType::WithoutBlock(CSSRule::Namespace(Arc::new(NamespaceRule { prefix: prefix, url: url, - })) + })))) } else { return Err(()) // "@namespace must be before any rule but @charset and @import" } }, + // @charset is removed by rust-cssparser if it’s the first rule in the stylesheet + // anything left is invalid. + "charset" => return Err(()), // (insert appropriate error message) _ => {} } @@ -502,7 +502,7 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { match_ignore_ascii_case! { name, "media" => { let media_queries = parse_media_query_list(input); - Ok(AtRuleType::WithBlock(AtRulePrelude::Media(media_queries))) + Ok(AtRuleType::WithBlock(AtRulePrelude::Media(Arc::new(media_queries)))) }, "font-face" => { Ok(AtRuleType::WithBlock(AtRulePrelude::FontFace)) @@ -530,22 +530,22 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { fn parse_block(&self, prelude: AtRulePrelude, input: &mut Parser) -> Result<CSSRule, ()> { match prelude { AtRulePrelude::FontFace => { - parse_font_face_block(self.context, input).map(CSSRule::FontFace) + Ok(CSSRule::FontFace(Arc::new(try!(parse_font_face_block(self.context, input))))) } AtRulePrelude::Media(media_queries) => { - Ok(CSSRule::Media(MediaRule { + Ok(CSSRule::Media(Arc::new(MediaRule { media_queries: media_queries, rules: parse_nested_rules(self.context, input), - })) + }))) } AtRulePrelude::Viewport => { - ViewportRule::parse(input, self.context).map(CSSRule::Viewport) + Ok(CSSRule::Viewport(Arc::new(try!(ViewportRule::parse(input, self.context))))) } AtRulePrelude::Keyframes(name) => { - Ok(CSSRule::Keyframes(KeyframesRule { + Ok(CSSRule::Keyframes(Arc::new(KeyframesRule { name: name, keyframes: parse_keyframe_list(&self.context, input), - })) + }))) } } } @@ -560,9 +560,9 @@ impl<'a, 'b> QualifiedRuleParser for NestedRuleParser<'a, 'b> { } fn parse_block(&self, prelude: Vec<Selector<TheSelectorImpl>>, input: &mut Parser) -> Result<CSSRule, ()> { - Ok(CSSRule::Style(StyleRule { + Ok(CSSRule::Style(Arc::new(StyleRule { selectors: prelude, - declarations: parse_property_declaration_list(self.context, input) - })) + declarations: Arc::new(parse_property_declaration_list(self.context, input)) + }))) } } diff --git a/ports/cef/Cargo.lock b/ports/cef/Cargo.lock index 57c402fc74f..1174b3037cc 100644 --- a/ports/cef/Cargo.lock +++ b/ports/cef/Cargo.lock @@ -1692,11 +1692,6 @@ dependencies = [ ] [[package]] -name = "ref_filter_map" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" - -[[package]] name = "ref_slice" version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -1781,7 +1776,6 @@ dependencies = [ "profile_traits 0.0.1", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "range 0.0.1", - "ref_filter_map 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "ref_slice 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "regex 0.1.73 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2720,7 +2714,6 @@ dependencies = [ "checksum quickersort 2.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e952ea7699262481636004bc4ab8afaccf2bc13f91b79d1aee6617bd8fc39651" "checksum rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)" = "2791d88c6defac799c3f20d74f094ca33b9332612d9aef9078519c82e4fe04a5" "checksum rayon 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8e501871917624668fe601ad12a730450414f9b0b64722a898b040ce3ae1b0fa" -"checksum ref_filter_map 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "2b5ceb840e4009da4841ed22a15eb49f64fdd00a2138945c5beacf506b2fb5ed" "checksum ref_slice 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "24c91f8f8903c37f0525112df98ef53b1985abca5702972e5e00854cd874baf2" "checksum regex 0.1.73 (registry+https://github.com/rust-lang/crates.io-index)" = "56b7ee9f764ecf412c6e2fff779bca4b22980517ae335a21aeaf4e32625a5df2" "checksum regex-syntax 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "31040aad7470ad9d8c46302dcffba337bb4289ca5da2e3cd6e37b64109a85199" diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index ae682ac72e0..0874b0a44b9 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -356,7 +356,7 @@ pub extern "C" fn Servo_StyleSet_Drop(data: *mut RawServoStyleSet) -> () { } pub struct GeckoDeclarationBlock { - pub declarations: Option<PropertyDeclarationBlock>, + pub declarations: Option<Arc<PropertyDeclarationBlock>>, // XXX The following two fields are made atomic to work around the // ownership system so that they can be changed inside a shared // instance. It wouldn't provide safety as Rust usually promises, @@ -377,7 +377,7 @@ pub extern "C" fn Servo_ParseStyleAttribute(bytes: *const u8, length: u32, -> ServoDeclarationBlockStrong { let value = unsafe { from_utf8_unchecked(slice::from_raw_parts(bytes, length as usize)) }; GeckoDeclarationBlock::from_arc(Arc::new(GeckoDeclarationBlock { - declarations: GeckoElement::parse_style_attribute(value), + declarations: GeckoElement::parse_style_attribute(value).map(Arc::new), cache: AtomicPtr::new(cache), immutable: AtomicBool::new(false), })) diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index 6a356b2e3c4..712febe446d 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -35,7 +35,6 @@ use snapshot::GeckoElementSnapshot; use snapshot_helpers; use std::fmt; use std::marker::PhantomData; -use std::mem::transmute; use std::ops::BitOr; use std::ptr; use std::sync::Arc; @@ -462,8 +461,6 @@ lazy_static! { }; } -static NO_STYLE_ATTRIBUTE: Option<PropertyDeclarationBlock> = None; - impl<'le> TElement for GeckoElement<'le> { type ConcreteNode = GeckoNode<'le>; type ConcreteDocument = GeckoDocument<'le>; @@ -472,14 +469,16 @@ impl<'le> TElement for GeckoElement<'le> { unsafe { GeckoNode::from_raw(self.element as *mut RawGeckoNode) } } - fn style_attribute(&self) -> &Option<PropertyDeclarationBlock> { + fn style_attribute(&self) -> Option<&Arc<PropertyDeclarationBlock>> { let declarations = unsafe { Gecko_GetServoDeclarationBlock(self.element) }; if declarations.is_null() { - &NO_STYLE_ATTRIBUTE + None } else { - GeckoDeclarationBlock::with(declarations, |declarations| { - unsafe { transmute(&declarations.declarations) } - }) + let opt_ptr = GeckoDeclarationBlock::with(declarations, |declarations| { + // Use a raw pointer to extend the lifetime + declarations.declarations.as_ref().map(|r| r as *const Arc<_>) + }); + opt_ptr.map(|ptr| unsafe { &*ptr }) } } diff --git a/tests/unit/script/size_of.rs b/tests/unit/script/size_of.rs index 96981ca001f..3930180a9cf 100644 --- a/tests/unit/script/size_of.rs +++ b/tests/unit/script/size_of.rs @@ -40,10 +40,10 @@ macro_rules! sizeof_checker ( // Update the sizes here sizeof_checker!(size_event_target, EventTarget, 40); sizeof_checker!(size_node, Node, 152); -sizeof_checker!(size_element, Element, 328); -sizeof_checker!(size_htmlelement, HTMLElement, 344); -sizeof_checker!(size_div, HTMLDivElement, 344); -sizeof_checker!(size_span, HTMLSpanElement, 344); +sizeof_checker!(size_element, Element, 320); +sizeof_checker!(size_htmlelement, HTMLElement, 336); +sizeof_checker!(size_div, HTMLDivElement, 336); +sizeof_checker!(size_span, HTMLSpanElement, 336); sizeof_checker!(size_text, Text, 184); sizeof_checker!(size_characterdata, CharacterData, 184); sizeof_checker!(size_servothreadsafelayoutnode, ServoThreadSafeLayoutNode, 16); diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index b802c5c4fbf..c43fc5e93c1 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -45,8 +45,7 @@ fn property_declaration_block_should_serialize_correctly() { ]; let block = PropertyDeclarationBlock { - declarations: Arc::new(declarations), - + declarations: declarations, important_count: 0, }; @@ -63,8 +62,7 @@ mod shorthand_serialization { pub fn shorthand_properties_to_string(properties: Vec<PropertyDeclaration>) -> String { let block = PropertyDeclarationBlock { - declarations: Arc::new(properties.into_iter().map(|d| (d, Importance::Normal)).collect()), - + declarations: properties.into_iter().map(|d| (d, Importance::Normal)).collect(), important_count: 0, }; diff --git a/tests/unit/style/selector_matching.rs b/tests/unit/style/selector_matching.rs index 22e11288c48..63a219ecd5e 100644 --- a/tests/unit/style/selector_matching.rs +++ b/tests/unit/style/selector_matching.rs @@ -6,7 +6,7 @@ use cssparser::Parser; use selectors::parser::{LocalName, ParserContext, parse_selector_list}; use std::sync::Arc; use string_cache::Atom; -use style::properties::Importance; +use style::properties::{Importance, PropertyDeclarationBlock}; use style::selector_matching::{DeclarationBlock, Rule, SelectorMap}; /// Helper method to get some Rules from selector strings. @@ -19,7 +19,10 @@ fn get_mock_rules(css_selectors: &[&str]) -> Vec<Vec<Rule>> { Rule { selector: s.complex_selector.clone(), declarations: DeclarationBlock { - mixed_declarations: Arc::new(Vec::new()), + mixed_declarations: Arc::new(PropertyDeclarationBlock { + declarations: Vec::new(), + important_count: 0, + }), importance: Importance::Normal, specificity: s.specificity, source_order: i, diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 528dcd67084..802ffb52f84 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -15,7 +15,7 @@ use style::parser::ParserContextExtraData; use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, DeclaredValue, longhands}; use style::properties::Importance; use style::properties::longhands::animation_play_state; -use style::stylesheets::{Stylesheet, CSSRule, StyleRule, KeyframesRule, Origin}; +use style::stylesheets::{Stylesheet, NamespaceRule, CSSRule, StyleRule, KeyframesRule, Origin}; use style::values::specified::{LengthOrPercentageOrAuto, Percentage}; use url::Url; @@ -65,11 +65,11 @@ fn test_parse_stylesheet() { media: None, dirty_on_viewport_size_change: false, rules: vec![ - CSSRule::Namespace { + CSSRule::Namespace(Arc::new(NamespaceRule { prefix: None, url: NsAtom(Atom::from("http://www.w3.org/1999/xhtml")) - }, - CSSRule::Style(StyleRule { + })), + CSSRule::Style(Arc::new(StyleRule { selectors: vec![ Selector { complex_selector: Arc::new(ComplexSelector { @@ -97,18 +97,18 @@ fn test_parse_stylesheet() { specificity: (0 << 20) + (1 << 10) + (1 << 0), }, ], - declarations: PropertyDeclarationBlock { - declarations: Arc::new(vec![ + declarations: Arc::new(PropertyDeclarationBlock { + declarations: vec![ (PropertyDeclaration::Display(DeclaredValue::Value( longhands::display::SpecifiedValue::none)), Importance::Important), (PropertyDeclaration::Custom(Atom::from("a"), DeclaredValue::Inherit), Importance::Important), - ]), + ], important_count: 2, - }, - }), - CSSRule::Style(StyleRule { + }), + })), + CSSRule::Style(Arc::new(StyleRule { selectors: vec![ Selector { complex_selector: Arc::new(ComplexSelector { @@ -145,16 +145,16 @@ fn test_parse_stylesheet() { specificity: (0 << 20) + (0 << 10) + (1 << 0), }, ], - declarations: PropertyDeclarationBlock { - declarations: Arc::new(vec![ + declarations: Arc::new(PropertyDeclarationBlock { + declarations: vec![ (PropertyDeclaration::Display(DeclaredValue::Value( longhands::display::SpecifiedValue::block)), Importance::Normal), - ]), + ], important_count: 0, - }, - }), - CSSRule::Style(StyleRule { + }), + })), + CSSRule::Style(Arc::new(StyleRule { selectors: vec![ Selector { complex_selector: Arc::new(ComplexSelector { @@ -180,8 +180,8 @@ fn test_parse_stylesheet() { specificity: (1 << 20) + (1 << 10) + (0 << 0), }, ], - declarations: PropertyDeclarationBlock { - declarations: Arc::new(vec![ + declarations: Arc::new(PropertyDeclarationBlock { + declarations: vec![ (PropertyDeclaration::BackgroundColor(DeclaredValue::Value( longhands::background_color::SpecifiedValue { authored: Some("blue".to_owned()), @@ -226,37 +226,43 @@ fn test_parse_stylesheet() { vec![longhands::background_clip::single_value ::get_initial_specified_value()]))), Importance::Normal), - ]), + ], important_count: 0, - }, - }), - CSSRule::Keyframes(KeyframesRule { + }), + })), + CSSRule::Keyframes(Arc::new(KeyframesRule { name: "foo".into(), keyframes: vec![ - Keyframe { + Arc::new(Keyframe { selector: KeyframeSelector::new_for_unit_testing( vec![KeyframePercentage::new(0.)]), - declarations: Arc::new(vec![ - (PropertyDeclaration::Width(DeclaredValue::Value( - LengthOrPercentageOrAuto::Percentage(Percentage(0.)))), - Importance::Normal), - ]), - }, - Keyframe { + block: Arc::new(PropertyDeclarationBlock { + declarations: vec![ + (PropertyDeclaration::Width(DeclaredValue::Value( + LengthOrPercentageOrAuto::Percentage(Percentage(0.)))), + Importance::Normal), + ], + important_count: 0, + }) + }), + Arc::new(Keyframe { selector: KeyframeSelector::new_for_unit_testing( vec![KeyframePercentage::new(1.)]), - declarations: Arc::new(vec![ - (PropertyDeclaration::Width(DeclaredValue::Value( - LengthOrPercentageOrAuto::Percentage(Percentage(1.)))), - Importance::Normal), - (PropertyDeclaration::AnimationPlayState(DeclaredValue::Value( - animation_play_state::SpecifiedValue( - vec![animation_play_state::SingleSpecifiedValue::running]))), - Importance::Normal), - ]), - }, + block: Arc::new(PropertyDeclarationBlock { + declarations: vec![ + (PropertyDeclaration::Width(DeclaredValue::Value( + LengthOrPercentageOrAuto::Percentage(Percentage(1.)))), + Importance::Normal), + (PropertyDeclaration::AnimationPlayState(DeclaredValue::Value( + animation_play_state::SpecifiedValue( + vec![animation_play_state::SingleSpecifiedValue::running]))), + Importance::Normal), + ], + important_count: 0, + }), + }), ] - }) + })) ], }); |