Auto merge of #15856 - servo:dedup, r=Manishearth

Deduplicate declarations on insertion, not at the end of parsing a block

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #15558 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15856)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-03-08 03:07:04 -08:00 committed by GitHub
commit 4fc7034370
18 changed files with 531 additions and 574 deletions

View file

@ -27,10 +27,7 @@ fn test_no_property_in_keyframe() {
let keyframes = vec![
Arc::new(RwLock::new(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]),
block: Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![],
important_count: 0,
}))
block: Arc::new(RwLock::new(PropertyDeclarationBlock::new()))
})),
];
let animation = KeyframesAnimation::from_keyframes(&keyframes);
@ -45,27 +42,26 @@ fn test_no_property_in_keyframe() {
#[test]
fn test_missing_property_in_initial_keyframe() {
let declarations_on_initial_keyframe =
Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::Width(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal),
],
important_count: 0,
}));
Arc::new(RwLock::new(PropertyDeclarationBlock::with_one(
PropertyDeclaration::Width(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal
)));
let declarations_on_final_keyframe =
Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::Width(
Arc::new(RwLock::new({
let mut block = PropertyDeclarationBlock::new();
block.push(
PropertyDeclaration::Width(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal),
(PropertyDeclaration::Height(
Importance::Normal
);
block.push(
PropertyDeclaration::Height(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal),
],
important_count: 0,
Importance::Normal
);
block
}));
let keyframes = vec![
@ -102,28 +98,27 @@ fn test_missing_property_in_initial_keyframe() {
#[test]
fn test_missing_property_in_final_keyframe() {
let declarations_on_initial_keyframe =
Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::Width(
Arc::new(RwLock::new({
let mut block = PropertyDeclarationBlock::new();
block.push(
PropertyDeclaration::Width(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal),
(PropertyDeclaration::Height(
Importance::Normal
);
block.push(
PropertyDeclaration::Height(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal),
],
important_count: 0,
Importance::Normal
);
block
}));
let declarations_on_final_keyframe =
Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::Height(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal),
],
important_count: 0,
}));
Arc::new(RwLock::new(PropertyDeclarationBlock::with_one(
PropertyDeclaration::Height(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal,
)));
let keyframes = vec![
Arc::new(RwLock::new(Keyframe {
@ -159,26 +154,25 @@ fn test_missing_property_in_final_keyframe() {
#[test]
fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() {
let declarations =
Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::Width(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal),
(PropertyDeclaration::Height(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal),
],
important_count: 0,
Arc::new(RwLock::new({
let mut block = PropertyDeclarationBlock::new();
block.push(
PropertyDeclaration::Width(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal
);
block.push(
PropertyDeclaration::Height(
DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))),
Importance::Normal
);
block
}));
let keyframes = vec![
Arc::new(RwLock::new(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]),
block: Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![],
important_count: 0,
}))
block: Arc::new(RwLock::new(PropertyDeclarationBlock::new()))
})),
Arc::new(RwLock::new(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.5)]),
@ -191,11 +185,10 @@ fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() {
KeyframesStep {
start_percentage: KeyframePercentage(0.),
value: KeyframesStepValue::Declarations {
block: Arc::new(RwLock::new(PropertyDeclarationBlock {
block: Arc::new(RwLock::new(
// XXX: Should we use ComputedValues in this case?
declarations: vec![],
important_count: 0,
}))
PropertyDeclarationBlock::new()
))
},
declared_timing_function: false,
},

View file

@ -17,6 +17,7 @@ use style::values::specified::{BorderStyle, BorderWidth, CSSColor, Length, NoCal
use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent};
use style::values::specified::url::SpecifiedUrl;
use style_traits::ToCss;
use stylesheets::block_from;
fn parse_declaration_block(css_properties: &str) -> PropertyDeclarationBlock {
let url = ServoUrl::parse("http://localhost").unwrap();
@ -56,10 +57,7 @@ fn property_declaration_block_should_serialize_correctly() {
Importance::Normal),
];
let block = PropertyDeclarationBlock {
declarations: declarations,
important_count: 0,
};
let block = block_from(declarations);
let css_string = block.to_css_string();
@ -73,10 +71,7 @@ mod shorthand_serialization {
pub use super::*;
pub fn shorthand_properties_to_string(properties: Vec<PropertyDeclaration>) -> String {
let block = PropertyDeclarationBlock {
declarations: properties.into_iter().map(|d| (d, Importance::Normal)).collect(),
important_count: 0,
};
let block = block_from(properties.into_iter().map(|d| (d, Importance::Normal)));
block.to_css_string()
}
@ -882,10 +877,7 @@ mod shorthand_serialization {
Importance::Normal)
];
let block = PropertyDeclarationBlock {
declarations: declarations,
important_count: 0
};
let block = block_from(declarations);
let mut s = String::new();
@ -905,10 +897,7 @@ mod shorthand_serialization {
Importance::Normal)
];
let block = PropertyDeclarationBlock {
declarations: declarations,
important_count: 0
};
let block = block_from(declarations);
let mut s = String::new();

View file

@ -17,7 +17,7 @@ use test::{self, Bencher};
struct ErrorringErrorReporter;
impl ParseErrorReporter for ErrorringErrorReporter {
fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str,
fn report_error(&self, _input: &mut Parser, position: SourcePosition, message: &str,
url: &ServoUrl) {
panic!("CSS error: {}\t\n{:?} {}", url.as_str(), position, message);
}
@ -68,14 +68,11 @@ fn test_insertion(rule_tree: &RuleTree, rules: Vec<(StyleSource, CascadeLevel)>)
fn test_insertion_style_attribute(rule_tree: &RuleTree, rules: &[(StyleSource, CascadeLevel)]) -> StrongRuleNode {
let mut rules = rules.to_vec();
rules.push((StyleSource::Declarations(Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::Display(DeclaredValue::Value(
longhands::display::SpecifiedValue::block)),
Importance::Normal),
],
important_count: 0,
}))), CascadeLevel::UserNormal));
rules.push((StyleSource::Declarations(Arc::new(RwLock::new(PropertyDeclarationBlock::with_one(
PropertyDeclaration::Display(DeclaredValue::Value(
longhands::display::SpecifiedValue::block)),
Importance::Normal
)))), CascadeLevel::UserNormal));
test_insertion(rule_tree, rules)
}

