Auto merge of #18692 - emilio:cleanups, r=nox

style: multiple cleanups.

<!-- 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/18692)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-10-01 13:30:18 -05:00 committed by GitHub
commit 4d95e2cd46
11 changed files with 124 additions and 171 deletions

View file

@ -206,7 +206,7 @@ impl CascadeInputs {
pub fn new_from_style(style: &ComputedValues) -> Self {
CascadeInputs {
rules: style.rules.clone(),
visited_rules: style.get_visited_style().and_then(|v| v.rules.clone()),
visited_rules: style.visited_style().and_then(|v| v.rules.clone()),
}
}
}

View file

@ -448,7 +448,7 @@ fn parse_var_function<'i, 't>(
/// name was already there.
pub fn cascade<'a>(
custom_properties: &mut Option<OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>>,
inherited: &'a Option<Arc<CustomPropertiesMap>>,
inherited: Option<&'a Arc<CustomPropertiesMap>>,
seen: &mut PrecomputedHashSet<&'a Name>,
name: &'a Name,
specified_value: DeclaredValue<'a, Box<SpecifiedValue>>
@ -462,7 +462,7 @@ pub fn cascade<'a>(
Some(ref mut map) => map,
None => {
let mut map = OrderedMap::new();
if let Some(ref inherited) = *inherited {
if let Some(inherited) = inherited {
for name in &inherited.index {
let inherited_value = inherited.get(name).unwrap();
map.insert(name, BorrowedSpecifiedValue {
@ -503,14 +503,15 @@ pub fn cascade<'a>(
/// to remove any potential cycles, and wrap it in an arc.
///
/// Otherwise, just use the inherited custom properties map.
pub fn finish_cascade(specified_values_map: Option<OrderedMap<&Name, BorrowedSpecifiedValue>>,
inherited: &Option<Arc<CustomPropertiesMap>>)
-> Option<Arc<CustomPropertiesMap>> {
pub fn finish_cascade(
specified_values_map: Option<OrderedMap<&Name, BorrowedSpecifiedValue>>,
inherited: Option<&Arc<CustomPropertiesMap>>,
) -> Option<Arc<CustomPropertiesMap>> {
if let Some(mut map) = specified_values_map {
remove_cycles(&mut map);
Some(Arc::new(substitute_all(map)))
} else {
inherited.clone()
inherited.cloned()
}
}
@ -732,16 +733,18 @@ fn substitute_block<'i, 't, F>(input: &mut Parser<'i, 't>,
/// Replace `var()` functions for a non-custom property.
/// Return `Err(())` for invalid at computed time.
pub fn substitute<'i>(input: &'i str, first_token_type: TokenSerializationType,
computed_values_map: &Option<Arc<CustomPropertiesMap>>)
-> Result<String, ParseError<'i>> {
pub fn substitute<'i>(
input: &'i str,
first_token_type: TokenSerializationType,
computed_values_map: Option<&Arc<CustomPropertiesMap>>,
) -> Result<String, ParseError<'i>> {
let mut substituted = ComputedValue::empty();
let mut input = ParserInput::new(input);
let mut input = Parser::new(&mut input);
let mut position = (input.position(), first_token_type);
let last_token_type = substitute_block(
&mut input, &mut position, &mut substituted, &mut |name, substituted| {
if let Some(value) = computed_values_map.as_ref().and_then(|map| map.get(name)) {
if let Some(value) = computed_values_map.and_then(|map| map.get(name)) {
substituted.push_variable(value);
Ok(value.last_token_type)
} else {

View file

@ -126,7 +126,7 @@ trait PrivateMatchMethods: TElement {
let inputs =
CascadeInputs {
rules: Some(without_transition_rules),
visited_rules: primary_style.get_visited_style().and_then(|s| s.rules.clone()),
visited_rules: primary_style.visited_rules().cloned()
};
// Actually `PseudoElementResolution` doesn't really matter.

View file

@ -152,7 +152,7 @@ pub struct AnimationValueIterator<'a, 'cx, 'cx_a:'cx> {
context: &'cx mut Context<'cx_a>,
default_values: &'a ComputedValues,
/// Custom properties in a keyframe if exists.
extra_custom_properties: &'a Option<Arc<::custom_properties::CustomPropertiesMap>>,
extra_custom_properties: Option<&'a Arc<::custom_properties::CustomPropertiesMap>>,
}
impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> {
@ -160,7 +160,7 @@ impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> {
declarations: &'a PropertyDeclarationBlock,
context: &'cx mut Context<'cx_a>,
default_values: &'a ComputedValues,
extra_custom_properties: &'a Option<Arc<::custom_properties::CustomPropertiesMap>>,
extra_custom_properties: Option<&'a Arc<::custom_properties::CustomPropertiesMap>>,
) -> AnimationValueIterator<'a, 'cx, 'cx_a> {
AnimationValueIterator {
iter: declarations.declaration_importance_iter(),
@ -259,7 +259,7 @@ impl PropertyDeclarationBlock {
&'a self,
context: &'cx mut Context<'cx_a>,
default_values: &'a ComputedValues,
extra_custom_properties: &'a Option<Arc<::custom_properties::CustomPropertiesMap>>,
extra_custom_properties: Option<&'a Arc<::custom_properties::CustomPropertiesMap>>,
) -> AnimationValueIterator<'a, 'cx, 'cx_a> {
AnimationValueIterator::new(self, context, default_values, extra_custom_properties)
}
@ -592,12 +592,14 @@ impl PropertyDeclarationBlock {
if self.declarations.len() == 1 {
let declaration = &self.declarations[0];
let custom_properties = if let Some(cv) = computed_values {
// If there are extra custom properties for this declaration block,
// factor them in too.
// If there are extra custom properties for this
// declaration block, factor them in too.
if let Some(block) = custom_properties_block {
// FIXME(emilio): This is not super-efficient
// here...
block.cascade_custom_properties(cv.custom_properties())
} else {
cv.custom_properties()
cv.custom_properties().cloned()
}
} else {
None
@ -610,13 +612,14 @@ impl PropertyDeclarationBlock {
// |computed_values| is supplied, we use it to expand such variable
// declarations. This will be fixed properly in Gecko bug 1391537.
(&PropertyDeclaration::WithVariables(id, ref unparsed),
Some(ref _computed_values)) => unparsed
.substitute_variables(
Some(ref _computed_values)) => {
unparsed.substitute_variables(
id,
&custom_properties,
custom_properties.as_ref(),
QuirksMode::NoQuirks,
)
.to_css(dest),
.to_css(dest)
},
(ref d, _) => d.to_css(dest),
}
} else {
@ -690,7 +693,7 @@ impl PropertyDeclarationBlock {
/// properties.
pub fn cascade_custom_properties(
&self,
inherited_custom_properties: Option<Arc<::custom_properties::CustomPropertiesMap>>,
inherited_custom_properties: Option<&Arc<::custom_properties::CustomPropertiesMap>>,
) -> Option<Arc<::custom_properties::CustomPropertiesMap>> {
let mut custom_properties = None;
let mut seen_custom = PrecomputedHashSet::default();
@ -698,12 +701,18 @@ impl PropertyDeclarationBlock {
for declaration in self.normal_declaration_iter() {
if let PropertyDeclaration::Custom(ref name, ref value) = *declaration {
::custom_properties::cascade(
&mut custom_properties, &inherited_custom_properties,
&mut seen_custom, name, value.borrow());
&mut custom_properties,
inherited_custom_properties,
&mut seen_custom,
name,
value.borrow(),
);
}
}
::custom_properties::finish_cascade(
custom_properties, &inherited_custom_properties)
custom_properties,
inherited_custom_properties,
)
}
}

View file

@ -253,9 +253,10 @@ impl ops::DerefMut for ComputedValues {
}
impl ComputedValuesInner {
/// Whether we're a visited style.
pub fn is_style_if_visited(&self) -> bool {
self.flags.contains(IS_STYLE_IF_VISITED)
/// Clone the visited style. Used for inheriting parent styles in
/// StyleBuilder::for_inheritance.
pub fn clone_visited_style(&self) -> Option<Arc<ComputedValues>> {
self.visited_style.as_ref().map(|x| x.clone_arc())
}
#[inline]
@ -291,46 +292,11 @@ impl ComputedValuesInner {
}
% endfor
/// Gets a reference to the rule node. Panic if no rule node exists.
pub fn rules(&self) -> &StrongRuleNode {
self.rules.as_ref().unwrap()
}
/// Whether there is a visited style.
pub fn has_visited_style(&self) -> bool {
self.visited_style.is_some()
}
/// Gets a reference to the visited style, if any.
pub fn get_visited_style(&self) -> Option< & ComputedValues> {
self.visited_style.as_ref().map(|x| &**x)
}
/// Gets the raw visited style. Useful for memory reporting.
pub fn get_raw_visited_style(&self) -> &Option<RawOffsetArc<ComputedValues>> {
&self.visited_style
}
/// Gets a reference to the visited style. Panic if no visited style exists.
pub fn visited_style(&self) -> &ComputedValues {
self.get_visited_style().unwrap()
}
/// Clone the visited style. Used for inheriting parent styles in
/// StyleBuilder::for_inheritance.
pub fn clone_visited_style(&self) -> Option<Arc<ComputedValues>> {
self.visited_style.as_ref().map(|x| x.clone_arc())
}
/// Gets a reference to the custom properties map (if one exists).
pub fn get_custom_properties(&self) -> Option<<&::custom_properties::CustomPropertiesMap> {
self.custom_properties.as_ref().map(|x| &**x)
}
pub fn custom_properties(&self) -> Option<Arc<CustomPropertiesMap>> {
self.custom_properties.clone()
}
#[allow(non_snake_case)]
pub fn has_moz_binding(&self) -> bool {
!self.get_box().gecko.mBinding.mPtr.mRawPtr.is_null()

View file

@ -402,7 +402,7 @@ impl AnimationValue {
pub fn from_declaration(
decl: &PropertyDeclaration,
context: &mut Context,
extra_custom_properties: &Option<Arc<::custom_properties::CustomPropertiesMap>>,
extra_custom_properties: Option<<&Arc<::custom_properties::CustomPropertiesMap>>,
initial: &ComputedValues
) -> Option<Self> {
use properties::LonghandId;
@ -476,14 +476,22 @@ impl AnimationValue {
}
},
PropertyDeclaration::WithVariables(id, ref unparsed) => {
let substituted = if extra_custom_properties.is_some() {
let substituted = {
let custom_properties =
extra_custom_properties.or_else(|| context.style().custom_properties());
unparsed.substitute_variables(
id, &extra_custom_properties, context.quirks_mode)
} else {
unparsed.substitute_variables(
id, &context.style().custom_properties(), context.quirks_mode)
id,
custom_properties,
context.quirks_mode
)
};
AnimationValue::from_declaration(&substituted, context, extra_custom_properties, initial)
AnimationValue::from_declaration(
&substituted,
context,
extra_custom_properties,
initial,
)
},
_ => None // non animatable properties will get included because of shorthands. ignore.
}

View file

@ -938,19 +938,23 @@ pub struct UnparsedValue {
}
impl UnparsedValue {
fn substitute_variables(&self, longhand_id: LonghandId,
custom_properties: &Option<Arc<::custom_properties::CustomPropertiesMap>>,
quirks_mode: QuirksMode)
-> PropertyDeclaration {
fn substitute_variables(
&self,
longhand_id: LonghandId,
custom_properties: Option<<&Arc<::custom_properties::CustomPropertiesMap>>,
quirks_mode: QuirksMode,
) -> PropertyDeclaration {
::custom_properties::substitute(&self.css, self.first_token_type, custom_properties)
.ok()
.and_then(|css| {
// As of this writing, only the base URL is used for property values:
let context = ParserContext::new(Origin::Author,
let context = ParserContext::new(
Origin::Author,
&self.url_data,
None,
PARSING_MODE_DEFAULT,
quirks_mode);
quirks_mode,
);
let mut input = ParserInput::new(&css);
Parser::new(&mut input).parse_entirely(|input| {
match self.from_shorthand {
@ -2086,6 +2090,21 @@ pub struct ComputedValues {
}
impl ComputedValues {
/// Whether we're a visited style.
pub fn is_style_if_visited(&self) -> bool {
self.flags.contains(IS_STYLE_IF_VISITED)
}
/// Gets a reference to the rule node. Panic if no rule node exists.
pub fn rules(&self) -> &StrongRuleNode {
self.rules.as_ref().unwrap()
}
/// Returns the visited style, if any.
pub fn visited_style(&self) -> Option<<&ComputedValues> {
self.visited_style.as_ref().map(|s| &**s)
}
/// Returns the visited rules, if applicable.
pub fn visited_rules(&self) -> Option<<&StrongRuleNode> {
self.visited_style.as_ref().and_then(|s| s.rules.as_ref())
@ -2097,6 +2116,11 @@ impl ComputedValues {
self.flags.contains(IS_IN_DISPLAY_NONE_SUBTREE)
}
/// Gets a reference to the custom properties map (if one exists).
pub fn custom_properties(&self) -> Option<<&Arc<::custom_properties::CustomPropertiesMap>> {
self.custom_properties.as_ref()
}
}
#[cfg(feature = "servo")]
@ -2116,16 +2140,16 @@ impl ComputedValues {
% endfor
) -> Arc<Self> {
Arc::new(Self {
inner: ComputedValuesInner::new(
inner: ComputedValuesInner {
custom_properties,
writing_mode,
flags,
rules,
visited_style,
flags,
% for style_struct in data.active_style_structs():
${style_struct.ident},
% endfor
)
}
})
}
@ -2133,32 +2157,6 @@ impl ComputedValues {
pub fn initial_values() -> &'static Self { &*INITIAL_SERVO_VALUES }
}
#[cfg(feature = "servo")]
impl ComputedValuesInner {
/// Construct a `ComputedValuesInner` instance.
pub fn new(
custom_properties: Option<Arc<::custom_properties::CustomPropertiesMap>>,
writing_mode: WritingMode,
flags: ComputedValueFlags,
rules: Option<StrongRuleNode>,
visited_style: Option<Arc<ComputedValues>>,
% for style_struct in data.active_style_structs():
${style_struct.ident}: Arc<style_structs::${style_struct.name}>,
% endfor
) -> Self {
ComputedValuesInner {
custom_properties: custom_properties,
writing_mode: writing_mode,
rules: rules,
visited_style: visited_style,
flags: flags,
% for style_struct in data.active_style_structs():
${style_struct.ident}: ${style_struct.ident},
% endfor
}
}
}
#[cfg(feature = "servo")]
impl ops::Deref for ComputedValues {
type Target = ComputedValuesInner;
@ -2207,20 +2205,9 @@ impl ComputedValuesInner {
self.rules.as_ref().unwrap()
}
/// Whether there is a visited style.
pub fn has_visited_style(&self) -> bool {
self.visited_style.is_some()
}
/// Gets a reference to the visited style, if any.
pub fn get_visited_style(&self) -> Option< & ComputedValues> {
self.visited_style.as_ref().map(|x| &**x)
}
/// Gets a reference to the visited style. Panic if no visited style exists.
pub fn visited_style(&self) -> &ComputedValues {
self.get_visited_style().unwrap()
}
/// Whether this style has a -moz-binding value. This is always false for
/// Servo for obvious reasons.
pub fn has_moz_binding(&self) -> bool { false }
/// Clone the visited style. Used for inheriting parent styles in
/// StyleBuilder::for_inheritance.
@ -2228,30 +2215,6 @@ impl ComputedValuesInner {
self.visited_style.clone()
}
// Aah! The << in the return type below is not valid syntax, but we must
// escape < that way for Mako.
/// Gets a reference to the custom properties map (if one exists).
pub fn get_custom_properties(&self) -> Option<<&::custom_properties::CustomPropertiesMap> {
self.custom_properties.as_ref().map(|x| &**x)
}
/// Get the custom properties map if necessary.
///
/// Cloning the Arc here is fine because it only happens in the case where
/// we have custom properties, and those are both rare and expensive.
pub fn custom_properties(&self) -> Option<Arc<::custom_properties::CustomPropertiesMap>> {
self.custom_properties.clone()
}
/// Whether this style has a -moz-binding value. This is always false for
/// Servo for obvious reasons.
pub fn has_moz_binding(&self) -> bool { false }
/// Whether we're a visited style.
pub fn is_style_if_visited(&self) -> bool {
self.flags.contains(IS_STYLE_IF_VISITED)
}
/// Returns whether this style's display value is equal to contents.
///
/// Since this isn't supported in Servo, this is always false for Servo.
@ -2769,7 +2732,7 @@ impl<'a> StyleBuilder<'a> {
pseudo,
modified_reset: false,
rules: None, // FIXME(emilio): Dubious...
custom_properties: style_to_derive_from.custom_properties(),
custom_properties: style_to_derive_from.custom_properties().cloned(),
writing_mode: style_to_derive_from.writing_mode,
flags: style_to_derive_from.flags,
visited_style: style_to_derive_from.clone_visited_style(),
@ -2888,7 +2851,7 @@ impl<'a> StyleBuilder<'a> {
pseudo,
CascadeFlags::empty(),
/* rules = */ None,
parent.custom_properties(),
parent.custom_properties().cloned(),
parent.writing_mode,
parent.flags,
parent.clone_visited_style()
@ -3009,8 +2972,8 @@ impl<'a> StyleBuilder<'a> {
///
/// Cloning the Arc here is fine because it only happens in the case where
/// we have custom properties, and those are both rare and expensive.
fn custom_properties(&self) -> Option<Arc<::custom_properties::CustomPropertiesMap>> {
self.custom_properties.clone()
fn custom_properties(&self) -> Option<<&Arc<::custom_properties::CustomPropertiesMap>> {
self.custom_properties.as_ref()
}
/// Access to various information about our inherited styles. We don't
@ -3276,14 +3239,20 @@ where
for (declaration, _cascade_level) in iter_declarations() {
if let PropertyDeclaration::Custom(ref name, ref value) = *declaration {
::custom_properties::cascade(
&mut custom_properties, &inherited_custom_properties,
&mut seen_custom, name, value.borrow());
&mut custom_properties,
inherited_custom_properties,
&mut seen_custom,
name,
value.borrow(),
);
}
}
let custom_properties =
::custom_properties::finish_cascade(
custom_properties, &inherited_custom_properties);
custom_properties,
inherited_custom_properties,
);
let mut context = computed::Context {
is_root_element: flags.contains(IS_ROOT_ELEMENT),
@ -3348,7 +3317,7 @@ where
}
Cow::Owned(unparsed.substitute_variables(
id,
&context.builder.custom_properties,
context.builder.custom_properties.as_ref(),
context.quirks_mode
))
}

View file

@ -281,7 +281,7 @@ fn compute_damage(old: &ComputedValues, new: &ComputedValues) -> ServoRestyleDam
// Paint worklets may depend on custom properties,
// so if they have changed we should repaint.
if old.get_custom_properties() != new.get_custom_properties() {
if old.custom_properties() != new.custom_properties() {
damage.insert(REPAINT);
}

View file

@ -293,7 +293,7 @@ where
pseudo: Option<&PseudoElement>,
) -> ResolvedStyle {
let mut style_if_visited = None;
if parent_style.map_or(false, |s| s.get_visited_style().is_some()) ||
if parent_style.map_or(false, |s| s.visited_style().is_some()) ||
inputs.visited_rules.is_some() {
style_if_visited = Some(self.cascade_style(
inputs.visited_rules.as_ref().or(inputs.rules.as_ref()),
@ -384,7 +384,7 @@ where
};
let mut visited_rules = None;
if originating_element_style.style().get_visited_style().is_some() {
if originating_element_style.style().visited_style().is_some() {
visited_rules = self.match_pseudo(
originating_element_style.style(),
pseudo,
@ -567,7 +567,7 @@ where
// visitedness of the relevant link should influence style.
if pseudo.is_some() || !self.element.is_link() {
parent_style = parent_style.map(|s| {
s.get_visited_style().unwrap_or(s)
s.visited_style().unwrap_or(s)
});
}
cascade_flags.insert(VISITED_DEPENDENT_ONLY);

View file

@ -893,7 +893,7 @@ impl Stylist {
) -> Arc<ComputedValues> {
// We need to compute visited values if we have visited rules or if our
// parent has visited values.
let visited_values = if inputs.visited_rules.is_some() || parent_style.get_visited_style().is_some() {
let visited_values = if inputs.visited_rules.is_some() || parent_style.visited_style().is_some() {
// Slightly annoying: we know that inputs has either rules or
// visited rules, but we can't do inputs.rules() up front because
// maybe it just has visited rules, so can't unwrap_or.
@ -913,11 +913,11 @@ impl Stylist {
// We want to use the visited bits (if any) from our parent
// style as our parent.
inherited_style =
parent_style.get_visited_style().unwrap_or(parent_style);
parent_style.visited_style().unwrap_or(parent_style);
inherited_style_ignoring_first_line =
parent_style_ignoring_first_line.get_visited_style().unwrap_or(parent_style_ignoring_first_line);
parent_style_ignoring_first_line.visited_style().unwrap_or(parent_style_ignoring_first_line);
layout_parent_style_for_visited =
layout_parent_style.get_visited_style().unwrap_or(layout_parent_style);
layout_parent_style.visited_style().unwrap_or(layout_parent_style);
}
Some(properties::cascade(

View file

@ -2044,7 +2044,7 @@ pub extern "C" fn Servo_ComputedValues_EqualCustomProperties(
first: ServoComputedDataBorrowed,
second: ServoComputedDataBorrowed
) -> bool {
first.get_custom_properties() == second.get_custom_properties()
first.custom_properties == second.custom_properties
}
#[no_mangle]
@ -3436,7 +3436,7 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeLis
let iter = guard.to_animation_value_iter(
&mut context,
&default_values,
&custom_properties,
custom_properties.as_ref(),
);
for value in iter {
@ -3479,11 +3479,10 @@ pub extern "C" fn Servo_GetAnimationValues(declarations: RawServoDeclarationBloc
let declarations = Locked::<PropertyDeclarationBlock>::as_arc(&declarations);
let guard = declarations.read_with(&guard);
let no_extra_custom_properties = None; // SMIL has no extra custom properties.
let iter = guard.to_animation_value_iter(
&mut context,
&default_values,
&no_extra_custom_properties,
None, // SMIL has no extra custom properties.
);
for (index, anim) in iter.enumerate() {
unsafe { animation_values.set_len((index + 1) as u32) };
@ -3524,11 +3523,10 @@ pub extern "C" fn Servo_AnimationValue_Compute(element: RawGeckoElementBorrowed,
// We only compute the first element in declarations.
match declarations.read_with(&guard).declaration_importance_iter().next() {
Some((decl, imp)) if imp == Importance::Normal => {
let no_extra_custom_properties = None; // No extra custom properties for devtools.
let animation = AnimationValue::from_declaration(
decl,
&mut context,
&no_extra_custom_properties,
None, // No extra custom properties for devtools.
default_values,
);
animation.map_or(RawServoAnimationValueStrong::null(), |value| {