From 333afb61f6edbc86966aa98e09bed36f00ca653c Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Wed, 12 Oct 2016 15:44:44 +0800 Subject: [PATCH 1/3] Use UrlExtraData::make_from to avoid manually cloning the ParserContextExtraData contents. MozReview-Commit-ID: FoCmI6swp5Q --- components/style/properties/longhand/box.mako.rs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index 378cd88bc45..45e64f88f7c 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -968,17 +968,8 @@ ${helpers.single_keyword("-moz-appearance", } let url = context.parse_url(&*try!(input.expect_url())); - match context.extra_data { - ParserContextExtraData { - base: Some(ref base), - referrer: Some(ref referrer), - principal: Some(ref principal), - } => { - let extra_data = UrlExtraData { - base: base.clone(), - referrer: referrer.clone(), - principal: principal.clone(), - }; + match UrlExtraData::make_from(context) { + Some(extra_data) => { Ok(SpecifiedValue::Url(url, extra_data)) }, _ => { From 75e97c02dcce99f2d4776ce0bb0ac78781b7a8a2 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Wed, 12 Oct 2016 16:25:16 +0800 Subject: [PATCH 2/3] Factor out a UrlOrNone value type and make -moz-binding use it. MozReview-Commit-ID: L57QEf40e2m --- components/style/properties/gecko.mako.rs | 2 +- components/style/properties/helpers.mako.rs | 11 +++- .../style/properties/longhand/box.mako.rs | 66 ++----------------- components/style/values/computed/mod.rs | 2 +- components/style/values/specified/mod.rs | 47 +++++++++++++ 5 files changed, 62 insertions(+), 66 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index a44956b9f04..bbd94e1ca29 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -893,7 +893,7 @@ fn static_assert() { #[allow(non_snake_case)] pub fn set__moz_binding(&mut self, v: longhands::_moz_binding::computed_value::T) { - use properties::longhands::_moz_binding::SpecifiedValue as BindingValue; + use properties::longhands::_moz_binding::computed_value::T as BindingValue; match v { BindingValue::None => debug_assert!(self.gecko.mBinding.mRawPtr.is_null()), BindingValue::Url(ref url, ref extra_data) => { diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 4dea502a5d5..b34175bcebe 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -16,19 +16,24 @@ -<%def name="predefined_type(name, type, initial_value, parse_method='parse', **kwargs)"> +<%def name="predefined_type(name, type, initial_value, parse_method='parse', needs_context=False, **kwargs)"> <%call expr="longhand(name, predefined_type=type, **kwargs)"> #[allow(unused_imports)] use app_units::Au; use cssparser::{Color as CSSParserColor, RGBA}; - pub type SpecifiedValue = specified::${type}; + pub use values::specified::${type} as SpecifiedValue; pub mod computed_value { pub use values::computed::${type} as T; } #[inline] pub fn get_initial_value() -> computed_value::T { ${initial_value} } - #[inline] pub fn parse(_context: &ParserContext, input: &mut Parser) + #[allow(unused_variables)] + #[inline] pub fn parse(context: &ParserContext, input: &mut Parser) -> Result { + % if needs_context: + specified::${type}::${parse_method}(context, input) + % else: specified::${type}::${parse_method}(input) + % endif } diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index 45e64f88f7c..067c37e6251 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -920,64 +920,8 @@ ${helpers.single_keyword("-moz-appearance", animatable=False)} // Non-standard: https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-binding -<%helpers:longhand name="-moz-binding" products="gecko" animatable="False" disable_when_testing="True"> - use cssparser::{CssStringWriter, ToCss}; - use gecko_bindings::sugar::refptr::{GeckoArcPrincipal, GeckoArcURI}; - use std::fmt::{self, Write}; - use url::Url; - use values::specified::UrlExtraData; - use values::computed::ComputedValueAsSpecified; - use values::NoViewportPercentage; - - #[derive(PartialEq, Clone, Debug)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub enum SpecifiedValue { - Url(Url, UrlExtraData), - None, - } - - impl ComputedValueAsSpecified for SpecifiedValue {} - impl NoViewportPercentage for SpecifiedValue {} - - impl ToCss for SpecifiedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - use values::LocalToCss; - match *self { - SpecifiedValue::Url(ref url, _) => { - url.to_css(dest) - } - SpecifiedValue::None => { - try!(dest.write_str("none")); - Ok(()) - } - } - } - } - - pub mod computed_value { - pub type T = super::SpecifiedValue; - } - - #[inline] pub fn get_initial_value() -> SpecifiedValue { - SpecifiedValue::None - } - - pub fn parse(context: &ParserContext, input: &mut Parser) -> Result { - if input.try(|input| input.expect_ident_matching("none")).is_ok() { - return Ok(SpecifiedValue::None); - } - - let url = context.parse_url(&*try!(input.expect_url())); - match UrlExtraData::make_from(context) { - Some(extra_data) => { - Ok(SpecifiedValue::Url(url, extra_data)) - }, - _ => { - // FIXME(heycam) should ensure we always have a principal, etc., when parsing - // style attributes and re-parsing due to CSS Variables. - println!("stylo: skipping -moz-binding declaration without ParserContextExtraData"); - Err(()) - }, - } - } - +${helpers.predefined_type("-moz-binding", "UrlOrNone", "computed_value::T::None", + needs_context=True, + products="gecko", + animatable="False", + disable_when_testing="True")} diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 90a1462d834..42abdf5b933 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -13,7 +13,7 @@ use super::LocalToCss; pub use cssparser::Color as CSSColor; pub use self::image::{EndingShape as GradientShape, Gradient, GradientKind, Image}; pub use self::image::{LengthOrKeyword, LengthOrPercentageOrKeyword}; -pub use super::specified::{Angle, BorderStyle, Time, UrlExtraData}; +pub use super::specified::{Angle, BorderStyle, Time, UrlExtraData, UrlOrNone}; pub mod basic_shape; pub mod image; diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index f8a456d2076..436dc65d7d9 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -18,6 +18,7 @@ use std::ops::Mul; use style_traits::values::specified::AllowedNumericType; use super::{CSSFloat, FONT_MEDIUM_PX, HasViewportPercentage, LocalToCss, NoViewportPercentage}; use super::computed::{self, ComputedValueAsSpecified, Context, ToComputedValue}; +use url::Url; pub use self::image::{AngleOrCorner, ColorStop, EndingShape as GradientEndingShape, Gradient}; pub use self::image::{GradientKind, HorizontalDirection, Image, LengthOrKeyword, LengthOrPercentageOrKeyword}; @@ -1452,3 +1453,49 @@ impl ToCss for Opacity { self.0.to_css(dest) } } + +#[derive(PartialEq, Clone, Debug)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub enum UrlOrNone { + Url(Url, UrlExtraData), + None, +} + +impl ComputedValueAsSpecified for UrlOrNone {} +impl NoViewportPercentage for UrlOrNone {} + +impl ToCss for UrlOrNone { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + use values::LocalToCss; + match *self { + UrlOrNone::Url(ref url, _) => { + url.to_css(dest) + } + UrlOrNone::None => { + try!(dest.write_str("none")); + Ok(()) + } + } + } +} + +impl UrlOrNone { + pub fn parse(context: &ParserContext, input: &mut Parser) -> Result { + if input.try(|input| input.expect_ident_matching("none")).is_ok() { + return Ok(UrlOrNone::None); + } + + let url = context.parse_url(&*try!(input.expect_url())); + match UrlExtraData::make_from(context) { + Some(extra_data) => { + Ok(UrlOrNone::Url(url, extra_data)) + }, + _ => { + // FIXME(heycam) should ensure we always have a principal, etc., when parsing + // style attributes and re-parsing due to CSS Variables. + println!("stylo: skipping UrlOrNone declaration without ParserContextExtraData"); + Err(()) + }, + } + } +} From 47b4648e62cb8bf4935b631d67540d1750820af6 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Wed, 12 Oct 2016 16:39:50 +0800 Subject: [PATCH 3/3] Make list-style-image use UrlOrNone. MozReview-Commit-ID: 6E82Tq5nrfC --- components/layout/construct.rs | 8 +- .../style/properties/longhand/list.mako.rs | 80 +------------------ 2 files changed, 7 insertions(+), 81 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index 5cc982fcc3f..f9b2d61ab91 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -43,7 +43,7 @@ use std::marker::PhantomData; use std::mem; use std::sync::Arc; use std::sync::atomic::Ordering; -use style::computed_values::{caption_side, display, empty_cells, float, list_style_position}; +use style::computed_values::{caption_side, display, empty_cells, float, list_style_image, list_style_position}; use style::computed_values::content::ContentItem; use style::computed_values::position; use style::context::SharedStyleContext; @@ -1264,14 +1264,14 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> fn build_flow_for_list_item(&mut self, node: &ConcreteThreadSafeLayoutNode, flotation: float::T) -> ConstructionResult { let flotation = FloatKind::from_property(flotation); - let marker_fragments = match node.style(self.style_context()).get_list().list_style_image.0 { - Some(ref url) => { + let marker_fragments = match node.style(self.style_context()).get_list().list_style_image { + list_style_image::T::Url(ref url, ref _extra_data) => { let image_info = box ImageFragmentInfo::new(node, Some((*url).clone()), &self.layout_context.shared); vec![Fragment::new(node, SpecificFragmentInfo::Image(image_info), self.layout_context)] } - None => { + list_style_image::T::None => { match ListStyleTypeContent::from_list_style_type(node.style(self.style_context()) .get_list() .list_style_type) { diff --git a/components/style/properties/longhand/list.mako.rs b/components/style/properties/longhand/list.mako.rs index 77e0af5797a..b8afa7e9249 100644 --- a/components/style/properties/longhand/list.mako.rs +++ b/components/style/properties/longhand/list.mako.rs @@ -26,83 +26,9 @@ ${helpers.single_keyword("list-style-type", """ gecko_constant_prefix="NS_STYLE_LIST_STYLE", animatable=False)} -<%helpers:longhand name="list-style-image" animatable="False"> - use cssparser::{ToCss, Token}; - use std::fmt; - use url::Url; - use values::LocalToCss; - use values::NoViewportPercentage; - - impl NoViewportPercentage for SpecifiedValue {} - - #[derive(Debug, Clone, PartialEq, Eq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub enum SpecifiedValue { - None, - Url(Url), - } - - impl ToCss for SpecifiedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - match *self { - SpecifiedValue::None => dest.write_str("none"), - SpecifiedValue::Url(ref url) => url.to_css(dest), - } - } - } - - pub mod computed_value { - use cssparser::{ToCss, Token}; - use std::fmt; - use url::Url; - use values::LocalToCss; - - #[derive(Debug, Clone, PartialEq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct T(pub Option); - - impl ToCss for T { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - match self.0 { - None => dest.write_str("none"), - Some(ref url) => url.to_css(dest), - } - } - } - } - - impl ToComputedValue for SpecifiedValue { - type ComputedValue = computed_value::T; - - #[inline] - fn to_computed_value(&self, _context: &Context) -> computed_value::T { - match *self { - SpecifiedValue::None => computed_value::T(None), - SpecifiedValue::Url(ref url) => computed_value::T(Some(url.clone())), - } - } - - #[inline] - fn from_computed_value(computed: &computed_value::T) -> Self { - match *computed { - computed_value::T(None) => SpecifiedValue::None, - computed_value::T(Some(ref url)) => SpecifiedValue::Url(url.clone()), - } - } - } - - pub fn parse(context: &ParserContext, input: &mut Parser) -> Result { - if input.try(|input| input.expect_ident_matching("none")).is_ok() { - Ok(SpecifiedValue::None) - } else { - Ok(SpecifiedValue::Url(context.parse_url(&*try!(input.expect_url())))) - } - } - #[inline] - pub fn get_initial_value() -> computed_value::T { - computed_value::T(None) - } - +${helpers.predefined_type("list-style-image", "UrlOrNone", "computed_value::T::None", + needs_context=True, + animatable="False")} <%helpers:longhand name="quotes" animatable="False"> use std::borrow::Cow;