diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index c048446d813..81a38c22551 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use cssparser::ToCss; +use dom::bindings::cell::DOMRefCell; use dom::bindings::codegen::Bindings::CSSStyleDeclarationBinding::{self, CSSStyleDeclarationMethods}; use dom::bindings::error::{Error, ErrorResult, Fallible}; use dom::bindings::global::GlobalRef; @@ -14,13 +15,11 @@ use dom::element::Element; use dom::node::{Node, NodeDamage, window_from_node}; use dom::window::Window; use std::ascii::AsciiExt; -use std::slice; use std::sync::Arc; use string_cache::Atom; use style::parser::ParserContextExtraData; -use style::properties::{PropertyDeclaration, Shorthand, Importance}; +use style::properties::{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 @@ -93,7 +92,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { fn Length(&self) -> u32 { let elem = self.owner.upcast::(); let len = match *elem.style_attribute().borrow() { - Some(ref declarations) => declarations.declarations.len(), + Some(ref declarations) => declarations.borrow().declarations.len(), None => 0, }; len as u32 @@ -119,43 +118,42 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // Step 2 if let Some(shorthand) = Shorthand::from_name(&property) { + let style_attribute = owner.style_attribute().borrow(); + let style_attribute = if let Some(ref style_attribute) = *style_attribute { + style_attribute.borrow() + } else { + // shorthand.longhands() is never empty, so with no style attribute + // step 2.2.2 would do this: + return DOMString::new() + }; + // Step 2.1 let mut list = vec![]; // Step 2.2 for longhand in shorthand.longhands() { // Step 2.2.1 - let declaration = owner.get_inline_style_declaration(&Atom::from(*longhand)); + let declaration = style_attribute.get(longhand); // Step 2.2.2 & 2.2.3 match declaration { - Some(declaration) => list.push(declaration), + Some(&(ref declaration, _importance)) => list.push(declaration), None => return DOMString::new(), } } // Step 2.3 - // Work around closures not being Clone - #[derive(Clone)] - struct Map<'a, 'b: 'a>(slice::Iter<'a, Ref<'b, (PropertyDeclaration, Importance)>>); - impl<'a, 'b> Iterator for Map<'a, 'b> { - type Item = &'a PropertyDeclaration; - fn next(&mut self) -> Option { - self.0.next().map(|r| &r.0) - } - } - // TODO: important is hardcoded to false because method does not implement it yet let serialized_value = shorthand.serialize_shorthand_value_to_string( - Map(list.iter()), Importance::Normal); + list, Importance::Normal); return DOMString::from(serialized_value); } // Step 3 & 4 - match owner.get_inline_style_declaration(&property) { + owner.get_inline_style_declaration(&property, |d| match d { Some(declaration) => DOMString::from(declaration.0.value()), None => DOMString::new(), - } + }) } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority @@ -172,13 +170,18 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { .all(|priority| priority == "important") { return DOMString::from("important"); } - // Step 3 } else { - if let Some(decl) = self.owner.get_inline_style_declaration(&property) { - if decl.1.important() { - return DOMString::from("important"); + // Step 3 + return self.owner.get_inline_style_declaration(&property, |d| { + if let Some(decl) = d { + if decl.1.important() { + return DOMString::from("important"); + } } - } + + // Step 4 + DOMString::new() + }) } // Step 4 @@ -328,13 +331,14 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { let elem = self.owner.upcast::(); let style_attribute = elem.style_attribute().borrow(); style_attribute.as_ref().and_then(|declarations| { - declarations.declarations.get(index) - }).map(|&(ref declaration, importance)| { - let mut css = declaration.to_css_string(); - if importance.important() { - css += " !important"; - } - DOMString::from(css) + declarations.borrow().declarations.get(index).map(|entry| { + let (ref declaration, importance) = *entry; + let mut css = declaration.to_css_string(); + if importance.important() { + css += " !important"; + } + DOMString::from(css) + }) }) } @@ -344,7 +348,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { let style_attribute = elem.style_attribute().borrow(); if let Some(declarations) = style_attribute.as_ref() { - DOMString::from(declarations.to_css_string()) + DOMString::from(declarations.borrow().to_css_string()) } else { DOMString::new() } @@ -366,7 +370,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { *element.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() { None // Step 2 } else { - Some(Arc::new(decl_block)) + Some(Arc::new(DOMRefCell::new(decl_block))) }; element.sync_property_with_attrs_style(); let node = element.upcast::(); diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index ca0aea778fa..9f43aa1b8a9 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -110,7 +110,7 @@ pub struct Element { attrs: DOMRefCell>>, id_attribute: DOMRefCell>, #[ignore_heap_size_of = "Arc"] - style_attribute: DOMRefCell>>, + style_attribute: DOMRefCell>>>, attr_list: MutNullableHeap>, class_list: MutNullableHeap>, state: Cell, @@ -298,7 +298,7 @@ pub trait LayoutElementHelpers { #[allow(unsafe_code)] unsafe fn html_element_in_html_document_for_layout(&self) -> bool; fn id_attribute(&self) -> *const Option; - fn style_attribute(&self) -> *const Option>; + fn style_attribute(&self) -> *const Option>>; fn local_name(&self) -> &Atom; fn namespace(&self) -> &Namespace; fn get_checked_state_for_layout(&self) -> bool; @@ -330,10 +330,10 @@ impl LayoutElementHelpers for LayoutJS { #[inline] fn from_declaration(rule: PropertyDeclaration) -> ApplicableDeclarationBlock { ApplicableDeclarationBlock::from_declarations( - Arc::new(PropertyDeclarationBlock { + Arc::new(DOMRefCell::new(PropertyDeclarationBlock { declarations: vec![(rule, Importance::Normal)], important_count: 0, - }), + })), Importance::Normal) } @@ -619,7 +619,7 @@ impl LayoutElementHelpers for LayoutJS { } #[allow(unsafe_code)] - fn style_attribute(&self) -> *const Option> { + fn style_attribute(&self) -> *const Option>> { unsafe { (*self.unsafe_get()).style_attribute.borrow_for_layout() } @@ -708,7 +708,7 @@ impl Element { self.attrs.borrow() } - pub fn style_attribute(&self) -> &DOMRefCell>> { + pub fn style_attribute(&self) -> &DOMRefCell>>> { &self.style_attribute } @@ -738,7 +738,7 @@ impl Element { // therefore, it should not trigger subsequent mutation events pub fn sync_property_with_attrs_style(&self) { let style_str = if let &Some(ref declarations) = &*self.style_attribute().borrow() { - declarations.to_css_string() + declarations.borrow().to_css_string() } else { String::new() }; @@ -770,7 +770,7 @@ impl Element { let mut inline_declarations = element.style_attribute.borrow_mut(); if let &mut Some(ref mut declarations) = &mut *inline_declarations { let mut importance = None; - let index = declarations.declarations.iter().position(|&(ref decl, i)| { + let index = declarations.borrow().declarations.iter().position(|&(ref decl, i)| { let matching = decl.matches(property); if matching { importance = Some(i) @@ -778,7 +778,7 @@ impl Element { matching }); if let Some(index) = index { - let declarations = Arc::make_mut(declarations); + let mut declarations = Arc::make_mut(declarations).borrow_mut(); declarations.declarations.remove(index); if importance.unwrap().important() { declarations.important_count -= 1; @@ -801,7 +801,8 @@ impl Element { { // Usually, the reference count will be 1 here. But transitions could make it greater // than that. - let declaration_block = Arc::make_mut(declaration_block); + let mut declaration_block = Arc::make_mut(declaration_block).borrow_mut(); + let declaration_block = &mut *declaration_block; let existing_declarations = &mut declaration_block.declarations; 'outer: for incoming_declaration in declarations { @@ -835,10 +836,10 @@ impl Element { 0 }; - *inline_declarations = Some(Arc::new(PropertyDeclarationBlock { + *inline_declarations = Some(Arc::new(DOMRefCell::new(PropertyDeclarationBlock { declarations: declarations.into_iter().map(|d| (d, importance)).collect(), important_count: important_count, - })); + }))); } update(self, declarations, importance); @@ -853,7 +854,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 block = Arc::make_mut(block); + let mut block = Arc::make_mut(block).borrow_mut(); + let block = &mut *block; let declarations = &mut block.declarations; for &mut (ref declaration, ref mut importance) in declarations { if properties.iter().any(|p| declaration.name() == **p) { @@ -875,16 +877,15 @@ impl Element { self.sync_property_with_attrs_style(); } - pub fn get_inline_style_declaration(&self, - property: &Atom) - -> Option> { - Ref::filter_map(self.style_attribute.borrow(), |inline_declarations| { - inline_declarations.as_ref().and_then(|declarations| { - declarations.declarations - .iter() - .find(|&&(ref decl, _)| decl.matches(&property)) - }) - }) + pub fn get_inline_style_declaration(&self, property: &str, f: F) -> R + where F: FnOnce(Option<&(PropertyDeclaration, Importance)>) -> R { + let style_attr = self.style_attribute.borrow(); + if let Some(ref block) = *style_attr { + let block = block.borrow(); + f(block.get(property)) + } else { + f(None) + } } pub fn serialize(&self, traversal_scope: TraversalScope) -> Fallible { @@ -2130,11 +2131,11 @@ impl VirtualMethods for Element { *self.style_attribute.borrow_mut() = mutation.new_value(attr).map(|value| { let win = window_from_node(self); - Arc::new(parse_style_attribute( + Arc::new(DOMRefCell::new(parse_style_attribute( &value, &doc.base_url(), win.css_error_reporter(), - ParserContextExtraData::default())) + ParserContextExtraData::default()))) }); if node.is_in_doc() { node.dirty(NodeDamage::NodeStyleDamaged); diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index b609a545277..4111bb49413 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -30,6 +30,7 @@ #![allow(unsafe_code)] +use dom::bindings::cell::DOMRefCell; use dom::bindings::inheritance::{CharacterDataTypeId, ElementTypeId}; use dom::bindings::inheritance::{HTMLElementTypeId, NodeTypeId}; use dom::bindings::js::LayoutJS; @@ -461,7 +462,7 @@ impl<'le> TElement for ServoLayoutElement<'le> { ServoLayoutNode::from_layout_js(self.element.upcast()) } - fn style_attribute(&self) -> Option<&Arc> { + fn style_attribute(&self) -> Option<&Arc>> { unsafe { (*self.element.style_attribute()).as_ref() } diff --git a/components/style/dom.rs b/components/style/dom.rs index a8da0d72fc3..6a90cd7c62b 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -8,6 +8,7 @@ use atomic_refcell::{AtomicRef, AtomicRefMut}; use data::PersistentStyleData; +use domrefcell::DOMRefCell; use element_state::ElementState; use properties::{ComputedValues, PropertyDeclarationBlock}; use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint}; @@ -202,7 +203,7 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre fn as_node(&self) -> Self::ConcreteNode; - fn style_attribute(&self) -> Option<&Arc>; + fn style_attribute(&self) -> Option<&Arc>>; fn get_state(&self) -> ElementState; diff --git a/components/style/domrefcell.rs b/components/style/domrefcell.rs index a61a2a0d7fe..f33d20b2191 100644 --- a/components/style/domrefcell.rs +++ b/components/style/domrefcell.rs @@ -11,12 +11,19 @@ use thread_state; /// /// This extends the API of `core::cell::RefCell` to allow unsafe access in /// certain situations, with dynamic checking in debug builds. -#[derive(Clone)] +#[derive(Clone, PartialEq, Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct DOMRefCell { value: RefCell, } +// FIXME: These two impls make promises that are not quite true, +// but maybe the debug_assert! makes it close enough. +#[allow(unsafe_code)] +unsafe impl Send for DOMRefCell {} +#[allow(unsafe_code)] +unsafe impl Sync for DOMRefCell {} + // Functionality specific to Servo's `DOMRefCell` type // =================================================== diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 6212e0da05f..92336364a3d 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -46,6 +46,7 @@ use std::ptr; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicPtr}; use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace}; +use style::domrefcell::DOMRefCell; use url::Url; pub struct NonOpaqueStyleData(AtomicRefCell); @@ -468,7 +469,7 @@ impl<'le> TElement for GeckoElement<'le> { unsafe { GeckoNode(&*(self.0 as *const _ as *const RawGeckoNode)) } } - fn style_attribute(&self) -> Option<&Arc> { + fn style_attribute(&self) -> Option<&Arc>> { let declarations = unsafe { Gecko_GetServoDeclarationBlock(self.0) }; if declarations.is_null() { None diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index 69d783fa57f..0d59fc5730e 100644 --- a/components/style/keyframes.rs +++ b/components/style/keyframes.rs @@ -4,6 +4,7 @@ use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser}; use cssparser::{DeclarationListParser, DeclarationParser}; +use domrefcell::DOMRefCell; use parser::{ParserContext, log_css_error}; use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock}; use properties::PropertyDeclarationParseResult; @@ -78,7 +79,7 @@ pub struct Keyframe { /// But including them enables `compute_style_for_animation_step` to create a `ApplicableDeclarationBlock` /// by cloning an `Arc<_>` (incrementing a reference count) rather than re-creating a `Vec<_>`. #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] - pub block: Arc, + pub block: Arc>, } /// A keyframes step value. This can be a synthetised keyframes animation, that @@ -89,7 +90,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), + Declarations(Arc>), ComputedValues, } @@ -110,12 +111,14 @@ pub struct KeyframesStep { } impl KeyframesStep { + #[allow(unsafe_code)] #[inline] fn new(percentage: KeyframePercentage, value: KeyframesStepValue) -> Self { let declared_timing_function = match value { KeyframesStepValue::Declarations(ref block) => { - block.declarations.iter().any(|&(ref prop_decl, _)| { + // FIXME: Is this thread-safe? + unsafe { block.borrow_for_layout() }.declarations.iter().any(|&(ref prop_decl, _)| { match *prop_decl { PropertyDeclaration::AnimationTimingFunction(..) => true, _ => false, @@ -151,11 +154,13 @@ pub struct KeyframesAnimation { /// /// In practice, browsers seem to try to do their best job at it, so we might /// want to go through all the actual keyframes and deduplicate properties. +#[allow(unsafe_code)] fn get_animated_properties(keyframe: &Keyframe) -> Vec { let mut ret = vec![]; // NB: declarations are already deduplicated, so we don't have to check for // it here. - for &(ref declaration, _) in keyframe.block.declarations.iter() { + // FIXME: Is this thread-safe? + for &(ref declaration, _) in unsafe { keyframe.block.borrow_for_layout() }.declarations.iter() { if let Some(property) = TransitionProperty::from_declaration(declaration) { ret.push(property); } @@ -266,10 +271,10 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { } Ok(Arc::new(Keyframe { selector: prelude, - block: Arc::new(PropertyDeclarationBlock { + block: Arc::new(DOMRefCell::new(PropertyDeclarationBlock { declarations: declarations, important_count: 0, - }), + })), })) } } diff --git a/components/style/matching.rs b/components/style/matching.rs index 03f86226870..00660791a25 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -24,6 +24,7 @@ use sink::ForgetfulSink; use smallvec::SmallVec; use std::collections::HashMap; use std::hash::{BuildHasherDefault, Hash, Hasher}; +use std::ops::Deref; use std::slice::IterMut; use std::sync::Arc; use string_cache::Atom; @@ -139,7 +140,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 PropertyDeclarationBlock = &*declaration.mixed_declarations; + let ptr: *const _ = Arc::deref(&declaration.mixed_declarations); ptr.hash(state); declaration.importance.hash(state); } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index ead84567e45..be7fb9ec2fa 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -311,6 +311,10 @@ impl PropertyDeclarationBlock { pub fn any_normal(&self) -> bool { self.declarations.len() > self.important_count as usize } + + pub fn get(&self, property_name: &str) -> Option< &(PropertyDeclaration, Importance)> { + self.declarations.iter().find(|&&(ref decl, _)| decl.matches(property_name)) + } } impl ToCss for PropertyDeclarationBlock { @@ -741,7 +745,7 @@ impl Shorthand { /// Serializes possible shorthand value to String. pub fn serialize_shorthand_value_to_string<'a, I>(self, declarations: I, importance: Importance) -> String - where I: Iterator + Clone { + where I: IntoIterator, I::IntoIter: Clone { let appendable_value = self.get_shorthand_appendable_value(declarations).unwrap(); let mut result = String::new(); append_declaration_value(&mut result, appendable_value, importance).unwrap(); @@ -755,7 +759,7 @@ impl Shorthand { declarations: I, is_first_serialization: &mut bool) -> Result - where W: Write, I: Iterator + Clone { + where W: Write, I: IntoIterator, I::IntoIter: Clone { match self.get_shorthand_appendable_value(declarations) { None => Ok(false), Some(appendable_value) => { @@ -772,8 +776,10 @@ impl Shorthand { } } - fn get_shorthand_appendable_value<'a, I>(self, declarations: I) -> Option> - where I: Iterator + Clone { + fn get_shorthand_appendable_value<'a, I>(self, declarations: I) + -> Option> + where I: IntoIterator, I::IntoIter: Clone { + let declarations = declarations.into_iter(); // Only cloning iterators (a few pointers each) not declarations. let mut declarations2 = declarations.clone(); diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs index fa7654251d9..56a828d9767 100644 --- a/components/style/selector_matching.rs +++ b/components/style/selector_matching.rs @@ -5,6 +5,7 @@ //! Selector matching. use dom::PresentationalHintsSynthetizer; +use domrefcell::DOMRefCell; use element_state::*; use error_reporting::StdoutErrorReporter; use keyframes::KeyframesAnimation; @@ -178,7 +179,7 @@ impl Stylist { map.insert(Rule { selector: selector.complex_selector.clone(), - declarations: style_rule.declarations.clone(), + declarations: style_rule.block.clone(), specificity: selector.specificity, source_order: rules_source_order, }); @@ -327,11 +328,12 @@ impl Stylist { /// that is, whether the matched selectors are simple enough to allow the /// matching logic to be reduced to the logic in /// `css::matching::PrivateMatchMethods::candidate_element_allows_for_style_sharing`. + #[allow(unsafe_code)] pub fn push_applicable_declarations( &self, element: &E, parent_bf: Option<&BloomFilter>, - style_attribute: Option<&Arc>, + style_attribute: Option<&Arc>>, pseudo_element: Option<&PseudoElement>, applicable_declarations: &mut V, reason: MatchingReason) -> StyleRelations @@ -390,8 +392,9 @@ impl Stylist { debug!("author normal: {:?}", relations); // Step 4: Normal style attributes. - if let Some(sa) = style_attribute { - if sa.any_normal() { + if let Some(sa) = style_attribute { + // FIXME: Is this thread-safe? + if unsafe { sa.borrow_for_layout() }.any_normal() { relations |= AFFECTED_BY_STYLE_ATTRIBUTE; Push::push( applicable_declarations, @@ -413,7 +416,8 @@ impl Stylist { // Step 6: `!important` style attributes. if let Some(sa) = style_attribute { - if sa.any_important() { + // FIXME: Is this thread-safe? + if unsafe { sa.borrow_for_layout() }.any_important() { relations |= AFFECTED_BY_STYLE_ATTRIBUTE; Push::push( applicable_declarations, @@ -691,6 +695,7 @@ impl SelectorMap { /// Append to `rule_list` all universal Rules (rules with selector `*|*`) in /// `self` sorted by specifity and source order. + #[allow(unsafe_code)] pub fn get_universal_rules(&self, matching_rules_list: &mut V) where V: VecLike @@ -704,11 +709,13 @@ impl SelectorMap { for rule in self.other_rules.iter() { if rule.selector.compound_selector.is_empty() && rule.selector.next.is_none() { - if rule.declarations.any_normal() { + // FIXME: Is this thread-safe? + let block = unsafe { rule.declarations.borrow_for_layout() }; + if block.any_normal() { matching_rules_list.push( rule.to_applicable_declaration_block(Importance::Normal)); } - if rule.declarations.any_important() { + if block.any_important() { matching_rules_list.push( rule.to_applicable_declaration_block(Importance::Important)); } @@ -745,6 +752,7 @@ impl SelectorMap { } /// Adds rules in `rules` that match `element` to the `matching_rules` list. + #[allow(unsafe_code)] fn get_matching_rules(element: &E, parent_bf: Option<&BloomFilter>, rules: &[Rule], @@ -756,7 +764,8 @@ impl SelectorMap { V: VecLike { for rule in rules.iter() { - let block = &rule.declarations; + // FIXME: Is this thread-safe? + let block = unsafe { rule.declarations.borrow_for_layout() }; let any_declaration_for_importance = if importance.important() { block.any_important() } else { @@ -844,7 +853,7 @@ pub struct Rule { #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] pub selector: Arc>, #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] - pub declarations: Arc, + pub declarations: Arc>, pub source_order: usize, pub specificity: u32, } @@ -869,7 +878,7 @@ pub struct ApplicableDeclarationBlock { /// Contains declarations of either importance, but only those of self.importance are relevant. /// Use ApplicableDeclarationBlock::iter #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] - pub mixed_declarations: Arc, + pub mixed_declarations: Arc>, pub importance: Importance, pub source_order: usize, pub specificity: u32, @@ -877,7 +886,7 @@ pub struct ApplicableDeclarationBlock { impl ApplicableDeclarationBlock { #[inline] - pub fn from_declarations(declarations: Arc, + pub fn from_declarations(declarations: Arc>, importance: Importance) -> Self { ApplicableDeclarationBlock { @@ -888,9 +897,11 @@ impl ApplicableDeclarationBlock { } } + #[allow(unsafe_code)] pub fn iter(&self) -> ApplicableDeclarationBlockIter { ApplicableDeclarationBlockIter { - iter: self.mixed_declarations.declarations.iter(), + // FIXME: Is this thread-safe? + iter: unsafe { self.mixed_declarations.borrow_for_layout() }.declarations.iter(), importance: self.importance, } } diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index c9186dde04c..a1bb692a804 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -6,6 +6,7 @@ use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, decode_stylesheet_bytes}; use cssparser::{AtRuleType, RuleListParser, Token}; +use domrefcell::DOMRefCell; use encoding::EncodingRef; use error_reporting::ParseErrorReporter; use font_face::{FontFaceRule, parse_font_face_block}; @@ -106,7 +107,7 @@ impl MediaRule { #[derive(Debug, PartialEq)] pub struct StyleRule { pub selectors: Vec>, - pub declarations: Arc, + pub block: Arc>, } @@ -558,7 +559,7 @@ impl<'a, 'b> QualifiedRuleParser for NestedRuleParser<'a, 'b> { -> Result { Ok(CSSRule::Style(Arc::new(StyleRule { selectors: prelude, - declarations: Arc::new(parse_property_declaration_list(self.context, input)) + block: Arc::new(DOMRefCell::new(parse_property_declaration_list(self.context, input))) }))) } } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index b6768b6a256..368b4216982 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -16,6 +16,7 @@ use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; use style::arc_ptr_eq; use style::context::{LocalStyleContextCreationInfo, ReflowGoal, SharedStyleContext}; use style::dom::{NodeInfo, TDocument, TElement, TNode}; +use style::domrefcell::DOMRefCell; use style::error_reporting::StdoutErrorReporter; use style::gecko::data::{NUM_THREADS, PerDocumentStyleData}; use style::gecko::selector_impl::{GeckoSelectorImpl, PseudoElement}; @@ -344,7 +345,9 @@ 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)) }; Arc::new(GeckoDeclarationBlock { - declarations: GeckoElement::parse_style_attribute(value).map(Arc::new), + declarations: GeckoElement::parse_style_attribute(value).map(|block| { + Arc::new(DOMRefCell::new(block)) + }), cache: AtomicPtr::new(cache), immutable: AtomicBool::new(false), }).into_strong() diff --git a/tests/unit/style/selector_matching.rs b/tests/unit/style/selector_matching.rs index b6422a75c09..e5fbc156711 100644 --- a/tests/unit/style/selector_matching.rs +++ b/tests/unit/style/selector_matching.rs @@ -6,9 +6,11 @@ use cssparser::Parser; use selectors::parser::{LocalName, ParserContext, parse_selector_list}; use std::sync::Arc; use string_cache::Atom; +use style::domrefcell::DOMRefCell; use style::properties::{PropertyDeclarationBlock, PropertyDeclaration, DeclaredValue}; use style::properties::{longhands, Importance}; use style::selector_matching::{Rule, SelectorMap}; +use style::thread_state; /// Helper method to get some Rules from selector strings. /// Each sublist of the result contains the Rules for one StyleRule. @@ -19,14 +21,14 @@ fn get_mock_rules(css_selectors: &[&str]) -> Vec> { .unwrap().into_iter().map(|s| { Rule { selector: s.complex_selector.clone(), - declarations: Arc::new(PropertyDeclarationBlock { + declarations: Arc::new(DOMRefCell::new(PropertyDeclarationBlock { declarations: vec![ (PropertyDeclaration::Display(DeclaredValue::Value( longhands::display::SpecifiedValue::block)), Importance::Normal), ], important_count: 0, - }), + })), specificity: s.specificity, source_order: i, } @@ -99,6 +101,7 @@ fn test_insert() { #[test] fn test_get_universal_rules() { + thread_state::initialize(thread_state::LAYOUT); let map = get_mock_map(&["*|*", "#foo > *|*", ".klass", "#id"]); let mut decls = vec![]; diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 802ffb52f84..0845045c99f 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -9,6 +9,7 @@ use std::borrow::ToOwned; use std::sync::Arc; use std::sync::Mutex; use string_cache::{Atom, Namespace as NsAtom}; +use style::domrefcell::DOMRefCell; use style::error_reporting::ParseErrorReporter; use style::keyframes::{Keyframe, KeyframeSelector, KeyframePercentage}; use style::parser::ParserContextExtraData; @@ -97,7 +98,7 @@ fn test_parse_stylesheet() { specificity: (0 << 20) + (1 << 10) + (1 << 0), }, ], - declarations: Arc::new(PropertyDeclarationBlock { + block: Arc::new(DOMRefCell::new(PropertyDeclarationBlock { declarations: vec![ (PropertyDeclaration::Display(DeclaredValue::Value( longhands::display::SpecifiedValue::none)), @@ -106,7 +107,7 @@ fn test_parse_stylesheet() { Importance::Important), ], important_count: 2, - }), + })), })), CSSRule::Style(Arc::new(StyleRule { selectors: vec![ @@ -145,14 +146,14 @@ fn test_parse_stylesheet() { specificity: (0 << 20) + (0 << 10) + (1 << 0), }, ], - declarations: Arc::new(PropertyDeclarationBlock { + block: Arc::new(DOMRefCell::new(PropertyDeclarationBlock { declarations: vec![ (PropertyDeclaration::Display(DeclaredValue::Value( longhands::display::SpecifiedValue::block)), Importance::Normal), ], important_count: 0, - }), + })), })), CSSRule::Style(Arc::new(StyleRule { selectors: vec![ @@ -180,7 +181,7 @@ fn test_parse_stylesheet() { specificity: (1 << 20) + (1 << 10) + (0 << 0), }, ], - declarations: Arc::new(PropertyDeclarationBlock { + block: Arc::new(DOMRefCell::new(PropertyDeclarationBlock { declarations: vec![ (PropertyDeclaration::BackgroundColor(DeclaredValue::Value( longhands::background_color::SpecifiedValue { @@ -228,7 +229,7 @@ fn test_parse_stylesheet() { Importance::Normal), ], important_count: 0, - }), + })), })), CSSRule::Keyframes(Arc::new(KeyframesRule { name: "foo".into(), @@ -236,19 +237,19 @@ fn test_parse_stylesheet() { Arc::new(Keyframe { selector: KeyframeSelector::new_for_unit_testing( vec![KeyframePercentage::new(0.)]), - block: Arc::new(PropertyDeclarationBlock { + block: Arc::new(DOMRefCell::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.)]), - block: Arc::new(PropertyDeclarationBlock { + block: Arc::new(DOMRefCell::new(PropertyDeclarationBlock { declarations: vec![ (PropertyDeclaration::Width(DeclaredValue::Value( LengthOrPercentageOrAuto::Percentage(Percentage(1.)))), @@ -259,7 +260,7 @@ fn test_parse_stylesheet() { Importance::Normal), ], important_count: 0, - }), + })), }), ] }))