Replace RwLock<Keyframe> with Locked<Keyframe>

This commit is contained in:
Simon Sapin 2017-03-17 19:48:37 +01:00
parent adb97d4cbe
commit 57724e5a37
8 changed files with 64 additions and 44 deletions

View file

@ -514,7 +514,7 @@ unsafe impl JSTraceable for StyleLocked<CssRules> {
} }
} }
unsafe impl JSTraceable for RwLock<Keyframe> { unsafe impl JSTraceable for StyleLocked<Keyframe> {
unsafe fn trace(&self, _trc: *mut JSTracer) { unsafe fn trace(&self, _trc: *mut JSTracer) {
// Do nothing. // Do nothing.
} }

View file

@ -12,21 +12,21 @@ use dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration, CSSSt
use dom::cssstylesheet::CSSStyleSheet; use dom::cssstylesheet::CSSStyleSheet;
use dom::window::Window; use dom::window::Window;
use dom_struct::dom_struct; use dom_struct::dom_struct;
use parking_lot::RwLock;
use std::sync::Arc; use std::sync::Arc;
use style::keyframes::Keyframe; use style::keyframes::Keyframe;
use style::shared_lock::Locked;
use style_traits::ToCss; use style_traits::ToCss;
#[dom_struct] #[dom_struct]
pub struct CSSKeyframeRule { pub struct CSSKeyframeRule {
cssrule: CSSRule, cssrule: CSSRule,
#[ignore_heap_size_of = "Arc"] #[ignore_heap_size_of = "Arc"]
keyframerule: Arc<RwLock<Keyframe>>, keyframerule: Arc<Locked<Keyframe>>,
style_decl: MutNullableJS<CSSStyleDeclaration>, style_decl: MutNullableJS<CSSStyleDeclaration>,
} }
impl CSSKeyframeRule { impl CSSKeyframeRule {
fn new_inherited(parent_stylesheet: &CSSStyleSheet, keyframerule: Arc<RwLock<Keyframe>>) fn new_inherited(parent_stylesheet: &CSSStyleSheet, keyframerule: Arc<Locked<Keyframe>>)
-> CSSKeyframeRule { -> CSSKeyframeRule {
CSSKeyframeRule { CSSKeyframeRule {
cssrule: CSSRule::new_inherited(parent_stylesheet), cssrule: CSSRule::new_inherited(parent_stylesheet),
@ -37,7 +37,7 @@ impl CSSKeyframeRule {
#[allow(unrooted_must_root)] #[allow(unrooted_must_root)]
pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet,
keyframerule: Arc<RwLock<Keyframe>>) -> Root<CSSKeyframeRule> { keyframerule: Arc<Locked<Keyframe>>) -> Root<CSSKeyframeRule> {
reflect_dom_object(box CSSKeyframeRule::new_inherited(parent_stylesheet, keyframerule), reflect_dom_object(box CSSKeyframeRule::new_inherited(parent_stylesheet, keyframerule),
window, window,
CSSKeyframeRuleBinding::Wrap) CSSKeyframeRuleBinding::Wrap)
@ -48,11 +48,16 @@ impl CSSKeyframeRuleMethods for CSSKeyframeRule {
// https://drafts.csswg.org/css-animations/#dom-csskeyframerule-style // https://drafts.csswg.org/css-animations/#dom-csskeyframerule-style
fn Style(&self) -> Root<CSSStyleDeclaration> { fn Style(&self) -> Root<CSSStyleDeclaration> {
self.style_decl.or_init(|| { self.style_decl.or_init(|| {
CSSStyleDeclaration::new(self.global().as_window(), let guard = self.cssrule.shared_lock().read();
CSSStyleOwner::CSSRule(JS::from_ref(self.upcast()), CSSStyleDeclaration::new(
self.keyframerule.read().block.clone()), self.global().as_window(),
None, CSSStyleOwner::CSSRule(
CSSModificationAccess::ReadWrite) JS::from_ref(self.upcast()),
self.keyframerule.read_with(&guard).block.clone(),
),
None,
CSSModificationAccess::ReadWrite,
)
}) })
} }
} }
@ -64,6 +69,7 @@ impl SpecificCSSRule for CSSKeyframeRule {
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {
self.keyframerule.read().to_css_string().into() let guard = self.cssrule.shared_lock().read();
self.keyframerule.read_with(&guard).to_css_string().into()
} }
} }

View file

@ -67,7 +67,7 @@ impl CSSKeyframesRule {
// because that's the rule that applies. Thus, rposition // because that's the rule that applies. Thus, rposition
self.keyframesrule.read_with(&guard) self.keyframesrule.read_with(&guard)
.keyframes.iter().rposition(|frame| { .keyframes.iter().rposition(|frame| {
frame.read().selector == sel frame.read_with(&guard).selector == sel
}) })
} else { } else {
None None

View file

@ -15,6 +15,7 @@ use properties::{PropertyDeclarationId, LonghandId, ParsedDeclaration};
use properties::LonghandIdSet; use properties::LonghandIdSet;
use properties::animated_properties::TransitionProperty; use properties::animated_properties::TransitionProperty;
use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction; use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction;
use shared_lock::{SharedRwLock, SharedRwLockReadGuard, Locked};
use std::fmt; use std::fmt;
use std::sync::Arc; use std::sync::Arc;
use style_traits::ToCss; use style_traits::ToCss;
@ -125,7 +126,7 @@ impl Keyframe {
pub fn parse(css: &str, pub fn parse(css: &str,
parent_stylesheet: &Stylesheet, parent_stylesheet: &Stylesheet,
extra_data: ParserContextExtraData) extra_data: ParserContextExtraData)
-> Result<Arc<RwLock<Self>>, ()> { -> Result<Arc<Locked<Self>>, ()> {
let error_reporter = MemoryHoleReporter; let error_reporter = MemoryHoleReporter;
let context = ParserContext::new_with_extra_data(parent_stylesheet.origin, let context = ParserContext::new_with_extra_data(parent_stylesheet.origin,
&parent_stylesheet.base_url, &parent_stylesheet.base_url,
@ -135,6 +136,7 @@ impl Keyframe {
let mut rule_parser = KeyframeListParser { let mut rule_parser = KeyframeListParser {
context: &context, context: &context,
shared_lock: &parent_stylesheet.shared_lock,
}; };
parse_one_rule(&mut input, &mut rule_parser) parse_one_rule(&mut input, &mut rule_parser)
} }
@ -239,13 +241,14 @@ pub struct KeyframesAnimation {
} }
/// Get all the animated properties in a keyframes animation. /// Get all the animated properties in a keyframes animation.
fn get_animated_properties(keyframes: &[Arc<RwLock<Keyframe>>]) -> Vec<TransitionProperty> { fn get_animated_properties(keyframes: &[Arc<Locked<Keyframe>>], guard: &SharedRwLockReadGuard)
-> Vec<TransitionProperty> {
let mut ret = vec![]; let mut ret = vec![];
let mut seen = LonghandIdSet::new(); let mut seen = LonghandIdSet::new();
// NB: declarations are already deduplicated, so we don't have to check for // NB: declarations are already deduplicated, so we don't have to check for
// it here. // it here.
for keyframe in keyframes { for keyframe in keyframes {
let keyframe = keyframe.read(); let keyframe = keyframe.read_with(&guard);
for &(ref declaration, importance) in keyframe.block.read().declarations().iter() { for &(ref declaration, importance) in keyframe.block.read().declarations().iter() {
assert!(!importance.important()); assert!(!importance.important());
@ -270,7 +273,8 @@ impl KeyframesAnimation {
/// ///
/// Otherwise, this will compute and sort the steps used for the animation, /// Otherwise, this will compute and sort the steps used for the animation,
/// and return the animation object. /// and return the animation object.
pub fn from_keyframes(keyframes: &[Arc<RwLock<Keyframe>>]) -> Self { pub fn from_keyframes(keyframes: &[Arc<Locked<Keyframe>>], guard: &SharedRwLockReadGuard)
-> Self {
let mut result = KeyframesAnimation { let mut result = KeyframesAnimation {
steps: vec![], steps: vec![],
properties_changed: vec![], properties_changed: vec![],
@ -280,13 +284,13 @@ impl KeyframesAnimation {
return result; return result;
} }
result.properties_changed = get_animated_properties(keyframes); result.properties_changed = get_animated_properties(keyframes, guard);
if result.properties_changed.is_empty() { if result.properties_changed.is_empty() {
return result; return result;
} }
for keyframe in keyframes { for keyframe in keyframes {
let keyframe = keyframe.read(); let keyframe = keyframe.read_with(&guard);
for percentage in keyframe.selector.0.iter() { for percentage in keyframe.selector.0.iter() {
result.steps.push(KeyframesStep::new(*percentage, KeyframesStepValue::Declarations { result.steps.push(KeyframesStep::new(*percentage, KeyframesStepValue::Declarations {
block: keyframe.block.clone(), block: keyframe.block.clone(),
@ -322,24 +326,27 @@ impl KeyframesAnimation {
/// } /// }
struct KeyframeListParser<'a> { struct KeyframeListParser<'a> {
context: &'a ParserContext<'a>, context: &'a ParserContext<'a>,
shared_lock: &'a SharedRwLock,
} }
/// Parses a keyframe list from CSS input. /// Parses a keyframe list from CSS input.
pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser) -> Vec<Arc<RwLock<Keyframe>>> { pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser, shared_lock: &SharedRwLock)
RuleListParser::new_for_nested_rule(input, KeyframeListParser { context: context }) -> Vec<Arc<Locked<Keyframe>>> {
.filter_map(Result::ok) RuleListParser::new_for_nested_rule(input, KeyframeListParser {
.collect() context: context,
shared_lock: shared_lock,
}).filter_map(Result::ok).collect()
} }
enum Void {} enum Void {}
impl<'a> AtRuleParser for KeyframeListParser<'a> { impl<'a> AtRuleParser for KeyframeListParser<'a> {
type Prelude = Void; type Prelude = Void;
type AtRule = Arc<RwLock<Keyframe>>; type AtRule = Arc<Locked<Keyframe>>;
} }
impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { impl<'a> QualifiedRuleParser for KeyframeListParser<'a> {
type Prelude = KeyframeSelector; type Prelude = KeyframeSelector;
type QualifiedRule = Arc<RwLock<Keyframe>>; type QualifiedRule = Arc<Locked<Keyframe>>;
fn parse_prelude(&mut self, input: &mut Parser) -> Result<Self::Prelude, ()> { fn parse_prelude(&mut self, input: &mut Parser) -> Result<Self::Prelude, ()> {
let start = input.position(); let start = input.position();
@ -372,7 +379,7 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> {
} }
// `parse_important` is not called here, `!important` is not allowed in keyframe blocks. // `parse_important` is not called here, `!important` is not allowed in keyframe blocks.
} }
Ok(Arc::new(RwLock::new(Keyframe { Ok(Arc::new(self.shared_lock.wrap(Keyframe {
selector: prelude, selector: prelude,
block: Arc::new(RwLock::new(block)), block: Arc::new(RwLock::new(block)),
}))) })))

View file

@ -451,12 +451,12 @@ pub struct KeyframesRule {
/// The name of the current animation. /// The name of the current animation.
pub name: Atom, pub name: Atom,
/// The keyframes specified for this CSS rule. /// The keyframes specified for this CSS rule.
pub keyframes: Vec<Arc<RwLock<Keyframe>>>, pub keyframes: Vec<Arc<Locked<Keyframe>>>,
} }
impl ToCssWithGuard for KeyframesRule { impl ToCssWithGuard for KeyframesRule {
// Serialization of KeyframesRule is not specced. // Serialization of KeyframesRule is not specced.
fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
where W: fmt::Write { where W: fmt::Write {
try!(dest.write_str("@keyframes ")); try!(dest.write_str("@keyframes "));
try!(dest.write_str(&*self.name.to_string())); try!(dest.write_str(&*self.name.to_string()));
@ -468,7 +468,7 @@ impl ToCssWithGuard for KeyframesRule {
try!(dest.write_str(" ")); try!(dest.write_str(" "));
} }
first = false; first = false;
let keyframe = lock.read(); let keyframe = lock.read_with(&guard);
try!(keyframe.to_css(dest)); try!(keyframe.to_css(dest));
} }
dest.write_str(" }") dest.write_str(" }")
@ -1009,7 +1009,7 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> {
AtRulePrelude::Keyframes(name) => { AtRulePrelude::Keyframes(name) => {
Ok(CssRule::Keyframes(Arc::new(self.shared_lock.wrap(KeyframesRule { Ok(CssRule::Keyframes(Arc::new(self.shared_lock.wrap(KeyframesRule {
name: name, name: name,
keyframes: parse_keyframe_list(&self.context, input), keyframes: parse_keyframe_list(&self.context, input, self.shared_lock),
})))) }))))
} }
} }

View file

@ -280,7 +280,8 @@ impl Stylist {
CssRule::Keyframes(ref keyframes_rule) => { CssRule::Keyframes(ref keyframes_rule) => {
let keyframes_rule = keyframes_rule.read_with(guard); let keyframes_rule = keyframes_rule.read_with(guard);
debug!("Found valid keyframes rule: {:?}", *keyframes_rule); debug!("Found valid keyframes rule: {:?}", *keyframes_rule);
let animation = KeyframesAnimation::from_keyframes(&keyframes_rule.keyframes); let animation = KeyframesAnimation::from_keyframes(
&keyframes_rule.keyframes, guard);
debug!("Found valid keyframe animation: {:?}", animation); debug!("Found valid keyframe animation: {:?}", animation);
self.animations.insert(keyframes_rule.name.clone(), animation); self.animations.insert(keyframes_rule.name.clone(), animation);
} }

View file

@ -8,12 +8,14 @@ use style::keyframes::{Keyframe, KeyframesAnimation, KeyframePercentage, Keyfra
use style::keyframes::{KeyframesStep, KeyframesStepValue}; use style::keyframes::{KeyframesStep, KeyframesStepValue};
use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, Importance}; use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, Importance};
use style::properties::animated_properties::TransitionProperty; use style::properties::animated_properties::TransitionProperty;
use style::shared_lock::SharedRwLock;
use style::values::specified::{LengthOrPercentageOrAuto, NoCalcLength}; use style::values::specified::{LengthOrPercentageOrAuto, NoCalcLength};
#[test] #[test]
fn test_empty_keyframe() { fn test_empty_keyframe() {
let shared_lock = SharedRwLock::new();
let keyframes = vec![]; let keyframes = vec![];
let animation = KeyframesAnimation::from_keyframes(&keyframes); let animation = KeyframesAnimation::from_keyframes(&keyframes, &shared_lock.read());
let expected = KeyframesAnimation { let expected = KeyframesAnimation {
steps: vec![], steps: vec![],
properties_changed: vec![], properties_changed: vec![],
@ -24,13 +26,14 @@ fn test_empty_keyframe() {
#[test] #[test]
fn test_no_property_in_keyframe() { fn test_no_property_in_keyframe() {
let shared_lock = SharedRwLock::new();
let keyframes = vec![ let keyframes = vec![
Arc::new(RwLock::new(Keyframe { Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]), selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]),
block: Arc::new(RwLock::new(PropertyDeclarationBlock::new())) block: Arc::new(RwLock::new(PropertyDeclarationBlock::new()))
})), })),
]; ];
let animation = KeyframesAnimation::from_keyframes(&keyframes); let animation = KeyframesAnimation::from_keyframes(&keyframes, &shared_lock.read());
let expected = KeyframesAnimation { let expected = KeyframesAnimation {
steps: vec![], steps: vec![],
properties_changed: vec![], properties_changed: vec![],
@ -41,6 +44,7 @@ fn test_no_property_in_keyframe() {
#[test] #[test]
fn test_missing_property_in_initial_keyframe() { fn test_missing_property_in_initial_keyframe() {
let shared_lock = SharedRwLock::new();
let declarations_on_initial_keyframe = let declarations_on_initial_keyframe =
Arc::new(RwLock::new(PropertyDeclarationBlock::with_one( Arc::new(RwLock::new(PropertyDeclarationBlock::with_one(
PropertyDeclaration::Width( PropertyDeclaration::Width(
@ -65,17 +69,17 @@ fn test_missing_property_in_initial_keyframe() {
})); }));
let keyframes = vec![ let keyframes = vec![
Arc::new(RwLock::new(Keyframe { Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]), selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]),
block: declarations_on_initial_keyframe.clone(), block: declarations_on_initial_keyframe.clone(),
})), })),
Arc::new(RwLock::new(Keyframe { Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]), selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]),
block: declarations_on_final_keyframe.clone(), block: declarations_on_final_keyframe.clone(),
})), })),
]; ];
let animation = KeyframesAnimation::from_keyframes(&keyframes); let animation = KeyframesAnimation::from_keyframes(&keyframes, &shared_lock.read());
let expected = KeyframesAnimation { let expected = KeyframesAnimation {
steps: vec![ steps: vec![
KeyframesStep { KeyframesStep {
@ -97,6 +101,7 @@ fn test_missing_property_in_initial_keyframe() {
#[test] #[test]
fn test_missing_property_in_final_keyframe() { fn test_missing_property_in_final_keyframe() {
let shared_lock = SharedRwLock::new();
let declarations_on_initial_keyframe = let declarations_on_initial_keyframe =
Arc::new(RwLock::new({ Arc::new(RwLock::new({
let mut block = PropertyDeclarationBlock::new(); let mut block = PropertyDeclarationBlock::new();
@ -121,17 +126,17 @@ fn test_missing_property_in_final_keyframe() {
))); )));
let keyframes = vec![ let keyframes = vec![
Arc::new(RwLock::new(Keyframe { Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]), selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]),
block: declarations_on_initial_keyframe.clone(), block: declarations_on_initial_keyframe.clone(),
})), })),
Arc::new(RwLock::new(Keyframe { Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]), selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]),
block: declarations_on_final_keyframe.clone(), block: declarations_on_final_keyframe.clone(),
})), })),
]; ];
let animation = KeyframesAnimation::from_keyframes(&keyframes); let animation = KeyframesAnimation::from_keyframes(&keyframes, &shared_lock.read());
let expected = KeyframesAnimation { let expected = KeyframesAnimation {
steps: vec![ steps: vec![
KeyframesStep { KeyframesStep {
@ -153,6 +158,7 @@ fn test_missing_property_in_final_keyframe() {
#[test] #[test]
fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() { fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() {
let shared_lock = SharedRwLock::new();
let declarations = let declarations =
Arc::new(RwLock::new({ Arc::new(RwLock::new({
let mut block = PropertyDeclarationBlock::new(); let mut block = PropertyDeclarationBlock::new();
@ -170,16 +176,16 @@ fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() {
})); }));
let keyframes = vec![ let keyframes = vec![
Arc::new(RwLock::new(Keyframe { Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]), selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]),
block: Arc::new(RwLock::new(PropertyDeclarationBlock::new())) block: Arc::new(RwLock::new(PropertyDeclarationBlock::new()))
})), })),
Arc::new(RwLock::new(Keyframe { Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.5)]), selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.5)]),
block: declarations.clone(), block: declarations.clone(),
})), })),
]; ];
let animation = KeyframesAnimation::from_keyframes(&keyframes); let animation = KeyframesAnimation::from_keyframes(&keyframes, &shared_lock.read());
let expected = KeyframesAnimation { let expected = KeyframesAnimation {
steps: vec![ steps: vec![
KeyframesStep { KeyframesStep {

View file

@ -238,7 +238,7 @@ fn test_parse_stylesheet() {
CssRule::Keyframes(Arc::new(stylesheet.shared_lock.wrap(KeyframesRule { CssRule::Keyframes(Arc::new(stylesheet.shared_lock.wrap(KeyframesRule {
name: "foo".into(), name: "foo".into(),
keyframes: vec![ keyframes: vec![
Arc::new(RwLock::new(Keyframe { Arc::new(stylesheet.shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing( selector: KeyframeSelector::new_for_unit_testing(
vec![KeyframePercentage::new(0.)]), vec![KeyframePercentage::new(0.)]),
block: Arc::new(RwLock::new(block_from(vec![ block: Arc::new(RwLock::new(block_from(vec![
@ -247,7 +247,7 @@ fn test_parse_stylesheet() {
Importance::Normal), Importance::Normal),
]))) ])))
})), })),
Arc::new(RwLock::new(Keyframe { Arc::new(stylesheet.shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing( selector: KeyframeSelector::new_for_unit_testing(
vec![KeyframePercentage::new(1.)]), vec![KeyframePercentage::new(1.)]),
block: Arc::new(RwLock::new(block_from(vec![ block: Arc::new(RwLock::new(block_from(vec![