From 9204f2aaad6d391b507860dbaca86cb5057166b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Wed, 8 Feb 2017 23:46:33 +0300 Subject: [PATCH 1/5] Add a test to check the size of PropertyDeclaration --- tests/unit/style/lib.rs | 1 + tests/unit/style/size_of.rs | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tests/unit/style/size_of.rs diff --git a/tests/unit/style/lib.rs b/tests/unit/style/lib.rs index 26d242b26b8..e9b0133538f 100644 --- a/tests/unit/style/lib.rs +++ b/tests/unit/style/lib.rs @@ -32,6 +32,7 @@ mod owning_handle; mod parsing; mod properties; mod rule_tree; +mod size_of; mod str; mod stylesheets; mod stylist; diff --git a/tests/unit/style/size_of.rs b/tests/unit/style/size_of.rs new file mode 100644 index 00000000000..5da4463db30 --- /dev/null +++ b/tests/unit/style/size_of.rs @@ -0,0 +1,24 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use std::mem::size_of; +use style::properties::PropertyDeclaration; + +#[test] +fn size_of_property_declaration() { + let old = 240; + let new = size_of::(); + if new < old { + panic!("Your changes have decreased the stack size of PropertyDeclaration enum from {} to {}. \ + Good work! Please update the size in tests/unit/style/size_of.rs.", + old, new) + } else if new > old { + panic!("Your changes have increased the stack size of PropertyDeclaration enum from {} to {}. \ + These enum is present in large quantities in the style, and increasing the size \ + may dramatically affect our memory footprint. Please consider using `boxed=\"True\"` in \ + the longhand If you feel that the increase is necessary, update to the new size in \ + tests/unit/style/size_of.rs.", + old, new) + } +} From abc40f61c0dc1703028a744b42c0a9b5b45339fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Wed, 8 Feb 2017 12:29:41 +0300 Subject: [PATCH 2/5] Replace NoViewportPercentage with a macro --- components/style/properties/helpers.mako.rs | 4 +- .../properties/longhand/background.mako.rs | 4 +- .../style/properties/longhand/border.mako.rs | 12 +++--- .../style/properties/longhand/box.mako.rs | 40 +++++++++---------- .../style/properties/longhand/column.mako.rs | 4 +- .../properties/longhand/counters.mako.rs | 8 ++-- .../style/properties/longhand/font.mako.rs | 24 +++++------ .../properties/longhand/inherited_box.mako.rs | 12 +++--- .../longhand/inherited_text.mako.rs | 20 +++++----- .../style/properties/longhand/list.mako.rs | 4 +- .../style/properties/longhand/outline.mako.rs | 1 - .../properties/longhand/pointing.mako.rs | 4 +- .../properties/longhand/position.mako.rs | 4 +- .../style/properties/longhand/svg.mako.rs | 8 ++-- .../style/properties/longhand/text.mako.rs | 8 ++-- components/style/values/mod.rs | 26 ++++++------ components/style/values/specified/grid.rs | 4 +- components/style/values/specified/mod.rs | 16 ++++---- components/style/values/specified/url.rs | 4 +- 19 files changed, 104 insertions(+), 103 deletions(-) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 7156e9f9585..1c0c441ac63 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -333,9 +333,9 @@ <%def name="single_keyword(name, values, vector=False, **kwargs)"> <%call expr="single_keyword_computed(name, values, vector, **kwargs)"> use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; + use values::HasViewportPercentage; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); diff --git a/components/style/properties/longhand/background.mako.rs b/components/style/properties/longhand/background.mako.rs index ca790ab884c..e5cffa016c2 100644 --- a/components/style/properties/longhand/background.mako.rs +++ b/components/style/properties/longhand/background.mako.rs @@ -16,8 +16,8 @@ ${helpers.predefined_type("background-color", "CSSColor", has_uncacheable_values="${product == 'gecko'}"> use std::fmt; use style_traits::ToCss; + use values::HasViewportPercentage; use values::specified::Image; - use values::NoViewportPercentage; pub mod computed_value { use values::computed; @@ -35,7 +35,7 @@ ${helpers.predefined_type("background-color", "CSSColor", } } - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); #[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] diff --git a/components/style/properties/longhand/border.mako.rs b/components/style/properties/longhand/border.mako.rs index b5c4ff6b1d3..55688015ed8 100644 --- a/components/style/properties/longhand/border.mako.rs +++ b/components/style/properties/longhand/border.mako.rs @@ -87,10 +87,10 @@ ${helpers.single_keyword("-moz-float-edge", "content-box margin-box", spec="https://drafts.csswg.org/css-backgrounds/#border-image-source"> use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::specified::Image; - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { use values::computed; @@ -279,9 +279,9 @@ ${helpers.single_keyword("-moz-float-edge", "content-box margin-box", spec="https://drafts.csswg.org/css-backgrounds/#border-image-repeat"> use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { pub use super::RepeatKeyword; @@ -557,10 +557,10 @@ ${helpers.single_keyword("-moz-float-edge", "content-box margin-box", spec="https://drafts.csswg.org/css-backgrounds/#border-image-slice"> use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::specified::{Number, Percentage}; - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { use values::computed::Number; diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index 0e4abaaa1dc..aa30a5551ad 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -31,8 +31,8 @@ %> use values::computed::ComputedValueAsSpecified; use style_traits::ToCss; - use values::NoViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + use values::HasViewportPercentage; + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { pub use super::SpecifiedValue as T; @@ -113,8 +113,8 @@ ${helpers.single_keyword("-moz-top-layer", "none top", } } - use values::NoViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + use values::HasViewportPercentage; + no_viewport_percentage!(SpecifiedValue); impl ToComputedValue for SpecifiedValue { type ComputedValue = computed_value::T; @@ -152,8 +152,8 @@ ${helpers.single_keyword("-moz-top-layer", "none top", gecko_inexhaustive="True" gecko_ffi_name="mFloat" spec="https://drafts.csswg.org/css-box/#propdef-float"> - use values::NoViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + use values::HasViewportPercentage; + no_viewport_percentage!(SpecifiedValue); impl ToComputedValue for SpecifiedValue { type ComputedValue = computed_value::T; @@ -194,8 +194,8 @@ ${helpers.single_keyword("-moz-top-layer", "none top", gecko_enum_prefix="StyleClear" gecko_ffi_name="mBreakType" spec="https://www.w3.org/TR/CSS2/visuren.html#flow-control"> - use values::NoViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + use values::HasViewportPercentage; + no_viewport_percentage!(SpecifiedValue); impl ToComputedValue for SpecifiedValue { type ComputedValue = computed_value::T; @@ -396,9 +396,9 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", use std::fmt; use style_traits::ToCss; use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; + use values::HasViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); impl ToCss for SpecifiedValue { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { @@ -442,8 +442,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", use values::specified::Time; pub use values::specified::Time as SpecifiedValue; - use values::NoViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + use values::HasViewportPercentage; + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { pub use values::computed::Time as T; @@ -703,8 +703,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", } } - use values::NoViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + use values::HasViewportPercentage; + no_viewport_percentage!(SpecifiedValue); #[inline] pub fn get_initial_value() -> computed_value::T { @@ -745,8 +745,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", SpecifiedValue::parse(input) } - use values::NoViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + use values::HasViewportPercentage; + no_viewport_percentage!(SpecifiedValue); impl ComputedValueAsSpecified for SpecifiedValue { } @@ -773,7 +773,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", use std::ops::Deref; use style_traits::ToCss; use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; + use values::HasViewportPercentage; pub mod computed_value { pub use super::SpecifiedValue as T; @@ -805,7 +805,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", }) } } - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub fn parse(context: &ParserContext, input: &mut Parser) -> Result { SpecifiedValue::parse(context, input) @@ -847,7 +847,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", use std::fmt; use style_traits::ToCss; use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; + use values::HasViewportPercentage; pub mod computed_value { pub use super::SpecifiedValue as T; @@ -885,7 +885,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", } } - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); #[inline] pub fn get_initial_value() -> computed_value::T { diff --git a/components/style/properties/longhand/column.mako.rs b/components/style/properties/longhand/column.mako.rs index e10a69e45d1..f55524f5201 100644 --- a/components/style/properties/longhand/column.mako.rs +++ b/components/style/properties/longhand/column.mako.rs @@ -22,9 +22,9 @@ ${helpers.predefined_type("column-width", spec="https://drafts.csswg.org/css-multicol/#propdef-column-count"> use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); #[derive(Debug, Clone, Copy, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] diff --git a/components/style/properties/longhand/counters.mako.rs b/components/style/properties/longhand/counters.mako.rs index 78c9fa8fc9e..265e4b9bfe0 100644 --- a/components/style/properties/longhand/counters.mako.rs +++ b/components/style/properties/longhand/counters.mako.rs @@ -10,7 +10,7 @@ use cssparser::Token; use std::ascii::AsciiExt; use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use super::list_style_type; @@ -18,7 +18,7 @@ pub use self::computed_value::ContentItem; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { use super::super::list_style_type; @@ -179,7 +179,7 @@ use std::fmt; use style_traits::ToCss; use super::content; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; use cssparser::{Token, serialize_identifier}; @@ -199,7 +199,7 @@ } impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); impl ToCss for SpecifiedValue { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { diff --git a/components/style/properties/longhand/font.mako.rs b/components/style/properties/longhand/font.mako.rs index 42387415c1b..a52a7e1f498 100644 --- a/components/style/properties/longhand/font.mako.rs +++ b/components/style/properties/longhand/font.mako.rs @@ -11,12 +11,12 @@ <%helpers:longhand name="font-family" animatable="False" need_index="True" spec="https://drafts.csswg.org/css-fonts/#propdef-font-family"> use self::computed_value::{FontFamily, FamilyName}; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; pub use self::computed_value::T as SpecifiedValue; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { use cssparser::CssStringWriter; @@ -223,9 +223,9 @@ ${helpers.single_keyword("font-variant-caps", spec="https://drafts.csswg.org/css-fonts/#propdef-font-weight"> use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); #[derive(Debug, Clone, PartialEq, Eq, Copy)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] @@ -458,12 +458,12 @@ ${helpers.single_keyword("font-variant-caps", <%helpers:longhand products="gecko" name="font-size-adjust" animatable="True" spec="https://drafts.csswg.org/css-fonts/#propdef-font-size-adjust"> - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; use values::specified::Number; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); #[derive(Copy, Clone, Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] @@ -520,11 +520,11 @@ ${helpers.single_keyword("font-variant-caps", spec="https://drafts.csswg.org/css-fonts/#propdef-font-synthesis"> use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { pub use super::SpecifiedValue as T; @@ -610,12 +610,12 @@ ${helpers.single_keyword("font-variant-position", spec="https://drafts.csswg.org/css-fonts/#propdef-font-feature-settings"> use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; pub use self::computed_value::T as SpecifiedValue; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { use cssparser::Parser; @@ -719,12 +719,12 @@ ${helpers.single_keyword("font-variant-position", // https://www.w3.org/TR/css-fonts-3/#propdef-font-language-override <%helpers:longhand name="font-language-override" products="none" animatable="False" extra_prefixes="moz" spec="https://drafts.csswg.org/css-fonts-3/#propdef-font-language-override"> - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; pub use self::computed_value::T as SpecifiedValue; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { use std::fmt; diff --git a/components/style/properties/longhand/inherited_box.mako.rs b/components/style/properties/longhand/inherited_box.mako.rs index 6a87bad5d15..ef19dedaca8 100644 --- a/components/style/properties/longhand/inherited_box.mako.rs +++ b/components/style/properties/longhand/inherited_box.mako.rs @@ -35,8 +35,8 @@ ${helpers.single_keyword("direction", "ltr rtl", need_clone=True, animatable=Fal animatable="False" spec="https://drafts.csswg.org/css-writing-modes/#propdef-text-orientation" > - use values::NoViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + use values::HasViewportPercentage; + no_viewport_percentage!(SpecifiedValue); impl ToComputedValue for SpecifiedValue { type ComputedValue = computed_value::T; @@ -91,8 +91,8 @@ ${helpers.single_keyword("image-rendering", use style_traits::ToCss; use values::specified::Angle; - use values::NoViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + use values::HasViewportPercentage; + no_viewport_percentage!(SpecifiedValue); use std::f32::consts::PI; use values::CSSFloat; @@ -225,9 +225,9 @@ ${helpers.single_keyword("image-rendering", use std::fmt; use style_traits::ToCss; use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; + use values::HasViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); #[derive(Copy, Clone, Debug, Eq, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))] diff --git a/components/style/properties/longhand/inherited_text.mako.rs b/components/style/properties/longhand/inherited_text.mako.rs index 03b5eb2e33b..900dec4846c 100644 --- a/components/style/properties/longhand/inherited_text.mako.rs +++ b/components/style/properties/longhand/inherited_text.mako.rs @@ -204,9 +204,9 @@ ${helpers.single_keyword("text-align-last", <%helpers:longhand name="text-align" animatable="False" spec="https://drafts.csswg.org/css-text/#propdef-text-align"> pub use self::computed_value::T as SpecifiedValue; use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; + use values::HasViewportPercentage; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { use style_traits::ToCss; macro_rules! define_text_align { @@ -434,11 +434,11 @@ ${helpers.single_keyword("text-align-last", use cssparser::RGBA; use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); #[derive(Clone, PartialEq, Copy, Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] @@ -518,9 +518,9 @@ ${helpers.single_keyword("text-align-last", animatable="False" spec="https://drafts.csswg.org/css-text/#propdef-white-space"> use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; + use values::HasViewportPercentage; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); impl SpecifiedValue { pub fn allow_wrap(&self) -> bool { @@ -771,9 +771,9 @@ ${helpers.single_keyword("text-align-last", use std::fmt; use style_traits::ToCss; use unicode_segmentation::UnicodeSegmentation; - use values::NoViewportPercentage; + use values::HasViewportPercentage; - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { #[derive(Debug, Clone, PartialEq)] @@ -977,7 +977,7 @@ ${helpers.single_keyword("text-align-last", spec="https://drafts.csswg.org/css-text-decor/#propdef-text-emphasis-position"> use std::fmt; use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use style_traits::ToCss; define_css_keyword_enum!(HorizontalWritingModeValue: @@ -996,7 +996,7 @@ ${helpers.single_keyword("text-align-last", } impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub fn get_initial_value() -> computed_value::T { SpecifiedValue(HorizontalWritingModeValue::Over, VerticalWritingModeValue::Right) diff --git a/components/style/properties/longhand/list.mako.rs b/components/style/properties/longhand/list.mako.rs index 9f19d80aec7..b0cfc32f56c 100644 --- a/components/style/properties/longhand/list.mako.rs +++ b/components/style/properties/longhand/list.mako.rs @@ -39,7 +39,7 @@ ${helpers.predefined_type("list-style-image", "UrlOrNone", "Either::Second(None_ use std::fmt; use style_traits::ToCss; use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; + use values::HasViewportPercentage; pub use self::computed_value::T as SpecifiedValue; @@ -50,7 +50,7 @@ ${helpers.predefined_type("list-style-image", "UrlOrNone", "Either::Second(None_ } impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); impl ToCss for SpecifiedValue { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { diff --git a/components/style/properties/longhand/outline.mako.rs b/components/style/properties/longhand/outline.mako.rs index e4fb0512d85..9780ac24110 100644 --- a/components/style/properties/longhand/outline.mako.rs +++ b/components/style/properties/longhand/outline.mako.rs @@ -20,7 +20,6 @@ ${helpers.predefined_type("outline-color", "CSSColor", "::cssparser::Color::Curr use std::fmt; use style_traits::ToCss; use values::specified::BorderStyle; - use values::NoViewportPercentage; use values::computed::ComputedValueAsSpecified; pub type SpecifiedValue = Either; diff --git a/components/style/properties/longhand/pointing.mako.rs b/components/style/properties/longhand/pointing.mako.rs index 71c43b2cc2b..a6219a32d1f 100644 --- a/components/style/properties/longhand/pointing.mako.rs +++ b/components/style/properties/longhand/pointing.mako.rs @@ -8,12 +8,12 @@ <%helpers:longhand name="cursor" animatable="False" spec="https://drafts.csswg.org/css-ui/#cursor"> pub use self::computed_value::T as SpecifiedValue; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; use values::specified::url::SpecifiedUrl; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub mod computed_value { use std::fmt; diff --git a/components/style/properties/longhand/position.mako.rs b/components/style/properties/longhand/position.mako.rs index adb66cdc3a4..73b11432aaa 100644 --- a/components/style/properties/longhand/position.mako.rs +++ b/components/style/properties/longhand/position.mako.rs @@ -24,11 +24,11 @@ % endfor <%helpers:longhand name="z-index" spec="https://www.w3.org/TR/CSS2/visuren.html#z-index" animatable="True"> - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); pub type SpecifiedValue = computed_value::T; pub mod computed_value { use std::fmt; diff --git a/components/style/properties/longhand/svg.mako.rs b/components/style/properties/longhand/svg.mako.rs index b75a9c26255..9e68d5386af 100644 --- a/components/style/properties/longhand/svg.mako.rs +++ b/components/style/properties/longhand/svg.mako.rs @@ -62,7 +62,7 @@ ${helpers.single_keyword("mask-type", "luminance alpha", spec="https://drafts.fxtf.org/css-masking/#propdef-clip-path"> use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::specified::basic_shape::{ShapeSource, GeometryBox}; pub mod computed_value { @@ -83,7 +83,7 @@ ${helpers.single_keyword("mask-type", "luminance alpha", ShapeSource::parse(context, input) } - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); ${helpers.single_keyword("mask-mode", @@ -196,7 +196,7 @@ ${helpers.single_keyword("mask-composite", use std::sync::Arc; use values::specified::Image; use values::specified::url::SpecifiedUrl; - use values::NoViewportPercentage; + use values::HasViewportPercentage; pub mod computed_value { use std::fmt; @@ -222,7 +222,7 @@ ${helpers.single_keyword("mask-composite", } } - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); #[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] diff --git a/components/style/properties/longhand/text.mako.rs b/components/style/properties/longhand/text.mako.rs index a4325c7a0cc..8d1792c046b 100644 --- a/components/style/properties/longhand/text.mako.rs +++ b/components/style/properties/longhand/text.mako.rs @@ -16,12 +16,12 @@ spec="https://drafts.csswg.org/css-ui/#propdef-text-overflow"> use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; use cssparser; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); #[derive(PartialEq, Eq, Clone, Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] @@ -108,11 +108,11 @@ ${helpers.single_keyword("unicode-bidi", spec="https://drafts.csswg.org/css-text-decor/#propdef-text-decoration-line"> use std::fmt; use style_traits::ToCss; - use values::NoViewportPercentage; + use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} + no_viewport_percentage!(SpecifiedValue); #[derive(PartialEq, Eq, Copy, Clone, Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs index 7a7d7baafea..4665b37ad82 100644 --- a/components/style/values/mod.rs +++ b/components/style/values/mod.rs @@ -47,6 +47,19 @@ macro_rules! define_numbered_css_keyword_enum { } } +/// A macro used to implement HasViewportPercentage trait +/// for a given type that may never contain viewport units. +macro_rules! no_viewport_percentage { + ($name: ident) => { + impl HasViewportPercentage for $name { + #[inline] + fn has_viewport_percentage(&self) -> bool { + false + } + } + }; +} + pub mod computed; pub mod specified; @@ -62,18 +75,7 @@ pub trait HasViewportPercentage { fn has_viewport_percentage(&self) -> bool; } -/// A trait used as a marker to represent that a given type may never contain -/// viewport units. -pub trait NoViewportPercentage {} -impl HasViewportPercentage for T - where T: NoViewportPercentage, -{ - #[inline] - fn has_viewport_percentage(&self) -> bool { - false - } -} use self::computed::ComputedValueAsSpecified; @@ -103,7 +105,7 @@ macro_rules! define_keyword_type { } impl ComputedValueAsSpecified for $name {} - impl NoViewportPercentage for $name {} + no_viewport_percentage!($name); }; } diff --git a/components/style/values/specified/grid.rs b/components/style/values/specified/grid.rs index 1e414799ed9..4f3e64142e7 100644 --- a/components/style/values/specified/grid.rs +++ b/components/style/values/specified/grid.rs @@ -8,7 +8,7 @@ use cssparser::Parser; use parser::{Parse, ParserContext}; use std::fmt; use style_traits::ToCss; -use values::NoViewportPercentage; +use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; #[derive(PartialEq, Clone, Debug)] @@ -96,4 +96,4 @@ impl Parse for GridLine { } impl ComputedValueAsSpecified for GridLine {} -impl NoViewportPercentage for GridLine {} +no_viewport_percentage!(GridLine); diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 076324cebe6..dac10f661f7 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -16,7 +16,7 @@ use std::f32::consts::PI; use std::fmt; use std::ops::Mul; use style_traits::ToCss; -use super::{CSSFloat, HasViewportPercentage, NoViewportPercentage, Either, None_}; +use super::{CSSFloat, HasViewportPercentage, Either, None_}; use super::computed::{ComputedValueAsSpecified, Context, ToComputedValue}; use super::computed::Shadow as ComputedShadow; @@ -35,7 +35,7 @@ pub mod length; pub mod position; pub mod url; -impl NoViewportPercentage for i32 {} // For PropertyDeclaration::Order +no_viewport_percentage!(i32); // For PropertyDeclaration::Order #[derive(Clone, PartialEq, Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] @@ -60,7 +60,7 @@ impl Parse for CSSColor { } } -impl NoViewportPercentage for CSSColor {} +no_viewport_percentage!(CSSColor); impl ToCss for CSSColor { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { @@ -79,7 +79,7 @@ pub struct CSSRGBA { pub authored: Option, } -impl NoViewportPercentage for CSSRGBA {} +no_viewport_percentage!(CSSRGBA); impl ToCss for CSSRGBA { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { @@ -193,7 +193,7 @@ pub fn parse_number(input: &mut Parser) -> Result { #[allow(missing_docs)] pub struct BorderRadiusSize(pub Size2D); -impl NoViewportPercentage for BorderRadiusSize {} +no_viewport_percentage!(BorderRadiusSize); impl BorderRadiusSize { #[allow(missing_docs)] @@ -402,7 +402,7 @@ define_numbered_css_keyword_enum! { BorderStyle: "outset" => outset = 2, } -impl NoViewportPercentage for BorderStyle {} +no_viewport_percentage!(BorderStyle); impl BorderStyle { /// Whether this border style is either none or hidden. @@ -462,7 +462,7 @@ impl ToCss for Time { #[allow(missing_docs)] pub struct Number(pub CSSFloat); -impl NoViewportPercentage for Number {} +no_viewport_percentage!(Number); impl Parse for Number { fn parse(_context: &ParserContext, input: &mut Parser) -> Result { @@ -512,7 +512,7 @@ impl ToCss for Number { #[allow(missing_docs)] pub struct Opacity(pub CSSFloat); -impl NoViewportPercentage for Opacity {} +no_viewport_percentage!(Opacity); impl Parse for Opacity { fn parse(_context: &ParserContext, input: &mut Parser) -> Result { diff --git a/components/style/values/specified/url.rs b/components/style/values/specified/url.rs index de58714c74f..9ce58980050 100644 --- a/components/style/values/specified/url.rs +++ b/components/style/values/specified/url.rs @@ -15,7 +15,7 @@ use std::borrow::Cow; use std::fmt::{self, Write}; use std::sync::Arc; use style_traits::ToCss; -use values::NoViewportPercentage; +use values::HasViewportPercentage; use values::computed::ComputedValueAsSpecified; /// A set of data needed in Gecko to represent a URL. @@ -202,4 +202,4 @@ impl ToCss for SpecifiedUrl { // TODO(emilio): Maybe consider ComputedUrl to save a word in style structs? impl ComputedValueAsSpecified for SpecifiedUrl {} -impl NoViewportPercentage for SpecifiedUrl {} +no_viewport_percentage!(SpecifiedUrl); From 78afe2b8d1dd170dc0870608aa10160b07b77a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 6 Feb 2017 21:53:59 +0300 Subject: [PATCH 3/5] Box larger specified values to avoid memmove impact --- components/script/dom/element.rs | 10 ++--- components/style/properties/data.py | 3 +- components/style/properties/gecko.mako.rs | 10 ++++- components/style/properties/helpers.mako.rs | 29 +++++++++--- .../helpers/animated_properties.mako.rs | 6 ++- .../properties/longhand/background.mako.rs | 2 +- .../style/properties/longhand/border.mako.rs | 4 +- .../style/properties/longhand/box.mako.rs | 5 ++- .../style/properties/longhand/color.mako.rs | 8 ++-- .../style/properties/longhand/column.mako.rs | 2 +- .../style/properties/longhand/effects.mako.rs | 2 +- .../longhand/inherited_table.mako.rs | 2 +- .../longhand/inherited_text.mako.rs | 7 +-- .../style/properties/longhand/list.mako.rs | 2 +- .../style/properties/longhand/outline.mako.rs | 2 +- .../properties/longhand/position.mako.rs | 3 +- .../style/properties/longhand/svg.mako.rs | 5 ++- .../style/properties/longhand/text.mako.rs | 3 +- .../style/properties/properties.mako.rs | 45 ++++++++++++++++--- .../properties/shorthand/serialize.mako.rs | 4 +- components/style/values/mod.rs | 7 ++- components/style_traits/values.rs | 8 ++++ ports/geckolib/glue.rs | 4 +- tests/unit/style/properties/serialization.rs | 40 ++++++++--------- tests/unit/style/size_of.rs | 24 +++++++++- tests/unit/style/stylesheets.rs | 4 +- 26 files changed, 174 insertions(+), 67 deletions(-) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index cf770674e74..cf4373fbeb6 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -407,7 +407,7 @@ impl LayoutElementHelpers for LayoutJS { if let Some(color) = bgcolor { hints.push(from_declaration( PropertyDeclaration::BackgroundColor(DeclaredValue::Value( - CSSColor { parsed: Color::RGBA(color), authored: None })))); + Box::new(CSSColor { parsed: Color::RGBA(color), authored: None }))))); } let background = if let Some(this) = self.downcast::() { @@ -440,10 +440,10 @@ impl LayoutElementHelpers for LayoutJS { if let Some(color) = color { hints.push(from_declaration( - PropertyDeclaration::Color(DeclaredValue::Value(CSSRGBA { + PropertyDeclaration::Color(DeclaredValue::Value(Box::new(CSSRGBA { parsed: color, authored: None, - })))); + }))))); } let font_family = if let Some(this) = self.downcast::() { @@ -480,10 +480,10 @@ impl LayoutElementHelpers for LayoutJS { let width_value = specified::Length::from_px(cellspacing as f32); hints.push(from_declaration( PropertyDeclaration::BorderSpacing(DeclaredValue::Value( - border_spacing::SpecifiedValue { + Box::new(border_spacing::SpecifiedValue { horizontal: width_value.clone(), vertical: width_value, - })))); + }))))); } diff --git a/components/style/properties/data.py b/components/style/properties/data.py index abec5947041..1219600a5d3 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -96,7 +96,7 @@ class Longhand(object): predefined_type=None, custom_cascade=False, experimental=False, internal=False, need_clone=False, need_index=False, gecko_ffi_name=None, depend_on_viewport_size=False, allowed_in_keyframe_block=True, complex_color=False, cast_type='u8', - has_uncacheable_values=False, logical=False, alias=None, extra_prefixes=None): + has_uncacheable_values=False, logical=False, alias=None, extra_prefixes=None, boxed=False): self.name = name if not spec: raise TypeError("Spec should be specified for %s" % name) @@ -119,6 +119,7 @@ class Longhand(object): self.logical = arg_to_bool(logical) self.alias = alias.split() if alias else [] self.extra_prefixes = extra_prefixes.split() if extra_prefixes else [] + self.boxed = arg_to_bool(boxed) # https://drafts.csswg.org/css-animations/#keyframes # > The inside of accepts any CSS property diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 00605214b06..70363eba25c 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -168,8 +168,16 @@ impl ComputedValues { PropertyDeclarationBlock { declarations: vec![ (PropertyDeclaration::${prop.camel_case}(DeclaredValue::Value( + % if prop.boxed: + Box::new( + % endif longhands::${prop.ident}::SpecifiedValue::from_computed_value( - &self.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}()))), + &self.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}()) + % if prop.boxed: + ) + % endif + + )), Importance::Normal) ], important_count: 0 diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 1c0c441ac63..fa6ccbd862b 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -9,8 +9,13 @@ ${caller.body()} % if not data.longhands_by_name[name].derived_from: pub fn parse_specified(context: &ParserContext, input: &mut Parser) - -> Result, ()> { - parse(context, input).map(DeclaredValue::Value) + % if data.longhands_by_name[name].boxed: + -> Result>, ()> { + parse(context, input).map(|result| DeclaredValue::Value(Box::new(result))) + % else: + -> Result, ()> { + parse(context, input).map(DeclaredValue::Value) + % endif } % endif @@ -298,7 +303,11 @@ } % if not property.derived_from: pub fn parse_declared(context: &ParserContext, input: &mut Parser) - -> Result, ()> { + % if property.boxed: + -> Result>, ()> { + % else: + -> Result, ()> { + % endif match input.try(|i| CSSWideKeyword::parse(context, i)) { Ok(CSSWideKeyword::InheritKeyword) => Ok(DeclaredValue::Inherit), Ok(CSSWideKeyword::InitialKeyword) => Ok(DeclaredValue::Initial), @@ -429,7 +438,13 @@ /// correspond to a shorthand. pub struct LonghandsToSerialize<'a> { % for sub_property in shorthand.sub_properties: - pub ${sub_property.ident}: &'a DeclaredValue, + % if sub_property.boxed: + pub ${sub_property.ident}: + &'a DeclaredValue>, + % else: + pub ${sub_property.ident}: + &'a DeclaredValue, + % endif % endfor } @@ -529,7 +544,11 @@ % for sub_property in shorthand.sub_properties: declarations.push((PropertyDeclaration::${sub_property.camel_case}( match value.${sub_property.ident} { - Some(value) => DeclaredValue::Value(value), + % if sub_property.boxed: + Some(value) => DeclaredValue::Value(Box::new(value)), + % else: + Some(value) => DeclaredValue::Value(value), + % endif None => DeclaredValue::Initial, } ), Importance::Normal)); diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 4ad97be978a..af0079a7f33 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -278,7 +278,11 @@ impl AnimationValue { AnimationValue::${prop.camel_case}(ref from) => { PropertyDeclaration::${prop.camel_case}( DeclaredValue::Value( - longhands::${prop.ident}::SpecifiedValue::from_computed_value(from))) + % if prop.boxed: + Box::new(longhands::${prop.ident}::SpecifiedValue::from_computed_value(from)))) + % else: + longhands::${prop.ident}::SpecifiedValue::from_computed_value(from))) + % endif } % endif % endfor diff --git a/components/style/properties/longhand/background.mako.rs b/components/style/properties/longhand/background.mako.rs index e5cffa016c2..57c193bedeb 100644 --- a/components/style/properties/longhand/background.mako.rs +++ b/components/style/properties/longhand/background.mako.rs @@ -9,7 +9,7 @@ ${helpers.predefined_type("background-color", "CSSColor", "::cssparser::Color::RGBA(::cssparser::RGBA { red: 0., green: 0., blue: 0., alpha: 0. }) /* transparent */", spec="https://drafts.csswg.org/css-backgrounds/#background-color", - animatable=True, complex_color=True)} + animatable=True, complex_color=True, boxed=True)} <%helpers:vector_longhand name="background-image" animatable="False" spec="https://drafts.csswg.org/css-backgrounds/#the-background-image" diff --git a/components/style/properties/longhand/border.mako.rs b/components/style/properties/longhand/border.mako.rs index 55688015ed8..624e8ac7926 100644 --- a/components/style/properties/longhand/border.mako.rs +++ b/components/style/properties/longhand/border.mako.rs @@ -20,7 +20,7 @@ "::cssparser::Color::CurrentColor", alias=maybe_moz_logical_alias(product, side, "-moz-border-%s-color"), spec=maybe_logical_spec(side, "color"), - animatable=True, logical = side[1])} + animatable=True, logical = side[1], boxed=True)} % endfor % for side in ALL_SIDES: @@ -83,7 +83,7 @@ ${helpers.single_keyword("-moz-float-edge", "content-box margin-box", spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-float-edge)", animatable=False)} -<%helpers:longhand name="border-image-source" products="gecko" animatable="False" +<%helpers:longhand name="border-image-source" products="gecko" animatable="False" boxed="True" spec="https://drafts.csswg.org/css-backgrounds/#border-image-source"> use std::fmt; use style_traits::ToCss; diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index aa30a5551ad..95d23dbf0e4 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -1755,7 +1755,7 @@ ${helpers.single_keyword("transform-style", extra_prefixes="moz webkit", animatable=False)} -<%helpers:longhand name="transform-origin" animatable="True" extra_prefixes="moz webkit" +<%helpers:longhand name="transform-origin" animatable="True" extra_prefixes="moz webkit" boxed="True" spec="https://drafts.csswg.org/css-transforms/#transform-origin-property"> use app_units::Au; use std::fmt; @@ -1896,7 +1896,8 @@ ${helpers.predefined_type("-moz-binding", "UrlOrNone", "Either::Second(None_)", products="gecko", animatable="False", spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-binding)", - disable_when_testing="True")} + disable_when_testing="True", + boxed=True)} ${helpers.single_keyword("-moz-orient", "inline block horizontal vertical", diff --git a/components/style/properties/longhand/color.mako.rs b/components/style/properties/longhand/color.mako.rs index 8012defab54..a2d9c8975cd 100644 --- a/components/style/properties/longhand/color.mako.rs +++ b/components/style/properties/longhand/color.mako.rs @@ -6,7 +6,7 @@ <% data.new_style_struct("Color", inherited=True) %> -<%helpers:raw_longhand name="color" need_clone="True" animatable="True" +<%helpers:raw_longhand name="color" need_clone="True" animatable="True" boxed="True" spec="https://drafts.csswg.org/css-color/#color"> use cssparser::Color as CSSParserColor; use cssparser::RGBA; @@ -39,15 +39,15 @@ RGBA { red: 0., green: 0., blue: 0., alpha: 1. } /* black */ } pub fn parse_specified(context: &ParserContext, input: &mut Parser) - -> Result, ()> { + -> Result>, ()> { let value = try!(CSSColor::parse(context, input)); let rgba = match value.parsed { CSSParserColor::RGBA(rgba) => rgba, CSSParserColor::CurrentColor => return Ok(DeclaredValue::Inherit) }; - Ok(DeclaredValue::Value(CSSRGBA { + Ok(DeclaredValue::Value(Box::new(CSSRGBA { parsed: rgba, authored: value.authored, - })) + }))) } diff --git a/components/style/properties/longhand/column.mako.rs b/components/style/properties/longhand/column.mako.rs index f55524f5201..f06ea2b51f7 100644 --- a/components/style/properties/longhand/column.mako.rs +++ b/components/style/properties/longhand/column.mako.rs @@ -147,7 +147,7 @@ ${helpers.single_keyword("column-fill", "auto balance", extra_prefixes="moz", ${helpers.predefined_type("column-rule-color", "CSSColor", "::cssparser::Color::CurrentColor", products="gecko", animatable=True, extra_prefixes="moz", - complex_color=True, need_clone=True, + complex_color=True, need_clone=True, boxed=True, spec="https://drafts.csswg.org/css-multicol/#propdef-column-rule-color")} // It's not implemented in servo or gecko yet. diff --git a/components/style/properties/longhand/effects.mako.rs b/components/style/properties/longhand/effects.mako.rs index 9bc62bc5e7d..b01874e8095 100644 --- a/components/style/properties/longhand/effects.mako.rs +++ b/components/style/properties/longhand/effects.mako.rs @@ -77,7 +77,7 @@ ${helpers.predefined_type("opacity", // FIXME: This prop should be animatable -<%helpers:longhand name="clip" products="servo" animatable="False" +<%helpers:longhand name="clip" products="servo" animatable="False" boxed="True" spec="https://drafts.fxtf.org/css-masking/#clip-property"> use std::fmt; use style_traits::ToCss; diff --git a/components/style/properties/longhand/inherited_table.mako.rs b/components/style/properties/longhand/inherited_table.mako.rs index 62d23a70b49..21930ed0114 100644 --- a/components/style/properties/longhand/inherited_table.mako.rs +++ b/components/style/properties/longhand/inherited_table.mako.rs @@ -19,7 +19,7 @@ ${helpers.single_keyword("caption-side", "top bottom", animatable=False, spec="https://drafts.csswg.org/css-tables/#propdef-caption-side")} -<%helpers:longhand name="border-spacing" animatable="False" +<%helpers:longhand name="border-spacing" animatable="False" boxed="True" spec="https://drafts.csswg.org/css-tables/#propdef-border-spacing"> use app_units::Au; use std::fmt; diff --git a/components/style/properties/longhand/inherited_text.mako.rs b/components/style/properties/longhand/inherited_text.mako.rs index 900dec4846c..335ad1f1ca1 100644 --- a/components/style/properties/longhand/inherited_text.mako.rs +++ b/components/style/properties/longhand/inherited_text.mako.rs @@ -429,7 +429,7 @@ ${helpers.single_keyword("text-align-last", <%helpers:longhand name="-servo-text-decorations-in-effect" derived_from="display text-decoration" need_clone="True" products="servo" - animatable="False" + animatable="False" boxed="True" spec="Nonstandard (Internal property used by Servo)"> use cssparser::RGBA; use std::fmt; @@ -1027,6 +1027,7 @@ ${helpers.predefined_type("text-emphasis-color", "CSSColor", "::cssparser::Color::CurrentColor", products="gecko",animatable=True, complex_color=True, need_clone=True, + boxed=True, spec="https://drafts.csswg.org/css-text-decor/#propdef-text-emphasis-color")} // CSS Compatibility @@ -1035,14 +1036,14 @@ ${helpers.predefined_type( "-webkit-text-fill-color", "CSSColor", "CSSParserColor::CurrentColor", products="gecko", animatable=True, - complex_color=True, need_clone=True, + complex_color=True, need_clone=True, boxed=True, spec="https://compat.spec.whatwg.org/#the-webkit-text-fill-color")} ${helpers.predefined_type( "-webkit-text-stroke-color", "CSSColor", "CSSParserColor::CurrentColor", products="gecko", animatable=True, - complex_color=True, need_clone=True, + complex_color=True, need_clone=True, boxed=True, spec="https://compat.spec.whatwg.org/#the-webkit-text-stroke-color")} <%helpers:longhand products="gecko" name="-webkit-text-stroke-width" animatable="False" diff --git a/components/style/properties/longhand/list.mako.rs b/components/style/properties/longhand/list.mako.rs index b0cfc32f56c..f4f84f9e884 100644 --- a/components/style/properties/longhand/list.mako.rs +++ b/components/style/properties/longhand/list.mako.rs @@ -29,7 +29,7 @@ ${helpers.single_keyword("list-style-type", """ spec="https://drafts.csswg.org/css-lists/#propdef-list-style-type")} ${helpers.predefined_type("list-style-image", "UrlOrNone", "Either::Second(None_)", - animatable="False", + animatable=False, spec="https://drafts.csswg.org/css-lists/#propdef-list-style-image")} <%helpers:longhand name="quotes" animatable="False" diff --git a/components/style/properties/longhand/outline.mako.rs b/components/style/properties/longhand/outline.mako.rs index 9780ac24110..b47f45d7a68 100644 --- a/components/style/properties/longhand/outline.mako.rs +++ b/components/style/properties/longhand/outline.mako.rs @@ -11,7 +11,7 @@ // TODO(pcwalton): `invert` ${helpers.predefined_type("outline-color", "CSSColor", "::cssparser::Color::CurrentColor", - animatable=True, complex_color=True, need_clone=True, + animatable=True, complex_color=True, need_clone=True, boxed=True, spec="https://drafts.csswg.org/css-ui/#propdef-outline-color")} <%helpers:longhand name="outline-style" need_clone="True" animatable="False" diff --git a/components/style/properties/longhand/position.mako.rs b/components/style/properties/longhand/position.mako.rs index 73b11432aaa..3e55cb2bd3b 100644 --- a/components/style/properties/longhand/position.mako.rs +++ b/components/style/properties/longhand/position.mako.rs @@ -218,5 +218,6 @@ ${helpers.single_keyword("object-fit", "fill contain cover none scale-down", "Default::default()", animatable=False, spec="https://drafts.csswg.org/css-grid/#propdef-%s" % longhand, - products="gecko")} + products="gecko", + boxed=True)} % endfor diff --git a/components/style/properties/longhand/svg.mako.rs b/components/style/properties/longhand/svg.mako.rs index 9e68d5386af..f4ea79eb5ae 100644 --- a/components/style/properties/longhand/svg.mako.rs +++ b/components/style/properties/longhand/svg.mako.rs @@ -25,6 +25,7 @@ ${helpers.predefined_type( "CSSParserColor::RGBA(RGBA { red: 0.0, green: 0.0, blue: 0.0, alpha: 1.0 })", products="gecko", animatable=False, + boxed=True, spec="https://www.w3.org/TR/SVGTiny12/painting.html#StopColorProperty")} ${helpers.predefined_type("stop-opacity", "Opacity", "1.0", @@ -39,6 +40,7 @@ ${helpers.predefined_type( "CSSParserColor::RGBA(RGBA { red: 0.0, green: 0.0, blue: 0.0, alpha: 1.0 })", products="gecko", animatable=False, + boxed=True, spec="https://www.w3.org/TR/SVG/filters.html#FloodColorProperty")} ${helpers.predefined_type("flood-opacity", "Opacity", @@ -50,6 +52,7 @@ ${helpers.predefined_type( "CSSParserColor::RGBA(RGBA { red: 1.0, green: 1.0, blue: 1.0, alpha: 1.0 })", products="gecko", animatable=False, + boxed=True, spec="https://www.w3.org/TR/SVG/filters.html#LightingColorProperty")} // CSS Masking Module Level 1 @@ -58,7 +61,7 @@ ${helpers.single_keyword("mask-type", "luminance alpha", products="gecko", animatable=False, spec="https://drafts.fxtf.org/css-masking/#propdef-mask-type")} -<%helpers:longhand name="clip-path" animatable="False" products="gecko" +<%helpers:longhand name="clip-path" animatable="False" products="gecko" boxed="True" spec="https://drafts.fxtf.org/css-masking/#propdef-clip-path"> use std::fmt; use style_traits::ToCss; diff --git a/components/style/properties/longhand/text.mako.rs b/components/style/properties/longhand/text.mako.rs index 8d1792c046b..2ea249924d5 100644 --- a/components/style/properties/longhand/text.mako.rs +++ b/components/style/properties/longhand/text.mako.rs @@ -12,7 +12,7 @@ Method("has_overline", "bool"), Method("has_line_through", "bool")]) %> -<%helpers:longhand name="text-overflow" animatable="False" +<%helpers:longhand name="text-overflow" animatable="False" boxed="True" spec="https://drafts.csswg.org/css-ui/#propdef-text-overflow"> use std::fmt; use style_traits::ToCss; @@ -214,4 +214,5 @@ ${helpers.predefined_type( complex_color=True, products="gecko", animatable=True, + boxed=True, spec="https://drafts.csswg.org/css-text-decor/#propdef-text-decoration-color")} diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 2db64837374..d8b7586a799 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -246,11 +246,19 @@ mod property_bit_field { /// the resulting declared value. #[allow(non_snake_case)] fn substitute_variables_${property.ident}( - value: &DeclaredValue, + % if property.boxed: + value: &DeclaredValue>, + % else: + value: &DeclaredValue, + % endif custom_properties: &Option>, f: F, error_reporter: &mut StdBox) - where F: FnOnce(&DeclaredValue) + % if property.boxed: + where F: FnOnce(&DeclaredValue>) + % else: + where F: FnOnce(&DeclaredValue) + % endif { if let DeclaredValue::WithVariables { ref css, first_token_type, ref base_url, from_shorthand @@ -283,7 +291,11 @@ mod property_bit_field { f: F, error_reporter: &mut StdBox, extra_data: ParserContextExtraData) - where F: FnOnce(&DeclaredValue) + % if property.boxed: + where F: FnOnce(&DeclaredValue>) + % else: + where F: FnOnce(&DeclaredValue) + % endif { f(& ::custom_properties::substitute(css, first_token_type, custom_properties) @@ -305,7 +317,11 @@ mod property_bit_field { Some(ShorthandId::${shorthand.camel_case}) => { shorthands::${shorthand.ident}::parse_value(&context, input) .map(|result| match result.${property.ident} { - Some(value) => DeclaredValue::Value(value), + % if property.boxed: + Some(value) => DeclaredValue::Value(Box::new(value)), + % else: + Some(value) => DeclaredValue::Value(value), + % endif None => DeclaredValue::Initial, }) } @@ -826,7 +842,11 @@ impl PropertyId { pub enum PropertyDeclaration { % for property in data.longhands: /// ${property.name} - ${property.camel_case}(DeclaredValue), + % if property.boxed: + ${property.camel_case}(DeclaredValue>), + % else: + ${property.camel_case}(DeclaredValue), + % endif % endfor /// A custom property declaration, with the property name and the declared /// value. @@ -2300,3 +2320,18 @@ macro_rules! longhand_properties_idents { } } } + +/// Retuns all longhands SpecifiedValue sizes. This is used in unit tests. +#[cfg(feature = "testing")] +pub fn specified_value_sizes() -> Vec<(&'static str, usize, bool)> { + use std::mem::size_of; + let mut sizes = vec![]; + + % for property in data.longhands: + sizes.push(("${property.name}", + size_of::(), + ${"true" if property.boxed else "false"})); + % endfor + + sizes +} diff --git a/components/style/properties/shorthand/serialize.mako.rs b/components/style/properties/shorthand/serialize.mako.rs index e8b7a76b0cf..a9913165e2c 100644 --- a/components/style/properties/shorthand/serialize.mako.rs +++ b/components/style/properties/shorthand/serialize.mako.rs @@ -58,10 +58,10 @@ pub fn serialize_four_sides(dest: &mut W, Ok(()) } -fn serialize_directional_border(dest: &mut W, +fn serialize_directional_border(dest: &mut W, width: &DeclaredValue, style: &DeclaredValue, - color: &DeclaredValue) + color: &DeclaredValue>) -> fmt::Result where W: fmt::Write, I: ToCss { match *width { DeclaredValue::Value(ref width) => { diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs index 4665b37ad82..a1885f7552e 100644 --- a/components/style/values/mod.rs +++ b/components/style/values/mod.rs @@ -75,7 +75,12 @@ pub trait HasViewportPercentage { fn has_viewport_percentage(&self) -> bool; } - +impl HasViewportPercentage for Box { + #[inline] + fn has_viewport_percentage(&self) -> bool { + (**self).has_viewport_percentage() + } +} use self::computed::ComputedValueAsSpecified; diff --git a/components/style_traits/values.rs b/components/style_traits/values.rs index d59520c626d..ab730508bce 100644 --- a/components/style_traits/values.rs +++ b/components/style_traits/values.rs @@ -39,6 +39,14 @@ impl ToCss for Vec where T: ToCss + OneOrMoreCommaSeparated { } } +impl ToCss for Box { + fn to_css(&self, dest: &mut W) -> fmt::Result + where W: fmt::Write, + { + (**self).to_css(dest) + } +} + impl ToCss for Au { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { write!(dest, "{}px", self.to_f64_px()) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 6ba9b8148de..ce1e1d123ca 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -1007,12 +1007,12 @@ pub extern "C" fn Servo_DeclarationBlock_AddPresValue(declarations: RawServoDecl } LonghandId::Color => { if let Some(color) = css_value.color_value() { - PropertyDeclaration::Color(DeclaredValue::Value( + PropertyDeclaration::Color(DeclaredValue::Value(Box::new( specified::CSSRGBA { parsed: convert_nscolor_to_rgba(color), authored: None } - )) + ))) } else { error!("stylo: got unexpected non-integer value for color presentation attribute"); return diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index 0952fc970db..da2574a575d 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -231,15 +231,15 @@ mod shorthand_serialization { fn border_color_should_serialize_correctly() { let mut properties = Vec::new(); - let red = DeclaredValue::Value(CSSColor { + let red = DeclaredValue::Value(Box::new(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), authored: None - }); + })); - let blue = DeclaredValue::Value(CSSColor { + let blue = DeclaredValue::Value(Box::new(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 0f32, green: 0f32, blue: 1f32, alpha: 1f32 }), authored: None - }); + })); properties.push(PropertyDeclaration::BorderTopColor(blue.clone())); properties.push(PropertyDeclaration::BorderRightColor(red.clone())); @@ -281,10 +281,10 @@ mod shorthand_serialization { let width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let style = DeclaredValue::Value(BorderStyle::solid); - let color = DeclaredValue::Value(CSSColor { + let color = DeclaredValue::Value(Box::new(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), authored: None - }); + })); properties.push(PropertyDeclaration::BorderTopWidth(width)); properties.push(PropertyDeclaration::BorderTopStyle(style)); @@ -300,10 +300,10 @@ mod shorthand_serialization { let width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let style = DeclaredValue::Initial; - let color = DeclaredValue::Value(CSSColor { + let color = DeclaredValue::Value(Box::new(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), authored: None - }); + })); properties.push(PropertyDeclaration::BorderTopWidth(width)); properties.push(PropertyDeclaration::BorderTopStyle(style)); @@ -469,10 +469,10 @@ mod shorthand_serialization { let width = DeclaredValue::Value(WidthContainer(Length::from_px(4f32))); let style = DeclaredValue::Value(Either::Second(BorderStyle::solid)); - let color = DeclaredValue::Value(CSSColor { + let color = DeclaredValue::Value(Box::new(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), authored: None - }); + })); properties.push(PropertyDeclaration::OutlineWidth(width)); properties.push(PropertyDeclaration::OutlineStyle(style)); @@ -504,10 +504,10 @@ mod shorthand_serialization { let width = DeclaredValue::Value(WidthContainer(Length::from_px(4f32))); let style = DeclaredValue::Initial; - let color = DeclaredValue::Value(CSSColor { + let color = DeclaredValue::Value(Box::new(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), authored: None - }); + })); properties.push(PropertyDeclaration::OutlineWidth(width)); properties.push(PropertyDeclaration::OutlineStyle(style)); properties.push(PropertyDeclaration::OutlineColor(color)); @@ -522,10 +522,10 @@ mod shorthand_serialization { let width = DeclaredValue::Value(WidthContainer(Length::from_px(4f32))); let style = DeclaredValue::Value(Either::First(Auto)); - let color = DeclaredValue::Value(CSSColor { + let color = DeclaredValue::Value(Box::new(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), authored: None - }); + })); properties.push(PropertyDeclaration::OutlineWidth(width)); properties.push(PropertyDeclaration::OutlineStyle(style)); properties.push(PropertyDeclaration::OutlineColor(color)); @@ -730,10 +730,10 @@ mod shorthand_serialization { fn background_should_serialize_all_available_properties_when_specified() { let mut properties = Vec::new(); - let color = DeclaredValue::Value(CSSColor { + let color = DeclaredValue::Value(Box::new(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), authored: None - }); + })); let position_x = single_vec_value_typedef!(position_x, HorizontalPosition { @@ -790,10 +790,10 @@ mod shorthand_serialization { fn background_should_combine_origin_and_clip_properties_when_equal() { let mut properties = Vec::new(); - let color = DeclaredValue::Value(CSSColor { + let color = DeclaredValue::Value(Box::new(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), authored: None - }); + })); let position_x = single_vec_value_typedef!(position_x, HorizontalPosition { @@ -849,10 +849,10 @@ mod shorthand_serialization { fn background_should_always_print_color_and_url_and_repeat_and_attachment_and_position() { let mut properties = Vec::new(); - let color = DeclaredValue::Value(CSSColor { + let color = DeclaredValue::Value(Box::new(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), authored: None - }); + })); let position_x = single_vec_value_typedef!(position_x, HorizontalPosition { diff --git a/tests/unit/style/size_of.rs b/tests/unit/style/size_of.rs index 5da4463db30..00005485175 100644 --- a/tests/unit/style/size_of.rs +++ b/tests/unit/style/size_of.rs @@ -3,11 +3,11 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use std::mem::size_of; -use style::properties::PropertyDeclaration; +use style::properties::{PropertyDeclaration, specified_value_sizes}; #[test] fn size_of_property_declaration() { - let old = 240; + let old = 96; let new = size_of::(); if new < old { panic!("Your changes have decreased the stack size of PropertyDeclaration enum from {} to {}. \ @@ -22,3 +22,23 @@ fn size_of_property_declaration() { old, new) } } + +#[test] +fn size_of_specified_values() { + let threshold = 40; + let longhands = specified_value_sizes(); + + for specified_value in longhands { + if specified_value.1 >= threshold && !specified_value.2 { + panic!("Your changes have increased the size of {} SpecifiedValue to {}. The threshold is \ + currently {}. SpecifiedValues are affect size of PropertyDeclaration enum and \ + increasing the size may dramatically affect our memory footprint. Please consider \ + using `boxed=\"True\"` in this longhand.", + specified_value.0, specified_value.1, threshold) + } else if specified_value.1 < threshold && specified_value.2 { + panic!("Your changes have decreased the size of {} SpecifiedValue to {}. Good work! \ + The threshold is currently {}. Please consider removing `boxed=\"True\"` from this longhand.", + specified_value.0, specified_value.1, threshold) + } + } +} diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 40ad06d8e4f..f2ff5d0163b 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -183,12 +183,12 @@ fn test_parse_stylesheet() { block: Arc::new(RwLock::new(PropertyDeclarationBlock { declarations: vec![ (PropertyDeclaration::BackgroundColor(DeclaredValue::Value( - longhands::background_color::SpecifiedValue { + Box::new(longhands::background_color::SpecifiedValue { authored: Some("blue".to_owned()), parsed: cssparser::Color::RGBA(cssparser::RGBA { red: 0., green: 0., blue: 1., alpha: 1. }), - } + }) )), Importance::Normal), (PropertyDeclaration::BackgroundPositionX(DeclaredValue::Value( From 43731c5757f5d91179beab9eae19dd0511b4ae42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 9 Feb 2017 01:05:44 +0300 Subject: [PATCH 4/5] Box custom property --- components/style/custom_properties.rs | 13 +++++++------ components/style/properties/properties.mako.rs | 5 +++-- tests/unit/style/size_of.rs | 2 +- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 8c6c3675c02..fcd319a290c 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -8,7 +8,7 @@ use Atom; use cssparser::{Delimiter, Parser, SourcePosition, Token, TokenSerializationType}; -use parser::{Parse, ParserContext}; +use parser::ParserContext; use properties::DeclaredValue; use std::ascii::AsciiExt; use std::borrow::Cow; @@ -129,16 +129,17 @@ impl ComputedValue { } } -impl Parse for SpecifiedValue { - fn parse(_context: &ParserContext, input: &mut Parser) -> Result { +impl SpecifiedValue { + /// Parse a custom property SpecifiedValue. + pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result, ()> { let mut references = Some(HashSet::new()); let (first, css, last) = try!(parse_self_contained_declaration_value(input, &mut references)); - Ok(SpecifiedValue { + Ok(Box::new(SpecifiedValue { css: css.into_owned(), first_token_type: first, last_token_type: last, references: references.unwrap(), - }) + })) } } @@ -328,7 +329,7 @@ pub fn cascade<'a>(custom_properties: &mut Option>>, seen: &mut HashSet<&'a Name>, name: &'a Name, - specified_value: &'a DeclaredValue) { + specified_value: &'a DeclaredValue>) { let was_already_present = !seen.insert(name); if was_already_present { return; diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index d8b7586a799..6440deab04b 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -850,7 +850,7 @@ pub enum PropertyDeclaration { % endfor /// A custom property declaration, with the property name and the declared /// value. - Custom(::custom_properties::Name, DeclaredValue<::custom_properties::SpecifiedValue>), + Custom(::custom_properties::Name, DeclaredValue>), } impl HasViewportPercentage for PropertyDeclaration { @@ -1029,7 +1029,8 @@ impl PropertyDeclaration { Err(()) => return PropertyDeclarationParseResult::InvalidValue, } }; - result_list.push((PropertyDeclaration::Custom(name, value), Importance::Normal)); + result_list.push((PropertyDeclaration::Custom(name, value), + Importance::Normal)); return PropertyDeclarationParseResult::ValidOrIgnoredDeclaration; } PropertyId::Longhand(id) => match id { diff --git a/tests/unit/style/size_of.rs b/tests/unit/style/size_of.rs index 00005485175..86f43fa7b51 100644 --- a/tests/unit/style/size_of.rs +++ b/tests/unit/style/size_of.rs @@ -7,7 +7,7 @@ use style::properties::{PropertyDeclaration, specified_value_sizes}; #[test] fn size_of_property_declaration() { - let old = 96; + let old = 72; let new = size_of::(); if new < old { panic!("Your changes have decreased the stack size of PropertyDeclaration enum from {} to {}. \ From adb6d202937457bb6ff6d24694f6c903d77370f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 9 Feb 2017 03:23:50 +0300 Subject: [PATCH 5/5] Box DeclaredValue::WithVariables --- components/style/custom_properties.rs | 2 +- components/style/properties/helpers.mako.rs | 17 ++--- .../helpers/animated_properties.mako.rs | 2 +- .../style/properties/properties.mako.rs | 68 +++++++++++-------- .../style/properties/shorthand/box.mako.rs | 6 +- tests/unit/style/size_of.rs | 2 +- 6 files changed, 53 insertions(+), 44 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index fcd319a290c..9edeb7a04e3 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -361,7 +361,7 @@ pub fn cascade<'a>(custom_properties: &mut Option unreachable!(), + DeclaredValue::WithVariables(_) => unreachable!(), DeclaredValue::Initial => { map.remove(&name); } diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index fa6ccbd862b..30c6873a8a2 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -196,7 +196,7 @@ % if not property.derived_from: use cssparser::Parser; use parser::{Parse, ParserContext, ParserContextExtraData}; - use properties::{CSSWideKeyword, DeclaredValue, ShorthandId}; + use properties::{CSSWideKeyword, DeclaredValue, UnparsedValue, ShorthandId}; % endif use values::{Auto, Either, None_, Normal}; use cascade_info::CascadeInfo; @@ -259,7 +259,7 @@ .set_${property.ident}(computed ${maybe_wm}); % endif } - DeclaredValue::WithVariables { .. } => unreachable!(), + DeclaredValue::WithVariables(_) => unreachable!(), % if not data.current_style_struct.inherited: DeclaredValue::Unset | % endif @@ -324,12 +324,12 @@ input.reset(start); let (first_token_type, css) = try!( ::custom_properties::parse_non_custom_with_var(input)); - return Ok(DeclaredValue::WithVariables { + return Ok(DeclaredValue::WithVariables(Box::new(UnparsedValue { css: css.into_owned(), first_token_type: first_token_type, base_url: context.base_url.clone(), from_shorthand: None, - }) + }))) } specified } @@ -421,7 +421,8 @@ #[allow(unused_imports)] use cssparser::Parser; use parser::ParserContext; - use properties::{longhands, PropertyDeclaration, DeclaredValue, ShorthandId}; + use properties::{DeclaredValue, PropertyDeclaration, UnparsedValue}; + use properties::{ShorthandId, longhands}; use properties::declaration_block::Importance; use std::fmt; use style_traits::ToCss; @@ -504,7 +505,7 @@ DeclaredValue::Initial => all_flags &= ALL_INITIAL, DeclaredValue::Inherit => all_flags &= ALL_INHERIT, DeclaredValue::Unset => all_flags &= ALL_UNSET, - DeclaredValue::WithVariables {..} => with_variables = true, + DeclaredValue::WithVariables(_) => with_variables = true, DeclaredValue::Value(..) => { all_flags = SerializeFlags::empty(); } @@ -560,12 +561,12 @@ ::custom_properties::parse_non_custom_with_var(input)); % for sub_property in shorthand.sub_properties: declarations.push((PropertyDeclaration::${sub_property.camel_case}( - DeclaredValue::WithVariables { + DeclaredValue::WithVariables(Box::new(UnparsedValue { css: css.clone().into_owned(), first_token_type: first_token_type, base_url: context.base_url.clone(), from_shorthand: Some(ShorthandId::${shorthand.camel_case}), - } + })) ), Importance::Normal)); % endfor Ok(()) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index af0079a7f33..35fb529c851 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -297,7 +297,7 @@ impl AnimationValue { PropertyDeclaration::${prop.camel_case}(ref val) => { let computed = match *val { // https://bugzilla.mozilla.org/show_bug.cgi?id=1326131 - DeclaredValue::WithVariables{..} => unimplemented!(), + DeclaredValue::WithVariables(_) => unimplemented!(), DeclaredValue::Value(ref val) => val.to_computed_value(context), % if not prop.style_struct.inherited: DeclaredValue::Unset | diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 6440deab04b..3f014d3368f 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -260,17 +260,15 @@ mod property_bit_field { where F: FnOnce(&DeclaredValue) % endif { - if let DeclaredValue::WithVariables { - ref css, first_token_type, ref base_url, from_shorthand - } = *value { + if let DeclaredValue::WithVariables(ref with_variables) = *value { // FIXME(heycam): A ParserContextExtraData should be built from data // stored in the WithVariables, in case variable expansion results in // a url() value. let extra_data = ParserContextExtraData::default(); - substitute_variables_${property.ident}_slow(css, - first_token_type, - base_url, - from_shorthand, + substitute_variables_${property.ident}_slow(&with_variables.css, + with_variables.first_token_type, + &with_variables.base_url, + with_variables.from_shorthand, custom_properties, f, error_reporter, @@ -614,17 +612,8 @@ impl ShorthandId { pub enum DeclaredValue { /// A known specified value from the stylesheet. Value(T), - /// A value that contained any css variables. - WithVariables { - /// The css serialization for this value. - css: String, - /// The first token type for this serialization. - first_token_type: TokenSerializationType, - /// The base url. - base_url: ServoUrl, - /// The shorthand this came from. - from_shorthand: Option, - }, + /// An unparsed value that contains `var()` functions. + WithVariables(Box), /// The `initial` keyword. Initial, /// The `inherit` keyword. @@ -633,11 +622,25 @@ pub enum DeclaredValue { Unset, } +/// An unparsed property value that contains `var()` functions. +#[derive(Clone, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub struct UnparsedValue { + /// The css serialization for this value. + css: String, + /// The first token type for this serialization. + first_token_type: TokenSerializationType, + /// The base url. + base_url: ServoUrl, + /// The shorthand this came from. + from_shorthand: Option, +} + impl HasViewportPercentage for DeclaredValue { fn has_viewport_percentage(&self) -> bool { match *self { DeclaredValue::Value(ref v) => v.has_viewport_percentage(), - DeclaredValue::WithVariables { .. } => { + DeclaredValue::WithVariables(_) => { panic!("DeclaredValue::has_viewport_percentage without \ resolving variables!") }, @@ -654,11 +657,13 @@ impl ToCss for DeclaredValue { { match *self { DeclaredValue::Value(ref inner) => inner.to_css(dest), - DeclaredValue::WithVariables { ref css, from_shorthand: None, .. } => { - dest.write_str(css) - } - // https://drafts.csswg.org/css-variables/#variables-in-shorthands - DeclaredValue::WithVariables { .. } => Ok(()), + DeclaredValue::WithVariables(ref with_variables) => { + // https://drafts.csswg.org/css-variables/#variables-in-shorthands + if with_variables.from_shorthand.is_none() { + dest.write_str(&*with_variables.css)? + } + Ok(()) + }, DeclaredValue::Initial => dest.write_str("initial"), DeclaredValue::Inherit => dest.write_str("inherit"), DeclaredValue::Unset => dest.write_str("unset"), @@ -959,9 +964,12 @@ impl PropertyDeclaration { match *self { % for property in data.longhands: PropertyDeclaration::${property.camel_case}(ref value) => match *value { - DeclaredValue::WithVariables { ref css, from_shorthand: Some(s), .. } - if s == shorthand => { - Some(&**css) + DeclaredValue::WithVariables(ref with_variables) => { + if let Some(s) = with_variables.from_shorthand { + if s == shorthand { + Some(&*with_variables.css) + } else { None } + } else { None } } _ => None }, @@ -976,12 +984,12 @@ impl PropertyDeclaration { match *self { % for property in data.longhands: PropertyDeclaration::${property.camel_case}(ref value) => match *value { - DeclaredValue::WithVariables { .. } => true, + DeclaredValue::WithVariables(_) => true, _ => false, }, % endfor PropertyDeclaration::Custom(_, ref value) => match *value { - DeclaredValue::WithVariables { .. } => true, + DeclaredValue::WithVariables(_) => true, _ => false, } } @@ -997,7 +1005,7 @@ impl PropertyDeclaration { match *self { % for property in data.longhands: PropertyDeclaration::${property.camel_case}(ref value) => { - matches!(*value, DeclaredValue::WithVariables { .. }) + matches!(*value, DeclaredValue::WithVariables(_)) }, % endfor PropertyDeclaration::Custom(..) => true diff --git a/components/style/properties/shorthand/box.mako.rs b/components/style/properties/shorthand/box.mako.rs index 207aefc6c5c..f28e3b0371c 100644 --- a/components/style/properties/shorthand/box.mako.rs +++ b/components/style/properties/shorthand/box.mako.rs @@ -22,7 +22,7 @@ (&DeclaredValue::Value(ref x_value), &DeclaredValue::Value(ref y_container)) => { *x_value == y_container.0 }, - (&DeclaredValue::WithVariables { .. }, &DeclaredValue::WithVariables { .. }) => true, + (&DeclaredValue::WithVariables(_), &DeclaredValue::WithVariables(_)) => true, (&DeclaredValue::Initial, &DeclaredValue::Initial) => true, (&DeclaredValue::Inherit, &DeclaredValue::Inherit) => true, (&DeclaredValue::Unset, &DeclaredValue::Unset) => true, @@ -43,8 +43,8 @@ (&DeclaredValue::Value(ref x_value), &DeclaredValue::Value(ref y_container)) => { *x_value == y_container.0 }, - (_, &DeclaredValue::WithVariables { .. }) | - (&DeclaredValue::WithVariables { .. }, _) => { + (_, &DeclaredValue::WithVariables(_)) | + (&DeclaredValue::WithVariables(_), _) => { // We don't serialize shorthands with variables return dest.write_str(""); }, diff --git a/tests/unit/style/size_of.rs b/tests/unit/style/size_of.rs index 86f43fa7b51..fb001763be4 100644 --- a/tests/unit/style/size_of.rs +++ b/tests/unit/style/size_of.rs @@ -7,7 +7,7 @@ use style::properties::{PropertyDeclaration, specified_value_sizes}; #[test] fn size_of_property_declaration() { - let old = 72; + let old = 48; let new = size_of::(); if new < old { panic!("Your changes have decreased the stack size of PropertyDeclaration enum from {} to {}. \