Unify specified Color type between Stylo and Servo. r?manishearth draft
authorXidorn Quan <me@upsuper.org>
Tue, 06 Jun 2017 17:42:29 +1000
changeset 590137 e143b9906704165f1a6374d3a0fde5e57736b9f7
parent 590136 1000b2c31a1d4e8cb1dd5cbcc15abd45b9a3f484
child 590138 0869b6408b7d7d1d2a6deffbbd3dae169a0552d2
push id62604
push userxquan@mozilla.com
push dateWed, 07 Jun 2017 07:07:54 +0000
reviewersmanishearth
milestone55.0a1
Unify specified Color type between Stylo and Servo. r?manishearth MozReview-Commit-ID: 9AEkujJXPEK
servo/components/script/dom/element.rs
servo/components/style/values/specified/color.rs
servo/tests/unit/style/parsing/ui.rs
servo/tests/unit/style/properties/serialization.rs
servo/tests/unit/style/stylesheets.rs
--- a/servo/components/script/dom/element.rs
+++ b/servo/components/script/dom/element.rs
@@ -1,15 +1,14 @@
 /* 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/. */
 
 //! Element nodes.
 
-use cssparser::Color;
 use devtools_traits::AttrInfo;
 use dom::activation::Activatable;
 use dom::attr::{Attr, AttrHelpersForLayout};
 use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::AttrBinding::AttrMethods;
 use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods;
 use dom::bindings::codegen::Bindings::ElementBinding;
 use dom::bindings::codegen::Bindings::ElementBinding::ElementMethods;
@@ -107,17 +106,17 @@ use style::restyle_hints::RestyleHint;
 use style::rule_tree::CascadeLevel;
 use style::selector_parser::{NonTSPseudoClass, PseudoElement, RestyleDamage, SelectorImpl, SelectorParser};
 use style::shared_lock::{SharedRwLock, Locked};
 use style::sink::Push;
 use style::stylearc::Arc;
 use style::stylist::ApplicableDeclarationBlock;
 use style::thread_state;
 use style::values::{CSSFloat, Either};