View file

@ -24,6 +24,15 @@ use style::stylesheets::{Origin, Namespaces};
use style::stylesheets::{Stylesheet, NamespaceRule, CssRule, CssRules, StyleRule, KeyframesRule};
use style::values::specified::{LengthOrPercentageOrAuto, Percentage};
pub fn block_from<I>(iterable: I) -> PropertyDeclarationBlock
where I: IntoIterator<Item=(PropertyDeclaration, Importance)> {
let mut block = PropertyDeclarationBlock::new();
for (d, i) in iterable {
block.push(d, i)
}
block
}
#[test]
fn test_parse_stylesheet() {
let css = r"
@ -98,17 +107,14 @@ fn test_parse_stylesheet() {
specificity: (0 << 20) + (1 << 10) + (1 << 0),
},
]),
block: Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::Display(DeclaredValue::Value(
longhands::display::SpecifiedValue::none)),
Importance::Important),
(PropertyDeclaration::Custom(Atom::from("a"),
DeclaredValue::CSSWideKeyword(CSSWideKeyword::Inherit)),
Importance::Important),
],
important_count: 2,
})),
block: Arc::new(RwLock::new(block_from(vec![
(PropertyDeclaration::Display(DeclaredValue::Value(
longhands::display::SpecifiedValue::none)),
Importance::Important),
(PropertyDeclaration::Custom(Atom::from("a"),
DeclaredValue::CSSWideKeyword(CSSWideKeyword::Inherit)),
Importance::Important),
]))),
}))),
CssRule::Style(Arc::new(RwLock::new(StyleRule {
selectors: SelectorList(vec![
@ -147,14 +153,11 @@ fn test_parse_stylesheet() {
specificity: (0 << 20) + (0 << 10) + (1 << 0),
},
]),
block: Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::Display(DeclaredValue::Value(
longhands::display::SpecifiedValue::block)),
Importance::Normal),
],
important_count: 0,
})),
block: Arc::new(RwLock::new(block_from(vec![
(PropertyDeclaration::Display(DeclaredValue::Value(
longhands::display::SpecifiedValue::block)),
Importance::Normal),
]))),
}))),
CssRule::Style(Arc::new(RwLock::new(StyleRule {
selectors: SelectorList(vec![
@ -182,58 +185,55 @@ fn test_parse_stylesheet() {
specificity: (1 << 20) + (1 << 10) + (0 << 0),
},
]),
block: Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::BackgroundColor(DeclaredValue::Value(
longhands::background_color::SpecifiedValue {
authored: Some("blue".to_owned().into_boxed_str()),
parsed: cssparser::Color::RGBA(cssparser::RGBA::new(0, 0, 255, 255)),
}
)),
Importance::Normal),
(PropertyDeclaration::BackgroundPositionX(DeclaredValue::Value(
longhands::background_position_x::SpecifiedValue(
vec![longhands::background_position_x::single_value
::get_initial_position_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundPositionY(DeclaredValue::Value(
longhands::background_position_y::SpecifiedValue(
vec![longhands::background_position_y::single_value
::get_initial_position_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundRepeat(DeclaredValue::Value(
longhands::background_repeat::SpecifiedValue(
vec![longhands::background_repeat::single_value
::get_initial_specified_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundAttachment(DeclaredValue::Value(
longhands::background_attachment::SpecifiedValue(
vec![longhands::background_attachment::single_value
::get_initial_specified_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundImage(DeclaredValue::Value(
longhands::background_image::SpecifiedValue(
vec![longhands::background_image::single_value
::get_initial_specified_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundSize(DeclaredValue::Value(
longhands::background_size::SpecifiedValue(
vec![longhands::background_size::single_value
::get_initial_specified_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundOrigin(DeclaredValue::Value(
longhands::background_origin::SpecifiedValue(
vec![longhands::background_origin::single_value
::get_initial_specified_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundClip(DeclaredValue::Value(
longhands::background_clip::SpecifiedValue(
vec![longhands::background_clip::single_value
::get_initial_specified_value()]))),
Importance::Normal),
],
important_count: 0,
})),
block: Arc::new(RwLock::new(block_from(vec![
(PropertyDeclaration::BackgroundColor(DeclaredValue::Value(
longhands::background_color::SpecifiedValue {
authored: Some("blue".to_owned().into_boxed_str()),
parsed: cssparser::Color::RGBA(cssparser::RGBA::new(0, 0, 255, 255)),
}
)),
Importance::Normal),
(PropertyDeclaration::BackgroundPositionX(DeclaredValue::Value(
longhands::background_position_x::SpecifiedValue(
vec![longhands::background_position_x::single_value
::get_initial_position_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundPositionY(DeclaredValue::Value(
longhands::background_position_y::SpecifiedValue(
vec![longhands::background_position_y::single_value
::get_initial_position_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundRepeat(DeclaredValue::Value(
longhands::background_repeat::SpecifiedValue(
vec![longhands::background_repeat::single_value
::get_initial_specified_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundAttachment(DeclaredValue::Value(
longhands::background_attachment::SpecifiedValue(
vec![longhands::background_attachment::single_value
::get_initial_specified_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundImage(DeclaredValue::Value(
longhands::background_image::SpecifiedValue(
vec![longhands::background_image::single_value
::get_initial_specified_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundSize(DeclaredValue::Value(
longhands::background_size::SpecifiedValue(
vec![longhands::background_size::single_value
::get_initial_specified_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundOrigin(DeclaredValue::Value(
longhands::background_origin::SpecifiedValue(
vec![longhands::background_origin::single_value
::get_initial_specified_value()]))),
Importance::Normal),
(PropertyDeclaration::BackgroundClip(DeclaredValue::Value(
longhands::background_clip::SpecifiedValue(
vec![longhands::background_clip::single_value
::get_initial_specified_value()]))),
Importance::Normal),
]))),
}))),
CssRule::Keyframes(Arc::new(RwLock::new(KeyframesRule {
name: "foo".into(),
@ -241,30 +241,24 @@ fn test_parse_stylesheet() {
Arc::new(RwLock::new(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(
vec![KeyframePercentage::new(0.)]),
block: Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::Width(DeclaredValue::Value(
LengthOrPercentageOrAuto::Percentage(Percentage(0.)))),
Importance::Normal),
],
important_count: 0,
}))
block: Arc::new(RwLock::new(block_from(vec![
(PropertyDeclaration::Width(DeclaredValue::Value(
LengthOrPercentageOrAuto::Percentage(Percentage(0.)))),
Importance::Normal),
])))
})),
Arc::new(RwLock::new(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(
vec![KeyframePercentage::new(1.)]),
block: Arc::new(RwLock::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,
})),
block: Arc::new(RwLock::new(block_from(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),
]))),
})),
]
})))

View file

@ -23,14 +23,11 @@ fn get_mock_rules(css_selectors: &[&str]) -> Vec<Vec<Rule>> {
let rule = Arc::new(RwLock::new(StyleRule {
selectors: selectors,
block: Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: vec![
(PropertyDeclaration::Display(DeclaredValue::Value(
longhands::display::SpecifiedValue::block)),
Importance::Normal),
],
important_count: 0,
})),
block: Arc::new(RwLock::new(PropertyDeclarationBlock::with_one(
PropertyDeclaration::Display(DeclaredValue::Value(
longhands::display::SpecifiedValue::block)),
Importance::Normal
))),
}));
let guard = rule.read();