Auto merge of #19953 - emilio:cleanup, r=wafflespeanut

style: Various cleanups

See each commit for details.

<!-- 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/19953)
<!-- Reviewable:end -->

components/style/custom_properties.rs

@@ -8,6 +8,7 @@
 
 use Atom;
 use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType};
+use hash::map::Entry;
 use precomputed_hash::PrecomputedHash;
 use properties::{CSSWideKeyword, DeclaredValue};
 use selector_map::{PrecomputedHashSet, PrecomputedHashMap};
@@ -105,11 +106,21 @@ where
     }
 
     /// Insert a new key-value pair.
+    ///
+    /// TODO(emilio): Remove unused_mut when Gecko and Servo agree in whether
+    /// it's necessary.
+    #[allow(unused_mut)]
     pub fn insert(&mut self, key: K, value: V) {
-        if !self.values.contains_key(&key) {
+        let OrderedMap { ref mut index, ref mut values } = *self;
-            self.index.push(key.clone());
+        match values.entry(key) {
+            Entry::Vacant(mut entry) => {
+                index.push(entry.key().clone());
+                entry.insert(value);
+            }
+            Entry::Occupied(mut entry) => {
+                entry.insert(value);
+            }
         }
-        self.values.insert(key, value);
     }
 
     /// Get a value given its key.
@@ -248,9 +259,7 @@ impl VariableValue {
         debug_assert!(variable.references.is_empty());
         self.push(&variable.css, variable.first_token_type, variable.last_token_type)
     }
-}
 
-impl VariableValue {
     /// Parse a custom property value.
     pub fn parse<'i, 't>(
         input: &mut Parser<'i, 't>,
@@ -666,12 +675,10 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap) {
     ///   been completely resolved.
     /// * There is no such variable at all.
     fn traverse<'a>(name: Name, context: &mut Context<'a>) -> Option<usize> {
-        use hash::map::Entry;
-
         // Some shortcut checks.
         let (name, value) = if let Some(value) = context.map.get(&name) {
             // This variable has been resolved. Return the signal value.
-            if value.references.is_empty()  || context.invalid.contains(&name) {
+            if value.references.is_empty() || context.invalid.contains(&name) {
                 return None;
             }
             // Whether this variable has been visited in this traversal.
@@ -694,7 +701,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap) {
         // Add new entry to the information table.
         let index = context.count;
         context.count += 1;
-        debug_assert!(index == context.var_info.len());
+        debug_assert_eq!(index, context.var_info.len());
         context.var_info.push(VarInfo {
             name: Some(name),
             lowlink: index,

components/style/values/generics/basic_shape.rs

@@ -29,7 +29,7 @@ pub enum GeometryBox {
 /// A float area shape, for `shape-outside`.
 pub type FloatAreaShape<BasicShape, Image> = ShapeSource<BasicShape, ShapeBox, Image>;
 
-// https://drafts.csswg.org/css-shapes-1/#typedef-shape-box
+/// https://drafts.csswg.org/css-shapes-1/#typedef-shape-box
 #[allow(missing_docs)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq)]

components/style/values/specified/basic_shape.rs

@@ -129,8 +129,10 @@ impl Parse for InsetRect {
 
 impl InsetRect {
     /// Parse the inner function arguments of `inset()`
-    pub fn parse_function_arguments<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+    fn parse_function_arguments<'i, 't>(
-                                            -> Result<Self, ParseError<'i>> {
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
         let rect = Rect::parse_with(context, input, LengthOrPercentage::parse)?;
         let round = if input.try(|i| i.expect_ident_matching("round")).is_ok() {
             Some(BorderRadius::parse(context, input)?)
@@ -153,9 +155,10 @@ impl Parse for Circle {
 }
 
 impl Circle {
-    #[allow(missing_docs)]
+    fn parse_function_arguments<'i, 't>(
-    pub fn parse_function_arguments<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+        context: &ParserContext,
-                                            -> Result<Self, ParseError<'i>> {
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
         let radius = input.try(|i| ShapeRadius::parse(context, i)).unwrap_or_default();
         let position = if input.try(|i| i.expect_ident_matching("at")).is_ok() {
             Position::parse(context, input)?
@@ -163,10 +166,7 @@ impl Circle {
             Position::center()
         };
 
-        Ok(GenericCircle {
+        Ok(GenericCircle { radius, position })
-            radius: radius,
-            position: position,
-        })
     }
 }
 
@@ -195,9 +195,10 @@ impl Parse for Ellipse {
 }
 
 impl Ellipse {
-    #[allow(missing_docs)]
+    fn parse_function_arguments<'i, 't>(
-    pub fn parse_function_arguments<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+        context: &ParserContext,
-                                            -> Result<Self, ParseError<'i>> {
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
         let (a, b) = input.try(|i| -> Result<_, ParseError> {
             Ok((ShapeRadius::parse(context, i)?, ShapeRadius::parse(context, i)?))
         }).unwrap_or_default();
@@ -331,8 +332,10 @@ impl Parse for Polygon {
 
 impl Polygon {
     /// Parse the inner arguments of a `polygon` function.
-    pub fn parse_function_arguments<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+    fn parse_function_arguments<'i, 't>(
-                                            -> Result<Self, ParseError<'i>> {
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
         let fill = input.try(|i| -> Result<_, ParseError> {
             let fill = FillRule::parse(i)?;
             i.expect_comma()?;      // only eat the comma if there is something before it

components/style/values/specified/position.rs

@@ -67,10 +67,11 @@ impl Parse for Position {
 
 impl Position {
     /// Parses a `<position>`, with quirks.
-    pub fn parse_quirky<'i, 't>(context: &ParserContext,
+    pub fn parse_quirky<'i, 't>(
-                                input: &mut Parser<'i, 't>,
+        context: &ParserContext,
-                                allow_quirks: AllowQuirks)
+        input: &mut Parser<'i, 't>,
-                                -> Result<Self, ParseError<'i>> {
+        allow_quirks: AllowQuirks,
+    ) -> Result<Self, ParseError<'i>> {
         match input.try(|i| PositionComponent::parse_quirky(context, i, allow_quirks)) {
             Ok(x_pos @ PositionComponent::Center) => {
                 if let Ok(y_pos) = input.try(|i|
@@ -304,10 +305,11 @@ impl Parse for LegacyPosition {
 
 impl LegacyPosition {
     /// Parses a `<position>`, with quirks.
-    pub fn parse_quirky<'i, 't>(context: &ParserContext,
+    pub fn parse_quirky<'i, 't>(
-                                input: &mut Parser<'i, 't>,
+        context: &ParserContext,
-                                allow_quirks: AllowQuirks)
+        input: &mut Parser<'i, 't>,
-                                -> Result<Self, ParseError<'i>> {
+        allow_quirks: AllowQuirks,
+    ) -> Result<Self, ParseError<'i>> {
         match input.try(|i| OriginComponent::parse(context, i)) {
             Ok(x_pos @ OriginComponent::Center) => {
                 if let Ok(y_pos) = input.try(|i|