-use style::values::specified::{self, CSSColor};
+use style::values::specified::{self, CSSColor, Color};
 use stylesheet_loader::StylesheetOwner;
 
 // TODO: Update focus state when the top-level browsing context gains or loses system focus,
 // and when the element enters or leaves a browsing context container.
 // https://html.spec.whatwg.org/multipage/#selector-focus
 
 #[dom_struct]
 pub struct Element {
--- a/servo/components/style/values/specified/color.rs
+++ b/servo/components/style/values/specified/color.rs
@@ -2,106 +2,98 @@
  * 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/. */
 
 //! Specified color values.
 
 use cssparser::{self, Color as CSSParserColor, Parser, RGBA, Token};
 use itoa;
 use parser::{ParserContext, Parse};
+#[cfg(feature = "gecko")]
+use properties::longhands::color::SystemColor;
 use std::fmt;
 use std::io::Write;
 use style_traits::ToCss;
 use super::AllowQuirks;
 use values::computed::{Context, ToComputedValue};
 
-#[cfg(not(feature = "gecko"))] pub use self::servo::Color;
-#[cfg(feature = "gecko")] pub use self::gecko::Color;
+/// Specified color value
+#[derive(Clone, Copy, PartialEq, Debug)]
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+pub enum Color {
+    /// The 'currentColor' keyword
+    CurrentColor,
+    /// A specific RGBA color
+    RGBA(RGBA),
 
-#[cfg(not(feature = "gecko"))]
-mod servo {
-    pub use cssparser::Color;
-    use cssparser::Parser;
-    use parser::{Parse, ParserContext};
+    /// A system color
+    #[cfg(feature = "gecko")]
+    System(SystemColor),
+    /// A special color keyword value used in Gecko
+    #[cfg(feature = "gecko")]
+    Special(gecko::SpecialColorKeyword),
+    /// Quirksmode-only rule for inheriting color from the body
+    #[cfg(feature = "gecko")]
+    InheritFromBodyQuirk,
+}
 
-    impl Parse for Color {
-        fn parse(_: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-            Color::parse(input)
-        }
-    }
-}
+no_viewport_percentage!(Color);
 
 #[cfg(feature = "gecko")]
 mod gecko {
-    use cssparser::{Color as CSSParserColor, Parser, RGBA};
-    use parser::{Parse, ParserContext};
-    use properties::longhands::color::SystemColor;
-    use std::fmt;
     use style_traits::ToCss;
 
     define_css_keyword_enum! { SpecialColorKeyword:
         "-moz-default-color" => MozDefaultColor,
         "-moz-default-background-color" => MozDefaultBackgroundColor,
         "-moz-hyperlinktext" => MozHyperlinktext,
         "-moz-activehyperlinktext" => MozActiveHyperlinktext,
         "-moz-visitedhyperlinktext" => MozVisitedHyperlinktext,
     }
+}
 
-    /// Color value including non-standard -moz prefixed values.
-    #[derive(Clone, Copy, PartialEq, Debug)]
-    pub enum Color {
-        /// The 'currentColor' keyword
-        CurrentColor,
-        /// A specific RGBA color
-        RGBA(RGBA),
-        /// A system color
-        System(SystemColor),
-        /// A special color keyword value used in Gecko
-        Special(SpecialColorKeyword),
-        /// Quirksmode-only rule for inheriting color from the body
-        InheritFromBodyQuirk,
-    }
-
-    no_viewport_percentage!(Color);
-
-    impl From<CSSParserColor> for Color {
-        fn from(value: CSSParserColor) -> Self {
-            match value {
-                CSSParserColor::CurrentColor => Color::CurrentColor,
-                CSSParserColor::RGBA(x) => Color::RGBA(x),
-            }
+impl From<CSSParserColor> for Color {
+    fn from(value: CSSParserColor) -> Self {
+        match value {
+            CSSParserColor::CurrentColor => Color::CurrentColor,
+            CSSParserColor::RGBA(x) => Color::RGBA(x),
         }
     }
+}
 
-    impl Parse for Color {
-        fn parse(_: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-            if let Ok(value) = input.try(CSSParserColor::parse) {
-                Ok(value.into())
-            } else if let Ok(system) = input.try(SystemColor::parse) {
-                Ok(Color::System(system))
-            } else if let Ok(special) = input.try(SpecialColorKeyword::parse) {
-                Ok(Color::Special(special))
-            } else {
+impl Parse for Color {
+    fn parse(_: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+        if let Ok(value) = input.try(CSSParserColor::parse) {
+            Ok(value.into())
+        } else {
+            #[cfg(feature = "gecko")] {
+                if let Ok(system) = input.try(SystemColor::parse) {
+                    Ok(Color::System(system))
+                } else {
+                    gecko::SpecialColorKeyword::parse(input).map(Color::Special)
+                }
+            }
+            #[cfg(not(feature = "gecko"))] {
                 Err(())
             }
         }
     }
+}
 
-    impl ToCss for Color {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            match *self {
-                // Standard values:
-                Color::CurrentColor => CSSParserColor::CurrentColor.to_css(dest),
-                Color::RGBA(rgba) => rgba.to_css(dest),
-                Color::System(system) => system.to_css(dest),
-
-                // Non-standard values:
-                Color::Special(special) => special.to_css(dest),
-                Color::InheritFromBodyQuirk => Ok(()),
-            }
+impl ToCss for Color {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+        match *self {
+            Color::CurrentColor => CSSParserColor::CurrentColor.to_css(dest),
+            Color::RGBA(rgba) => rgba.to_css(dest),
+            #[cfg(feature = "gecko")]
+            Color::System(system) => system.to_css(dest),
+            #[cfg(feature = "gecko")]
+            Color::Special(special) => special.to_css(dest),
+            #[cfg(feature = "gecko")]
+            Color::InheritFromBodyQuirk => Ok(()),
         }
     }
 }
 
 #[derive(Clone, PartialEq, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[allow(missing_docs)]
 pub struct CSSColor {
@@ -240,47 +232,42 @@ impl CSSColor {
         // We should probably set authored to "transparent", but maybe it doesn't matter.
         Color::RGBA(cssparser::RGBA::transparent()).into()
     }
 }
 
 impl ToComputedValue for Color {
     type ComputedValue = RGBA;
 
-    #[cfg(not(feature = "gecko"))]
     fn to_computed_value(&self, context: &Context) -> RGBA {
+        #[cfg(feature = "gecko")]
+        use gecko::values::convert_nscolor_to_rgba as to_rgba;
         match *self {
             Color::RGBA(rgba) => rgba,
             Color::CurrentColor => context.inherited_style.get_color().clone_color(),
-        }
-    }
-
-    #[cfg(feature = "gecko")]
-    fn to_computed_value(&self, context: &Context) -> RGBA {
-        use gecko::values::convert_nscolor_to_rgba as to_rgba;
-        // It's safe to access the nsPresContext immutably during style computation.
-        let pres_context = unsafe { &*context.device.pres_context };
-        match *self {
-            Color::RGBA(rgba) => rgba,
+            #[cfg(feature = "gecko")]
             Color::System(system) => to_rgba(system.to_computed_value(context)),
-            Color::CurrentColor => context.inherited_style.get_color().clone_color(),
+            #[cfg(feature = "gecko")]
             Color::Special(special) => {
                 use self::gecko::SpecialColorKeyword as Keyword;
+                let pres_context = unsafe { &*context.device.pres_context };
                 to_rgba(match special {
                     Keyword::MozDefaultColor => pres_context.mDefaultColor,
                     Keyword::MozDefaultBackgroundColor => pres_context.mBackgroundColor,
                     Keyword::MozHyperlinktext => pres_context.mLinkColor,
                     Keyword::MozActiveHyperlinktext => pres_context.mActiveLinkColor,
                     Keyword::MozVisitedHyperlinktext => pres_context.mVisitedLinkColor,
                 })
             }
+            #[cfg(feature = "gecko")]
             Color::InheritFromBodyQuirk => {
                 use dom::TElement;
                 use gecko::wrapper::GeckoElement;
                 use gecko_bindings::bindings::Gecko_GetBody;
+                let pres_context = unsafe { &*context.device.pres_context };
                 let body = unsafe {
                     Gecko_GetBody(pres_context)
                 };
                 if let Some(body) = body {
                     let wrap = GeckoElement(body);
                     let borrow = wrap.borrow_data();
                     borrow.as_ref().unwrap()
                           .styles().primary.values()
@@ -296,30 +283,24 @@ impl ToComputedValue for Color {
     fn from_computed_value(computed: &RGBA) -> Self {
         Color::RGBA(*computed)
     }
 }
 
 impl ToComputedValue for CSSColor {
     type ComputedValue = CSSParserColor;
 
-    #[cfg(not(feature = "gecko"))]
     #[inline]
     fn to_computed_value(&self, _context: &Context) -> CSSParserColor {
-        self.parsed
-    }
-
-    #[cfg(feature = "gecko")]
-    #[inline]
-    fn to_computed_value(&self, context: &Context) -> CSSParserColor {
         match self.parsed {
             Color::RGBA(rgba) => CSSParserColor::RGBA(rgba),
             Color::CurrentColor => CSSParserColor::CurrentColor,
             // Resolve non-standard -moz keywords to RGBA:
-            non_standard => CSSParserColor::RGBA(non_standard.to_computed_value(context)),
+            #[cfg(feature = "gecko")]
+            non_standard => CSSParserColor::RGBA(non_standard.to_computed_value(_context)),
         }
     }
 
     #[inline]
     fn from_computed_value(computed: &CSSParserColor) -> Self {
         (match *computed {
             CSSParserColor::RGBA(rgba) => Color::RGBA(rgba),
             CSSParserColor::CurrentColor => Color::CurrentColor,
--- a/servo/tests/unit/style/parsing/ui.rs
+++ b/servo/tests/unit/style/parsing/ui.rs
@@ -1,16 +1,16 @@
 /* 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 cssparser::{Color, RGBA};
+use cssparser::RGBA;
 use parsing::parse;
 use style::values::{Auto, Either};
-use style::values::specified::CSSColor;
+use style::values::specified::{CSSColor, Color};
 use style_traits::ToCss;
 
 #[test]
 fn test_moz_user_select() {
     use style::properties::longhands::_moz_user_select;
 
     assert_roundtrip_with_context!(_moz_user_select::parse, "auto");
     assert_roundtrip_with_context!(_moz_user_select::parse, "text");
--- a/servo/tests/unit/style/properties/serialization.rs
+++ b/servo/tests/unit/style/properties/serialization.rs
@@ -1,20 +1,20 @@
 /* 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 properties::parse;
 use style::computed_values::display::T::inline_block;
 use style::properties::{PropertyDeclaration, Importance, PropertyId};
-use style::properties::longhands::outline_color::computed_value::T as ComputedColor;
 use style::properties::parse_property_declaration_list;
 use style::values::{RGBA, Auto};
 use style::values::CustomIdent;
-use style::values::specified::{BorderStyle, BorderSideWidth, CSSColor, Length, LengthOrPercentage};
+use style::values::specified::{BorderStyle, BorderSideWidth, CSSColor, Color};
+use style::values::specified::{Length, LengthOrPercentage};
 use style::values::specified::{LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent};
 use style::values::specified::{NoCalcLength, PositionComponent};
 use style::values::specified::position::Y;
 use style::values::specified::url::SpecifiedUrl;
 use style_traits::ToCss;
 use stylesheets::block_from;
 
 #[test]
@@ -106,17 +106,17 @@ mod shorthand_serialization {
 
         #[test]
         fn text_decoration_should_show_all_properties_when_set() {
             let mut properties = Vec::new();
 
             let line = TextDecorationLine::OVERLINE;
             let style = TextDecorationStyle::dotted;
             let color = CSSColor {
-                parsed: ComputedColor::RGBA(RGBA::new(128, 0, 128, 255)),
+                parsed: Color::RGBA(RGBA::new(128, 0, 128, 255)),
                 authored: None
             };
 
             properties.push(PropertyDeclaration::TextDecorationLine(line));
             properties.push(PropertyDeclaration::TextDecorationStyle(style));
             properties.push(PropertyDeclaration::TextDecorationColor(color));
 
             let serialization = shorthand_properties_to_string(properties);
@@ -225,17 +225,17 @@ mod shorthand_serialization {
           let px_10 = BorderSideWidth::Length(Length::from_px(10f32));
 
           properties.push(PropertyDeclaration::BorderTopWidth(px_30.clone()));
           properties.push(PropertyDeclaration::BorderRightWidth(px_30.clone()));
           properties.push(PropertyDeclaration::BorderBottomWidth(px_30.clone()));
           properties.push(PropertyDeclaration::BorderLeftWidth(px_10.clone()));
 
           let blue = CSSColor {
-              parsed: ComputedColor::RGBA(RGBA::new(0, 0, 255, 255)),
+              parsed: Color::RGBA(RGBA::new(0, 0, 255, 255)),
               authored: None
           };
 
           properties.push(PropertyDeclaration::BorderTopColor(blue.clone()));
           properties.push(PropertyDeclaration::BorderRightColor(blue.clone()));
           properties.push(PropertyDeclaration::BorderBottomColor(blue.clone()));
           properties.push(PropertyDeclaration::BorderLeftColor(blue.clone()));
 
@@ -258,17 +258,17 @@ mod shorthand_serialization {
           let px_30 = BorderSideWidth::Length(Length::from_px(30f32));
 
           properties.push(PropertyDeclaration::BorderTopWidth(px_30.clone()));
           properties.push(PropertyDeclaration::BorderRightWidth(px_30.clone()));
           properties.push(PropertyDeclaration::BorderBottomWidth(px_30.clone()));
           properties.push(PropertyDeclaration::BorderLeftWidth(px_30.clone()));
 
           let blue = CSSColor {
-              parsed: ComputedColor::RGBA(RGBA::new(0, 0, 255, 255)),
+              parsed: Color::RGBA(RGBA::new(0, 0, 255, 255)),
               authored: None
           };
 
           properties.push(PropertyDeclaration::BorderTopColor(blue.clone()));
           properties.push(PropertyDeclaration::BorderRightColor(blue.clone()));
           properties.push(PropertyDeclaration::BorderBottomColor(blue.clone()));
           properties.push(PropertyDeclaration::BorderLeftColor(blue.clone()));
 
@@ -328,22 +328,22 @@ mod shorthand_serialization {
             assert_eq!(serialization, "border-width: thin medium thick 15px;");
         }
 
         #[test]
         fn border_color_should_serialize_correctly() {
             let mut properties = Vec::new();
 
             let red = CSSColor {
-                parsed: ComputedColor::RGBA(RGBA::new(255, 0, 0, 255)),
+                parsed: Color::RGBA(RGBA::new(255, 0, 0, 255)),
                 authored: None
             };
 
             let blue = CSSColor {
-                parsed: ComputedColor::RGBA(RGBA::new(0, 0, 255, 255)),
+                parsed: Color::RGBA(RGBA::new(0, 0, 255, 255)),
                 authored: None
             };
 
             properties.push(PropertyDeclaration::BorderTopColor(blue.clone()));
             properties.push(PropertyDeclaration::BorderRightColor(red.clone()));
             properties.push(PropertyDeclaration::BorderBottomColor(blue));
             properties.push(PropertyDeclaration::BorderLeftColor(red));
 
@@ -401,17 +401,17 @@ mod shorthand_serialization {
 
         #[test]
         fn directional_border_should_show_all_properties_when_values_are_set() {
             let mut properties = Vec::new();
 
             let width = BorderSideWidth::Length(Length::from_px(4f32));
             let style = BorderStyle::solid;
             let color = CSSColor {
-                parsed: ComputedColor::RGBA(RGBA::new(255, 0, 0, 255)),
+                parsed: Color::RGBA(RGBA::new(255, 0, 0, 255)),
                 authored: None
             };
 
             properties.push(PropertyDeclaration::BorderTopWidth(width));
             properties.push(PropertyDeclaration::BorderTopStyle(style));
             properties.push(PropertyDeclaration::BorderTopColor(color));
 
             let serialization = shorthand_properties_to_string(properties);
@@ -528,17 +528,17 @@ mod shorthand_serialization {
 
         #[test]
         fn outline_should_show_all_properties_when_set() {
             let mut properties = Vec::new();
 
             let width = BorderSideWidth::Length(Length::from_px(4f32));
             let style = Either::Second(BorderStyle::solid);
             let color = CSSColor {
-                parsed: ComputedColor::RGBA(RGBA::new(255, 0, 0, 255)),
+                parsed: Color::RGBA(RGBA::new(255, 0, 0, 255)),
                 authored: None
             };
 
             properties.push(PropertyDeclaration::OutlineWidth(width));
             properties.push(PropertyDeclaration::OutlineStyle(style));
             properties.push(PropertyDeclaration::OutlineColor(color));
 
             let serialization = shorthand_properties_to_string(properties);
@@ -547,17 +547,17 @@ mod shorthand_serialization {
 
         #[test]
         fn outline_should_serialize_correctly_when_style_is_auto() {
             let mut properties = Vec::new();
 
             let width = BorderSideWidth::Length(Length::from_px(4f32));
             let style = Either::First(Auto);
             let color = CSSColor {
-                parsed: ComputedColor::RGBA(RGBA::new(255, 0, 0, 255)),
+                parsed: Color::RGBA(RGBA::new(255, 0, 0, 255)),
                 authored: None
             };
             properties.push(PropertyDeclaration::OutlineWidth(width));
             properties.push(PropertyDeclaration::OutlineStyle(style));
             properties.push(PropertyDeclaration::OutlineColor(color));
 
             let serialization = shorthand_properties_to_string(properties);
             assert_eq!(serialization, "outline: 4px auto rgb(255, 0, 0);");
--- a/servo/tests/unit/style/stylesheets.rs
+++ b/servo/tests/unit/style/stylesheets.rs
@@ -21,17 +21,17 @@ use style::properties::{CSSWideKeyword, 
 use style::properties::longhands;
 use style::properties::longhands::animation_play_state;
 use style::shared_lock::SharedRwLock;
 use style::stylearc::Arc;
 use style::stylesheets::{Origin, Namespaces};
 use style::stylesheets::{Stylesheet, NamespaceRule, CssRule, CssRules, StyleRule, KeyframesRule};
 use style::stylesheets::keyframes_rule::{Keyframe, KeyframeSelector, KeyframePercentage};
 use style::values::{KeyframesName, CustomIdent};
-use style::values::specified::{LengthOrPercentageOrAuto, Percentage, PositionComponent};
+use style::values::specified::{Color, LengthOrPercentageOrAuto, Percentage, PositionComponent};
 
 pub fn block_from<I>(iterable: I) -> PropertyDeclarationBlock
 where I: IntoIterator<Item=(PropertyDeclaration, Importance)> {
     let mut block = PropertyDeclarationBlock::new();
     for (d, i) in iterable {
         block.push(d, i)
     }
     block
@@ -155,17 +155,17 @@ fn test_parse_stylesheet() {
                         Component::DefaultNamespace(NsAtom::from("http://www.w3.org/1999/xhtml")),
                         Component::Class(Atom::from("ok"))
                     ), (1 << 20) + (1 << 10) + (0 << 0))
                 )),
                 block: Arc::new(stylesheet.shared_lock.wrap(block_from(vec![
                     (PropertyDeclaration::BackgroundColor(
                         longhands::background_color::SpecifiedValue {
                             authored: Some("blue".to_owned().into_boxed_str()),
-                            parsed: cssparser::Color::RGBA(cssparser::RGBA::new(0, 0, 255, 255)),
+                            parsed: Color::RGBA(cssparser::RGBA::new(0, 0, 255, 255)),
                         }
                      ),
                      Importance::Normal),
                     (PropertyDeclaration::BackgroundPositionX(
                         longhands::background_position_x::SpecifiedValue(
                         vec![PositionComponent::zero()])),
                      Importance::Normal),
                     (PropertyDeclaration::BackgroundPositionY(