Auto merge of #14370 - thiagopnts:master, r=Wafflespeanut

use Either type for UrlOrNone

Use the Either type for UrlOrNone

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #14298 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14370)
<!-- Reviewable:end -->

components/layout/construct.rs

@@ -45,7 +45,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_image, list_style_position};
+use style::computed_values::{caption_side, display, empty_cells, float, list_style_position};
 use style::computed_values::content::ContentItem;
 use style::computed_values::position;
 use style::context::SharedStyleContext;
@@ -54,6 +54,7 @@ use style::properties::{self, ServoComputedValues};
 use style::selector_parser::{PseudoElement, RestyleDamage};
 use style::servo::restyle_damage::{BUBBLE_ISIZES, RECONSTRUCT_FLOW};
 use style::stylist::Stylist;
+use style::values::Either;
 use table::TableFlow;
 use table_caption::TableCaptionFlow;
 use table_cell::TableCellFlow;
@@ -1205,13 +1206,13 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
                                 -> ConstructionResult {
         let flotation = FloatKind::from_property(flotation);
         let marker_fragments = match node.style(self.style_context()).get_list().list_style_image {
-            list_style_image::T::Url(ref url_value) => {
+            Either::First(ref url_value) => {
                 let image_info = box ImageFragmentInfo::new(node,
                                                             url_value.url().map(|u| u.clone()),
                                                             &self.layout_context.shared);
                 vec![Fragment::new(node, SpecificFragmentInfo::Image(image_info), self.layout_context)]
             }
-            list_style_image::T::None => {
+            Either::Second(_none) => {
                 match ListStyleTypeContent::from_list_style_type(node.style(self.style_context())
                                                                      .get_list()
                                                                      .list_style_type) {

components/style/font_face.rs

@@ -8,7 +8,7 @@
 
 use computed_values::font_family::FontFamily;
 use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser};
-use parser::{ParserContext, log_css_error};
+use parser::{ParserContext, log_css_error, Parse};
 use properties::longhands::font_family::parse_one_family;
 use std::fmt;
 use std::iter;

components/style/properties/gecko.mako.rs

@@ -1089,10 +1089,10 @@ 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::computed_value::T as BindingValue;
+        use values::Either;
         match v {
-            BindingValue::None => debug_assert!(self.gecko.mBinding.mRawPtr.is_null()),
+            Either::Second(_none) => debug_assert!(self.gecko.mBinding.mRawPtr.is_null()),
-            BindingValue::Url(ref url) => {
+            Either::First(ref url) => {
                 let extra_data = url.extra_data();
                 let (ptr, len) = url.as_slice_components();
                 unsafe {
@@ -1617,14 +1617,14 @@ fn static_assert() {
                   skip_additionals="*">
 
     pub fn set_list_style_image(&mut self, image: longhands::list_style_image::computed_value::T) {
-        use values::computed::UrlOrNone;
+        use values::Either;
         match image {
-            UrlOrNone::None => {
+            Either::Second(_none) => {
                 unsafe {
                     Gecko_SetListStyleImageNone(&mut self.gecko);
                 }
             }
-            UrlOrNone::Url(ref url) => {
+            Either::First(ref url) => {
                 let (ptr, len) = url.as_slice_components();
                 let extra_data = url.extra_data();
                 unsafe {

components/style/properties/longhand/box.mako.rs

@@ -1551,7 +1551,7 @@ ${helpers.single_keyword("-moz-appearance",
                          animatable=False)}
 
 // Non-standard: https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-binding
-${helpers.predefined_type("-moz-binding", "UrlOrNone", "computed_value::T::None",
+${helpers.predefined_type("-moz-binding", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
                           animatable="False",
                           disable_when_testing="True")}

components/style/properties/longhand/list.mako.rs

@@ -26,7 +26,7 @@ ${helpers.single_keyword("list-style-type", """
     gecko_constant_prefix="NS_STYLE_LIST_STYLE",
     animatable=False)}
 
-${helpers.predefined_type("list-style-image", "UrlOrNone", "computed_value::T::None",
+${helpers.predefined_type("list-style-image", "UrlOrNone", "Either::Second(None_)",
                           animatable="False")}
 
 <%helpers:longhand name="quotes" animatable="False">

components/style/properties/shorthand/list.mako.rs

@@ -7,6 +7,7 @@
 <%helpers:shorthand name="list-style"
                     sub_properties="list-style-image list-style-position list-style-type">
     use properties::longhands::{list_style_image, list_style_position, list_style_type};
+    use values::{Either, None_};
 
     pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> {
         // `none` is ambiguous until we've finished parsing the shorthands, so we count the number
@@ -56,7 +57,7 @@
             (true, 2, None, None) => {
                 Ok(Longhands {
                     list_style_position: position,
-                    list_style_image: Some(list_style_image::SpecifiedValue::None),
+                    list_style_image: Some(Either::Second(None_)),
                     list_style_type: Some(list_style_type::SpecifiedValue::none),
                 })
             }
@@ -70,14 +71,14 @@
             (true, 1, Some(list_style_type), None) => {
                 Ok(Longhands {
                     list_style_position: position,
-                    list_style_image: Some(list_style_image::SpecifiedValue::None),
+                    list_style_image: Some(Either::Second(None_)),
                     list_style_type: Some(list_style_type),
                 })
             }
             (true, 1, None, None) => {
                 Ok(Longhands {
                     list_style_position: position,
-                    list_style_image: Some(list_style_image::SpecifiedValue::None),
+                    list_style_image: Some(Either::Second(None_)),
                     list_style_type: Some(list_style_type::SpecifiedValue::none),
                 })
             }

components/style/values/specified/mod.rs

@@ -5,14 +5,14 @@
 use app_units::Au;
 use cssparser::{self, Parser, Token};
 use euclid::size::Size2D;
-use parser::{Parse, ParserContext};
+use parser::{ParserContext, Parse};
 use self::url::SpecifiedUrl;
 use std::ascii::AsciiExt;
 use std::f32::consts::PI;
 use std::fmt;
 use std::ops::Mul;
 use style_traits::ToCss;
-use super::{CSSFloat, HasViewportPercentage, NoViewportPercentage};
+use super::{CSSFloat, HasViewportPercentage, NoViewportPercentage, Either, None_};
 use super::computed::{ComputedValueAsSpecified, Context, ToComputedValue};
 
 pub use self::image::{AngleOrCorner, ColorStop, EndingShape as GradientEndingShape, Gradient};
@@ -515,31 +515,5 @@ impl ToCss for Opacity {
     }
 }
 
-#[derive(PartialEq, Clone, Debug)]
+pub type UrlOrNone = Either<SpecifiedUrl, None_>;
-#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-pub enum UrlOrNone {
-    Url(SpecifiedUrl),
-    None,
-}
 
-impl ComputedValueAsSpecified for UrlOrNone {}
-impl NoViewportPercentage for UrlOrNone {}
-
-impl ToCss for UrlOrNone {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-        match *self {
-            UrlOrNone::Url(ref url) => url.to_css(dest),
-            UrlOrNone::None => dest.write_str("none"),
-        }
-    }
-}
-
-impl UrlOrNone {
-    pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<UrlOrNone, ()> {
-        if input.try(|input| input.expect_ident_matching("none")).is_ok() {
-            return Ok(UrlOrNone::None);
-        }
-
-        Ok(UrlOrNone::Url(try!(SpecifiedUrl::parse(context, input))))
-    }
-}

components/style/values/specified/url.rs

@@ -7,7 +7,7 @@
 use cssparser::{CssStringWriter, Parser};
 #[cfg(feature = "gecko")]
 use gecko_bindings::sugar::refptr::{GeckoArcPrincipal, GeckoArcURI};
-use parser::ParserContext;
+use parser::{Parse, ParserContext};
 #[cfg(feature = "gecko")]
 use parser::ParserContextExtraData;
 use servo_url::ServoUrl;
@@ -15,6 +15,7 @@ use std::fmt::{self, Write};
 use std::ptr;
 use std::sync::Arc;
 use style_traits::ToCss;
+use values::NoViewportPercentage;
 use values::computed::ComputedValueAsSpecified;
 
 #[derive(PartialEq, Clone, Debug)]
@@ -72,8 +73,8 @@ pub struct SpecifiedUrl {
     extra_data: UrlExtraData,
 }
 
-impl SpecifiedUrl {
+impl Parse for SpecifiedUrl {
-    pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+    fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
         let url = try!(input.expect_url());
 
         let extra_data = match UrlExtraData::make_from(context) {
@@ -95,7 +96,9 @@ impl SpecifiedUrl {
             extra_data: extra_data,
         })
     }
+}
 
+impl SpecifiedUrl {
     pub fn extra_data(&self) -> &UrlExtraData {
         &self.extra_data
     }
@@ -171,3 +174,5 @@ impl ToCss for SpecifiedUrl {
 
 // TODO(emilio): Maybe consider ComputedUrl to save a word in style structs?
 impl ComputedValueAsSpecified for SpecifiedUrl {}
+
+impl NoViewportPercentage for SpecifiedUrl {}

tests/unit/style/properties/serialization.rs

@@ -418,9 +418,9 @@ mod shorthand_serialization {
     }
 
     mod list_style {
-        use style::properties::longhands::list_style_image::SpecifiedValue as ListStyleImage;
         use style::properties::longhands::list_style_position::computed_value::T as ListStylePosition;
         use style::properties::longhands::list_style_type::computed_value::T as ListStyleType;
+        use style::values::Either;
         use super::*;
 
         #[test]
@@ -428,7 +428,7 @@ mod shorthand_serialization {
             let mut properties = Vec::new();
 
             let position = DeclaredValue::Value(ListStylePosition::inside);
-            let image = DeclaredValue::Value(ListStyleImage::Url(
+            let image = DeclaredValue::Value(Either::First(
                 SpecifiedUrl::new_for_testing("http://servo/test.png")));
             let style_type = DeclaredValue::Value(ListStyleType::disc);