From ade76f10b7add2a5d4dc12f38238b6b06547e4b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Fri, 14 Jul 2017 15:41:26 -0700 Subject: [PATCH] Convert TrackList's 'values' field to store only TrackSize. It was storing both TrackSize and TrackRepeat before and TrackRepeat have to be converted into sequence of TrackSize during serialization. Instead of doing this in serialization process(which is hard and hacky), we converted to do this in parsing process. We were doing this conversion in the ComputedValue conversion. So we only had to move this process to parsing. --- .../properties/shorthand/position.mako.rs | 2 +- components/style/values/computed/mod.rs | 4 +- components/style/values/generics/grid.rs | 51 +++--- components/style/values/specified/grid.rs | 160 +++++++----------- components/style/values/specified/mod.rs | 5 +- 5 files changed, 92 insertions(+), 130 deletions(-) diff --git a/components/style/properties/shorthand/position.mako.rs b/components/style/properties/shorthand/position.mako.rs index 0e630b555ad..af4f218ee44 100644 --- a/components/style/properties/shorthand/position.mako.rs +++ b/components/style/properties/shorthand/position.mako.rs @@ -289,7 +289,7 @@ line_names.push(names); strings.push(string); let size = input.try(|i| TrackSize::parse(context, i)).unwrap_or_default(); - values.push(Either::First(size)); + values.push(size); names = input.try(parse_line_names).unwrap_or(vec![]); if let Ok(v) = input.try(parse_line_names) { names.extend(v); diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index db63c87cbb1..e7810c82e25 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -549,10 +549,10 @@ pub type TrackSize = GenericTrackSize; /// The computed value of a grid `` /// (could also be `` or ``) -pub type TrackList = GenericTrackList; +pub type TrackList = GenericTrackList; /// ` | ` -pub type GridTemplateComponent = GenericGridTemplateComponent; +pub type GridTemplateComponent = GenericGridTemplateComponent; impl ClipRectOrAuto { /// Return an auto (default for clip-rect and image-region) value diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index b627231ecd5..2b20269ecae 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -10,7 +10,7 @@ use parser::{Parse, ParserContext}; use std::{fmt, mem, usize}; use style_traits::{ToCss, ParseError, StyleParseError}; use values::{CSSFloat, CustomIdent}; -use values::computed::{self, ComputedValueAsSpecified, Context, ToComputedValue}; +use values::computed::{ComputedValueAsSpecified, Context, ToComputedValue}; use values::specified::Integer; use values::specified::grid::parse_line_names; @@ -411,13 +411,9 @@ impl ToCss for TrackRepeat { Ok(()) } } - -impl ToComputedValue for TrackRepeat { - type ComputedValue = TrackRepeat; - - #[inline] - fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { - // If the repeat count is numeric, then expand the values and merge accordingly. +impl TrackRepeat { + /// If the repeat count is numeric, then expand the values and merge accordingly. + pub fn expand(&self) -> Self { if let RepeatCount::Number(num) = self.count { let mut line_names = vec![]; let mut track_sizes = vec![]; @@ -428,7 +424,7 @@ impl ToComputedValue for TrackRepeat { for (size, names) in self.track_sizes.iter().zip(&mut names_iter) { prev_names.extend_from_slice(&names); line_names.push(mem::replace(&mut prev_names, vec![])); - track_sizes.push(size.to_computed_value(context)); + track_sizes.push(size.clone()); } if let Some(names) = names_iter.next() { @@ -446,13 +442,25 @@ impl ToComputedValue for TrackRepeat { } else { // if it's auto-fit/auto-fill, then it's left to the layout. TrackRepeat { count: self.count, - track_sizes: self.track_sizes.iter() - .map(|l| l.to_computed_value(context)) - .collect(), + track_sizes: self.track_sizes.clone(), line_names: self.line_names.clone(), } } } +} +impl ToComputedValue for TrackRepeat { + type ComputedValue = TrackRepeat; + + #[inline] + fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { + TrackRepeat { + count: self.count, + track_sizes: self.track_sizes.iter() + .map(|val| val.to_computed_value(context)) + .collect(), + line_names: self.line_names.clone(), + } + } #[inline] fn from_computed_value(computed: &Self::ComputedValue) -> Self { @@ -489,6 +497,8 @@ pub enum TrackListType { Explicit, } +impl ComputedValueAsSpecified for TrackListType {} + /// A grid `` type. /// /// https://drafts.csswg.org/css-grid/#typedef-track-list @@ -500,22 +510,16 @@ pub struct TrackList { /// In order to avoid parsing the same value multiple times, this does a single traversal /// and arrives at the type of value it has parsed (or bails out gracefully with an error). pub list_type: TrackListType, - /// A vector of ` | ` values. In its specified form, it may contain - /// any value, but once it's computed, it contains only `` values. - /// - /// Note that this may also contain `` at an index. If it exists, it's - /// given by the index in `TrackListType::Auto` - pub values: Vec, + /// A vector of `` values. + pub values: Vec>, /// `` accompanying ` | ` values. /// /// If there's no ``, then it's represented by an empty vector. /// For N values, there will be N+1 ``, and so this vector's /// length is always one value more than that of the ``. pub line_names: Vec>, - /// `` value after computation. This field is necessary, because - /// the `values` field (after computation) will only contain `` values, and - /// we need something to represent this function. - pub auto_repeat: Option>, + /// `` value. There can only be one `` in a TrackList. + pub auto_repeat: Option>, } impl ToCss for TrackList { @@ -552,7 +556,8 @@ impl ToCss for TrackList { }, } - if values_iter.peek().is_some() || line_names_iter.peek().map_or(false, |v| !v.is_empty()) { + if values_iter.peek().is_some() || line_names_iter.peek().map_or(false, |v| !v.is_empty()) || + (idx + 1 == auto_idx) { dest.write_str(" ")?; } } diff --git a/components/style/values/specified/grid.rs b/components/style/values/specified/grid.rs index be0ced6cd56..0f1769ee1d1 100644 --- a/components/style/values/specified/grid.rs +++ b/components/style/values/specified/grid.rs @@ -7,10 +7,10 @@ use cssparser::{Parser, Token, BasicParseError}; use parser::{Parse, ParserContext}; -use std::{mem, usize}; use std::ascii::AsciiExt; +use std::mem; use style_traits::{HasViewportPercentage, ParseError, StyleParseError}; -use values::{CSSFloat, CustomIdent, Either}; +use values::{CSSFloat, CustomIdent}; use values::computed::{self, Context, ToComputedValue}; use values::generics::grid::{GridTemplateComponent, RepeatCount, TrackBreadth, TrackKeyword, TrackRepeat}; use values::generics::grid::{LineNameList, TrackSize, TrackList, TrackListType}; @@ -183,37 +183,40 @@ impl HasViewportPercentage for TrackRepeat { } } -/// Either a `` or `` component of `` -/// -/// This is required only for the specified form of ``, and will become -/// `TrackSize` in its computed form. -pub type TrackSizeOrRepeat = Either, TrackRepeat>; - -impl Parse for TrackList { +impl Parse for TrackList { fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result> { - let mut current_names; + // Merge the line names while parsing values. The resulting values will + // all be bunch of `` and one . + // + // For example, + // `[a b] 100px [c d] repeat(1, 30px [g]) [h]` will be merged as `[a b] 100px [c d] 30px [g h]` + // whereas, `[a b] repeat(2, [c] 50px [d]) [e f] repeat(auto-fill, [g] 12px) 10px [h]` will be merged as + // `[a b c] 50px [d c] 50px [d e f] repeat(auto-fill, [g] 12px) 10px [h]`, with the `` value + // set in the `auto_repeat` field, and the `idx` in TrackListType::Auto pointing to the values after + // `` (in this case, `10px [h]`). + let mut current_names = vec![]; let mut names = vec![]; let mut values = vec![]; let mut list_type = TrackListType::Explicit; // assume it's the simplest case - // marker to check whether we've already encountered along the way - let mut is_auto = false; + // holds value. It can only be only one in a TrackList. + let mut auto_repeat = None; // assume that everything is . This flag is useful when we encounter let mut atleast_one_not_fixed = false; loop { - current_names = input.try(parse_line_names).unwrap_or(vec![]); + current_names.append(&mut input.try(parse_line_names).unwrap_or(vec![])); if let Ok(track_size) = input.try(|i| TrackSize::parse(context, i)) { if !track_size.is_fixed() { atleast_one_not_fixed = true; - if is_auto { + if auto_repeat.is_some() { // only accepts and return Err(StyleParseError::UnspecifiedError.into()) } } - names.push(current_names); - values.push(Either::First(track_size)); + names.push(mem::replace(&mut current_names, vec![])); + values.push(track_size); } else if let Ok((repeat, type_)) = input.try(|i| TrackRepeat::parse_with_repeat_type(context, i)) { if list_type == TrackListType::Explicit { list_type = TrackListType::Normal; // doesn't contain repeat() @@ -222,26 +225,38 @@ impl Parse for TrackList { match type_ { RepeatType::Normal => { atleast_one_not_fixed = true; - if is_auto { // only + if auto_repeat.is_some() { // only return Err(StyleParseError::UnspecifiedError.into()) } }, RepeatType::Auto => { - if is_auto || atleast_one_not_fixed { + if auto_repeat.is_some() || atleast_one_not_fixed { // We've either seen earlier, or there's at least one non-fixed value return Err(StyleParseError::UnspecifiedError.into()) } - is_auto = true; list_type = TrackListType::Auto(values.len() as u16); + auto_repeat = Some(repeat); + names.push(mem::replace(&mut current_names, vec![])); + continue }, RepeatType::Fixed => (), } - names.push(current_names); - values.push(Either::Second(repeat)); + // If the repeat count is numeric, we axpand and merge the values. + let mut repeat = repeat.expand(); + let mut repeat_names_iter = repeat.line_names.drain(..); + for (size, repeat_names) in repeat.track_sizes.drain(..).zip(&mut repeat_names_iter) { + current_names.extend_from_slice(&repeat_names); + names.push(mem::replace(&mut current_names, vec![])); + values.push(size); + } + + if let Some(names) = repeat_names_iter.next() { + current_names.extend_from_slice(&names); + } } else { - if values.is_empty() { + if values.is_empty() && auto_repeat.is_none() { return Err(StyleParseError::UnspecifiedError.into()) } @@ -254,113 +269,56 @@ impl Parse for TrackList { list_type: list_type, values: values, line_names: names, - auto_repeat: None, // filled only in computation + auto_repeat: auto_repeat, }) } } -impl HasViewportPercentage for TrackList { +impl HasViewportPercentage for TrackList { #[inline] fn has_viewport_percentage(&self) -> bool { self.values.iter().any(|ref v| v.has_viewport_percentage()) } } -impl ToComputedValue for TrackList { - type ComputedValue = TrackList>; + +impl ToComputedValue for TrackList { + type ComputedValue = TrackList; #[inline] fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { - // Merge the line names while computing values. The resulting values will - // all be a bunch of ``. - // - // For example, - // `[a b] 100px [c d] repeat(1, 30px [g]) [h]` will be merged as `[a b] 100px [c d] 30px [g h]` - // whereas, `[a b] repeat(2, [c] 50px [d]) [e f] repeat(auto-fill, [g] 12px) 10px [h]` will be merged as - // `[a b c] 50px [d c] 50px [d e f] repeat(auto-fill, [g] 12px) 10px [h]`, with the `` value - // set in the `auto_repeat` field, and the `idx` in TrackListType::Auto pointing to the values after - // `` (in this case, `10px [h]`). - let mut line_names = vec![]; - let mut list_type = self.list_type; - let mut values = vec![]; - let mut prev_names = vec![]; - let mut auto_repeat = None; - - let mut names_iter = self.line_names.iter(); - for (size_or_repeat, names) in self.values.iter().zip(&mut names_iter) { - prev_names.extend_from_slice(names); - - match *size_or_repeat { - Either::First(ref size) => values.push(size.to_computed_value(context)), - Either::Second(ref repeat) => { - let mut computed = repeat.to_computed_value(context); - if computed.count == RepeatCount::AutoFit || computed.count == RepeatCount::AutoFill { - line_names.push(mem::replace(&mut prev_names, vec![])); // don't merge for auto - list_type = TrackListType::Auto(values.len() as u16); - auto_repeat = Some(computed); - continue - } - - let mut repeat_names_iter = computed.line_names.drain(..); - for (size, mut names) in computed.track_sizes.drain(..).zip(&mut repeat_names_iter) { - prev_names.append(&mut names); - line_names.push(mem::replace(&mut prev_names, vec![])); - values.push(size); - } - - if let Some(mut names) = repeat_names_iter.next() { - prev_names.append(&mut names); - } - - continue // last `` in repeat() may merge with the next set - } - } - - line_names.push(mem::replace(&mut prev_names, vec![])); + let mut values = Vec::with_capacity(self.values.len() + 1); + for value in self.values.iter().map(|val| val.to_computed_value(context)) { + values.push(value); } - if let Some(names) = names_iter.next() { - prev_names.extend_from_slice(names); - } - - line_names.push(mem::replace(&mut prev_names, vec![])); - TrackList { - list_type: list_type, + list_type: self.list_type.to_computed_value(context), values: values, - line_names: line_names, - auto_repeat: auto_repeat, + line_names: self.line_names.clone(), + auto_repeat: self.auto_repeat.clone().map(|repeat| repeat.to_computed_value(context)), } } #[inline] fn from_computed_value(computed: &Self::ComputedValue) -> Self { - let auto_idx = if let TrackListType::Auto(idx) = computed.list_type { - idx as usize - } else { - usize::MAX - }; - let mut values = Vec::with_capacity(computed.values.len() + 1); - for (i, value) in computed.values.iter().map(ToComputedValue::from_computed_value).enumerate() { - if i == auto_idx { - let value = TrackRepeat::from_computed_value(computed.auto_repeat.as_ref().unwrap()); - values.push(Either::Second(value)); - } - - values.push(Either::First(value)); + for value in computed.values.iter().map(ToComputedValue::from_computed_value) { + values.push(value); } TrackList { list_type: computed.list_type, values: values, line_names: computed.line_names.clone(), - auto_repeat: None, + auto_repeat: computed.auto_repeat.clone().map(|ref repeat| TrackRepeat::from_computed_value(repeat)), } } } -impl Parse for GridTemplateComponent { // FIXME: Derive Parse (probably with None_) + +impl Parse for GridTemplateComponent { + // FIXME: Derive Parse (probably with None_) fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result> { if input.try(|i| i.expect_ident_matching("none")).is_ok() { return Ok(GridTemplateComponent::None) @@ -370,8 +328,8 @@ impl Parse for GridTemplateComponent { // FIXME: Derive Par } } -impl GridTemplateComponent { - /// Parses a `GridTemplateComponent` except `none` keyword. +impl GridTemplateComponent { + /// Parses a `GridTemplateComponent` except `none` keyword. pub fn parse_without_none<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result> { if let Ok(t) = input.try(|i| TrackList::parse(context, i)) { @@ -382,7 +340,7 @@ impl GridTemplateComponent { } } -impl HasViewportPercentage for GridTemplateComponent { +impl HasViewportPercentage for GridTemplateComponent { #[inline] fn has_viewport_percentage(&self) -> bool { match *self { @@ -392,8 +350,8 @@ impl HasViewportPercentage for GridTemplateComponent { } } -impl ToComputedValue for GridTemplateComponent { - type ComputedValue = GridTemplateComponent>; +impl ToComputedValue for GridTemplateComponent { + type ComputedValue = GridTemplateComponent; #[inline] fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index a5e20879af9..9bde152a8ce 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -10,7 +10,6 @@ use Namespace; use context::QuirksMode; use cssparser::{Parser, Token, serialize_identifier, BasicParseError}; use parser::{ParserContext, Parse}; -use self::grid::TrackSizeOrRepeat; use self::url::SpecifiedUrl; use std::ascii::AsciiExt; use std::borrow::Cow; @@ -685,10 +684,10 @@ pub type TrackSize = GenericTrackSize; /// The specified value of a grid `` /// (could also be `` or ``) -pub type TrackList = GenericTrackList; +pub type TrackList = GenericTrackList; /// ` | ` -pub type GridTemplateComponent = GenericGridTemplateComponent; +pub type GridTemplateComponent = GenericGridTemplateComponent; no_viewport_percentage!(SVGPaint);