From a40fd0bd5dadade69f0833e48c2d454428775976 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Mon, 3 Jun 2019 16:37:34 +0000
Subject: [PATCH 01/32] style: Add some very basic rule tree stats that can be
 enabled with trace-level logging.

This is useful to analyze the shape of the rule tree at a glance.

Differential Revision: https://phabricator.services.mozilla.com/D33350
---
 components/style/rule_tree/mod.rs | 49 +++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs
index 3b45035f46d..76f3cdf4bd7 100644
--- a/components/style/rule_tree/mod.rs
+++ b/components/style/rule_tree/mod.rs
@@ -382,9 +382,58 @@ impl RuleTree {
 
     /// This can only be called when no other threads is accessing this tree.
     pub unsafe fn maybe_gc(&self) {
+        #[cfg(debug_assertions)]
+        self.maybe_dump_stats();
+
         self.root.maybe_gc();
     }
 
+    #[cfg(debug_assertions)]
+    fn maybe_dump_stats(&self) {
+        use itertools::Itertools;
+        use std::cell::Cell;
+        use std::time::{Duration, Instant};
+
+        if !log_enabled!(log::Level::Trace) {
+            return;
+        }
+
+        const RULE_TREE_STATS_INTERVAL: Duration = Duration::from_secs(2);
+
+        thread_local! {
+            pub static LAST_STATS: Cell<Instant> = Cell::new(Instant::now());
+        };
+
+        let should_dump = LAST_STATS.with(|s| {
+            let now = Instant::now();
+            if now.duration_since(s.get()) < RULE_TREE_STATS_INTERVAL {
+                return false;
+            }
+            s.set(now);
+            true
+        });
+
+        if !should_dump {
+            return;
+        }
+
+        let mut children_count = FxHashMap::default();
+
+        let mut stack = SmallVec::<[_; 32]>::new();
+        stack.push(self.root.clone());
+        while let Some(node) = stack.pop() {
+            let children = node.get().children.read();
+            *children_count.entry(children.len()).or_insert(0) += 1;
+            stack.extend(children.iter().map(|(_k, v)| v.upgrade()))
+        }
+
+        trace!("Rule tree stats:");
+        let counts = children_count.keys().sorted();
+        for count in counts {
+            trace!(" {} - {}", count, children_count[count]);
+        }
+    }
+
     /// Replaces a rule in a given level (if present) for another rule.
     ///
     /// Returns the resulting node that represents the new path, or None if

From 9721bd7d0d590050c35e507a1cf3e7a80b134e88 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Mon, 3 Jun 2019 16:37:45 +0000
Subject: [PATCH 02/32] style: Inline one child in the rule tree.

It is indeed the most common case according to a bit of measurement.

A non-atypical example from GitHub for example:

> Rule tree stats:
>  0 - 340
>  1 - 1403
>  2 - 28
>  3 - 8
>  4 - 2
>  6 - 1
>  7 - 3
>  8 - 2
>  12 - 2
>  14 - 1
>  41 - 1
>  45 - 1
>  67 - 1
>  68 - 1

Differential Revision: https://phabricator.services.mozilla.com/D33351
---
 components/style/rule_tree/mod.rs | 198 +++++++++++++++++++++++++-----
 1 file changed, 168 insertions(+), 30 deletions(-)

diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs
index 76f3cdf4bd7..f4d62e4ce2a 100644
--- a/components/style/rule_tree/mod.rs
+++ b/components/style/rule_tree/mod.rs
@@ -83,20 +83,16 @@ impl MallocSizeOf for RuleTree {
 
         while let Some(node) = stack.pop() {
             n += unsafe { ops.malloc_size_of(node.ptr()) };
-            stack.extend(unsafe {
-                (*node.ptr())
-                    .children
-                    .read()
-                    .iter()
-                    .map(|(_k, v)| v.clone())
-            });
+            unsafe {
+                (*node.ptr()).children.read().each(|c| stack.push(c.clone()));
+            }
         }
 
         n
     }
 }
 
-#[derive(Debug, Eq, Hash, PartialEq)]
+#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
 struct ChildKey(CascadeLevel, ptr::NonNull<()>);
 
 unsafe impl Send for ChildKey {}
@@ -424,7 +420,7 @@ impl RuleTree {
         while let Some(node) = stack.pop() {
             let children = node.get().children.read();
             *children_count.entry(children.len()).or_insert(0) += 1;
-            stack.extend(children.iter().map(|(_k, v)| v.upgrade()))
+            children.each(|c| stack.push(c.upgrade()));
         }
 
         trace!("Rule tree stats:");
@@ -755,6 +751,149 @@ impl CascadeLevel {
     }
 }
 
+/// The children of a single rule node.
+///
+/// We optimize the case of no kids and a single child, since they're by far the
+/// most common case and it'd cause a bunch of bloat for no reason.
+///
+/// The children remove themselves when they go away, which means that it's ok
+/// for us to store weak pointers to them.
+enum RuleNodeChildren {
+    /// There are no kids.
+    Empty,
+    /// There's just one kid. This is an extremely common case, so we don't
+    /// bother allocating a map for it.
+    One(WeakRuleNode),
+    /// At least at one point in time there was more than one kid (that is to
+    /// say, we don't bother re-allocating if children are removed dynamically).
+    Map(Box<FxHashMap<ChildKey, WeakRuleNode>>),
+}
+
+impl Default for RuleNodeChildren {
+    fn default() -> Self {
+        RuleNodeChildren::Empty
+    }
+}
+
+impl RuleNodeChildren {
+    /// Executes a given function for each of the children.
+    fn each(&self, mut f: impl FnMut(&WeakRuleNode)) {
+        match *self {
+            RuleNodeChildren::Empty => {},
+            RuleNodeChildren::One(ref child) => f(child),
+            RuleNodeChildren::Map(ref map) => {
+                for (_key, kid) in map.iter() {
+                    f(kid)
+                }
+            },
+        }
+    }
+
+    fn len(&self) -> usize {
+        match *self {
+            RuleNodeChildren::Empty => 0,
+            RuleNodeChildren::One(..) => 1,
+            RuleNodeChildren::Map(ref map) => map.len(),
+        }
+    }
+
+    fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+
+    fn get(&self, key: &ChildKey) -> Option<&WeakRuleNode> {
+        match *self {
+            RuleNodeChildren::Empty => return None,
+            RuleNodeChildren::One(ref kid) => {
+                // We're read-locked, so no need to do refcount stuff, since the
+                // child is only removed from the main thread, _and_ it'd need
+                // to write-lock us anyway.
+                if unsafe { (*kid.ptr()).key() } == *key {
+                    Some(kid)
+                } else {
+                    None
+                }
+            },
+            RuleNodeChildren::Map(ref map) => map.get(&key),
+        }
+    }
+
+    fn get_or_insert_with(
+        &mut self,
+        key: ChildKey,
+        get_new_child: impl FnOnce() -> StrongRuleNode,
+    ) -> StrongRuleNode {
+        let existing_child_key = match *self {
+            RuleNodeChildren::Empty => {
+                let new = get_new_child();
+                debug_assert_eq!(new.get().key(), key);
+                *self = RuleNodeChildren::One(new.downgrade());
+                return new;
+            },
+            RuleNodeChildren::One(ref weak) => unsafe {
+                // We're locked necessarily, so it's fine to look at our
+                // weak-child without refcount-traffic.
+                let existing_child_key = (*weak.ptr()).key();
+                if existing_child_key == key {
+                    return weak.upgrade();
+                }
+                existing_child_key
+            },
+            RuleNodeChildren::Map(ref mut map) => {
+                return match map.entry(key) {
+                    hash::map::Entry::Occupied(ref occupied) => occupied.get().upgrade(),
+                    hash::map::Entry::Vacant(vacant) => {
+                        let new = get_new_child();
+
+                        debug_assert_eq!(new.get().key(), key);
+                        vacant.insert(new.downgrade());
+
+                        new
+                    },
+                };
+            },
+        };
+
+        let existing_child = match mem::replace(self, RuleNodeChildren::Empty) {
+            RuleNodeChildren::One(o) => o,
+            _ => unreachable!(),
+        };
+        // Two rule-nodes are still a not-totally-uncommon thing, so
+        // avoid over-allocating entries.
+        //
+        // TODO(emilio): Maybe just inline two kids too?
+        let mut children = Box::new(FxHashMap::with_capacity_and_hasher(2, Default::default()));
+        children.insert(existing_child_key, existing_child);
+
+        let new = get_new_child();
+        debug_assert_eq!(new.get().key(), key);
+        children.insert(key, new.downgrade());
+
+        *self = RuleNodeChildren::Map(children);
+
+        new
+    }
+
+    fn remove(&mut self, key: &ChildKey) -> Option<WeakRuleNode> {
+        match *self {
+            RuleNodeChildren::Empty => return None,
+            RuleNodeChildren::One(ref one) => {
+                if unsafe { (*one.ptr()).key() } != *key {
+                    return None;
+                }
+            },
+            RuleNodeChildren::Map(ref mut multiple) => {
+                return multiple.remove(key);
+            },
+        }
+
+        match mem::replace(self, RuleNodeChildren::Empty) {
+            RuleNodeChildren::One(o) => Some(o),
+            _ => unreachable!(),
+        }
+    }
+}
+
 /// A node in the rule tree.
 pub struct RuleNode {
     /// The root node. Only the root has no root pointer, for obvious reasons.
@@ -780,7 +919,7 @@ pub struct RuleNode {
 
     /// The children of a given rule node. Children remove themselves from here
     /// when they go away.
-    children: RwLock<FxHashMap<ChildKey, WeakRuleNode>>,
+    children: RwLock<RuleNodeChildren>,
 
     /// The next item in the rule tree free list, that starts on the root node.
     ///
@@ -871,8 +1010,14 @@ impl RuleNode {
         }
     }
 
-    fn key(&self) -> Option<ChildKey> {
-        Some(ChildKey(self.level, self.source.as_ref()?.key()))
+    fn key(&self) -> ChildKey {
+        ChildKey(
+            self.level,
+            self.source
+                .as_ref()
+                .expect("Called key() on the root node")
+                .key(),
+        )
     }
 
     fn is_root(&self) -> bool {
@@ -896,7 +1041,7 @@ impl RuleNode {
         );
 
         if let Some(parent) = self.parent.as_ref() {
-            let weak = parent.get().children.write().remove(&self.key().unwrap());
+            let weak = parent.get().children.write().remove(&self.key());
             assert_eq!(weak.unwrap().ptr() as *const _, self as *const _);
         }
     }
@@ -933,12 +1078,12 @@ impl RuleNode {
         }
 
         let _ = write!(writer, "\n");
-        for (_, child) in self.children.read().iter() {
+        self.children.read().each(|child| {
             child
                 .upgrade()
                 .get()
                 .dump(guards, writer, indent + INDENT_INCREMENT);
-        }
+        });
     }
 }
 
@@ -1000,21 +1145,14 @@ impl StrongRuleNode {
             return child.upgrade();
         }
 
-        match RwLockUpgradableReadGuard::upgrade(read_guard).entry(key) {
-            hash::map::Entry::Occupied(ref occupied) => occupied.get().upgrade(),
-            hash::map::Entry::Vacant(vacant) => {
-                let new_node = StrongRuleNode::new(Box::new(RuleNode::new(
-                    root,
-                    self.clone(),
-                    source.clone(),
-                    level,
-                )));
-
-                vacant.insert(new_node.downgrade());
-
-                new_node
-            },
-        }
+        RwLockUpgradableReadGuard::upgrade(read_guard).get_or_insert_with(key, move || {
+            StrongRuleNode::new(Box::new(RuleNode::new(
+                root,
+                self.clone(),
+                source.clone(),
+                level,
+            )))
+        })
     }
 
     /// Raw pointer to the RuleNode

From ef99ab1f08db3b9df7c6580b1984585cb7be6fff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Mon, 3 Jun 2019 16:37:53 +0000
Subject: [PATCH 03/32] style: Report heap size of rule tree heap allocations
 as well.

Differential Revision: https://phabricator.services.mozilla.com/D33353
---
 components/style/rule_tree/mod.rs | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs
index f4d62e4ce2a..8f255898fc8 100644
--- a/components/style/rule_tree/mod.rs
+++ b/components/style/rule_tree/mod.rs
@@ -14,8 +14,7 @@ use crate::properties::{Importance, LonghandIdSet, PropertyDeclarationBlock};
 use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
 use crate::stylesheets::{Origin, StyleRule};
 use crate::thread_state;
-#[cfg(feature = "gecko")]
-use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
+use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps};
 use parking_lot::RwLock;
 use servo_arc::{Arc, ArcBorrow, ArcUnion, ArcUnionBorrow};
 use smallvec::SmallVec;
@@ -46,7 +45,6 @@ use std::sync::atomic::{AtomicPtr, AtomicUsize, Ordering};
 /// logs from http://logs.glob.uno/?c=mozilla%23servo&s=3+Apr+2017&e=3+Apr+2017#c644094
 /// to se a discussion about the different memory orderings used here.
 #[derive(Debug)]
-#[cfg_attr(feature = "servo", derive(MallocSizeOf))]
 pub struct RuleTree {
     root: StrongRuleNode,
 }
@@ -74,7 +72,6 @@ impl Drop for RuleTree {
     }
 }
 
-#[cfg(feature = "gecko")]
 impl MallocSizeOf for RuleTree {
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         let mut n = 0;
@@ -83,9 +80,9 @@ impl MallocSizeOf for RuleTree {
 
         while let Some(node) = stack.pop() {
             n += unsafe { ops.malloc_size_of(node.ptr()) };
-            unsafe {
-                (*node.ptr()).children.read().each(|c| stack.push(c.clone()));
-            }
+            let children = unsafe { (*node.ptr()).children.read() };
+            children.shallow_size_of(ops);
+            children.each(|c| stack.push(c.clone()));
         }
 
         n
@@ -769,6 +766,18 @@ enum RuleNodeChildren {
     Map(Box<FxHashMap<ChildKey, WeakRuleNode>>),
 }
 
+impl MallocShallowSizeOf for RuleNodeChildren {
+    fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
+        match *self {
+            RuleNodeChildren::One(..) | RuleNodeChildren::Empty => 0,
+            RuleNodeChildren::Map(ref m) => {
+                // Want to account for both the box and the hashmap.
+                m.shallow_size_of(ops) + (**m).shallow_size_of(ops)
+            },
+        }
+    }
+}
+
 impl Default for RuleNodeChildren {
     fn default() -> Self {
         RuleNodeChildren::Empty

From 1f5bed447333ad83dd0af279a894621a198730c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Mon, 3 Jun 2019 17:32:48 +0000
Subject: [PATCH 04/32] style: Hide some appearance values from content.

They're not used internally either, so remove all ability to address them.

I haven't removed the implementation yet, as some of them are quite complex, and
I don't have a mac / windows build. We should do that when this hits release
though.

Differential Revision: https://phabricator.services.mozilla.com/D32488
---
 components/style/values/specified/box.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs
index 542976d45ba..a64216087d7 100644
--- a/components/style/values/specified/box.rs
+++ b/components/style/values/specified/box.rs
@@ -1129,11 +1129,13 @@ pub enum Appearance {
     ButtonArrowUp,
     /// A rectangular button that contains complex content
     /// like images (e.g. HTML <button> elements)
+    #[css(skip)]
     ButtonBevel,
     /// The focus outline box inside of a button.
     #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]
     ButtonFocus,
     /// The caret of a text area
+    #[css(skip)]
     Caret,
     /// A dual toolbar button (e.g., a Back button with a dropdown)
     #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]
@@ -1146,6 +1148,7 @@ pub enum Appearance {
     /// List boxes.
     Listbox,
     /// A listbox item.
+    #[css(skip)]
     Listitem,
     /// Menu Bar background
     #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]
@@ -1165,8 +1168,10 @@ pub enum Appearance {
     /// The dropdown button(s) that open up a dropdown list.
     MenulistButton,
     /// The text part of a dropdown list, to left of button.
+    #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]
     MenulistText,
     /// An editable textfield with a dropdown list (a combobox).
+    #[css(skip)]
     MenulistTextfield,
     /// Menu Popup background.
     #[parse(condition = "ParserContext::in_ua_or_chrome_sheet")]

From b6c88745e8b16434ffa4504f8673165e0edeb6fe Mon Sep 17 00:00:00 2001
From: Charlie Marlow <cmarlow@mozilla.com>
Date: Tue, 4 Jun 2019 22:04:14 +0000
Subject: [PATCH 05/32] style: adding support for CSS text underline offset to
 the style system.

Support for from-font listed in the CSS spec will be implemented in a later bug.

Differential Revision: https://phabricator.services.mozilla.com/D33233
---
 .../style/properties/longhands/inherited_text.mako.rs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/components/style/properties/longhands/inherited_text.mako.rs b/components/style/properties/longhands/inherited_text.mako.rs
index 58331efa6bd..cb319f18770 100644
--- a/components/style/properties/longhands/inherited_text.mako.rs
+++ b/components/style/properties/longhands/inherited_text.mako.rs
@@ -374,3 +374,14 @@ ${helpers.single_keyword(
     products="gecko",
     spec="Nonstandard",
 )}
+
+// text underline offset
+${helpers.predefined_type(
+    "text-underline-offset",
+    "LengthOrAuto",
+    "computed::LengthOrAuto::auto()",
+    products="gecko",
+    animation_value_type="ComputedValue",
+    gecko_pref="layout.css.text-underline-offset.enabled",
+    spec="https://drafts.csswg.org/css-text-decor-4/#underline-offset",
+)}

From e9b0d54a9505a0ae6085de992eef503644a567b8 Mon Sep 17 00:00:00 2001
From: Daniel Holbert <dholbert@cs.stanford.edu>
Date: Wed, 5 Jun 2019 18:23:02 +0000
Subject: [PATCH 06/32] style: Remove support for the
 layout.css.prefixes.webkit pref.

From this point on, the webkit-prefixed CSS features that were previously
protected by this pref will now be unconditionally enabled.

Differential Revision: https://phabricator.services.mozilla.com/D33807
---
 components/style/media_queries/media_feature_expression.rs  | 3 +--
 components/style/properties/data.py                         | 6 ------
 .../style/properties/longhands/inherited_text.mako.rs       | 3 ---
 .../style/properties/shorthands/inherited_text.mako.rs      | 1 -
 4 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/components/style/media_queries/media_feature_expression.rs b/components/style/media_queries/media_feature_expression.rs
index 3f8eefbfab5..ada30c4e895 100644
--- a/components/style/media_queries/media_feature_expression.rs
+++ b/components/style/media_queries/media_feature_expression.rs
@@ -298,8 +298,7 @@ impl MediaFeatureExpression {
 
                 #[cfg(feature = "gecko")]
                 {
-                    if unsafe { structs::StaticPrefs_sVarCache_layout_css_prefixes_webkit } &&
-                        starts_with_ignore_ascii_case(feature_name, "-webkit-")
+                    if starts_with_ignore_ascii_case(feature_name, "-webkit-")
                     {
                         feature_name = &feature_name[8..];
                         requirements.insert(ParsingRequirements::WEBKIT_PREFIX);
diff --git a/components/style/properties/data.py b/components/style/properties/data.py
index e771607b6d9..c363aa782cc 100644
--- a/components/style/properties/data.py
+++ b/components/style/properties/data.py
@@ -159,8 +159,6 @@ def parse_property_aliases(alias_list):
     if alias_list:
         for alias in alias_list.split():
             (name, _, pref) = alias.partition(":")
-            if name.startswith("-webkit-") and not pref:
-                pref = "layout.css.prefixes.webkit"
             result.append((name, pref))
     return result
 
@@ -542,10 +540,6 @@ class PropertiesData(object):
         #       See servo/servo#14941.
         if self.product == "gecko":
             for (prefix, pref) in property.extra_prefixes:
-                # All webkit prefixed properties are currently under
-                # control of this pref in Gecko currently.
-                if prefix == "webkit" and not pref:
-                    pref = "layout.css.prefixes.webkit"
                 property.alias.append(('-%s-%s' % (prefix, property.name), pref))
 
     def declare_longhand(self, name, products="gecko servo", **kwargs):
diff --git a/components/style/properties/longhands/inherited_text.mako.rs b/components/style/properties/longhands/inherited_text.mako.rs
index cb319f18770..45b3ee35aa1 100644
--- a/components/style/properties/longhands/inherited_text.mako.rs
+++ b/components/style/properties/longhands/inherited_text.mako.rs
@@ -291,7 +291,6 @@ ${helpers.predefined_type(
     "Color",
     "computed_value::T::currentcolor()",
     products="gecko",
-    gecko_pref="layout.css.prefixes.webkit",
     animation_value_type="AnimatedColor",
     ignored_when_colors_disabled=True,
     flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER",
@@ -306,7 +305,6 @@ ${helpers.predefined_type(
     products="gecko",
     animation_value_type="AnimatedColor",
     ignored_when_colors_disabled=True,
-    gecko_pref="layout.css.prefixes.webkit",
     flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER",
     spec="https://compat.spec.whatwg.org/#the-webkit-text-stroke-color",
 )}
@@ -318,7 +316,6 @@ ${helpers.predefined_type(
     initial_specified_value="specified::BorderSideWidth::zero()",
     computed_type="crate::values::computed::NonNegativeLength",
     products="gecko",
-    gecko_pref="layout.css.prefixes.webkit",
     flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER",
     spec="https://compat.spec.whatwg.org/#the-webkit-text-stroke-width",
     animation_value_type="discrete",
diff --git a/components/style/properties/shorthands/inherited_text.mako.rs b/components/style/properties/shorthands/inherited_text.mako.rs
index 091b5353b33..6c50ea0fb28 100644
--- a/components/style/properties/shorthands/inherited_text.mako.rs
+++ b/components/style/properties/shorthands/inherited_text.mako.rs
@@ -48,7 +48,6 @@
 <%helpers:shorthand name="-webkit-text-stroke"
                     sub_properties="-webkit-text-stroke-width
                                     -webkit-text-stroke-color"
-                    gecko_pref="layout.css.prefixes.webkit"
                     products="gecko"
                     derive_serialize="True"
                     spec="https://compat.spec.whatwg.org/#the-webkit-text-stroke">

From 3980dc31cd6e9d65deba57ea0cd5cf80b2ad8625 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Thu, 6 Jun 2019 04:08:52 +0000
Subject: [PATCH 07/32] style: Cleanup gradient parsing by removing the
 simple-moz-gradient parsing pref.

It looks like bug 1547939 will stick, given how fast the other regressions came
in for bug 1337655.

We haven't seen any regression from this, and it seems unlikely that we'd want
this code back.

This blocks further improvements to the style system. Simplifying this code
allows me to remove all the conversion code for gradients.

Let me know if you think it's premature and I'm happy to wait, but I really want
to see this code gone :)

Differential Revision: https://phabricator.services.mozilla.com/D33820
---
 components/style/gecko/conversions.rs         |  45 +--
 components/style/values/computed/image.rs     |  49 +---
 components/style/values/generics/image.rs     |  23 +-
 components/style/values/specified/image.rs    | 264 +++---------------
 components/style/values/specified/position.rs | 112 --------
 5 files changed, 50 insertions(+), 443 deletions(-)

diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs
index 0be8a81e66a..71e4de1819c 100644
--- a/components/style/gecko/conversions.rs
+++ b/components/style/gecko/conversions.rs
@@ -65,7 +65,6 @@ impl From<Angle> for CoordDataValue {
 }
 
 fn line_direction(horizontal: LengthPercentage, vertical: LengthPercentage) -> LineDirection {
-    use crate::values::computed::position::Position;
     use crate::values::specified::position::{X, Y};
 
     let horizontal_percentage = horizontal.as_percentage();
@@ -107,13 +106,7 @@ fn line_direction(horizontal: LengthPercentage, vertical: LengthPercentage) -> L
         }
     }
 
-    LineDirection::MozPosition(
-        Some(Position {
-            horizontal,
-            vertical,
-        }),
-        None,
-    )
+    unreachable!("Unexpected line direction");
 }
 
 impl nsStyleImage {
@@ -241,20 +234,10 @@ impl nsStyleImage {
                                 .set_value(CoordDataValue::Percent(percent_y));
                         }
                     },
-                    #[cfg(feature = "gecko")]
-                    LineDirection::MozPosition(position, angle) => unsafe {
-                        if let Some(position) = position {
-                            (*gecko_gradient).mBgPosX.set(position.horizontal);
-                            (*gecko_gradient).mBgPosY.set(position.vertical);
-                        }
-                        if let Some(angle) = angle {
-                            (*gecko_gradient).mAngle.set(angle);
-                        }
-                    },
                 }
                 gecko_gradient
             },
-            GradientKind::Radial(shape, position, angle) => {
+            GradientKind::Radial(shape, position) => {
                 let keyword_to_gecko_size = |keyword| match keyword {
                     ShapeExtent::ClosestSide => CLOSEST_SIDE,
                     ShapeExtent::FarthestSide => FARTHEST_SIDE,
@@ -294,13 +277,6 @@ impl nsStyleImage {
                     )
                 };
 
-                // Clear mBgPos field and set mAngle if angle is set. Otherwise clear it.
-                unsafe {
-                    if let Some(angle) = angle {
-                        (*gecko_gradient).mAngle.set(angle);
-                    }
-                }
-
                 // Setting radius values depending shape
                 match shape {
                     EndingShape::Circle(Circle::Radius(length)) => unsafe {
@@ -405,30 +381,19 @@ impl nsStyleImage {
         let gecko_gradient = bindings::Gecko_GetGradientImageValue(self)
             .as_ref()
             .unwrap();
-        let angle = Angle::from_gecko_style_coord(&gecko_gradient.mAngle);
         let horizontal_style = LengthPercentage::from_gecko_style_coord(&gecko_gradient.mBgPosX);
         let vertical_style = LengthPercentage::from_gecko_style_coord(&gecko_gradient.mBgPosY);
 
         let kind = match gecko_gradient.mShape as u32 {
             structs::NS_STYLE_GRADIENT_SHAPE_LINEAR => {
+                let angle = Angle::from_gecko_style_coord(&gecko_gradient.mAngle);
                 let line_direction = match (angle, horizontal_style, vertical_style) {
                     (Some(a), None, None) => LineDirection::Angle(a),
                     (None, Some(horizontal), Some(vertical)) => {
                         line_direction(horizontal, vertical)
                     },
-                    (Some(_), Some(horizontal), Some(vertical)) => LineDirection::MozPosition(
-                        Some(Position {
-                            horizontal,
-                            vertical,
-                        }),
-                        angle,
-                    ),
                     _ => {
-                        debug_assert!(
-                            horizontal_style.is_none() && vertical_style.is_none(),
-                            "Unexpected linear gradient direction"
-                        );
-                        LineDirection::MozPosition(None, None)
+                        unreachable!("unexpected line direction for linear gradient direction");
                     },
                 };
                 GradientKind::Linear(line_direction)
@@ -507,7 +472,7 @@ impl nsStyleImage {
                     },
                 };
 
-                GradientKind::Radial(shape, position, angle)
+                GradientKind::Radial(shape, position)
             },
         };
 
diff --git a/components/style/values/computed/image.rs b/components/style/values/computed/image.rs
index 70ecad6a04e..ea2f4e9530b 100644
--- a/components/style/values/computed/image.rs
+++ b/components/style/values/computed/image.rs
@@ -29,11 +29,11 @@ pub type Image = generic::Image<Gradient, MozImageRect, ComputedImageUrl>;
 /// Computed values for a CSS gradient.
 /// <https://drafts.csswg.org/css-images/#gradients>
 pub type Gradient =
-    generic::Gradient<LineDirection, Length, LengthPercentage, Position, Color, Angle>;
+    generic::Gradient<LineDirection, Length, LengthPercentage, Position, Color>;
 
 /// A computed gradient kind.
 pub type GradientKind =
-    generic::GradientKind<LineDirection, Length, LengthPercentage, Position, Angle>;
+    generic::GradientKind<LineDirection, Length, LengthPercentage, Position>;
 
 /// A computed gradient line direction.
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToResolvedValue)]
@@ -46,9 +46,6 @@ pub enum LineDirection {
     Vertical(Y),
     /// A corner.
     Corner(X, Y),
-    /// A Position and an Angle for legacy `-moz-` prefixed gradient.
-    #[cfg(feature = "gecko")]
-    MozPosition(Option<Position>, Option<Angle>),
 }
 
 /// A computed radial gradient ending shape.
@@ -69,19 +66,6 @@ impl generic::LineDirection for LineDirection {
             LineDirection::Angle(angle) => angle.radians() == PI,
             LineDirection::Vertical(Y::Bottom) if compat_mode == CompatMode::Modern => true,
             LineDirection::Vertical(Y::Top) if compat_mode != CompatMode::Modern => true,
-            LineDirection::Corner(..) => false,
-            #[cfg(feature = "gecko")]
-            LineDirection::MozPosition(
-                Some(Position {
-                    ref vertical,
-                    ref horizontal,
-                }),
-                None,
-            ) => {
-                // `50% 0%` is the default value for line direction.
-                horizontal.as_percentage().map_or(false, |p| p.0 == 0.5) &&
-                    vertical.as_percentage().map_or(false, |p| p.0 == 0.0)
-            },
             _ => false,
         }
     }
@@ -112,21 +96,6 @@ impl generic::LineDirection for LineDirection {
                 dest.write_str(" ")?;
                 y.to_css(dest)
             },
-            #[cfg(feature = "gecko")]
-            LineDirection::MozPosition(position, angle) => {
-                let mut need_space = false;
-                if let Some(position) = position {
-                    position.to_css(dest)?;
-                    need_space = true;
-                }
-                if let Some(angle) = angle {
-                    if need_space {
-                        dest.write_str(" ")?;
-                    }
-                    angle.to_css(dest)?;
-                }
-                Ok(())
-            },
         }
     }
 }
@@ -142,13 +111,6 @@ impl ToComputedValue for SpecifiedLineDirection {
             SpecifiedLineDirection::Horizontal(x) => LineDirection::Horizontal(x),
             SpecifiedLineDirection::Vertical(y) => LineDirection::Vertical(y),
             SpecifiedLineDirection::Corner(x, y) => LineDirection::Corner(x, y),
-            #[cfg(feature = "gecko")]
-            SpecifiedLineDirection::MozPosition(ref position, ref angle) => {
-                LineDirection::MozPosition(
-                    position.to_computed_value(context),
-                    angle.to_computed_value(context),
-                )
-            },
         }
     }
 
@@ -160,13 +122,6 @@ impl ToComputedValue for SpecifiedLineDirection {
             LineDirection::Horizontal(x) => SpecifiedLineDirection::Horizontal(x),
             LineDirection::Vertical(y) => SpecifiedLineDirection::Vertical(y),
             LineDirection::Corner(x, y) => SpecifiedLineDirection::Corner(x, y),
-            #[cfg(feature = "gecko")]
-            LineDirection::MozPosition(ref position, ref angle) => {
-                SpecifiedLineDirection::MozPosition(
-                    ToComputedValue::from_computed_value(position),
-                    ToComputedValue::from_computed_value(angle),
-                )
-            },
         }
     }
 }
diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs
index 1442ce604a2..c672efc2e7d 100644
--- a/components/style/values/generics/image.rs
+++ b/components/style/values/generics/image.rs
@@ -39,9 +39,9 @@ pub enum Image<Gradient, MozImageRect, ImageUrl> {
 /// A CSS gradient.
 /// <https://drafts.csswg.org/css-images/#gradients>
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
-pub struct Gradient<LineDirection, Length, LengthPercentage, Position, Color, Angle> {
+pub struct Gradient<LineDirection, Length, LengthPercentage, Position, Color> {
     /// Gradients can be linear or radial.
-    pub kind: GradientKind<LineDirection, Length, LengthPercentage, Position, Angle>,
+    pub kind: GradientKind<LineDirection, Length, LengthPercentage, Position>,
     /// The color stops and interpolation hints.
     pub items: Vec<GradientItem<Color, LengthPercentage>>,
     /// True if this is a repeating gradient.
@@ -63,15 +63,11 @@ pub enum CompatMode {
 
 /// A gradient kind.
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
-pub enum GradientKind<LineDirection, Length, LengthPercentage, Position, Angle> {
+pub enum GradientKind<LineDirection, Length, LengthPercentage, Position> {
     /// A linear gradient.
     Linear(LineDirection),
     /// A radial gradient.
-    Radial(
-        EndingShape<Length, LengthPercentage>,
-        Position,
-        Option<Angle>,
-    ),
+    Radial(EndingShape<Length, LengthPercentage>, Position),
 }
 
 /// A radial gradient's ending shape.
@@ -268,14 +264,13 @@ where
     }
 }
 
-impl<D, L, LoP, P, C, A> ToCss for Gradient<D, L, LoP, P, C, A>
+impl<D, L, LoP, P, C> ToCss for Gradient<D, L, LoP, P, C>
 where
     D: LineDirection,
     L: ToCss,
     LoP: ToCss,
     P: ToCss,
     C: ToCss,
-    A: ToCss,
 {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
@@ -300,7 +295,7 @@ where
                 direction.to_css(dest, self.compat_mode)?;
                 false
             },
-            GradientKind::Radial(ref shape, ref position, ref angle) => {
+            GradientKind::Radial(ref shape, ref position) => {
                 let omit_shape = match *shape {
                     EndingShape::Ellipse(Ellipse::Extent(ShapeExtent::Cover)) |
                     EndingShape::Ellipse(Ellipse::Extent(ShapeExtent::FarthestCorner)) => true,
@@ -315,10 +310,6 @@ where
                     position.to_css(dest)?;
                 } else {
                     position.to_css(dest)?;
-                    if let Some(ref a) = *angle {
-                        dest.write_str(" ")?;
-                        a.to_css(dest)?;
-                    }
                     if !omit_shape {
                         dest.write_str(", ")?;
                         shape.to_css(dest)?;
@@ -338,7 +329,7 @@ where
     }
 }
 
-impl<D, L, LoP, P, A> GradientKind<D, L, LoP, P, A> {
+impl<D, L, LoP, P> GradientKind<D, L, LoP, P> {
     fn label(&self) -> &str {
         match *self {
             GradientKind::Linear(..) => "linear",
diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs
index ca7f687bfea..9779c6fa2f6 100644
--- a/components/style/values/specified/image.rs
+++ b/components/style/values/specified/image.rs
@@ -8,16 +8,12 @@
 //! [image]: https://drafts.csswg.org/css-images/#image-values
 
 use crate::custom_properties::SpecifiedValue;
-#[cfg(feature = "gecko")]
-use crate::gecko_bindings::structs;
 use crate::parser::{Parse, ParserContext};
 use crate::stylesheets::CorsMode;
-#[cfg(feature = "gecko")]
-use crate::values::computed::{Context, Position as ComputedPosition, ToComputedValue};
 use crate::values::generics::image::PaintWorklet;
 use crate::values::generics::image::{self as generic, Circle, CompatMode, Ellipse, ShapeExtent};
 use crate::values::generics::position::Position as GenericPosition;
-use crate::values::specified::position::{LegacyPosition, Position, PositionComponent, Side, X, Y};
+use crate::values::specified::position::{Position, PositionComponent, Side, X, Y};
 use crate::values::specified::url::SpecifiedImageUrl;
 use crate::values::specified::{Angle, Color, Length, LengthPercentage};
 use crate::values::specified::{Number, NumberOrPercentage, Percentage};
@@ -55,15 +51,7 @@ pub type Image = generic::Image<Gradient, MozImageRect, SpecifiedImageUrl>;
 
 /// Specified values for a CSS gradient.
 /// <https://drafts.csswg.org/css-images/#gradients>
-#[cfg(not(feature = "gecko"))]
-pub type Gradient =
-    generic::Gradient<LineDirection, Length, LengthPercentage, Position, Color, Angle>;
-
-/// Specified values for a CSS gradient.
-/// <https://drafts.csswg.org/css-images/#gradients>
-#[cfg(feature = "gecko")]
-pub type Gradient =
-    generic::Gradient<LineDirection, Length, LengthPercentage, GradientPosition, Color, Angle>;
+pub type Gradient = generic::Gradient<LineDirection, Length, LengthPercentage, Position, Color>;
 
 impl SpecifiedValueInfo for Gradient {
     const SUPPORTED_TYPES: u8 = CssType::GRADIENT;
@@ -89,16 +77,11 @@ impl SpecifiedValueInfo for Gradient {
 }
 
 /// A specified gradient kind.
-#[cfg(not(feature = "gecko"))]
-pub type GradientKind =
-    generic::GradientKind<LineDirection, Length, LengthPercentage, Position, Angle>;
-
-/// A specified gradient kind.
-#[cfg(feature = "gecko")]
-pub type GradientKind =
-    generic::GradientKind<LineDirection, Length, LengthPercentage, GradientPosition, Angle>;
+pub type GradientKind = generic::GradientKind<LineDirection, Length, LengthPercentage, Position>;
 
 /// A specified gradient line direction.
+///
+/// FIXME(emilio): This should be generic over Angle.
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToShmem)]
 pub enum LineDirection {
     /// An angular direction.
@@ -109,22 +92,6 @@ pub enum LineDirection {
     Vertical(Y),
     /// A direction towards a corner of a box.
     Corner(X, Y),
-    /// A Position and an Angle for legacy `-moz-` prefixed gradient.
-    /// `-moz-` prefixed linear gradient can contain both a position and an angle but it
-    /// uses legacy syntax for position. That means we can't specify both keyword and
-    /// length for each horizontal/vertical components.
-    #[cfg(feature = "gecko")]
-    MozPosition(Option<LegacyPosition>, Option<Angle>),
-}
-
-/// A binary enum to hold either Position or LegacyPosition.
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss, ToShmem)]
-#[cfg(feature = "gecko")]
-pub enum GradientPosition {
-    /// 1, 2, 3, 4-valued <position>.
-    Modern(Position),
-    /// 1, 2-valued <position>.
-    Legacy(LegacyPosition),
 }
 
 /// A specified ending shape.
@@ -434,18 +401,8 @@ impl Gradient {
                 let shape = generic::EndingShape::Circle(rad);
                 let position: Position = point.into();
 
-                #[cfg(feature = "gecko")]
-                {
-                    let pos = GradientPosition::Modern(position);
-                    let kind = generic::GradientKind::Radial(shape, pos, None);
-                    (kind, reverse_stops)
-                }
-
-                #[cfg(not(feature = "gecko"))]
-                {
-                    let kind = generic::GradientKind::Radial(shape, position, None);
-                    (kind, reverse_stops)
-                }
+                let kind = generic::GradientKind::Radial(shape, position);
+                (kind, reverse_stops)
             },
             _ => {
                 let e = SelectorParseErrorKind::UnexpectedIdent(ident.clone());
@@ -544,16 +501,6 @@ impl Gradient {
     }
 }
 
-#[inline]
-fn simple_moz_gradient() -> bool {
-    #[cfg(feature = "gecko")]
-    unsafe {
-        return structs::StaticPrefs_sVarCache_layout_css_simple_moz_gradient_enabled;
-    }
-    #[cfg(not(feature = "gecko"))]
-    return false;
-}
-
 impl GradientKind {
     /// Parses a linear gradient.
     /// CompatMode can change during `-moz-` prefixed gradient parsing if it come across a `to` keyword.
@@ -574,46 +521,19 @@ impl GradientKind {
         };
         Ok(generic::GradientKind::Linear(direction))
     }
-
     fn parse_radial<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
         compat_mode: &mut CompatMode,
     ) -> Result<Self, ParseError<'i>> {
-        let (shape, position, angle, moz_position) = match *compat_mode {
+        let (shape, position) = match *compat_mode {
             CompatMode::Modern => {
                 let shape = input.try(|i| EndingShape::parse(context, i, *compat_mode));
                 let position = input.try(|i| {
                     i.expect_ident_matching("at")?;
                     Position::parse(context, i)
                 });
-                (shape, position.ok(), None, None)
-            },
-            // The syntax of `-moz-` prefixed radial gradient is:
-            // -moz-radial-gradient(
-            //   [ [ <position> || <angle> ]?  [ ellipse | [ <length> | <percentage> ]{2} ] , |
-            //     [ <position> || <angle> ]?  [ [ circle | ellipse ] | <extent-keyword> ] , |
-            //   ]?
-            //   <color-stop> [ , <color-stop> ]+
-            // )
-            // where <extent-keyword> = closest-corner | closest-side | farthest-corner | farthest-side |
-            //                          cover | contain
-            // and <color-stop>     = <color> [ <percentage> | <length> ]?
-            CompatMode::Moz if !simple_moz_gradient() => {
-                let mut position = input.try(|i| LegacyPosition::parse(context, i));
-                let angle = input.try(|i| Angle::parse(context, i)).ok();
-                if position.is_err() {
-                    position = input.try(|i| LegacyPosition::parse(context, i));
-                }
-
-                let shape = input.try(|i| {
-                    if position.is_ok() || angle.is_some() {
-                        i.expect_comma()?;
-                    }
-                    EndingShape::parse(context, i, *compat_mode)
-                });
-
-                (shape, None, angle, position.ok())
+                (shape, position.ok())
             },
             _ => {
                 let position = input.try(|i| Position::parse(context, i));
@@ -623,11 +543,11 @@ impl GradientKind {
                     }
                     EndingShape::parse(context, i, *compat_mode)
                 });
-                (shape, position.ok(), None, None)
+                (shape, position.ok())
             },
         };
 
-        if shape.is_ok() || position.is_some() || angle.is_some() || moz_position.is_some() {
+        if shape.is_ok() || position.is_some() {
             input.expect_comma()?;
         }
 
@@ -635,35 +555,8 @@ impl GradientKind {
             generic::EndingShape::Ellipse(Ellipse::Extent(ShapeExtent::FarthestCorner))
         });
 
-        #[cfg(feature = "gecko")]
-        {
-            if *compat_mode == CompatMode::Moz && !simple_moz_gradient() {
-                // If this form can be represented in Modern mode, then convert the compat_mode to Modern.
-                if angle.is_none() {
-                    *compat_mode = CompatMode::Modern;
-                }
-                let position = moz_position.unwrap_or(LegacyPosition::center());
-                return Ok(generic::GradientKind::Radial(
-                    shape,
-                    GradientPosition::Legacy(position),
-                    angle,
-                ));
-            }
-        }
-
         let position = position.unwrap_or(Position::center());
-        #[cfg(feature = "gecko")]
-        {
-            return Ok(generic::GradientKind::Radial(
-                shape,
-                GradientPosition::Modern(position),
-                angle,
-            ));
-        }
-        #[cfg(not(feature = "gecko"))]
-        {
-            return Ok(generic::GradientKind::Radial(shape, position, angle));
-        }
+        Ok(generic::GradientKind::Radial(shape, position))
     }
 }
 
@@ -673,35 +566,6 @@ impl generic::LineDirection for LineDirection {
             LineDirection::Angle(ref angle) => angle.degrees() == 180.0,
             LineDirection::Vertical(Y::Bottom) if compat_mode == CompatMode::Modern => true,
             LineDirection::Vertical(Y::Top) if compat_mode != CompatMode::Modern => true,
-            #[cfg(feature = "gecko")]
-            LineDirection::MozPosition(
-                Some(LegacyPosition {
-                    horizontal: ref x,
-                    vertical: ref y,
-                }),
-                None,
-            ) => {
-                use crate::values::computed::Percentage as ComputedPercentage;
-                use crate::values::specified::transform::OriginComponent;
-
-                // `50% 0%` is the default value for line direction.
-                // These percentage values can also be keywords.
-                let x = match *x {
-                    OriginComponent::Center => true,
-                    OriginComponent::Length(LengthPercentage::Percentage(ComputedPercentage(
-                        val,
-                    ))) => val == 0.5,
-                    _ => false,
-                };
-                let y = match *y {
-                    OriginComponent::Side(Y::Top) => true,
-                    OriginComponent::Length(LengthPercentage::Percentage(ComputedPercentage(
-                        val,
-                    ))) => val == 0.0,
-                    _ => false,
-                };
-                x && y
-            },
             _ => false,
         }
     }
@@ -732,21 +596,6 @@ impl generic::LineDirection for LineDirection {
                 dest.write_str(" ")?;
                 y.to_css(dest)
             },
-            #[cfg(feature = "gecko")]
-            LineDirection::MozPosition(ref position, ref angle) => {
-                let mut need_space = false;
-                if let Some(ref position) = *position {
-                    position.to_css(dest)?;
-                    need_space = true;
-                }
-                if let Some(ref angle) = *angle {
-                    if need_space {
-                        dest.write_str(" ")?;
-                    }
-                    angle.to_css(dest)?;
-                }
-                Ok(())
-            },
         }
     }
 }
@@ -757,16 +606,11 @@ impl LineDirection {
         input: &mut Parser<'i, 't>,
         compat_mode: &mut CompatMode,
     ) -> Result<Self, ParseError<'i>> {
-        let mut _angle = if *compat_mode == CompatMode::Moz && !simple_moz_gradient() {
-            input.try(|i| Angle::parse(context, i)).ok()
-        } else {
-            // Gradients allow unitless zero angles as an exception, see:
-            // https://github.com/w3c/csswg-drafts/issues/1162
-            if let Ok(angle) = input.try(|i| Angle::parse_with_unitless(context, i)) {
-                return Ok(LineDirection::Angle(angle));
-            }
-            None
-        };
+        // Gradients allow unitless zero angles as an exception, see:
+        // https://github.com/w3c/csswg-drafts/issues/1162
+        if let Ok(angle) = input.try(|i| Angle::parse_with_unitless(context, i)) {
+            return Ok(LineDirection::Angle(angle));
+        }
 
         input.try(|i| {
             let to_ident = i.try(|i| i.expect_ident_matching("to"));
@@ -787,22 +631,6 @@ impl LineDirection {
                 _ => {},
             }
 
-            #[cfg(feature = "gecko")]
-            {
-                // `-moz-` prefixed linear gradient can be both Angle and Position.
-                if *compat_mode == CompatMode::Moz && !simple_moz_gradient() {
-                    let position = i.try(|i| LegacyPosition::parse(context, i)).ok();
-                    if _angle.is_none() {
-                        _angle = i.try(|i| Angle::parse(context, i)).ok();
-                    };
-
-                    if _angle.is_none() && position.is_none() {
-                        return Err(i.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-                    }
-                    return Ok(LineDirection::MozPosition(position, _angle));
-                }
-            }
-
             if let Ok(x) = i.try(X::parse) {
                 if let Ok(y) = i.try(Y::parse) {
                     return Ok(LineDirection::Corner(x, y));
@@ -818,22 +646,6 @@ impl LineDirection {
     }
 }
 
-#[cfg(feature = "gecko")]
-impl ToComputedValue for GradientPosition {
-    type ComputedValue = ComputedPosition;
-
-    fn to_computed_value(&self, context: &Context) -> ComputedPosition {
-        match *self {
-            GradientPosition::Modern(ref pos) => pos.to_computed_value(context),
-            GradientPosition::Legacy(ref pos) => pos.to_computed_value(context),
-        }
-    }
-
-    fn from_computed_value(computed: &ComputedPosition) -> Self {
-        GradientPosition::Modern(ToComputedValue::from_computed_value(computed))
-    }
-}
-
 impl EndingShape {
     fn parse<'i, 't>(
         context: &ParserContext,
@@ -878,35 +690,31 @@ impl EndingShape {
                 ShapeExtent::FarthestCorner,
             )));
         }
-        // -moz- prefixed radial gradient doesn't allow EndingShape's Length or LengthPercentage
-        // to come before shape keyword. Otherwise it conflicts with <position>.
-        if compat_mode != CompatMode::Moz || simple_moz_gradient() {
-            if let Ok(length) = input.try(|i| Length::parse(context, i)) {
-                if let Ok(y) = input.try(|i| LengthPercentage::parse(context, i)) {
-                    if compat_mode == CompatMode::Modern {
-                        let _ = input.try(|i| i.expect_ident_matching("ellipse"));
-                    }
+        if let Ok(length) = input.try(|i| Length::parse(context, i)) {
+            if let Ok(y) = input.try(|i| LengthPercentage::parse(context, i)) {
+                if compat_mode == CompatMode::Modern {
+                    let _ = input.try(|i| i.expect_ident_matching("ellipse"));
+                }
+                return Ok(generic::EndingShape::Ellipse(Ellipse::Radii(
+                    length.into(),
+                    y,
+                )));
+            }
+            if compat_mode == CompatMode::Modern {
+                let y = input.try(|i| {
+                    i.expect_ident_matching("ellipse")?;
+                    LengthPercentage::parse(context, i)
+                });
+                if let Ok(y) = y {
                     return Ok(generic::EndingShape::Ellipse(Ellipse::Radii(
                         length.into(),
                         y,
                     )));
                 }
-                if compat_mode == CompatMode::Modern {
-                    let y = input.try(|i| {
-                        i.expect_ident_matching("ellipse")?;
-                        LengthPercentage::parse(context, i)
-                    });
-                    if let Ok(y) = y {
-                        return Ok(generic::EndingShape::Ellipse(Ellipse::Radii(
-                            length.into(),
-                            y,
-                        )));
-                    }
-                    let _ = input.try(|i| i.expect_ident_matching("circle"));
-                }
-
-                return Ok(generic::EndingShape::Circle(Circle::Radius(length)));
+                let _ = input.try(|i| i.expect_ident_matching("circle"));
             }
+
+            return Ok(generic::EndingShape::Circle(Circle::Radius(length)));
         }
         input.try(|i| {
             let x = Percentage::parse(context, i)?;
diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs
index c0ac56c9822..d9d75343331 100644
--- a/components/style/values/specified/position.rs
+++ b/components/style/values/specified/position.rs
@@ -14,7 +14,6 @@ use crate::values::computed::LengthPercentage as ComputedLengthPercentage;
 use crate::values::computed::{Context, Percentage, ToComputedValue};
 use crate::values::generics::position::Position as GenericPosition;
 use crate::values::generics::position::ZIndex as GenericZIndex;
-use crate::values::specified::transform::OriginComponent;
 use crate::values::specified::{AllowQuirks, Integer, LengthPercentage};
 use crate::values::{Either, None_};
 use crate::Zero;
@@ -326,117 +325,6 @@ impl Side for Y {
     }
 }
 
-/// The specified value of a legacy CSS `<position>`
-/// Modern position syntax supports 3 and 4-value syntax. That means:
-/// If three or four values are given, then each <percentage> or <length> represents an offset
-/// and must be preceded by a keyword, which specifies from which edge the offset is given.
-/// For example, `bottom 10px right 20px` represents a `10px` vertical
-/// offset up from the bottom edge and a `20px` horizontal offset leftward from the right edge.
-/// If three values are given, the missing offset is assumed to be zero.
-/// But for some historical reasons we need to keep CSS Level 2 syntax which only supports up to
-/// 2-value. This type represents this 2-value syntax.
-pub type LegacyPosition = GenericPosition<LegacyHPosition, LegacyVPosition>;
-
-/// The specified value of a horizontal position.
-pub type LegacyHPosition = OriginComponent<X>;
-
-/// The specified value of a vertical position.
-pub type LegacyVPosition = OriginComponent<Y>;
-
-impl Parse for LegacyPosition {
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        Self::parse_quirky(context, input, AllowQuirks::No)
-    }
-}
-
-impl LegacyPosition {
-    /// Parses a `<position>`, with quirks.
-    pub fn parse_quirky<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-        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| OriginComponent::parse(context, i)) {
-                    return Ok(Self::new(x_pos, y_pos));
-                }
-                let x_pos = input
-                    .try(|i| OriginComponent::parse(context, i))
-                    .unwrap_or(x_pos);
-                let y_pos = OriginComponent::Center;
-                return Ok(Self::new(x_pos, y_pos));
-            },
-            Ok(OriginComponent::Side(x_keyword)) => {
-                if let Ok(y_keyword) = input.try(Y::parse) {
-                    let x_pos = OriginComponent::Side(x_keyword);
-                    let y_pos = OriginComponent::Side(y_keyword);
-                    return Ok(Self::new(x_pos, y_pos));
-                }
-                let x_pos = OriginComponent::Side(x_keyword);
-                if let Ok(y_lp) =
-                    input.try(|i| LengthPercentage::parse_quirky(context, i, allow_quirks))
-                {
-                    return Ok(Self::new(x_pos, OriginComponent::Length(y_lp)));
-                }
-                let _ = input.try(|i| i.expect_ident_matching("center"));
-                return Ok(Self::new(x_pos, OriginComponent::Center));
-            },
-            Ok(x_pos @ OriginComponent::Length(_)) => {
-                if let Ok(y_keyword) = input.try(Y::parse) {
-                    let y_pos = OriginComponent::Side(y_keyword);
-                    return Ok(Self::new(x_pos, y_pos));
-                }
-                if let Ok(y_lp) =
-                    input.try(|i| LengthPercentage::parse_quirky(context, i, allow_quirks))
-                {
-                    let y_pos = OriginComponent::Length(y_lp);
-                    return Ok(Self::new(x_pos, y_pos));
-                }
-                let _ = input.try(|i| i.expect_ident_matching("center"));
-                return Ok(Self::new(x_pos, OriginComponent::Center));
-            },
-            Err(_) => {},
-        }
-        let y_keyword = Y::parse(input)?;
-        let x_pos: Result<_, ParseError> = input.try(|i| {
-            if let Ok(x_keyword) = i.try(X::parse) {
-                let x_pos = OriginComponent::Side(x_keyword);
-                return Ok(x_pos);
-            }
-            i.expect_ident_matching("center")?;
-            Ok(OriginComponent::Center)
-        });
-        if let Ok(x_pos) = x_pos {
-            let y_pos = OriginComponent::Side(y_keyword);
-            return Ok(Self::new(x_pos, y_pos));
-        }
-        let x_pos = OriginComponent::Center;
-        let y_pos = OriginComponent::Side(y_keyword);
-        Ok(Self::new(x_pos, y_pos))
-    }
-
-    /// `center center`
-    #[inline]
-    pub fn center() -> Self {
-        Self::new(OriginComponent::Center, OriginComponent::Center)
-    }
-}
-
-impl ToCss for LegacyPosition {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        self.horizontal.to_css(dest)?;
-        dest.write_str(" ")?;
-        self.vertical.to_css(dest)
-    }
-}
-
 #[derive(
     Clone,
     Copy,

From b70a9fe56424f832e6ff8adc3675168971b2688d Mon Sep 17 00:00:00 2001
From: Charlie Marlow <cmarlow@mozilla.com>
Date: Thu, 6 Jun 2019 17:12:22 +0000
Subject: [PATCH 08/32] style: added text-decoration-width support to style
 system.

from-font support will be added later

Differential Revision: https://phabricator.services.mozilla.com/D33392
---
 components/style/properties/longhands/text.mako.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/components/style/properties/longhands/text.mako.rs b/components/style/properties/longhands/text.mako.rs
index 723e26ae22b..91412d2ee6e 100644
--- a/components/style/properties/longhands/text.mako.rs
+++ b/components/style/properties/longhands/text.mako.rs
@@ -69,3 +69,13 @@ ${helpers.predefined_type(
     gecko_pref="layout.css.initial-letter.enabled",
     spec="https://drafts.csswg.org/css-inline/#sizing-drop-initials",
 )}
+
+${helpers.predefined_type(
+   "text-decoration-width",
+   "LengthOrAuto",
+   "computed::LengthOrAuto::auto()",
+   products="gecko",
+   animation_value_type="ComputedValue",
+   gecko_pref="layout.css.text-decoration-width.enabled",
+   spec="https://drafts.csswg.org/css-text-decor-4/#text-decoration-width-property"
+)}

From ad142f8f2f035360eff197cea233741b5529f012 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Thu, 6 Jun 2019 19:18:28 -0400
Subject: [PATCH 09/32] style: Handle default angle in radians.

We were not copying the angle in this case for some reason, and I overlooked
this.
---
 components/style/gecko/conversions.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs
index 71e4de1819c..68f8d2327d5 100644
--- a/components/style/gecko/conversions.rs
+++ b/components/style/gecko/conversions.rs
@@ -389,6 +389,7 @@ impl nsStyleImage {
                 let angle = Angle::from_gecko_style_coord(&gecko_gradient.mAngle);
                 let line_direction = match (angle, horizontal_style, vertical_style) {
                     (Some(a), None, None) => LineDirection::Angle(a),
+                    (None, None, None) => LineDirection::Angle(Angle::from_radians(PI)),
                     (None, Some(horizontal), Some(vertical)) => {
                         line_direction(horizontal, vertical)
                     },

From a4690ce158a34553c781cf138b033fd2ec27e420 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Fri, 7 Jun 2019 14:13:17 +0000
Subject: [PATCH 10/32] style: Use cbindgen for gradients.

Differential Revision: https://phabricator.services.mozilla.com/D33901
---
 components/style/gecko/conversions.rs         | 347 +-----------------
 components/style/properties/gecko.mako.rs     |  17 +-
 .../properties/longhands/background.mako.rs   |   4 +-
 .../style/properties/longhands/border.mako.rs |   4 +-
 .../style/properties/longhands/svg.mako.rs    |   4 +-
 components/style/values/computed/image.rs     |  38 +-
 components/style/values/generics/image.rs     |  79 +++-
 components/style/values/specified/image.rs    | 127 +++----
 components/style/values/specified/position.rs |  30 +-
 .../style/values/specified/transform.rs       |  14 +-
 10 files changed, 190 insertions(+), 474 deletions(-)

diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs
index 68f8d2327d5..4ce44136279 100644
--- a/components/style/gecko/conversions.rs
+++ b/components/style/gecko/conversions.rs
@@ -16,18 +16,16 @@ use crate::gecko_bindings::structs::{self, nsStyleCoord_CalcValue, Matrix4x4Comp
 use crate::gecko_bindings::structs::{nsStyleImage, nsresult};
 use crate::gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataValue};
 use crate::stylesheets::RulesMutateError;
-use crate::values::computed::image::LineDirection;
 use crate::values::computed::transform::Matrix3D;
 use crate::values::computed::url::ComputedImageUrl;
 use crate::values::computed::{Angle, Gradient, Image};
 use crate::values::computed::{Integer, LengthPercentage};
 use crate::values::computed::{Length, Percentage, TextAlign};
 use crate::values::generics::grid::{TrackListValue, TrackSize};
-use crate::values::generics::image::{CompatMode, Image as GenericImage};
+use crate::values::generics::image::GenericImage;
 use crate::values::generics::rect::Rect;
 use crate::Zero;
 use app_units::Au;
-use std::f32::consts::PI;
 use style_traits::values::specified::AllowedNumericType;
 
 impl From<LengthPercentage> for nsStyleCoord_CalcValue {
@@ -64,56 +62,11 @@ impl From<Angle> for CoordDataValue {
     }
 }
 
-fn line_direction(horizontal: LengthPercentage, vertical: LengthPercentage) -> LineDirection {
-    use crate::values::specified::position::{X, Y};
-
-    let horizontal_percentage = horizontal.as_percentage();
-    let vertical_percentage = vertical.as_percentage();
-
-    let horizontal_as_corner = horizontal_percentage.and_then(|percentage| {
-        if percentage.0 == 0.0 {
-            Some(X::Left)
-        } else if percentage.0 == 1.0 {
-            Some(X::Right)
-        } else {
-            None
-        }
-    });
-
-    let vertical_as_corner = vertical_percentage.and_then(|percentage| {
-        if percentage.0 == 0.0 {
-            Some(Y::Top)
-        } else if percentage.0 == 1.0 {
-            Some(Y::Bottom)
-        } else {
-            None
-        }
-    });
-
-    if let (Some(hc), Some(vc)) = (horizontal_as_corner, vertical_as_corner) {
-        return LineDirection::Corner(hc, vc);
-    }
-
-    if let Some(hc) = horizontal_as_corner {
-        if vertical_percentage == Some(Percentage(0.5)) {
-            return LineDirection::Horizontal(hc);
-        }
-    }
-
-    if let Some(vc) = vertical_as_corner {
-        if horizontal_percentage == Some(Percentage(0.5)) {
-            return LineDirection::Vertical(vc);
-        }
-    }
-
-    unreachable!("Unexpected line direction");
-}
-
 impl nsStyleImage {
     /// Set a given Servo `Image` value into this `nsStyleImage`.
     pub fn set(&mut self, image: Image) {
         match image {
-            GenericImage::Gradient(boxed_gradient) => self.set_gradient(*boxed_gradient),
+            GenericImage::Gradient(boxed_gradient) => self.set_gradient(boxed_gradient),
             GenericImage::Url(ref url) => unsafe {
                 bindings::Gecko_SetLayerImageImageValue(self, url);
             },
@@ -144,172 +97,9 @@ impl nsStyleImage {
         }
     }
 
-    // FIXME(emilio): This is really complex, we should use cbindgen for this.
-    fn set_gradient(&mut self, gradient: Gradient) {
-        use self::structs::NS_STYLE_GRADIENT_SIZE_CLOSEST_CORNER as CLOSEST_CORNER;
-        use self::structs::NS_STYLE_GRADIENT_SIZE_CLOSEST_SIDE as CLOSEST_SIDE;
-        use self::structs::NS_STYLE_GRADIENT_SIZE_FARTHEST_CORNER as FARTHEST_CORNER;
-        use self::structs::NS_STYLE_GRADIENT_SIZE_FARTHEST_SIDE as FARTHEST_SIDE;
-        use crate::values::generics::image::{
-            Circle, Ellipse, EndingShape, GradientKind, ShapeExtent,
-        };
-        use crate::values::specified::position::{X, Y};
-
-        let stop_count = gradient.items.len();
-        if stop_count >= ::std::u32::MAX as usize {
-            warn!("stylo: Prevented overflow due to too many gradient stops");
-            return;
-        }
-
-        let gecko_gradient = match gradient.kind {
-            GradientKind::Linear(direction) => {
-                let gecko_gradient = unsafe {
-                    bindings::Gecko_CreateGradient(
-                        structs::NS_STYLE_GRADIENT_SHAPE_LINEAR as u8,
-                        structs::NS_STYLE_GRADIENT_SIZE_FARTHEST_CORNER as u8,
-                        gradient.repeating,
-                        gradient.compat_mode != CompatMode::Modern,
-                        gradient.compat_mode == CompatMode::Moz,
-                        stop_count as u32,
-                    )
-                };
-
-                match direction {
-                    LineDirection::Angle(angle) => {
-                        // PI radians (180deg) is ignored because it is the default value.
-                        if angle.radians() != PI {
-                            unsafe {
-                                (*gecko_gradient).mAngle.set(angle);
-                            }
-                        }
-                    },
-                    LineDirection::Horizontal(x) => {
-                        let x = match x {
-                            X::Left => 0.0,
-                            X::Right => 1.0,
-                        };
-
-                        unsafe {
-                            (*gecko_gradient)
-                                .mBgPosX
-                                .set_value(CoordDataValue::Percent(x));
-                            (*gecko_gradient)
-                                .mBgPosY
-                                .set_value(CoordDataValue::Percent(0.5));
-                        }
-                    },
-                    LineDirection::Vertical(y) => {
-                        // Although bottom is the default value, we can not ignore
-                        // it here, because the rendering code of Gecko relies on
-                        // this to behave correctly for legacy mode.
-                        let y = match y {
-                            Y::Top => 0.0,
-                            Y::Bottom => 1.0,
-                        };
-                        unsafe {
-                            (*gecko_gradient)
-                                .mBgPosX
-                                .set_value(CoordDataValue::Percent(0.5));
-                            (*gecko_gradient)
-                                .mBgPosY
-                                .set_value(CoordDataValue::Percent(y));
-                        }
-                    },
-                    LineDirection::Corner(horiz, vert) => {
-                        let percent_x = match horiz {
-                            X::Left => 0.0,
-                            X::Right => 1.0,
-                        };
-                        let percent_y = match vert {
-                            Y::Top => 0.0,
-                            Y::Bottom => 1.0,
-                        };
-
-                        unsafe {
-                            (*gecko_gradient)
-                                .mBgPosX
-                                .set_value(CoordDataValue::Percent(percent_x));
-                            (*gecko_gradient)
-                                .mBgPosY
-                                .set_value(CoordDataValue::Percent(percent_y));
-                        }
-                    },
-                }
-                gecko_gradient
-            },
-            GradientKind::Radial(shape, position) => {
-                let keyword_to_gecko_size = |keyword| match keyword {
-                    ShapeExtent::ClosestSide => CLOSEST_SIDE,
-                    ShapeExtent::FarthestSide => FARTHEST_SIDE,
-                    ShapeExtent::ClosestCorner => CLOSEST_CORNER,
-                    ShapeExtent::FarthestCorner => FARTHEST_CORNER,
-                    ShapeExtent::Contain => CLOSEST_SIDE,
-                    ShapeExtent::Cover => FARTHEST_CORNER,
-                };
-                let (gecko_shape, gecko_size) = match shape {
-                    EndingShape::Circle(ref circle) => {
-                        let size = match *circle {
-                            Circle::Extent(extent) => keyword_to_gecko_size(extent),
-                            _ => structs::NS_STYLE_GRADIENT_SIZE_EXPLICIT_SIZE,
-                        };
-                        (structs::NS_STYLE_GRADIENT_SHAPE_CIRCULAR as u8, size as u8)
-                    },
-                    EndingShape::Ellipse(ref ellipse) => {
-                        let size = match *ellipse {
-                            Ellipse::Extent(extent) => keyword_to_gecko_size(extent),
-                            _ => structs::NS_STYLE_GRADIENT_SIZE_EXPLICIT_SIZE,
-                        };
-                        (
-                            structs::NS_STYLE_GRADIENT_SHAPE_ELLIPTICAL as u8,
-                            size as u8,
-                        )
-                    },
-                };
-
-                let gecko_gradient = unsafe {
-                    bindings::Gecko_CreateGradient(
-                        gecko_shape,
-                        gecko_size,
-                        gradient.repeating,
-                        gradient.compat_mode == CompatMode::Moz,
-                        gradient.compat_mode == CompatMode::Moz,
-                        stop_count as u32,
-                    )
-                };
-
-                // Setting radius values depending shape
-                match shape {
-                    EndingShape::Circle(Circle::Radius(length)) => unsafe {
-                        let au = length.to_i32_au();
-                        (*gecko_gradient)
-                            .mRadiusX
-                            .set_value(CoordDataValue::Coord(au));
-                        (*gecko_gradient)
-                            .mRadiusY
-                            .set_value(CoordDataValue::Coord(au));
-                    },
-                    EndingShape::Ellipse(Ellipse::Radii(x, y)) => unsafe {
-                        (*gecko_gradient).mRadiusX.set(x);
-                        (*gecko_gradient).mRadiusY.set(y);
-                    },
-                    _ => {},
-                }
-                unsafe {
-                    (*gecko_gradient).mBgPosX.set(position.horizontal);
-                    (*gecko_gradient).mBgPosY.set(position.vertical);
-                }
-
-                gecko_gradient
-            },
-        };
-
-        for (index, item) in gradient.items.into_iter().enumerate() {
-            let gecko_stop = unsafe { &mut (*gecko_gradient).mStops[index] };
-            *gecko_stop = item;
-        }
-
+    fn set_gradient(&mut self, gradient: Box<Gradient>) {
         unsafe {
-            bindings::Gecko_SetGradientImageValue(self, gecko_gradient);
+             bindings::Gecko_SetGradientImageValue(self, Box::into_raw(gradient));
         }
     }
 
@@ -352,7 +142,9 @@ impl nsStyleImage {
                 }
             },
             nsStyleImageType::eStyleImageType_Gradient => {
-                Some(GenericImage::Gradient(self.get_gradient()))
+                let gradient: &Gradient =
+                    &**self.__bindgen_anon_1.mGradient.as_ref();
+                Some(GenericImage::Gradient(Box::new(gradient.clone())))
             },
             nsStyleImageType::eStyleImageType_Element => {
                 use crate::gecko_string_cache::Atom;
@@ -368,131 +160,6 @@ impl nsStyleImage {
             .expect("Null image request?");
         ComputedImageUrl::from_image_request(image_request)
     }
-
-    unsafe fn get_gradient(self: &nsStyleImage) -> Box<Gradient> {
-        use self::structs::NS_STYLE_GRADIENT_SIZE_CLOSEST_CORNER as CLOSEST_CORNER;
-        use self::structs::NS_STYLE_GRADIENT_SIZE_CLOSEST_SIDE as CLOSEST_SIDE;
-        use self::structs::NS_STYLE_GRADIENT_SIZE_FARTHEST_CORNER as FARTHEST_CORNER;
-        use self::structs::NS_STYLE_GRADIENT_SIZE_FARTHEST_SIDE as FARTHEST_SIDE;
-        use crate::values::computed::position::Position;
-        use crate::values::generics::image::{Circle, Ellipse};
-        use crate::values::generics::image::{EndingShape, GradientKind, ShapeExtent};
-
-        let gecko_gradient = bindings::Gecko_GetGradientImageValue(self)
-            .as_ref()
-            .unwrap();
-        let horizontal_style = LengthPercentage::from_gecko_style_coord(&gecko_gradient.mBgPosX);
-        let vertical_style = LengthPercentage::from_gecko_style_coord(&gecko_gradient.mBgPosY);
-
-        let kind = match gecko_gradient.mShape as u32 {
-            structs::NS_STYLE_GRADIENT_SHAPE_LINEAR => {
-                let angle = Angle::from_gecko_style_coord(&gecko_gradient.mAngle);
-                let line_direction = match (angle, horizontal_style, vertical_style) {
-                    (Some(a), None, None) => LineDirection::Angle(a),
-                    (None, None, None) => LineDirection::Angle(Angle::from_radians(PI)),
-                    (None, Some(horizontal), Some(vertical)) => {
-                        line_direction(horizontal, vertical)
-                    },
-                    _ => {
-                        unreachable!("unexpected line direction for linear gradient direction");
-                    },
-                };
-                GradientKind::Linear(line_direction)
-            },
-            _ => {
-                let gecko_size_to_keyword = |gecko_size| {
-                    match gecko_size {
-                        CLOSEST_SIDE => ShapeExtent::ClosestSide,
-                        FARTHEST_SIDE => ShapeExtent::FarthestSide,
-                        CLOSEST_CORNER => ShapeExtent::ClosestCorner,
-                        FARTHEST_CORNER => ShapeExtent::FarthestCorner,
-                        // FIXME: We should support ShapeExtent::Contain and ShapeExtent::Cover.
-                        // But we can't choose those yet since Gecko does not support both values.
-                        // https://bugzilla.mozilla.org/show_bug.cgi?id=1217664
-                        _ => panic!("Found unexpected gecko_size"),
-                    }
-                };
-
-                let shape = match gecko_gradient.mShape as u32 {
-                    structs::NS_STYLE_GRADIENT_SHAPE_CIRCULAR => {
-                        let circle = match gecko_gradient.mSize as u32 {
-                            structs::NS_STYLE_GRADIENT_SIZE_EXPLICIT_SIZE => {
-                                let radius =
-                                    Length::from_gecko_style_coord(&gecko_gradient.mRadiusX)
-                                        .expect("mRadiusX could not convert to Length");
-                                debug_assert_eq!(
-                                    radius,
-                                    Length::from_gecko_style_coord(&gecko_gradient.mRadiusY)
-                                        .unwrap()
-                                );
-                                Circle::Radius(radius)
-                            },
-                            size => Circle::Extent(gecko_size_to_keyword(size)),
-                        };
-                        EndingShape::Circle(circle)
-                    },
-                    structs::NS_STYLE_GRADIENT_SHAPE_ELLIPTICAL => {
-                        let length_percentage_keyword = match gecko_gradient.mSize as u32 {
-                            structs::NS_STYLE_GRADIENT_SIZE_EXPLICIT_SIZE => match (
-                                LengthPercentage::from_gecko_style_coord(&gecko_gradient.mRadiusX),
-                                LengthPercentage::from_gecko_style_coord(&gecko_gradient.mRadiusY),
-                            ) {
-                                (Some(x), Some(y)) => Ellipse::Radii(x, y),
-                                _ => {
-                                    debug_assert!(
-                                        false,
-                                        "mRadiusX, mRadiusY could not convert to LengthPercentage"
-                                    );
-                                    Ellipse::Radii(
-                                        LengthPercentage::zero(),
-                                        LengthPercentage::zero(),
-                                    )
-                                },
-                            },
-                            size => Ellipse::Extent(gecko_size_to_keyword(size)),
-                        };
-                        EndingShape::Ellipse(length_percentage_keyword)
-                    },
-                    _ => panic!("Found unexpected mShape"),
-                };
-
-                let position = match (horizontal_style, vertical_style) {
-                    (Some(horizontal), Some(vertical)) => Position {
-                        horizontal,
-                        vertical,
-                    },
-                    _ => {
-                        debug_assert!(
-                            false,
-                            "mRadiusX, mRadiusY could not convert to LengthPercentage"
-                        );
-                        Position {
-                            horizontal: LengthPercentage::zero(),
-                            vertical: LengthPercentage::zero(),
-                        }
-                    },
-                };
-
-                GradientKind::Radial(shape, position)
-            },
-        };
-
-        let items = gecko_gradient.mStops.iter().cloned().collect();
-        let compat_mode = if gecko_gradient.mMozLegacySyntax {
-            CompatMode::Moz
-        } else if gecko_gradient.mLegacySyntax {
-            CompatMode::WebKit
-        } else {
-            CompatMode::Modern
-        };
-
-        Box::new(Gradient {
-            items,
-            repeating: gecko_gradient.mRepeating,
-            kind,
-            compat_mode,
-        })
-    }
 }
 
 pub mod basic_shape {
diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index 9f56c42cc2b..3ff3be2a2ec 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -55,6 +55,7 @@ use crate::values::computed::url::ComputedImageUrl;
 use crate::values::computed::BorderStyle;
 use crate::values::computed::font::FontSize;
 use crate::values::generics::column::ColumnCount;
+use crate::values::generics::image::ImageLayer;
 use crate::values::generics::transform::TransformStyle;
 use crate::values::generics::url::UrlOrNone;
 
@@ -982,7 +983,7 @@ fn static_assert() {
             Gecko_SetNullImageValue(&mut self.gecko.mBorderImageSource);
         }
 
-        if let Either::Second(image) = image {
+        if let ImageLayer::Image(image) = image {
             self.gecko.mBorderImageSource.set(image);
         }
     }
@@ -999,11 +1000,9 @@ fn static_assert() {
     }
 
     pub fn clone_border_image_source(&self) -> longhands::border_image_source::computed_value::T {
-        use crate::values::None_;
-
         match unsafe { self.gecko.mBorderImageSource.into_image() } {
-            Some(image) => Either::Second(image),
-            None => Either::First(None_),
+            Some(image) => ImageLayer::Image(image),
+            None => ImageLayer::None,
         }
     }
 
@@ -2714,22 +2713,20 @@ fn static_assert() {
 
         for (image, geckoimage) in images.zip(self.gecko.${image_layers_field}
                                                   .mLayers.iter_mut()) {
-            if let Either::Second(image) = image {
+            if let ImageLayer::Image(image) = image {
                 geckoimage.mImage.set(image)
             }
         }
     }
 
     pub fn clone_${shorthand}_image(&self) -> longhands::${shorthand}_image::computed_value::T {
-        use crate::values::None_;
-
         longhands::${shorthand}_image::computed_value::List(
             self.gecko.${image_layers_field}.mLayers.iter()
                 .take(self.gecko.${image_layers_field}.mImageCount as usize)
                 .map(|ref layer| {
                     match unsafe { layer.mImage.into_image() } {
-                        Some(image) => Either::Second(image),
-                        None => Either::First(None_),
+                        Some(image) => ImageLayer::Image(image),
+                        None => ImageLayer::None,
                     }
             }).collect()
         )
diff --git a/components/style/properties/longhands/background.mako.rs b/components/style/properties/longhands/background.mako.rs
index 9c66e4676f9..e9e866983c0 100644
--- a/components/style/properties/longhands/background.mako.rs
+++ b/components/style/properties/longhands/background.mako.rs
@@ -22,8 +22,8 @@ ${helpers.predefined_type(
 ${helpers.predefined_type(
     "background-image",
     "ImageLayer",
-    initial_value="Either::First(None_)",
-    initial_specified_value="Either::First(None_)",
+    initial_value="computed::ImageLayer::none()",
+    initial_specified_value="specified::ImageLayer::none()",
     spec="https://drafts.csswg.org/css-backgrounds/#the-background-image",
     vector="True",
     animation_value_type="discrete",
diff --git a/components/style/properties/longhands/border.mako.rs b/components/style/properties/longhands/border.mako.rs
index 7909372be17..66544a44858 100644
--- a/components/style/properties/longhands/border.mako.rs
+++ b/components/style/properties/longhands/border.mako.rs
@@ -107,8 +107,8 @@ ${helpers.single_keyword(
 ${helpers.predefined_type(
     "border-image-source",
     "ImageLayer",
-    initial_value="Either::First(None_)",
-    initial_specified_value="Either::First(None_)",
+    initial_value="computed::ImageLayer::none()",
+    initial_specified_value="specified::ImageLayer::none()",
     spec="https://drafts.csswg.org/css-backgrounds/#the-background-image",
     vector=False,
     animation_value_type="discrete",
diff --git a/components/style/properties/longhands/svg.mako.rs b/components/style/properties/longhands/svg.mako.rs
index 8e93d03dbb1..9f0e0db9fbc 100644
--- a/components/style/properties/longhands/svg.mako.rs
+++ b/components/style/properties/longhands/svg.mako.rs
@@ -181,8 +181,8 @@ ${helpers.single_keyword(
 ${helpers.predefined_type(
     "mask-image",
     "ImageLayer",
-    "Either::First(None_)",
-    initial_specified_value="Either::First(None_)",
+    initial_value="computed::ImageLayer::none()",
+    initial_specified_value="specified::ImageLayer::none()",
     parse_method="parse_with_cors_anonymous",
     spec="https://drafts.fxtf.org/css-masking/#propdef-mask-image",
     vector=True,
diff --git a/components/style/values/computed/image.rs b/components/style/values/computed/image.rs
index ea2f4e9530b..3012eece0e7 100644
--- a/components/style/values/computed/image.rs
+++ b/components/style/values/computed/image.rs
@@ -11,41 +11,41 @@ use crate::values::computed::position::Position;
 use crate::values::computed::url::ComputedImageUrl;
 use crate::values::computed::{Angle, Color, Context};
 use crate::values::computed::{Length, LengthPercentage, NumberOrPercentage, ToComputedValue};
-use crate::values::generics::image::{self as generic, CompatMode};
+use crate::values::generics::image::{self as generic, GradientCompatMode};
 use crate::values::specified::image::LineDirection as SpecifiedLineDirection;
-use crate::values::specified::position::{X, Y};
-use crate::values::{Either, None_};
+use crate::values::specified::position::{HorizontalPositionKeyword, VerticalPositionKeyword};
 use std::f32::consts::PI;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};
 
 /// A computed image layer.
-pub type ImageLayer = Either<None_, Image>;
+pub type ImageLayer = generic::GenericImageLayer<Image>;
 
 /// Computed values for an image according to CSS-IMAGES.
 /// <https://drafts.csswg.org/css-images/#image-values>
-pub type Image = generic::Image<Gradient, MozImageRect, ComputedImageUrl>;
+pub type Image = generic::GenericImage<Gradient, MozImageRect, ComputedImageUrl>;
 
 /// Computed values for a CSS gradient.
 /// <https://drafts.csswg.org/css-images/#gradients>
 pub type Gradient =
-    generic::Gradient<LineDirection, Length, LengthPercentage, Position, Color>;
+    generic::GenericGradient<LineDirection, Length, LengthPercentage, Position, Color>;
 
 /// A computed gradient kind.
 pub type GradientKind =
-    generic::GradientKind<LineDirection, Length, LengthPercentage, Position>;
+    generic::GenericGradientKind<LineDirection, Length, LengthPercentage, Position>;
 
 /// A computed gradient line direction.
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToResolvedValue)]
+#[repr(C, u8)]
 pub enum LineDirection {
     /// An angle.
     Angle(Angle),
     /// A horizontal direction.
-    Horizontal(X),
+    Horizontal(HorizontalPositionKeyword),
     /// A vertical direction.
-    Vertical(Y),
+    Vertical(VerticalPositionKeyword),
     /// A corner.
-    Corner(X, Y),
+    Corner(HorizontalPositionKeyword, VerticalPositionKeyword),
 }
 
 /// A computed radial gradient ending shape.
@@ -61,35 +61,39 @@ pub type ColorStop = generic::ColorStop<Color, LengthPercentage>;
 pub type MozImageRect = generic::MozImageRect<NumberOrPercentage, ComputedImageUrl>;
 
 impl generic::LineDirection for LineDirection {
-    fn points_downwards(&self, compat_mode: CompatMode) -> bool {
+    fn points_downwards(&self, compat_mode: GradientCompatMode) -> bool {
         match *self {
             LineDirection::Angle(angle) => angle.radians() == PI,
-            LineDirection::Vertical(Y::Bottom) if compat_mode == CompatMode::Modern => true,
-            LineDirection::Vertical(Y::Top) if compat_mode != CompatMode::Modern => true,
+            LineDirection::Vertical(VerticalPositionKeyword::Bottom) => {
+                compat_mode == GradientCompatMode::Modern
+            }
+            LineDirection::Vertical(VerticalPositionKeyword::Top) => {
+                compat_mode != GradientCompatMode::Modern
+            }
             _ => false,
         }
     }
 
-    fn to_css<W>(&self, dest: &mut CssWriter<W>, compat_mode: CompatMode) -> fmt::Result
+    fn to_css<W>(&self, dest: &mut CssWriter<W>, compat_mode: GradientCompatMode) -> fmt::Result
     where
         W: Write,
     {
         match *self {
             LineDirection::Angle(ref angle) => angle.to_css(dest),
             LineDirection::Horizontal(x) => {
-                if compat_mode == CompatMode::Modern {
+                if compat_mode == GradientCompatMode::Modern {
                     dest.write_str("to ")?;
                 }
                 x.to_css(dest)
             },
             LineDirection::Vertical(y) => {
-                if compat_mode == CompatMode::Modern {
+                if compat_mode == GradientCompatMode::Modern {
                     dest.write_str("to ")?;
                 }
                 y.to_css(dest)
             },
             LineDirection::Corner(x, y) => {
-                if compat_mode == CompatMode::Modern {
+                if compat_mode == GradientCompatMode::Modern {
                     dest.write_str("to ")?;
                 }
                 x.to_css(dest)?;
diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs
index c672efc2e7d..3f126a6e20d 100644
--- a/components/style/values/generics/image.rs
+++ b/components/style/values/generics/image.rs
@@ -13,13 +13,35 @@ use servo_arc::Arc;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};
 
+/// An <image> | <none> (for background-image, for example).
+#[derive(
+    Clone, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss, ToResolvedValue, ToShmem,
+)]
+pub enum GenericImageLayer<Image> {
+    /// The `none` value.
+    None,
+    /// The `<image>` value.
+    Image(Image),
+}
+
+pub use self::GenericImageLayer as ImageLayer;
+
+impl<I> ImageLayer<I> {
+    /// Returns `none`.
+    #[inline]
+    pub fn none() -> Self {
+        ImageLayer::None
+    }
+}
+
 /// An [image].
 ///
 /// [image]: https://drafts.csswg.org/css-images/#image-values
 #[derive(
     Clone, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem,
 )]
-pub enum Image<Gradient, MozImageRect, ImageUrl> {
+#[repr(C, u8)]
+pub enum GenericImage<Gradient, MozImageRect, ImageUrl> {
     /// A `<url()>` image.
     Url(ImageUrl),
     /// A `<gradient>` image.  Gradients are rather large, and not nearly as
@@ -36,23 +58,29 @@ pub enum Image<Gradient, MozImageRect, ImageUrl> {
     PaintWorklet(PaintWorklet),
 }
 
+pub use self::GenericImage as Image;
+
 /// A CSS gradient.
 /// <https://drafts.csswg.org/css-images/#gradients>
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
-pub struct Gradient<LineDirection, Length, LengthPercentage, Position, Color> {
+#[repr(C)]
+pub struct GenericGradient<LineDirection, Length, LengthPercentage, Position, Color> {
     /// Gradients can be linear or radial.
-    pub kind: GradientKind<LineDirection, Length, LengthPercentage, Position>,
+    pub kind: GenericGradientKind<LineDirection, Length, LengthPercentage, Position>,
     /// The color stops and interpolation hints.
-    pub items: Vec<GradientItem<Color, LengthPercentage>>,
+    pub items: crate::OwnedSlice<GenericGradientItem<Color, LengthPercentage>>,
     /// True if this is a repeating gradient.
     pub repeating: bool,
     /// Compatibility mode.
-    pub compat_mode: CompatMode,
+    pub compat_mode: GradientCompatMode,
 }
 
+pub use self::GenericGradient as Gradient;
+
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
+#[repr(u8)]
 /// Whether we used the modern notation or the compatibility `-webkit`, `-moz` prefixes.
-pub enum CompatMode {
+pub enum GradientCompatMode {
     /// Modern syntax.
     Modern,
     /// `-webkit` prefix.
@@ -63,44 +91,56 @@ pub enum CompatMode {
 
 /// A gradient kind.
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
-pub enum GradientKind<LineDirection, Length, LengthPercentage, Position> {
+#[repr(C, u8)]
+pub enum GenericGradientKind<LineDirection, Length, LengthPercentage, Position> {
     /// A linear gradient.
     Linear(LineDirection),
     /// A radial gradient.
-    Radial(EndingShape<Length, LengthPercentage>, Position),
+    Radial(GenericEndingShape<Length, LengthPercentage>, Position),
 }
 
+pub use self::GenericGradientKind as GradientKind;
+
 /// A radial gradient's ending shape.
 #[derive(
     Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem,
 )]
-pub enum EndingShape<Length, LengthPercentage> {
+#[repr(C, u8)]
+pub enum GenericEndingShape<Length, LengthPercentage> {
     /// A circular gradient.
-    Circle(Circle<Length>),
+    Circle(GenericCircle<Length>),
     /// An elliptic gradient.
-    Ellipse(Ellipse<LengthPercentage>),
+    Ellipse(GenericEllipse<LengthPercentage>),
 }
 
+pub use self::GenericEndingShape as EndingShape;
+
 /// A circle shape.
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
-pub enum Circle<Length> {
+#[repr(C, u8)]
+pub enum GenericCircle<Length> {
     /// A circle radius.
     Radius(Length),
     /// A circle extent.
     Extent(ShapeExtent),
 }
 
+pub use self::GenericCircle as Circle;
+
 /// An ellipse shape.
 #[derive(
     Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem,
 )]
-pub enum Ellipse<LengthPercentage> {
+#[repr(C, u8)]
+pub enum GenericEllipse<LengthPercentage> {
     /// An ellipse pair of radii.
     Radii(LengthPercentage, LengthPercentage),
     /// An ellipse extent.
     Extent(ShapeExtent),
 }
 
+pub use self::GenericEllipse as Ellipse;
+
 /// <https://drafts.csswg.org/css-images/#typedef-extent-keyword>
 #[allow(missing_docs)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
@@ -117,6 +157,7 @@ pub enum Ellipse<LengthPercentage> {
     ToResolvedValue,
     ToShmem,
 )]
+#[repr(u8)]
 pub enum ShapeExtent {
     ClosestSide,
     FarthestSide,
@@ -277,8 +318,8 @@ where
         W: Write,
     {
         match self.compat_mode {
-            CompatMode::WebKit => dest.write_str("-webkit-")?,
-            CompatMode::Moz => dest.write_str("-moz-")?,
+            GradientCompatMode::WebKit => dest.write_str("-webkit-")?,
+            GradientCompatMode::Moz => dest.write_str("-moz-")?,
             _ => {},
         }
 
@@ -301,7 +342,7 @@ where
                     EndingShape::Ellipse(Ellipse::Extent(ShapeExtent::FarthestCorner)) => true,
                     _ => false,
                 };
-                if self.compat_mode == CompatMode::Modern {
+                if self.compat_mode == GradientCompatMode::Modern {
                     if !omit_shape {
                         shape.to_css(dest)?;
                         dest.write_str(" ")?;
@@ -318,7 +359,7 @@ where
                 false
             },
         };
-        for item in &self.items {
+        for item in &*self.items {
             if !skip_comma {
                 dest.write_str(", ")?;
             }
@@ -341,10 +382,10 @@ impl<D, L, LoP, P> GradientKind<D, L, LoP, P> {
 /// The direction of a linear gradient.
 pub trait LineDirection {
     /// Whether this direction points towards, and thus can be omitted.
-    fn points_downwards(&self, compat_mode: CompatMode) -> bool;
+    fn points_downwards(&self, compat_mode: GradientCompatMode) -> bool;
 
     /// Serialises this direction according to the compatibility mode.
-    fn to_css<W>(&self, dest: &mut CssWriter<W>, compat_mode: CompatMode) -> fmt::Result
+    fn to_css<W>(&self, dest: &mut CssWriter<W>, compat_mode: GradientCompatMode) -> fmt::Result
     where
         W: Write;
 }
diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs
index 9779c6fa2f6..f4f4cf9afce 100644
--- a/components/style/values/specified/image.rs
+++ b/components/style/values/specified/image.rs
@@ -11,13 +11,13 @@ use crate::custom_properties::SpecifiedValue;
 use crate::parser::{Parse, ParserContext};
 use crate::stylesheets::CorsMode;
 use crate::values::generics::image::PaintWorklet;
-use crate::values::generics::image::{self as generic, Circle, CompatMode, Ellipse, ShapeExtent};
+use crate::values::generics::image::{self as generic, Circle, GradientCompatMode, Ellipse, ShapeExtent};
 use crate::values::generics::position::Position as GenericPosition;
-use crate::values::specified::position::{Position, PositionComponent, Side, X, Y};
+use crate::values::specified::position::{Position, PositionComponent, Side};
+use crate::values::specified::position::{HorizontalPositionKeyword, VerticalPositionKeyword};
 use crate::values::specified::url::SpecifiedImageUrl;
 use crate::values::specified::{Angle, Color, Length, LengthPercentage};
 use crate::values::specified::{Number, NumberOrPercentage, Percentage};
-use crate::values::{Either, None_};
 use crate::Atom;
 use cssparser::{Delimiter, Parser, Token};
 use selectors::parser::SelectorParseErrorKind;
@@ -29,7 +29,7 @@ use style_traits::{CssType, CssWriter, KeywordsCollectFn, ParseError};
 use style_traits::{SpecifiedValueInfo, StyleParseErrorKind, ToCss};
 
 /// A specified image layer.
-pub type ImageLayer = Either<None_, Image>;
+pub type ImageLayer = generic::GenericImageLayer<Image>;
 
 impl ImageLayer {
     /// This is a specialization of Either with an alternative parse
@@ -38,10 +38,11 @@ impl ImageLayer {
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
-        if let Ok(v) = input.try(|i| None_::parse(context, i)) {
-            return Ok(Either::First(v));
+        if let Ok(v) = input.try(|i| Image::parse_with_cors_anonymous(context, i)) {
+            return Ok(generic::GenericImageLayer::Image(v));
         }
-        Image::parse_with_cors_anonymous(context, input).map(Either::Second)
+        input.expect_ident_matching("none")?;
+        Ok(generic::GenericImageLayer::None)
     }
 }
 
@@ -87,11 +88,11 @@ pub enum LineDirection {
     /// An angular direction.
     Angle(Angle),
     /// A horizontal direction.
-    Horizontal(X),
+    Horizontal(HorizontalPositionKeyword),
     /// A vertical direction.
-    Vertical(Y),
+    Vertical(VerticalPositionKeyword),
     /// A direction towards a corner of a box.
-    Corner(X, Y),
+    Corner(HorizontalPositionKeyword, VerticalPositionKeyword),
 }
 
 /// A specified ending shape.
@@ -182,44 +183,44 @@ impl Parse for Gradient {
         let func = input.expect_function()?.clone();
         let result = match_ignore_ascii_case! { &func,
             "linear-gradient" => {
-                Some((Shape::Linear, false, CompatMode::Modern))
+                Some((Shape::Linear, false, GradientCompatMode::Modern))
             },
             "-webkit-linear-gradient" => {
-                Some((Shape::Linear, false, CompatMode::WebKit))
+                Some((Shape::Linear, false, GradientCompatMode::WebKit))
             },
             #[cfg(feature = "gecko")]
             "-moz-linear-gradient" => {
-                Some((Shape::Linear, false, CompatMode::Moz))
+                Some((Shape::Linear, false, GradientCompatMode::Moz))
             },
             "repeating-linear-gradient" => {
-                Some((Shape::Linear, true, CompatMode::Modern))
+                Some((Shape::Linear, true, GradientCompatMode::Modern))
             },
             "-webkit-repeating-linear-gradient" => {
-                Some((Shape::Linear, true, CompatMode::WebKit))
+                Some((Shape::Linear, true, GradientCompatMode::WebKit))
             },
             #[cfg(feature = "gecko")]
             "-moz-repeating-linear-gradient" => {
-                Some((Shape::Linear, true, CompatMode::Moz))
+                Some((Shape::Linear, true, GradientCompatMode::Moz))
             },
             "radial-gradient" => {
-                Some((Shape::Radial, false, CompatMode::Modern))
+                Some((Shape::Radial, false, GradientCompatMode::Modern))
             },
             "-webkit-radial-gradient" => {
-                Some((Shape::Radial, false, CompatMode::WebKit))
+                Some((Shape::Radial, false, GradientCompatMode::WebKit))
             }
             #[cfg(feature = "gecko")]
             "-moz-radial-gradient" => {
-                Some((Shape::Radial, false, CompatMode::Moz))
+                Some((Shape::Radial, false, GradientCompatMode::Moz))
             },
             "repeating-radial-gradient" => {
-                Some((Shape::Radial, true, CompatMode::Modern))
+                Some((Shape::Radial, true, GradientCompatMode::Modern))
             },
             "-webkit-repeating-radial-gradient" => {
-                Some((Shape::Radial, true, CompatMode::WebKit))
+                Some((Shape::Radial, true, GradientCompatMode::WebKit))
             },
             #[cfg(feature = "gecko")]
             "-moz-repeating-radial-gradient" => {
-                Some((Shape::Radial, true, CompatMode::Moz))
+                Some((Shape::Radial, true, GradientCompatMode::Moz))
             },
             "-webkit-gradient" => {
                 return input.parse_nested_block(|i| {
@@ -249,12 +250,7 @@ impl Parse for Gradient {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
 
-        Ok(Gradient {
-            items: items,
-            repeating: repeating,
-            kind: kind,
-            compat_mode: compat_mode,
-        })
+        Ok(Gradient { items, repeating, kind, compat_mode })
     }
 }
 
@@ -263,6 +259,7 @@ impl Gradient {
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
+        use crate::values::specified::position::{HorizontalPositionKeyword as X, VerticalPositionKeyword as Y};
         type Point = GenericPosition<Component<X>, Component<Y>>;
 
         #[derive(Clone, Copy, Parse)]
@@ -493,21 +490,21 @@ impl Gradient {
         }
 
         Ok(generic::Gradient {
-            kind: kind,
-            items: items,
+            kind,
+            items: items.into(),
             repeating: false,
-            compat_mode: CompatMode::Modern,
+            compat_mode: GradientCompatMode::Modern,
         })
     }
 }
 
 impl GradientKind {
     /// Parses a linear gradient.
-    /// CompatMode can change during `-moz-` prefixed gradient parsing if it come across a `to` keyword.
+    /// GradientCompatMode can change during `-moz-` prefixed gradient parsing if it come across a `to` keyword.
     fn parse_linear<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
-        compat_mode: &mut CompatMode,
+        compat_mode: &mut GradientCompatMode,
     ) -> Result<Self, ParseError<'i>> {
         let direction = if let Ok(d) = input.try(|i| LineDirection::parse(context, i, compat_mode))
         {
@@ -515,8 +512,8 @@ impl GradientKind {
             d
         } else {
             match *compat_mode {
-                CompatMode::Modern => LineDirection::Vertical(Y::Bottom),
-                _ => LineDirection::Vertical(Y::Top),
+                GradientCompatMode::Modern => LineDirection::Vertical(VerticalPositionKeyword::Bottom),
+                _ => LineDirection::Vertical(VerticalPositionKeyword::Top),
             }
         };
         Ok(generic::GradientKind::Linear(direction))
@@ -524,10 +521,10 @@ impl GradientKind {
     fn parse_radial<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
-        compat_mode: &mut CompatMode,
+        compat_mode: &mut GradientCompatMode,
     ) -> Result<Self, ParseError<'i>> {
         let (shape, position) = match *compat_mode {
-            CompatMode::Modern => {
+            GradientCompatMode::Modern => {
                 let shape = input.try(|i| EndingShape::parse(context, i, *compat_mode));
                 let position = input.try(|i| {
                     i.expect_ident_matching("at")?;
@@ -561,35 +558,39 @@ impl GradientKind {
 }
 
 impl generic::LineDirection for LineDirection {
-    fn points_downwards(&self, compat_mode: CompatMode) -> bool {
+    fn points_downwards(&self, compat_mode: GradientCompatMode) -> bool {
         match *self {
             LineDirection::Angle(ref angle) => angle.degrees() == 180.0,
-            LineDirection::Vertical(Y::Bottom) if compat_mode == CompatMode::Modern => true,
-            LineDirection::Vertical(Y::Top) if compat_mode != CompatMode::Modern => true,
+            LineDirection::Vertical(VerticalPositionKeyword::Bottom) => {
+                compat_mode == GradientCompatMode::Modern
+            }
+            LineDirection::Vertical(VerticalPositionKeyword::Top) => {
+                compat_mode != GradientCompatMode::Modern
+            }
             _ => false,
         }
     }
 
-    fn to_css<W>(&self, dest: &mut CssWriter<W>, compat_mode: CompatMode) -> fmt::Result
+    fn to_css<W>(&self, dest: &mut CssWriter<W>, compat_mode: GradientCompatMode) -> fmt::Result
     where
         W: Write,
     {
         match *self {
             LineDirection::Angle(angle) => angle.to_css(dest),
             LineDirection::Horizontal(x) => {
-                if compat_mode == CompatMode::Modern {
+                if compat_mode == GradientCompatMode::Modern {
                     dest.write_str("to ")?;
                 }
                 x.to_css(dest)
             },
             LineDirection::Vertical(y) => {
-                if compat_mode == CompatMode::Modern {
+                if compat_mode == GradientCompatMode::Modern {
                     dest.write_str("to ")?;
                 }
                 y.to_css(dest)
             },
             LineDirection::Corner(x, y) => {
-                if compat_mode == CompatMode::Modern {
+                if compat_mode == GradientCompatMode::Modern {
                     dest.write_str("to ")?;
                 }
                 x.to_css(dest)?;
@@ -604,7 +605,7 @@ impl LineDirection {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
-        compat_mode: &mut CompatMode,
+        compat_mode: &mut GradientCompatMode,
     ) -> Result<Self, ParseError<'i>> {
         // Gradients allow unitless zero angles as an exception, see:
         // https://github.com/w3c/csswg-drafts/issues/1162
@@ -616,14 +617,14 @@ impl LineDirection {
             let to_ident = i.try(|i| i.expect_ident_matching("to"));
             match *compat_mode {
                 // `to` keyword is mandatory in modern syntax.
-                CompatMode::Modern => to_ident?,
+                GradientCompatMode::Modern => to_ident?,
                 // Fall back to Modern compatibility mode in case there is a `to` keyword.
                 // According to Gecko, `-moz-linear-gradient(to ...)` should serialize like
                 // `linear-gradient(to ...)`.
-                CompatMode::Moz if to_ident.is_ok() => *compat_mode = CompatMode::Modern,
+                GradientCompatMode::Moz if to_ident.is_ok() => *compat_mode = GradientCompatMode::Modern,
                 // There is no `to` keyword in webkit prefixed syntax. If it's consumed,
                 // parsing should throw an error.
-                CompatMode::WebKit if to_ident.is_ok() => {
+                GradientCompatMode::WebKit if to_ident.is_ok() => {
                     return Err(
                         i.new_custom_error(SelectorParseErrorKind::UnexpectedIdent("to".into()))
                     );
@@ -631,14 +632,14 @@ impl LineDirection {
                 _ => {},
             }
 
-            if let Ok(x) = i.try(X::parse) {
-                if let Ok(y) = i.try(Y::parse) {
+            if let Ok(x) = i.try(HorizontalPositionKeyword::parse) {
+                if let Ok(y) = i.try(VerticalPositionKeyword::parse) {
                     return Ok(LineDirection::Corner(x, y));
                 }
                 return Ok(LineDirection::Horizontal(x));
             }
-            let y = Y::parse(i)?;
-            if let Ok(x) = i.try(X::parse) {
+            let y = VerticalPositionKeyword::parse(i)?;
+            if let Ok(x) = i.try(HorizontalPositionKeyword::parse) {
                 return Ok(LineDirection::Corner(x, y));
             }
             Ok(LineDirection::Vertical(y))
@@ -650,7 +651,7 @@ impl EndingShape {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
-        compat_mode: CompatMode,
+        compat_mode: GradientCompatMode,
     ) -> Result<Self, ParseError<'i>> {
         if let Ok(extent) = input.try(|i| ShapeExtent::parse_with_compat_mode(i, compat_mode)) {
             if input.try(|i| i.expect_ident_matching("circle")).is_ok() {
@@ -663,7 +664,7 @@ impl EndingShape {
             if let Ok(extent) = input.try(|i| ShapeExtent::parse_with_compat_mode(i, compat_mode)) {
                 return Ok(generic::EndingShape::Circle(Circle::Extent(extent)));
             }
-            if compat_mode == CompatMode::Modern {
+            if compat_mode == GradientCompatMode::Modern {
                 if let Ok(length) = input.try(|i| Length::parse(context, i)) {
                     return Ok(generic::EndingShape::Circle(Circle::Radius(length)));
                 }
@@ -676,7 +677,7 @@ impl EndingShape {
             if let Ok(extent) = input.try(|i| ShapeExtent::parse_with_compat_mode(i, compat_mode)) {
                 return Ok(generic::EndingShape::Ellipse(Ellipse::Extent(extent)));
             }
-            if compat_mode == CompatMode::Modern {
+            if compat_mode == GradientCompatMode::Modern {
                 let pair: Result<_, ParseError> = input.try(|i| {
                     let x = LengthPercentage::parse(context, i)?;
                     let y = LengthPercentage::parse(context, i)?;
@@ -692,7 +693,7 @@ impl EndingShape {
         }
         if let Ok(length) = input.try(|i| Length::parse(context, i)) {
             if let Ok(y) = input.try(|i| LengthPercentage::parse(context, i)) {
-                if compat_mode == CompatMode::Modern {
+                if compat_mode == GradientCompatMode::Modern {
                     let _ = input.try(|i| i.expect_ident_matching("ellipse"));
                 }
                 return Ok(generic::EndingShape::Ellipse(Ellipse::Radii(
@@ -700,7 +701,7 @@ impl EndingShape {
                     y,
                 )));
             }
-            if compat_mode == CompatMode::Modern {
+            if compat_mode == GradientCompatMode::Modern {
                 let y = input.try(|i| {
                     i.expect_ident_matching("ellipse")?;
                     LengthPercentage::parse(context, i)
@@ -719,12 +720,12 @@ impl EndingShape {
         input.try(|i| {
             let x = Percentage::parse(context, i)?;
             let y = if let Ok(y) = i.try(|i| LengthPercentage::parse(context, i)) {
-                if compat_mode == CompatMode::Modern {
+                if compat_mode == GradientCompatMode::Modern {
                     let _ = i.try(|i| i.expect_ident_matching("ellipse"));
                 }
                 y
             } else {
-                if compat_mode == CompatMode::Modern {
+                if compat_mode == GradientCompatMode::Modern {
                     i.expect_ident_matching("ellipse")?;
                 }
                 LengthPercentage::parse(context, i)?
@@ -737,10 +738,10 @@ impl EndingShape {
 impl ShapeExtent {
     fn parse_with_compat_mode<'i, 't>(
         input: &mut Parser<'i, 't>,
-        compat_mode: CompatMode,
+        compat_mode: GradientCompatMode,
     ) -> Result<Self, ParseError<'i>> {
         match Self::parse(input)? {
-            ShapeExtent::Contain | ShapeExtent::Cover if compat_mode == CompatMode::Modern => {
+            ShapeExtent::Contain | ShapeExtent::Cover if compat_mode == GradientCompatMode::Modern => {
                 Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
             },
             ShapeExtent::Contain => Ok(ShapeExtent::ClosestSide),
@@ -754,7 +755,7 @@ impl GradientItem {
     fn parse_comma_separated<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
-    ) -> Result<Vec<Self>, ParseError<'i>> {
+    ) -> Result<crate::OwnedSlice<Self>, ParseError<'i>> {
         let mut items = Vec::new();
         let mut seen_stop = false;
 
@@ -798,7 +799,7 @@ impl GradientItem {
         if !seen_stop || items.len() < 2 {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
-        Ok(items)
+        Ok(items.into())
     }
 }
 
diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs
index d9d75343331..fed0c5d3098 100644
--- a/components/style/values/specified/position.rs
+++ b/components/style/values/specified/position.rs
@@ -28,10 +28,10 @@ use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 pub type Position = GenericPosition<HorizontalPosition, VerticalPosition>;
 
 /// The specified value of a horizontal position.
-pub type HorizontalPosition = PositionComponent<X>;
+pub type HorizontalPosition = PositionComponent<HorizontalPositionKeyword>;
 
 /// The specified value of a vertical position.
-pub type VerticalPosition = PositionComponent<Y>;
+pub type VerticalPosition = PositionComponent<VerticalPositionKeyword>;
 
 /// The specified value of a component of a CSS `<position>`.
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
@@ -61,7 +61,8 @@ pub enum PositionComponent<S> {
     ToShmem,
 )]
 #[allow(missing_docs)]
-pub enum X {
+#[repr(u8)]
+pub enum HorizontalPositionKeyword {
     Left,
     Right,
 }
@@ -83,7 +84,8 @@ pub enum X {
     ToShmem,
 )]
 #[allow(missing_docs)]
-pub enum Y {
+#[repr(u8)]
+pub enum VerticalPositionKeyword {
     Top,
     Bottom,
 }
@@ -123,7 +125,7 @@ impl Position {
                     let y_pos = PositionComponent::Center;
                     return Ok(Self::new(x_pos, y_pos));
                 }
-                if let Ok(y_keyword) = input.try(Y::parse) {
+                if let Ok(y_keyword) = input.try(VerticalPositionKeyword::parse) {
                     let y_lp = input
                         .try(|i| LengthPercentage::parse_quirky(context, i, allow_quirks))
                         .ok();
@@ -136,7 +138,7 @@ impl Position {
                 return Ok(Self::new(x_pos, y_pos));
             },
             Ok(x_pos @ PositionComponent::Length(_)) => {
-                if let Ok(y_keyword) = input.try(Y::parse) {
+                if let Ok(y_keyword) = input.try(VerticalPositionKeyword::parse) {
                     let y_pos = PositionComponent::Side(y_keyword, None);
                     return Ok(Self::new(x_pos, y_pos));
                 }
@@ -152,12 +154,12 @@ impl Position {
             },
             Err(_) => {},
         }
-        let y_keyword = Y::parse(input)?;
+        let y_keyword = VerticalPositionKeyword::parse(input)?;
         let lp_and_x_pos: Result<_, ParseError> = input.try(|i| {
             let y_lp = i
                 .try(|i| LengthPercentage::parse_quirky(context, i, allow_quirks))
                 .ok();
-            if let Ok(x_keyword) = i.try(X::parse) {
+            if let Ok(x_keyword) = i.try(HorizontalPositionKeyword::parse) {
                 let x_lp = i
                     .try(|i| LengthPercentage::parse_quirky(context, i, allow_quirks))
                     .ok();
@@ -301,27 +303,27 @@ pub trait Side {
     fn is_start(&self) -> bool;
 }
 
-impl Side for X {
+impl Side for HorizontalPositionKeyword {
     #[inline]
     fn start() -> Self {
-        X::Left
+        HorizontalPositionKeyword::Left
     }
 
     #[inline]
     fn is_start(&self) -> bool {
-        *self == X::Left
+        *self == Self::start()
     }
 }
 
-impl Side for Y {
+impl Side for VerticalPositionKeyword {
     #[inline]
     fn start() -> Self {
-        Y::Top
+        VerticalPositionKeyword::Top
     }
 
     #[inline]
     fn is_start(&self) -> bool {
-        *self == Y::Top
+        *self == Self::start()
     }
 }
 
diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs
index e942cf13085..d52aa69cd24 100644
--- a/components/style/values/specified/transform.rs
+++ b/components/style/values/specified/transform.rs
@@ -9,7 +9,7 @@ use crate::values::computed::{Context, LengthPercentage as ComputedLengthPercent
 use crate::values::computed::{Percentage as ComputedPercentage, ToComputedValue};
 use crate::values::generics::transform as generic;
 use crate::values::generics::transform::{Matrix, Matrix3D};
-use crate::values::specified::position::{Side, X, Y};
+use crate::values::specified::position::{Side, HorizontalPositionKeyword, VerticalPositionKeyword};
 use crate::values::specified::{self, Angle, Integer, Length, LengthPercentage, Number};
 use crate::Zero;
 use cssparser::Parser;
@@ -25,7 +25,11 @@ pub type TransformOperation =
 pub type Transform = generic::Transform<TransformOperation>;
 
 /// The specified value of a CSS `<transform-origin>`
-pub type TransformOrigin = generic::TransformOrigin<OriginComponent<X>, OriginComponent<Y>, Length>;
+pub type TransformOrigin = generic::TransformOrigin<
+    OriginComponent<HorizontalPositionKeyword>,
+    OriginComponent<VerticalPositionKeyword>,
+    Length,
+>;
 
 impl Transform {
     /// Internal parse function for deciding if we wish to accept prefixed values or not
@@ -263,7 +267,7 @@ impl Parse for TransformOrigin {
                     return Ok(Self::new(x_origin, y_origin, depth));
                 }
                 let y_origin = OriginComponent::Center;
-                if let Ok(x_keyword) = input.try(X::parse) {
+                if let Ok(x_keyword) = input.try(HorizontalPositionKeyword::parse) {
                     let x_origin = OriginComponent::Side(x_keyword);
                     let depth = parse_depth(input);
                     return Ok(Self::new(x_origin, y_origin, depth));
@@ -282,9 +286,9 @@ impl Parse for TransformOrigin {
             },
             Err(_) => {},
         }
-        let y_keyword = Y::parse(input)?;
+        let y_keyword = VerticalPositionKeyword::parse(input)?;
         let y_origin = OriginComponent::Side(y_keyword);
-        if let Ok(x_keyword) = input.try(X::parse) {
+        if let Ok(x_keyword) = input.try(HorizontalPositionKeyword::parse) {
             let x_origin = OriginComponent::Side(x_keyword);
             let depth = parse_depth(input);
             return Ok(Self::new(x_origin, y_origin, depth));

From f0bf7d6481a97bd75264af3d379bff5338d174b2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 11 Jun 2019 17:41:51 +0000
Subject: [PATCH 11/32] style: Add plumbing code to invalidate shadow parts.

Still does nothing, since we still do not collect part rules, but this is all
the plumbing that should allow us to invalidate parts when attributes or state
change on their ancestors.

Differential Revision: https://phabricator.services.mozilla.com/D32642
---
 components/style/dom.rs                       |  8 ++++
 components/style/gecko/wrapper.rs             | 15 +++++++
 .../invalidation/element/invalidation_map.rs  |  4 +-
 .../style/invalidation/element/invalidator.rs | 45 +++++++++++++++++--
 .../element/state_and_attributes.rs           |  4 ++
 5 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/components/style/dom.rs b/components/style/dom.rs
index 04d6a79b384..0a743c60207 100644
--- a/components/style/dom.rs
+++ b/components/style/dom.rs
@@ -346,6 +346,14 @@ pub trait TShadowRoot: Sized + Copy + Clone + PartialEq {
     where
         Self: 'a;
 
+    /// Get the list of shadow parts for this shadow root.
+    fn parts<'a>(&self) -> &[<Self::ConcreteNode as TNode>::ConcreteElement]
+    where
+        Self: 'a
+    {
+        &[]
+    }
+
     /// Get a list of elements with a given ID in this shadow root, sorted by
     /// tree position.
     ///
diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs
index 3ce9e6fe6ab..aa59df56bde 100644
--- a/components/style/gecko/wrapper.rs
+++ b/components/style/gecko/wrapper.rs
@@ -185,6 +185,21 @@ impl<'lr> TShadowRoot for GeckoShadowRoot<'lr> {
             bindings::Gecko_ShadowRoot_GetElementsWithId(self.0, id.as_ptr())
         }))
     }
+
+    #[inline]
+    fn parts<'a>(&self) -> &[<Self::ConcreteNode as TNode>::ConcreteElement]
+    where
+        Self: 'a
+    {
+        let slice: &[*const RawGeckoElement] = &*self.0.mParts;
+
+        #[allow(dead_code)]
+        unsafe fn static_assert() {
+            mem::transmute::<*const RawGeckoElement, GeckoElement<'static>>(0xbadc0de as *const _);
+        }
+
+        unsafe { mem::transmute(slice) }
+    }
 }
 
 /// A simple wrapper over a non-null Gecko node (`nsINode`) pointer.
diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs
index 63d6eb6acc4..e0b9cf8d747 100644
--- a/components/style/invalidation/element/invalidation_map.rs
+++ b/components/style/invalidation/element/invalidation_map.rs
@@ -66,6 +66,8 @@ pub enum DependencyInvalidationKind {
     Siblings,
     /// This dependency may affect slotted elements of the element that changed.
     SlottedElements,
+    /// This dependency may affect parts of the element that changed.
+    Parts,
 }
 
 impl Dependency {
@@ -98,7 +100,7 @@ impl Dependency {
             // an eager pseudo, and return only Descendants here if not.
             Some(Combinator::PseudoElement) => DependencyInvalidationKind::ElementAndDescendants,
             Some(Combinator::SlotAssignment) => DependencyInvalidationKind::SlottedElements,
-            Some(Combinator::Part) => unimplemented!("Need to add invalidation for shadow parts"),
+            Some(Combinator::Part) => DependencyInvalidationKind::Parts,
         }
     }
 }
diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs
index ec1548f4aa3..f00aa57fcff 100644
--- a/components/style/invalidation/element/invalidator.rs
+++ b/components/style/invalidation/element/invalidator.rs
@@ -72,11 +72,14 @@ pub struct DescendantInvalidationLists<'a> {
     pub dom_descendants: InvalidationVector<'a>,
     /// Invalidations for slotted children of an element.
     pub slotted_descendants: InvalidationVector<'a>,
+    /// Invalidations for ::part()s of an element.
+    pub parts: InvalidationVector<'a>,
 }
 
 impl<'a> DescendantInvalidationLists<'a> {
     fn is_empty(&self) -> bool {
-        self.dom_descendants.is_empty() && self.slotted_descendants.is_empty()
+        self.dom_descendants.is_empty() && self.slotted_descendants.is_empty() &&
+            self.parts.is_empty()
     }
 }
 
@@ -104,6 +107,8 @@ enum DescendantInvalidationKind {
     Dom,
     /// A ::slotted() descendant invalidation.
     Slotted,
+    /// A ::part() descendant invalidation.
+    Part,
 }
 
 /// The kind of invalidation we're processing.
@@ -175,7 +180,7 @@ impl<'a> Invalidation<'a> {
                 InvalidationKind::Descendant(DescendantInvalidationKind::Dom)
             },
             Combinator::Part => {
-                unimplemented!("Need to add invalidation for shadow parts");
+                InvalidationKind::Descendant(DescendantInvalidationKind::Part)
             },
             Combinator::SlotAssignment => {
                 InvalidationKind::Descendant(DescendantInvalidationKind::Slotted)
@@ -472,6 +477,36 @@ where
         any_descendant
     }
 
+    fn invalidate_parts(&mut self, invalidations: &[Invalidation<'b>]) -> bool {
+        if invalidations.is_empty() {
+            return false;
+        }
+
+        let shadow = match self.element.shadow_root() {
+            Some(s) => s,
+            None => return false,
+        };
+
+        let mut any = false;
+        let mut sibling_invalidations = InvalidationVector::new();
+        for element in shadow.parts() {
+            any |= self.invalidate_child(
+                *element,
+                invalidations,
+                &mut sibling_invalidations,
+                DescendantInvalidationKind::Part,
+            );
+            debug_assert!(
+                sibling_invalidations.is_empty(),
+                "::part() shouldn't have sibling combinators to the right, \
+                 this makes no sense! {:?}",
+                sibling_invalidations
+            );
+        }
+        any
+    }
+
+
     fn invalidate_slotted_elements(&mut self, invalidations: &[Invalidation<'b>]) -> bool {
         if invalidations.is_empty() {
             return false;
@@ -598,6 +633,7 @@ where
 
         any_descendant |= self.invalidate_non_slotted_descendants(&invalidations.dom_descendants);
         any_descendant |= self.invalidate_slotted_elements(&invalidations.slotted_descendants);
+        any_descendant |= self.invalidate_parts(&invalidations.parts);
 
         any_descendant
     }
@@ -672,7 +708,7 @@ where
                 debug_assert_eq!(
                     descendant_invalidation_kind,
                     DescendantInvalidationKind::Dom,
-                    "Slotted invalidations don't propagate."
+                    "Slotted or part invalidations don't propagate."
                 );
                 descendant_invalidations.dom_descendants.push(invalidation);
             }
@@ -860,6 +896,9 @@ where
                                 .dom_descendants
                                 .push(next_invalidation);
                         },
+                        InvalidationKind::Descendant(DescendantInvalidationKind::Part) => {
+                            descendant_invalidations.parts.push(next_invalidation);
+                        },
                         InvalidationKind::Descendant(DescendantInvalidationKind::Slotted) => {
                             descendant_invalidations
                                 .slotted_descendants
diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs
index a49bb6306c5..eccc5f60bbf 100644
--- a/components/style/invalidation/element/state_and_attributes.rs
+++ b/components/style/invalidation/element/state_and_attributes.rs
@@ -472,6 +472,9 @@ where
             DependencyInvalidationKind::Siblings => {
                 self.sibling_invalidations.push(invalidation);
             },
+            DependencyInvalidationKind::Parts => {
+                self.descendant_invalidations.parts.push(invalidation);
+            },
             DependencyInvalidationKind::SlottedElements => {
                 self.descendant_invalidations
                     .slotted_descendants
@@ -486,6 +489,7 @@ where
         match dependency.invalidation_kind() {
             DependencyInvalidationKind::Element => !self.invalidates_self,
             DependencyInvalidationKind::SlottedElements => self.element.is_html_slot_element(),
+            DependencyInvalidationKind::Parts => self.element.shadow_root().is_some(),
             DependencyInvalidationKind::ElementAndDescendants |
             DependencyInvalidationKind::Siblings |
             DependencyInvalidationKind::Descendants => true,

From 96594be65d131fed2aad55187fa291ab8a1d8934 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 11 Jun 2019 17:42:09 +0000
Subject: [PATCH 12/32] style: Don't go through all the part names if not
 there.

This uses the bit added for tracking part attributes in order to avoid doing
wasted work.

Differential Revision: https://phabricator.services.mozilla.com/D32644
---
 components/style/gecko/wrapper.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs
index aa59df56bde..70eaa552ed5 100644
--- a/components/style/gecko/wrapper.rs
+++ b/components/style/gecko/wrapper.rs
@@ -590,6 +590,9 @@ impl<'le> GeckoElement<'le> {
 
     #[inline(always)]
     fn get_part_attr(&self) -> Option<&structs::nsAttrValue> {
+        if !self.has_part_attr() {
+            return None;
+        }
         snapshot_helpers::find_attr(self.attrs(), &atom!("part"))
     }
 
@@ -623,6 +626,11 @@ impl<'le> GeckoElement<'le> {
         }
     }
 
+    #[inline]
+    fn has_part_attr(&self) -> bool {
+        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasPart)
+    }
+
     #[inline]
     fn may_have_anonymous_children(&self) -> bool {
         self.as_node()

From fac050325c35729e6f5ff137e259bfe0b301b925 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 11 Jun 2019 17:42:23 +0000
Subject: [PATCH 13/32] style: Add an extra flag to flag ::part() to selectors.

This grows the selector struct, but only in 32-bit, since in 64-bit we take
space from the alignment padding that we're paying due to having the size of the
slice as a word.

Differential Revision: https://phabricator.services.mozilla.com/D32645
---
 components/selectors/builder.rs | 50 ++++++++++++++++-------
 components/selectors/parser.rs  | 70 ++++++++++++++++++++++++++-------
 2 files changed, 92 insertions(+), 28 deletions(-)

diff --git a/components/selectors/builder.rs b/components/selectors/builder.rs
index c3e733cbeb0..cddaf7195a1 100644
--- a/components/selectors/builder.rs
+++ b/components/selectors/builder.rs
@@ -96,18 +96,24 @@ impl<Impl: SelectorImpl> SelectorBuilder<Impl> {
         &mut self,
         parsed_pseudo: bool,
         parsed_slotted: bool,
+        parsed_part: bool,
     ) -> ThinArc<SpecificityAndFlags, Component<Impl>> {
         // Compute the specificity and flags.
-        let mut spec = SpecificityAndFlags(specificity(self.simple_selectors.iter()));
+        let specificity = specificity(self.simple_selectors.iter());
+        let mut flags = SelectorFlags::empty();
         if parsed_pseudo {
-            spec.0 |= HAS_PSEUDO_BIT;
+            flags |= SelectorFlags::HAS_PSEUDO;
         }
-
         if parsed_slotted {
-            spec.0 |= HAS_SLOTTED_BIT;
+            flags |= SelectorFlags::HAS_SLOTTED;
         }
-
-        self.build_with_specificity_and_flags(spec)
+        if parsed_part {
+            flags |= SelectorFlags::HAS_PART;
+        }
+        self.build_with_specificity_and_flags(SpecificityAndFlags {
+            specificity,
+            flags,
+        })
     }
 
     /// Builds with an explicit SpecificityAndFlags. This is separated from build() so
@@ -188,28 +194,44 @@ fn split_from_end<T>(s: &[T], at: usize) -> (&[T], &[T]) {
     s.split_at(s.len() - at)
 }
 
-pub const HAS_PSEUDO_BIT: u32 = 1 << 30;
-pub const HAS_SLOTTED_BIT: u32 = 1 << 31;
+bitflags! {
+    /// Flags that indicate at which point of parsing a selector are we.
+    #[derive(Default, ToShmem)]
+    pub (crate) struct SelectorFlags : u8 {
+        const HAS_PSEUDO = 1 << 0;
+        const HAS_SLOTTED = 1 << 1;
+        const HAS_PART = 1 << 2;
+    }
+}
 
-/// We use ten bits for each specificity kind (id, class, element), and the two
-/// high bits for the pseudo and slotted flags.
 #[derive(Clone, Copy, Debug, Eq, PartialEq, ToShmem)]
-pub struct SpecificityAndFlags(pub u32);
+pub struct SpecificityAndFlags {
+    /// There are two free bits here, since we use ten bits for each specificity
+    /// kind (id, class, element).
+    pub (crate) specificity: u32,
+    /// There's padding after this field due to the size of the flags.
+    pub (crate) flags: SelectorFlags,
+}
 
 impl SpecificityAndFlags {
     #[inline]
     pub fn specificity(&self) -> u32 {
-        self.0 & !(HAS_PSEUDO_BIT | HAS_SLOTTED_BIT)
+        self.specificity
     }
 
     #[inline]
     pub fn has_pseudo_element(&self) -> bool {
-        (self.0 & HAS_PSEUDO_BIT) != 0
+        self.flags.intersects(SelectorFlags::HAS_PSEUDO)
     }
 
     #[inline]
     pub fn is_slotted(&self) -> bool {
-        (self.0 & HAS_SLOTTED_BIT) != 0
+        self.flags.intersects(SelectorFlags::HAS_SLOTTED)
+    }
+
+    #[inline]
+    pub fn is_part(&self) -> bool {
+        self.flags.intersects(SelectorFlags::HAS_PART)
     }
 }
 
diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs
index f049555730c..fa43aaa38a2 100644
--- a/components/selectors/parser.rs
+++ b/components/selectors/parser.rs
@@ -6,7 +6,7 @@ use crate::attr::{AttrSelectorOperator, AttrSelectorWithOptionalNamespace};
 use crate::attr::{NamespaceConstraint, ParsedAttrSelectorOperation};
 use crate::attr::{ParsedCaseSensitivity, SELECTOR_WHITESPACE};
 use crate::bloom::BLOOM_HASH_MASK;
-use crate::builder::{SelectorBuilder, SpecificityAndFlags};
+use crate::builder::{SelectorBuilder, SelectorFlags, SpecificityAndFlags};
 use crate::context::QuirksMode;
 use crate::sink::Push;
 pub use crate::visitor::{SelectorVisitor, Visit};
@@ -601,6 +601,11 @@ impl<Impl: SelectorImpl> Selector<Impl> {
         self.0.header.header.is_slotted()
     }
 
+    #[inline]
+    pub fn is_part(&self) -> bool {
+        self.0.header.header.is_part()
+    }
+
     #[inline]
     pub fn pseudo_element(&self) -> Option<&Impl::PseudoElement> {
         if !self.has_pseudo_element() {
@@ -720,7 +725,8 @@ impl<Impl: SelectorImpl> Selector<Impl> {
     }
 
     /// Creates a Selector from a vec of Components, specified in parse order. Used in tests.
-    pub fn from_vec(vec: Vec<Component<Impl>>, specificity_and_flags: u32) -> Self {
+    #[allow(unused)]
+    pub (crate) fn from_vec(vec: Vec<Component<Impl>>, specificity: u32, flags: SelectorFlags) -> Self {
         let mut builder = SelectorBuilder::default();
         for component in vec.into_iter() {
             if let Some(combinator) = component.as_combinator() {
@@ -729,7 +735,10 @@ impl<Impl: SelectorImpl> Selector<Impl> {
                 builder.push_simple_selector(component);
             }
         }
-        let spec = SpecificityAndFlags(specificity_and_flags);
+        let spec = SpecificityAndFlags {
+            specificity,
+            flags,
+        };
         Selector(builder.build_with_specificity_and_flags(spec))
     }
 
@@ -1445,6 +1454,7 @@ where
 
     let mut has_pseudo_element = false;
     let mut slotted = false;
+    let mut part = false;
     'outer_loop: loop {
         // Parse a sequence of simple selectors.
         let state = match parse_compound_selector(parser, input, &mut builder)? {
@@ -1461,7 +1471,7 @@ where
         if state.intersects(SelectorParsingState::AFTER_PSEUDO) {
             has_pseudo_element = state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT);
             slotted = state.intersects(SelectorParsingState::AFTER_SLOTTED);
-            let part = state.intersects(SelectorParsingState::AFTER_PART);
+            part = state.intersects(SelectorParsingState::AFTER_PART);
             debug_assert!(has_pseudo_element || slotted || part);
             break;
         }
@@ -1500,9 +1510,7 @@ where
         builder.push_combinator(combinator);
     }
 
-    // TODO(emilio): We'll have to flag part() somehow as well, but we need more
-    // bits!
-    Ok(Selector(builder.build(has_pseudo_element, slotted)))
+    Ok(Selector(builder.build(has_pseudo_element, slotted, part)))
 }
 
 impl<Impl: SelectorImpl> Selector<Impl> {
@@ -2249,7 +2257,7 @@ where
 #[cfg(test)]
 pub mod tests {
     use super::*;
-    use crate::builder::HAS_PSEUDO_BIT;
+    use crate::builder::SelectorFlags;
     use crate::parser;
     use cssparser::{serialize_identifier, Parser as CssParser, ParserInput, ToCss};
     use std::collections::HashMap;
@@ -2532,6 +2540,7 @@ pub mod tests {
                     lower_name: DummyAtom::from("eeÉ"),
                 })],
                 specificity(0, 0, 1),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2545,6 +2554,7 @@ pub mod tests {
                     }),
                 ],
                 specificity(0, 0, 1),
+                Default::default(),
             )]))
         );
         // When the default namespace is not set, *| should be elided.
@@ -2557,6 +2567,7 @@ pub mod tests {
                     lower_name: DummyAtom::from("e"),
                 })],
                 specificity(0, 0, 1),
+                Default::default(),
             )]))
         );
         // When the default namespace is set, *| should _not_ be elided (as foo
@@ -2577,13 +2588,15 @@ pub mod tests {
                     }),
                 ],
                 specificity(0, 0, 1),
+                Default::default(),
             )]))
         );
         assert_eq!(
             parse("*"),
             Ok(SelectorList::from_vec(vec![Selector::from_vec(
                 vec![Component::ExplicitUniversalType],
-                specificity(0, 0, 0)
+                specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2594,13 +2607,15 @@ pub mod tests {
                     Component::ExplicitUniversalType,
                 ],
                 specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
             parse_expected("*|*", Some("*")),
             Ok(SelectorList::from_vec(vec![Selector::from_vec(
                 vec![Component::ExplicitUniversalType],
-                specificity(0, 0, 0)
+                specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2614,6 +2629,7 @@ pub mod tests {
                     Component::ExplicitUniversalType,
                 ],
                 specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2624,6 +2640,7 @@ pub mod tests {
                     Component::NonTSPseudoClass(PseudoClass::Lang("en-US".to_owned())),
                 ],
                 specificity(0, 2, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2631,6 +2648,7 @@ pub mod tests {
             Ok(SelectorList::from_vec(vec![Selector::from_vec(
                 vec![Component::ID(DummyAtom::from("bar"))],
                 specificity(1, 0, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2645,6 +2663,7 @@ pub mod tests {
                     Component::ID(DummyAtom::from("bar")),
                 ],
                 specificity(1, 1, 1),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2660,6 +2679,7 @@ pub mod tests {
                     Component::ID(DummyAtom::from("bar")),
                 ],
                 specificity(1, 1, 1),
+                Default::default(),
             )]))
         );
         // Default namespace does not apply to attribute selectors
@@ -2673,6 +2693,7 @@ pub mod tests {
                     local_name_lower: DummyAtom::from("foo"),
                 }],
                 specificity(0, 1, 0),
+                Default::default(),
             )]))
         );
         assert!(parse_ns("svg|circle", &parser).is_err());
@@ -2690,6 +2711,7 @@ pub mod tests {
                     }),
                 ],
                 specificity(0, 0, 1),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2700,6 +2722,7 @@ pub mod tests {
                     Component::ExplicitUniversalType,
                 ],
                 specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
         // Default namespace does not apply to attribute selectors
@@ -2718,6 +2741,7 @@ pub mod tests {
                     },
                 ],
                 specificity(0, 1, 0),
+                Default::default(),
             )]))
         );
         // Default namespace does apply to type selectors
@@ -2732,6 +2756,7 @@ pub mod tests {
                     }),
                 ],
                 specificity(0, 0, 1),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2742,6 +2767,7 @@ pub mod tests {
                     Component::ExplicitUniversalType,
                 ],
                 specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2752,6 +2778,7 @@ pub mod tests {
                     Component::ExplicitUniversalType,
                 ],
                 specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
         // Default namespace applies to universal and type selectors inside :not and :matches,
@@ -2768,6 +2795,7 @@ pub mod tests {
                     ),
                 ],
                 specificity(0, 1, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2785,6 +2813,7 @@ pub mod tests {
                     ),
                 ],
                 specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2805,6 +2834,7 @@ pub mod tests {
                     ),
                 ],
                 specificity(0, 0, 1),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2818,6 +2848,7 @@ pub mod tests {
                     case_sensitivity: ParsedCaseSensitivity::CaseSensitive,
                 }],
                 specificity(0, 1, 0),
+                Default::default(),
             )]))
         );
         // https://github.com/mozilla/servo/issues/1723
@@ -2828,7 +2859,8 @@ pub mod tests {
                     Component::Combinator(Combinator::PseudoElement),
                     Component::PseudoElement(PseudoElement::Before),
                 ],
-                specificity(0, 0, 1) | HAS_PSEUDO_BIT,
+                specificity(0, 0, 1),
+                SelectorFlags::HAS_PSEUDO,
             )]))
         );
         assert_eq!(
@@ -2839,7 +2871,8 @@ pub mod tests {
                     Component::PseudoElement(PseudoElement::Before),
                     Component::NonTSPseudoClass(PseudoClass::Hover),
                 ],
-                specificity(0, 1, 1) | HAS_PSEUDO_BIT,
+                specificity(0, 1, 1),
+                SelectorFlags::HAS_PSEUDO,
             )]))
         );
         assert_eq!(
@@ -2851,7 +2884,8 @@ pub mod tests {
                     Component::NonTSPseudoClass(PseudoClass::Hover),
                     Component::NonTSPseudoClass(PseudoClass::Hover),
                 ],
-                specificity(0, 2, 1) | HAS_PSEUDO_BIT,
+                specificity(0, 2, 1),
+                SelectorFlags::HAS_PSEUDO,
             )]))
         );
         assert!(parse("::before:hover:lang(foo)").is_err());
@@ -2874,7 +2908,8 @@ pub mod tests {
                     Component::Combinator(Combinator::PseudoElement),
                     Component::PseudoElement(PseudoElement::After),
                 ],
-                specificity(0, 0, 2) | HAS_PSEUDO_BIT,
+                specificity(0, 0, 2),
+                SelectorFlags::HAS_PSEUDO,
             )]))
         );
         assert_eq!(
@@ -2886,6 +2921,7 @@ pub mod tests {
                     Component::Class(DummyAtom::from("ok")),
                 ],
                 (1 << 20) + (1 << 10) + (0 << 0),
+                Default::default(),
             )]))
         );
         parser.default_ns = None;
@@ -2901,6 +2937,7 @@ pub mod tests {
                         .into(),
                 )],
                 specificity(1, 0, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2918,6 +2955,7 @@ pub mod tests {
                     .into(),
                 )],
                 specificity(0, 0, 1),
+                Default::default(),
             )]))
         );
         // https://github.com/servo/servo/issues/16017
@@ -2930,6 +2968,7 @@ pub mod tests {
                         .into(),
                 )],
                 specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
         assert_eq!(
@@ -2944,6 +2983,7 @@ pub mod tests {
                     .into(),
                 )],
                 specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
         // *| should be elided if there is no default namespace.
@@ -2957,6 +2997,7 @@ pub mod tests {
                         .into(),
                 )],
                 specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
 
@@ -2972,6 +3013,7 @@ pub mod tests {
                     .into(),
                 )],
                 specificity(0, 0, 0),
+                Default::default(),
             )]))
         );
 

From 39de0a068ea3b1ff021ddf979575ce2feeaa6b32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 11 Jun 2019 17:42:32 +0000
Subject: [PATCH 14/32] style: Collect ::part() rules during CascadeData
 rebuilds.

Unlike for :host() or ::slotted(), or regular rules, we don't need a whole
SelectorMap<>, so gotta make the code a bit more generic.

Differential Revision: https://phabricator.services.mozilla.com/D32646
---
 components/selectors/parser.rs |  25 ++++++++
 components/style/stylist.rs    | 114 +++++++++++++++++++++++----------
 2 files changed, 104 insertions(+), 35 deletions(-)

diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs
index fa43aaa38a2..d76defe8131 100644
--- a/components/selectors/parser.rs
+++ b/components/selectors/parser.rs
@@ -606,6 +606,31 @@ impl<Impl: SelectorImpl> Selector<Impl> {
         self.0.header.header.is_part()
     }
 
+    #[inline]
+    pub fn part(&self) -> Option<&Impl::PartName> {
+        if !self.is_part() {
+            return None;
+        }
+
+        let mut iter = self.iter();
+        if self.has_pseudo_element() {
+            // Skip the pseudo-element.
+            for _ in &mut iter {}
+
+            let combinator = iter.next_sequence()?;
+            debug_assert_eq!(combinator, Combinator::PseudoElement);
+        }
+
+        for component in iter {
+            if let Component::Part(ref part) = *component {
+                return Some(part);
+            }
+        }
+
+        debug_assert!(false, "is_part() lied somehow?");
+        None
+    }
+
     #[inline]
     pub fn pseudo_element(&self) -> Option<&Impl::PseudoElement> {
         if !self.has_pseudo_element() {
diff --git a/components/style/stylist.rs b/components/style/stylist.rs
index a98b8a89eb5..c2601515002 100644
--- a/components/style/stylist.rs
+++ b/components/style/stylist.rs
@@ -33,6 +33,7 @@ use crate::stylesheets::{CounterStyleRule, FontFaceRule, FontFeatureValuesRule,
 use crate::stylesheets::{CssRule, Origin, OriginSet, PerOrigin, PerOriginIter};
 use crate::thread_state::{self, ThreadState};
 use crate::{Atom, LocalName, Namespace, WeakAtom};
+use fallible::FallibleVec;
 use hashglobe::FailedAllocationError;
 #[cfg(feature = "gecko")]
 use malloc_size_of::MallocUnconditionalShallowSizeOf;
@@ -1641,9 +1642,9 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
 
 /// A set of rules for element and pseudo-elements.
 #[derive(Debug, Default, MallocSizeOf)]
-struct ElementAndPseudoRules {
+struct GenericElementAndPseudoRules<Map> {
     /// Rules from stylesheets at this `CascadeData`'s origin.
-    element_map: SelectorMap<Rule>,
+    element_map: Map,
 
     /// Rules from stylesheets at this `CascadeData`'s origin that correspond
     /// to a given pseudo-element.
@@ -1651,39 +1652,30 @@ struct ElementAndPseudoRules {
     /// FIXME(emilio): There are a bunch of wasted entries here in practice.
     /// Figure out a good way to do a `PerNonAnonBox` and `PerAnonBox` (for
     /// `precomputed_values_for_pseudo`) without duplicating a lot of code.
-    pseudos_map: PerPseudoElementMap<Box<SelectorMap<Rule>>>,
+    pseudos_map: PerPseudoElementMap<Box<Map>>,
 }
 
-impl ElementAndPseudoRules {
+impl<Map: Default + MallocSizeOf> GenericElementAndPseudoRules<Map> {
     #[inline(always)]
-    fn insert(
-        &mut self,
-        rule: Rule,
-        pseudo_element: Option<&PseudoElement>,
-        quirks_mode: QuirksMode,
-    ) -> Result<(), FailedAllocationError> {
+    fn for_insertion(&mut self, pseudo_element: Option<&PseudoElement>) -> &mut Map {
         debug_assert!(
-            pseudo_element.map_or(true, |pseudo| !pseudo.is_precomputed() &&
-                !pseudo.is_unknown_webkit_pseudo_element())
+            pseudo_element.map_or(true, |pseudo| {
+                !pseudo.is_precomputed() && !pseudo.is_unknown_webkit_pseudo_element()
+            }),
+            "Precomputed pseudos should end up in precomputed_pseudo_element_decls, \
+             and unknown webkit pseudos should be discarded before getting here"
         );
 
-        let map = match pseudo_element {
+        match pseudo_element {
             None => &mut self.element_map,
             Some(pseudo) => self
                 .pseudos_map
-                .get_or_insert_with(pseudo, || Box::new(SelectorMap::new())),
-        };
-
-        map.insert(rule, quirks_mode)
-    }
-
-    fn clear(&mut self) {
-        self.element_map.clear();
-        self.pseudos_map.clear();
+                .get_or_insert_with(pseudo, || Box::new(Default::default())),
+        }
     }
 
     #[inline]
-    fn rules(&self, pseudo: Option<&PseudoElement>) -> Option<&SelectorMap<Rule>> {
+    fn rules(&self, pseudo: Option<&PseudoElement>) -> Option<&Map> {
         match pseudo {
             Some(pseudo) => self.pseudos_map.get(pseudo).map(|p| &**p),
             None => Some(&self.element_map),
@@ -1703,6 +1695,26 @@ impl ElementAndPseudoRules {
     }
 }
 
+type ElementAndPseudoRules = GenericElementAndPseudoRules<SelectorMap<Rule>>;
+type PartMap = PrecomputedHashMap<Atom, SmallVec<[Rule; 1]>>;
+type PartElementAndPseudoRules = GenericElementAndPseudoRules<PartMap>;
+
+impl ElementAndPseudoRules {
+    // TODO(emilio): Should we retain storage of these?
+    fn clear(&mut self) {
+        self.element_map.clear();
+        self.pseudos_map.clear();
+    }
+}
+
+impl PartElementAndPseudoRules {
+    // TODO(emilio): Should we retain storage of these?
+    fn clear(&mut self) {
+        self.element_map.clear();
+        self.pseudos_map.clear();
+    }
+}
+
 /// Data resulting from performing the CSS cascade that is specific to a given
 /// origin.
 ///
@@ -1727,6 +1739,12 @@ pub struct CascadeData {
     /// containing style scopes starting from the closest assigned slot.
     slotted_rules: Option<Box<ElementAndPseudoRules>>,
 
+    /// The data coming from ::part() pseudo-element rules.
+    ///
+    /// We need to store them separately because an element needs to match
+    /// ::part() pseudo-element rules in different shadow roots.
+    part_rules: Option<Box<PartElementAndPseudoRules>>,
+
     /// The invalidation map for these rules.
     invalidation_map: InvalidationMap,
 
@@ -1786,6 +1804,7 @@ impl CascadeData {
             normal_rules: ElementAndPseudoRules::default(),
             host_rules: None,
             slotted_rules: None,
+            part_rules: None,
             invalidation_map: InvalidationMap::new(),
             attribute_dependencies: PrecomputedHashSet::default(),
             state_dependencies: ElementState::empty(),
@@ -1876,6 +1895,12 @@ impl CascadeData {
         self.slotted_rules.as_ref().and_then(|d| d.rules(pseudo))
     }
 
+    /// Returns the parts rule map for a given pseudo-element.
+    #[inline]
+    pub fn part_rules(&self, pseudo: Option<&PseudoElement>) -> Option<&PartMap> {
+        self.part_rules.as_ref().and_then(|d| d.rules(pseudo))
+    }
+
     /// Collects all the applicable media query results into `results`.
     ///
     /// This duplicates part of the logic in `add_stylesheet`, which is
@@ -2005,20 +2030,33 @@ impl CascadeData {
                             }
                         }
 
-                        // NOTE(emilio): It's fine to look at :host and then at
-                        // ::slotted(..), since :host::slotted(..) could never
-                        // possibly match, as <slot> is not a valid shadow host.
-                        let rules = if selector.is_featureless_host_selector_or_pseudo_element() {
-                            self.host_rules
-                                .get_or_insert_with(|| Box::new(Default::default()))
-                        } else if selector.is_slotted() {
-                            self.slotted_rules
+                        // Part is special, since given it doesn't have any
+                        // selectors inside, it's not worth using a whole
+                        // SelectorMap for it.
+                        if let Some(part) = selector.part() {
+                            self.part_rules
                                 .get_or_insert_with(|| Box::new(Default::default()))
+                                .for_insertion(pseudo_element)
+                                .try_entry(part.clone())?
+                                .or_insert_with(SmallVec::new)
+                                .try_push(rule)?;
                         } else {
-                            &mut self.normal_rules
-                        };
-
-                        rules.insert(rule, pseudo_element, quirks_mode)?;
+                            // NOTE(emilio): It's fine to look at :host and then at
+                            // ::slotted(..), since :host::slotted(..) could never
+                            // possibly match, as <slot> is not a valid shadow host.
+                            let rules =
+                                if selector.is_featureless_host_selector_or_pseudo_element() {
+                                    self.host_rules
+                                        .get_or_insert_with(|| Box::new(Default::default()))
+                                } else if selector.is_slotted() {
+                                    self.slotted_rules
+                                        .get_or_insert_with(|| Box::new(Default::default()))
+                                } else {
+                                    &mut self.normal_rules
+                                }
+                                .for_insertion(pseudo_element);
+                            rules.insert(rule, quirks_mode)?;
+                        }
                     }
                     self.rules_source_order += 1;
                 },
@@ -2184,6 +2222,9 @@ impl CascadeData {
         if let Some(ref mut slotted_rules) = self.slotted_rules {
             slotted_rules.clear();
         }
+        if let Some(ref mut part_rules) = self.part_rules {
+            part_rules.clear();
+        }
         if let Some(ref mut host_rules) = self.host_rules {
             host_rules.clear();
         }
@@ -2212,6 +2253,9 @@ impl CascadeData {
         if let Some(ref slotted_rules) = self.slotted_rules {
             slotted_rules.add_size_of(ops, sizes);
         }
+        if let Some(ref part_rules) = self.part_rules {
+            part_rules.add_size_of(ops, sizes);
+        }
         if let Some(ref host_rules) = self.host_rules {
             host_rules.add_size_of(ops, sizes);
         }

From 7139a4185a583f190cab1e9e8fb056cd6149a815 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 11 Jun 2019 17:42:41 +0000
Subject: [PATCH 15/32] style: Add code to make part rules affect the style of
 the elements.

I still haven't implemented each_part(), so this will do nothing yet.

The cascade order stuff is fishy, I know, and I'll fix in a followup if it's
fine with you. I moved the sorting of the rules to rule_collector, since it
seemed to me it was better that way that duplicating the code, and those
SelectorMap functions only have a single caller anyway.

Differential Revision: https://phabricator.services.mozilla.com/D32647
---
 components/selectors/context.rs    |  4 +-
 components/style/dom.rs            | 10 ++++
 components/style/gecko/wrapper.rs  | 10 ++--
 components/style/rule_collector.rs | 91 ++++++++++++++++++++++++++----
 components/style/selector_map.rs   | 10 +---
 components/style/stylist.rs        |  2 +-
 6 files changed, 100 insertions(+), 27 deletions(-)

diff --git a/components/selectors/context.rs b/components/selectors/context.rs
index 3686513e79d..d159891ff02 100644
--- a/components/selectors/context.rs
+++ b/components/selectors/context.rs
@@ -279,13 +279,13 @@ where
     /// Runs F with a given shadow host which is the root of the tree whose
     /// rules we're matching.
     #[inline]
-    pub fn with_shadow_host<F, E, R>(&mut self, host: E, f: F) -> R
+    pub fn with_shadow_host<F, E, R>(&mut self, host: Option<E>, f: F) -> R
     where
         E: Element,
         F: FnOnce(&mut Self) -> R,
     {
         let original_host = self.current_host.take();
-        self.current_host = Some(host.opaque());
+        self.current_host = host.map(|h| h.opaque());
         let result = f(self);
         self.current_host = original_host;
         result
diff --git a/components/style/dom.rs b/components/style/dom.rs
index 0a743c60207..0e68e55c47f 100644
--- a/components/style/dom.rs
+++ b/components/style/dom.rs
@@ -520,6 +520,9 @@ pub trait TElement:
     /// Whether this element has an attribute with a given namespace.
     fn has_attr(&self, namespace: &Namespace, attr: &LocalName) -> bool;
 
+    /// Returns whether this element has a `part` attribute.
+    fn has_part_attr(&self) -> bool;
+
     /// The ID for this element.
     fn id(&self) -> Option<&WeakAtom>;
 
@@ -528,6 +531,13 @@ pub trait TElement:
     where
         F: FnMut(&Atom);
 
+    /// Internal iterator for the part names of this element.
+    fn each_part<F>(&self, _callback: F)
+    where
+        F: FnMut(&Atom)
+    {
+    }
+
     /// Whether a given element may generate a pseudo-element.
     ///
     /// This is useful to avoid computing, for example, pseudo styles for
diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs
index 70eaa552ed5..17c1cfaf35d 100644
--- a/components/style/gecko/wrapper.rs
+++ b/components/style/gecko/wrapper.rs
@@ -626,11 +626,6 @@ impl<'le> GeckoElement<'le> {
         }
     }
 
-    #[inline]
-    fn has_part_attr(&self) -> bool {
-        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasPart)
-    }
-
     #[inline]
     fn may_have_anonymous_children(&self) -> bool {
         self.as_node()
@@ -1353,6 +1348,11 @@ impl<'le> TElement for GeckoElement<'le> {
         unsafe { bindings::Gecko_HasAttr(self.0, namespace.0.as_ptr(), attr.as_ptr()) }
     }
 
+    #[inline]
+    fn has_part_attr(&self) -> bool {
+        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasPart)
+    }
+
     // FIXME(emilio): we should probably just return a reference to the Atom.
     #[inline]
     fn id(&self) -> Option<&WeakAtom> {
diff --git a/components/style/rule_collector.rs b/components/style/rule_collector.rs
index 360b8101ef1..dcd39c2b36c 100644
--- a/components/style/rule_collector.rs
+++ b/components/style/rule_collector.rs
@@ -54,6 +54,11 @@ pub fn containing_shadow_ignoring_svg_use<E: TElement>(
     }
 }
 
+#[inline]
+fn sort_rules_from(rules: &mut ApplicableDeclarationList, start: usize) {
+    rules[start..].sort_unstable_by_key(|block| (block.specificity, block.source_order()));
+}
+
 /// An object that we use with all the intermediate state needed for the
 /// cascade.
 ///
@@ -146,15 +151,7 @@ where
             None => return,
         };
 
-        map.get_all_matching_rules(
-            self.element,
-            self.rule_hash_target,
-            self.rules,
-            self.context,
-            self.flags_setter,
-            cascade_level,
-            0,
-        );
+        self.collect_rules_internal(None, map, cascade_level);
     }
 
     fn collect_user_agent_rules(&mut self) {
@@ -200,11 +197,23 @@ where
         cascade_level: CascadeLevel,
     ) {
         debug_assert!(shadow_host.shadow_root().is_some());
+        self.collect_rules_internal(Some(shadow_host), map, cascade_level);
+        self.shadow_cascade_order += 1;
+    }
+
+    #[inline]
+    fn collect_rules_internal(
+        &mut self,
+        shadow_host: Option<E>,
+        map: &SelectorMap<Rule>,
+        cascade_level: CascadeLevel,
+    ) {
         let element = self.element;
         let rule_hash_target = self.rule_hash_target;
         let rules = &mut self.rules;
         let flags_setter = &mut self.flags_setter;
         let shadow_cascade_order = self.shadow_cascade_order;
+        let start = rules.len();
         self.context.with_shadow_host(shadow_host, |context| {
             map.get_all_matching_rules(
                 element,
@@ -216,7 +225,7 @@ where
                 shadow_cascade_order,
             );
         });
-        self.shadow_cascade_order += 1;
+        sort_rules_from(rules, start);
     }
 
     /// Collects the rules for the ::slotted pseudo-element.
@@ -310,6 +319,67 @@ where
         self.collect_stylist_rules(Origin::Author);
     }
 
+    fn collect_part_rules(&mut self) {
+        if !self.rule_hash_target.has_part_attr() {
+            return;
+        }
+
+        let shadow = match self.rule_hash_target.containing_shadow() {
+            Some(s) => s,
+            None => return,
+        };
+
+        let host = shadow.host();
+        let containing_shadow = host.containing_shadow();
+        let part_rules = match containing_shadow {
+            Some(shadow) => {
+                shadow
+                    .style_data()
+                    .and_then(|data| data.part_rules(self.pseudo_element))
+            },
+            None => {
+                self.stylist
+                    .cascade_data()
+                    .borrow_for_origin(Origin::Author)
+                    .part_rules(self.pseudo_element)
+            }
+        };
+
+        // TODO(emilio): SameTreeAuthorNormal is a bit of a lie here, we may
+        // need an OuterTreeAuthorNormal cascade level or such, and change the
+        // cascade order, if we allow to forward parts to even outer trees.
+        //
+        // Though the current thing kinda works because we apply them after
+        // the outer tree, so as long as we don't allow forwarding we're
+        // good.
+        if let Some(part_rules) = part_rules {
+            let containing_host = containing_shadow.map(|s| s.host());
+            let element = self.element;
+            let rule_hash_target = self.rule_hash_target;
+            let rules = &mut self.rules;
+            let flags_setter = &mut self.flags_setter;
+            let shadow_cascade_order = self.shadow_cascade_order;
+            let cascade_level = CascadeLevel::SameTreeAuthorNormal;
+            let start = rules.len();
+            self.context.with_shadow_host(containing_host, |context| {
+                rule_hash_target.each_part(|p| {
+                    if let Some(part_rules) = part_rules.get(p) {
+                        SelectorMap::get_matching_rules(
+                            element,
+                            &part_rules,
+                            rules,
+                            context,
+                            flags_setter,
+                            cascade_level,
+                            shadow_cascade_order,
+                        );
+                    }
+                });
+            });
+            sort_rules_from(rules, start);
+        }
+    }
+
     fn collect_style_attribute_and_animation_rules(&mut self) {
         if let Some(sa) = self.style_attribute {
             self.rules
@@ -368,6 +438,7 @@ where
         self.collect_slotted_rules();
         self.collect_normal_rules_from_containing_shadow_tree();
         self.collect_document_author_rules();
+        self.collect_part_rules();
         self.collect_style_attribute_and_animation_rules();
     }
 }
diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs
index afae5450182..a1ef9d0c273 100644
--- a/components/style/selector_map.rs
+++ b/components/style/selector_map.rs
@@ -182,10 +182,6 @@ impl SelectorMap<Rule> {
 
         let quirks_mode = context.quirks_mode();
 
-        // At the end, we're going to sort the rules that we added, so remember
-        // where we began.
-        let init_len = matching_rules_list.len();
-
         if rule_hash_target.is_root() {
             SelectorMap::get_matching_rules(
                 element,
@@ -259,14 +255,10 @@ impl SelectorMap<Rule> {
             cascade_level,
             shadow_cascade_order,
         );
-
-        // Sort only the rules we just added.
-        matching_rules_list[init_len..]
-            .sort_unstable_by_key(|block| (block.specificity, block.source_order()));
     }
 
     /// Adds rules in `rules` that match `element` to the `matching_rules` list.
-    fn get_matching_rules<E, F>(
+    pub (crate) fn get_matching_rules<E, F>(
         element: E,
         rules: &[Rule],
         matching_rules: &mut ApplicableDeclarationList,
diff --git a/components/style/stylist.rs b/components/style/stylist.rs
index c2601515002..33ce450151b 100644
--- a/components/style/stylist.rs
+++ b/components/style/stylist.rs
@@ -1257,7 +1257,7 @@ impl Stylist {
 
         let matches_document_rules =
             element.each_applicable_non_document_style_rule_data(|data, host| {
-                matching_context.with_shadow_host(host, |matching_context| {
+                matching_context.with_shadow_host(Some(host), |matching_context| {
                     data.selectors_for_cache_revalidation.lookup(
                         element,
                         self.quirks_mode,

From e272bfed70819d25911285bebf9fb3397bb3dbae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 11 Jun 2019 17:42:49 +0000
Subject: [PATCH 16/32] style: Implement GeckoElement::each_part.

This should make all the pieces come together.

Note that we don't need to look at the snapshot for ::part() for now (other than
when selector-matching normally) because I decided to just restyle the element
for now when the part attribute changes.

::part() can't affect descendants anyway (as long as we don't do the forwarding
stuff), and eager pseudo-elements are handled during the normal element restyle,
so it seems to me that adding all the complexity that we have for classes to
part may not be worth it at least yet.

Differential Revision: https://phabricator.services.mozilla.com/D32648
---
 components/style/gecko/snapshot.rs         |  2 +-
 components/style/gecko/snapshot_helpers.rs |  4 ++--
 components/style/gecko/wrapper.rs          | 14 +++++++++++++-
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs
index d294939baeb..cffb78d3e9e 100644
--- a/components/style/gecko/snapshot.rs
+++ b/components/style/gecko/snapshot.rs
@@ -211,7 +211,7 @@ impl ElementSnapshot for GeckoElementSnapshot {
             return;
         }
 
-        snapshot_helpers::each_class(&self.mClass, callback)
+        snapshot_helpers::each_class_or_part(&self.mClass, callback)
     }
 
     #[inline]
diff --git a/components/style/gecko/snapshot_helpers.rs b/components/style/gecko/snapshot_helpers.rs
index aaa2254dbd7..b8b31bc87dd 100644
--- a/components/style/gecko/snapshot_helpers.rs
+++ b/components/style/gecko/snapshot_helpers.rs
@@ -107,9 +107,9 @@ pub fn has_class_or_part(
 }
 
 /// Given an item, a callback, and a getter, execute `callback` for each class
-/// this `item` has.
+/// or part name this `item` has.
 #[inline(always)]
-pub fn each_class<F>(attr: &structs::nsAttrValue, mut callback: F)
+pub fn each_class_or_part<F>(attr: &structs::nsAttrValue, mut callback: F)
 where
     F: FnMut(&Atom),
 {
diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs
index 17c1cfaf35d..ba2951f6f25 100644
--- a/components/style/gecko/wrapper.rs
+++ b/components/style/gecko/wrapper.rs
@@ -1372,7 +1372,19 @@ impl<'le> TElement for GeckoElement<'le> {
             None => return,
         };
 
-        snapshot_helpers::each_class(attr, callback)
+        snapshot_helpers::each_class_or_part(attr, callback)
+    }
+
+    fn each_part<F>(&self, callback: F)
+    where
+        F: FnMut(&Atom),
+    {
+        let attr = match self.get_part_attr() {
+            Some(c) => c,
+            None => return,
+        };
+
+        snapshot_helpers::each_class_or_part(attr, callback)
     }
 
     #[inline]

From 8b0d2b1e386c5bf9e513126de055205bc6853371 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 11 Jun 2019 17:42:58 +0000
Subject: [PATCH 17/32] style: Add a pref and enable Shadow Parts in Nightly
 and for chrome stylesheets.

I want to enable in Nightly to evaluate (in the medium term) shipping it without
the part forwarding, once the cascade order and importance issues are fixed, and
that we pass all the tests that don't involve forwarding.

That is, I want to monitor whether having ::part() causes compat issues or not.

Differential Revision: https://phabricator.services.mozilla.com/D32649
---
 components/style/gecko/selector_parser.rs | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs
index 18718446c1a..659200c3786 100644
--- a/components/style/gecko/selector_parser.rs
+++ b/components/style/gecko/selector_parser.rs
@@ -5,7 +5,7 @@
 //! Gecko-specific bits for selector-parsing.
 
 use crate::element_state::{DocumentState, ElementState};
-use crate::gecko_bindings::structs::RawServoSelectorList;
+use crate::gecko_bindings::structs::{self, RawServoSelectorList};
 use crate::gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI};
 use crate::invalidation::element::document_state::InvalidationMatchingData;
 use crate::selector_parser::{Direction, SelectorParser};
@@ -349,7 +349,14 @@ impl<'a, 'i> ::selectors::Parser<'i> for SelectorParser<'a> {
 
     #[inline]
     fn parse_host(&self) -> bool {
-        self.parse_slotted()
+        true
+    }
+
+    #[inline]
+    fn parse_part(&self) -> bool {
+        self.chrome_rules_enabled() || unsafe {
+            structs::StaticPrefs_sVarCache_layout_css_shadow_parts_enabled
+        }
     }
 
     fn parse_non_ts_pseudo_class(

From 05633886ab862ab647b4cf9cb8eade2944aec91c Mon Sep 17 00:00:00 2001
From: violet <violet.bugreport@gmail.com>
Date: Wed, 12 Jun 2019 10:23:49 +0000
Subject: [PATCH 18/32] style: Implement the break-spaces value of the
 white-space property.

Differential Revision: https://phabricator.services.mozilla.com/D34499
---
 .../style/properties/longhands/inherited_text.mako.rs | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/components/style/properties/longhands/inherited_text.mako.rs b/components/style/properties/longhands/inherited_text.mako.rs
index 45b3ee35aa1..3efd8123bd1 100644
--- a/components/style/properties/longhands/inherited_text.mako.rs
+++ b/components/style/properties/longhands/inherited_text.mako.rs
@@ -175,7 +175,7 @@ ${helpers.predefined_type(
 
 <%helpers:single_keyword
     name="white-space"
-    values="normal pre nowrap pre-wrap pre-line"
+    values="normal pre nowrap pre-wrap pre-line break-spaces"
     extra_gecko_values="-moz-pre-space"
     gecko_enum_prefix="StyleWhiteSpace"
     needs_conversion="True"
@@ -193,7 +193,8 @@ ${helpers.predefined_type(
                 SpecifiedValue::Pre => false,
                 SpecifiedValue::Normal |
                 SpecifiedValue::PreWrap |
-                SpecifiedValue::PreLine => true,
+                SpecifiedValue::PreLine |
+                SpecifiedValue::BreakSpaces => true,
             }
         }
 
@@ -203,7 +204,8 @@ ${helpers.predefined_type(
                 SpecifiedValue::Nowrap => false,
                 SpecifiedValue::Pre |
                 SpecifiedValue::PreWrap |
-                SpecifiedValue::PreLine => true,
+                SpecifiedValue::PreLine |
+                SpecifiedValue::BreakSpaces => true,
             }
         }
 
@@ -213,7 +215,8 @@ ${helpers.predefined_type(
                 SpecifiedValue::Nowrap |
                 SpecifiedValue::PreLine => false,
                 SpecifiedValue::Pre |
-                SpecifiedValue::PreWrap => true,
+                SpecifiedValue::PreWrap |
+                SpecifiedValue::BreakSpaces => true,
             }
         }
     }

From c7c1fed95c12eaef266a7de5657e702f9722a30d Mon Sep 17 00:00:00 2001
From: violet <violet.bugreport@gmail.com>
Date: Sat, 15 Jun 2019 13:09:16 +0000
Subject: [PATCH 19/32] style: Implement overflow-block and overflow-inline
 properties

Differential Revision: https://phabricator.services.mozilla.com/D34738
---
 components/style/properties/data.py           | 19 +++++----
 components/style/properties/helpers.mako.rs   | 24 ++++++++++--
 .../style/properties/longhands/box.mako.rs    | 39 ++++++++-----------
 3 files changed, 50 insertions(+), 32 deletions(-)

diff --git a/components/style/properties/data.py b/components/style/properties/data.py
index c363aa782cc..8113c402581 100644
--- a/components/style/properties/data.py
+++ b/components/style/properties/data.py
@@ -10,11 +10,14 @@ PHYSICAL_SIZES = ["width", "height"]
 LOGICAL_SIZES = ["block-size", "inline-size"]
 PHYSICAL_CORNERS = ["top-left", "top-right", "bottom-right", "bottom-left"]
 LOGICAL_CORNERS = ["start-start", "start-end", "end-start", "end-end"]
+PHYSICAL_AXES = ["x", "y"]
+LOGICAL_AXES = ["inline", "block"]
 
 # bool is True when logical
 ALL_SIDES = [(side, False) for side in PHYSICAL_SIDES] + [(side, True) for side in LOGICAL_SIDES]
 ALL_SIZES = [(size, False) for size in PHYSICAL_SIZES] + [(size, True) for size in LOGICAL_SIZES]
 ALL_CORNERS = [(corner, False) for corner in PHYSICAL_CORNERS] + [(corner, True) for corner in LOGICAL_CORNERS]
+ALL_AXES = [(axis, False) for axis in PHYSICAL_AXES] + [(axis, True) for axis in LOGICAL_AXES]
 
 SYSTEM_FONT_LONGHANDS = """font_family font_size font_style
                            font_variant_caps font_stretch font_kerning
@@ -162,6 +165,8 @@ def parse_property_aliases(alias_list):
             result.append((name, pref))
     return result
 
+def to_phys(name, logical, physical):
+    return name.replace(logical, physical).replace("inset-", "")
 
 class Longhand(object):
     def __init__(self, style_struct, name, spec=None, animation_value_type=None, keyword=None,
@@ -241,16 +246,16 @@ class Longhand(object):
     # property names corresponding to it.
     def all_physical_mapped_properties(self):
         assert self.logical
-        logical_side = None
-        for s in LOGICAL_SIDES + LOGICAL_SIZES + LOGICAL_CORNERS:
-            if s in self.name:
-                assert not logical_side
-                logical_side = s
-        assert logical_side
+        candidates = [s for s in LOGICAL_SIDES + LOGICAL_SIZES + LOGICAL_CORNERS
+                if s in self.name] + [s for s in LOGICAL_AXES if self.name.endswith(s)]
+        assert(len(candidates) == 1)
+        logical_side = candidates[0]
+
         physical = PHYSICAL_SIDES if logical_side in LOGICAL_SIDES \
             else PHYSICAL_SIZES if logical_side in LOGICAL_SIZES \
+            else PHYSICAL_AXES if logical_side in LOGICAL_AXES \
             else LOGICAL_CORNERS
-        return [self.name.replace(logical_side, physical_side).replace("inset-", "")
+        return [to_phys(self.name, logical_side, physical_side)
                 for physical_side in physical]
 
     def experimental(self, product):
diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs
index 800d50c6220..b29c323cf92 100644
--- a/components/style/properties/helpers.mako.rs
+++ b/components/style/properties/helpers.mako.rs
@@ -3,8 +3,9 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 <%!
-    from data import Keyword, to_rust_ident, to_camel_case, SYSTEM_FONT_LONGHANDS
-    from data import LOGICAL_CORNERS, PHYSICAL_CORNERS, LOGICAL_SIDES, PHYSICAL_SIDES, LOGICAL_SIZES
+    from data import Keyword, to_rust_ident, to_phys, to_camel_case, SYSTEM_FONT_LONGHANDS
+    from data import (LOGICAL_CORNERS, PHYSICAL_CORNERS, LOGICAL_SIDES,
+                      PHYSICAL_SIDES, LOGICAL_SIZES, LOGICAL_AXES)
 %>
 
 <%def name="predefined_type(name, type, initial_value, parse_method='parse',
@@ -1038,17 +1039,21 @@
         side = None
         size = None
         corner = None
+        axis = None
         maybe_side = [s for s in LOGICAL_SIDES if s in name]
         maybe_size = [s for s in LOGICAL_SIZES if s in name]
         maybe_corner = [s for s in LOGICAL_CORNERS if s in name]
+        maybe_axis = [s for s in LOGICAL_AXES if name.endswith(s)]
         if len(maybe_side) == 1:
             side = maybe_side[0]
         elif len(maybe_size) == 1:
             size = maybe_size[0]
         elif len(maybe_corner) == 1:
             corner = maybe_corner[0]
+        elif len(maybe_axis) == 1:
+            axis = maybe_axis[0]
         def phys_ident(side, phy_side):
-            return to_rust_ident(name.replace(side, phy_side).replace("inset-", ""))
+            return to_rust_ident(to_phys(name, side, phy_side))
     %>
     % if side is not None:
         use crate::logical_geometry::PhysicalSide;
@@ -1080,6 +1085,19 @@
         } else {
             ${caller.inner(physical_ident=phys_ident(size, physical_size[0]))}
         }
+    % elif axis is not None:
+        <%
+            if axis == "inline":
+                me, other = "x", "y"
+            else:
+                assert(axis == "block")
+                me, other = "y", "x"
+        %>
+        if wm.is_vertical() {
+            ${caller.inner(physical_ident=phys_ident(axis, other))}
+        } else {
+            ${caller.inner(physical_ident=phys_ident(axis, me))}
+        }
     % else:
         <% raise Exception("Don't know what to do with logical property %s" % name) %>
     % endif
diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs
index f37d5968b24..0a15047038a 100644
--- a/components/style/properties/longhands/box.mako.rs
+++ b/components/style/properties/longhands/box.mako.rs
@@ -3,7 +3,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
-<% from data import Keyword, Method, to_rust_ident, to_camel_case%>
+<% from data import ALL_AXES, Keyword, Method, to_rust_ident, to_camel_case%>
 
 <% data.new_style_struct("Box",
                          inherited=False,
@@ -114,27 +114,22 @@ ${helpers.single_keyword("-servo-overflow-clip-box", "padding-box content-box",
 // FIXME(pcwalton, #2742): Implement scrolling for `scroll` and `auto`.
 //
 // We allow it to apply to placeholders for UA sheets, which set it !important.
-${helpers.predefined_type(
-    "overflow-x",
-    "Overflow",
-    "computed::Overflow::Visible",
-    animation_value_type="discrete",
-    flags="APPLIES_TO_PLACEHOLDER",
-    spec="https://drafts.csswg.org/css-overflow/#propdef-overflow-x",
-    needs_context=False,
-    servo_restyle_damage = "reflow",
-)}
-
-${helpers.predefined_type(
-    "overflow-y",
-    "Overflow",
-    "computed::Overflow::Visible",
-    animation_value_type="discrete",
-    flags="APPLIES_TO_PLACEHOLDER",
-    spec="https://drafts.csswg.org/css-overflow/#propdef-overflow-y",
-    needs_context=False,
-    servo_restyle_damage = "reflow",
-)}
+% for (axis, logical) in ALL_AXES:
+    <% full_name = "overflow-{}".format(axis) %>
+    ${helpers.predefined_type(
+        full_name,
+        "Overflow",
+        "computed::Overflow::Visible",
+        logical_group="overflow",
+        logical=logical,
+        animation_value_type="discrete",
+        flags="APPLIES_TO_PLACEHOLDER",
+        spec="https://drafts.csswg.org/css-overflow-3/#propdef-{}".format(full_name),
+        needs_context=False,
+        servo_restyle_damage = "reflow",
+        gecko_pref="layout.css.overflow-logical.enabled" if logical else None,
+    )}
+% endfor
 
 ${helpers.predefined_type(
     "overflow-anchor",

From e1e82dbe5f62b997a84a56a63829848236380409 Mon Sep 17 00:00:00 2001
From: violet <violet.bugreport@gmail.com>
Date: Tue, 18 Jun 2019 11:54:41 +0000
Subject: [PATCH 20/32] style: Should not serialize default radius of circle.

Should not serialize default shape-outside circle() function radius.

The ToCss impl of Circle and Ellipse turn out to be identical in specified and computed value, thus move them to generics.

Differential Revision: https://phabricator.services.mozilla.com/D35183
---
 .../style/values/computed/basic_shape.rs      | 33 ---------------
 .../style/values/generics/basic_shape.rs      | 42 +++++++++++++++++++
 .../style/values/specified/basic_shape.rs     | 39 +----------------
 3 files changed, 43 insertions(+), 71 deletions(-)

diff --git a/components/style/values/computed/basic_shape.rs b/components/style/values/computed/basic_shape.rs
index 9a245aa77f1..27a091a1115 100644
--- a/components/style/values/computed/basic_shape.rs
+++ b/components/style/values/computed/basic_shape.rs
@@ -10,8 +10,6 @@
 use crate::values::computed::url::ComputedUrl;
 use crate::values::computed::{Image, LengthPercentage, NonNegativeLengthPercentage};
 use crate::values::generics::basic_shape as generic;
-use std::fmt::{self, Write};
-use style_traits::{CssWriter, ToCss};
 
 /// A computed alias for FillRule.
 pub use crate::values::generics::basic_shape::FillRule;
@@ -42,34 +40,3 @@ pub type Ellipse =
 
 /// The computed value of `ShapeRadius`
 pub type ShapeRadius = generic::GenericShapeRadius<NonNegativeLengthPercentage>;
-
-impl ToCss for Circle {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        dest.write_str("circle(")?;
-        self.radius.to_css(dest)?;
-        dest.write_str(" at ")?;
-        self.position.to_css(dest)?;
-        dest.write_str(")")
-    }
-}
-
-impl ToCss for Ellipse {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        dest.write_str("ellipse(")?;
-        if (self.semiaxis_x, self.semiaxis_y) != Default::default() {
-            self.semiaxis_x.to_css(dest)?;
-            dest.write_str(" ")?;
-            self.semiaxis_y.to_css(dest)?;
-            dest.write_str(" ")?;
-        }
-        dest.write_str("at ")?;
-        self.position.to_css(dest)?;
-        dest.write_str(")")
-    }
-}
diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs
index 8c63d064817..d91fa58eb0d 100644
--- a/components/style/values/generics/basic_shape.rs
+++ b/components/style/values/generics/basic_shape.rs
@@ -378,6 +378,48 @@ where
     }
 }
 
+impl<H, V, NonNegativeLengthPercentage> ToCss for Circle<H, V, NonNegativeLengthPercentage>
+where
+    GenericPosition<H, V>: ToCss,
+    NonNegativeLengthPercentage: ToCss + PartialEq,
+{
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: Write,
+    {
+        dest.write_str("circle(")?;
+        if self.radius != Default::default() {
+            self.radius.to_css(dest)?;
+            dest.write_str(" ")?;
+        }
+        dest.write_str("at ")?;
+        self.position.to_css(dest)?;
+        dest.write_str(")")
+    }
+}
+
+impl<H, V, NonNegativeLengthPercentage> ToCss for Ellipse<H, V, NonNegativeLengthPercentage>
+where
+    GenericPosition<H, V>: ToCss,
+    NonNegativeLengthPercentage: ToCss + PartialEq,
+{
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: Write,
+    {
+        dest.write_str("ellipse(")?;
+        if self.semiaxis_x != Default::default() || self.semiaxis_y != Default::default() {
+            self.semiaxis_x.to_css(dest)?;
+            dest.write_str(" ")?;
+            self.semiaxis_y.to_css(dest)?;
+            dest.write_str(" ")?;
+        }
+        dest.write_str("at ")?;
+        self.position.to_css(dest)?;
+        dest.write_str(")")
+    }
+}
+
 impl<L> Default for ShapeRadius<L> {
     #[inline]
     fn default() -> Self {
diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs
index 4f627ceb07f..7ee33d0580e 100644
--- a/components/style/values/specified/basic_shape.rs
+++ b/components/style/values/specified/basic_shape.rs
@@ -20,8 +20,7 @@ use crate::values::specified::SVGPathData;
 use crate::values::specified::{LengthPercentage, NonNegativeLengthPercentage};
 use crate::Zero;
 use cssparser::Parser;
-use std::fmt::{self, Write};
-use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
+use style_traits::{ParseError, StyleParseErrorKind};
 
 /// A specified alias for FillRule.
 pub use crate::values::generics::basic_shape::FillRule;
@@ -239,23 +238,6 @@ impl Circle {
     }
 }
 
-impl ToCss for Circle {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        dest.write_str("circle(")?;
-        if generic::ShapeRadius::ClosestSide != self.radius {
-            self.radius.to_css(dest)?;
-            dest.write_str(" ")?;
-        }
-
-        dest.write_str("at ")?;
-        self.position.to_css(dest)?;
-        dest.write_str(")")
-    }
-}
-
 impl Parse for Ellipse {
     fn parse<'i, 't>(
         context: &ParserContext,
@@ -293,25 +275,6 @@ impl Ellipse {
     }
 }
 
-impl ToCss for Ellipse {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        dest.write_str("ellipse(")?;
-        if self.semiaxis_x != ShapeRadius::default() || self.semiaxis_y != ShapeRadius::default() {
-            self.semiaxis_x.to_css(dest)?;
-            dest.write_str(" ")?;
-            self.semiaxis_y.to_css(dest)?;
-            dest.write_str(" ")?;
-        }
-
-        dest.write_str("at ")?;
-        self.position.to_css(dest)?;
-        dest.write_str(")")
-    }
-}
-
 impl Parse for ShapeRadius {
     fn parse<'i, 't>(
         context: &ParserContext,

From e8271ee9262f74b1374f4ca02086e9adfb406841 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Mon, 17 Jun 2019 15:44:21 +0000
Subject: [PATCH 21/32] style: Use more ffi-friendly types in grid template
 areas.

Differential Revision: https://phabricator.services.mozilla.com/D35116
---
 components/style/properties/gecko.mako.rs     | 25 ++++++-----
 .../properties/shorthands/position.mako.rs    |  6 +--
 components/style/values/specified/position.rs | 43 +++++++++++++------
 3 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index 3ff3be2a2ec..5a2418112c8 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -1433,7 +1433,7 @@ fn static_assert() {
                 Gecko_NewGridTemplateAreasValue(v.0.areas.len() as u32, v.0.strings.len() as u32, v.0.width))
         };
 
-        for (servo, gecko) in v.0.areas.into_iter().zip(refptr.mNamedAreas.iter_mut()) {
+        for (servo, gecko) in v.0.areas.iter().zip(refptr.mNamedAreas.iter_mut()) {
             gecko.mName.assign_str(&*servo.name);
             gecko.mColumnStart = servo.columns.start;
             gecko.mColumnEnd = servo.columns.end;
@@ -1441,7 +1441,7 @@ fn static_assert() {
             gecko.mRowEnd = servo.rows.end;
         }
 
-        for (servo, gecko) in v.0.strings.into_iter().zip(refptr.mTemplates.iter_mut()) {
+        for (servo, gecko) in v.0.strings.iter().zip(refptr.mTemplates.iter_mut()) {
             gecko.assign_str(&*servo);
         }
 
@@ -1457,9 +1457,8 @@ fn static_assert() {
     }
 
     pub fn clone_grid_template_areas(&self) -> values::computed::position::GridTemplateAreas {
-        use std::ops::Range;
         use crate::values::None_;
-        use crate::values::specified::position::{NamedArea, TemplateAreas, TemplateAreasArc};
+        use crate::values::specified::position::{NamedArea, TemplateAreas, TemplateAreasArc, UnsignedRange};
 
         if self.gecko.mGridTemplateAreas.mRawPtr.is_null() {
             return Either::Second(None_);
@@ -1469,33 +1468,33 @@ fn static_assert() {
         let areas = unsafe {
             let vec: Vec<NamedArea> =
                 (*gecko_grid_template_areas).mNamedAreas.iter().map(|gecko_name_area| {
-                    let name = gecko_name_area.mName.to_string().into_boxed_str();
-                    let rows = Range {
+                    let name = gecko_name_area.mName.to_string().into();
+                    let rows = UnsignedRange {
                         start: gecko_name_area.mRowStart,
                         end: gecko_name_area.mRowEnd
                     };
-                    let columns = Range {
+                    let columns = UnsignedRange {
                         start: gecko_name_area.mColumnStart,
                         end: gecko_name_area.mColumnEnd
                     };
-                    NamedArea{ name, rows, columns }
+                    NamedArea { name, rows, columns }
                 }).collect();
-            vec.into_boxed_slice()
+            vec.into()
         };
 
         let strings = unsafe {
-            let vec: Vec<Box<str>> =
+            let vec: Vec<crate::OwnedStr> =
                 (*gecko_grid_template_areas).mTemplates.iter().map(|gecko_template| {
-                    gecko_template.to_string().into_boxed_str()
+                    gecko_template.to_string().into()
                 }).collect();
-            vec.into_boxed_slice()
+            vec.into()
         };
 
         let width = unsafe {
             (*gecko_grid_template_areas).mNColumns
         };
 
-        Either::First(TemplateAreasArc(Arc::new(TemplateAreas{ areas, strings, width })))
+        Either::First(TemplateAreasArc(Arc::new(TemplateAreas { areas, strings, width })))
     }
 
 </%self:impl_trait>
diff --git a/components/style/properties/shorthands/position.mako.rs b/components/style/properties/shorthands/position.mako.rs
index 63298b86ba1..73d47e1bea7 100644
--- a/components/style/properties/shorthands/position.mako.rs
+++ b/components/style/properties/shorthands/position.mako.rs
@@ -290,7 +290,7 @@
         % endfor
 
         let first_line_names = input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice());
-        if let Ok(mut string) = input.try(|i| i.expect_string().map(|s| s.as_ref().into())) {
+        if let Ok(mut string) = input.try(|i| i.expect_string().map(|s| s.as_ref().to_owned().into())) {
             let mut strings = vec![];
             let mut values = vec![];
             let mut line_names = vec![];
@@ -305,7 +305,7 @@
                     names.extend(v.into_vec());
                 }
 
-                string = match input.try(|i| i.expect_string().map(|s| s.as_ref().into())) {
+                string = match input.try(|i| i.expect_string().map(|s| s.as_ref().to_owned().into())) {
                     Ok(s) => s,
                     _ => {      // only the named area determines whether we should bail out
                         line_names.push(names.into_boxed_slice());
@@ -323,7 +323,7 @@
                 .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError))?;
             let template_rows = TrackList {
                 list_type: TrackListType::Normal,
-                values: values,
+                values,
                 line_names: line_names.into_boxed_slice(),
                 auto_repeat: None,
             };
diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs
index fed0c5d3098..9f1fa7f2ee4 100644
--- a/components/style/values/specified/position.rs
+++ b/components/style/values/specified/position.rs
@@ -21,7 +21,6 @@ use cssparser::Parser;
 use selectors::parser::SelectorParseErrorKind;
 use servo_arc::Arc;
 use std::fmt::{self, Write};
-use std::ops::Range;
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 
 /// The specified value of a CSS `<position>`
@@ -482,10 +481,10 @@ impl From<GridAutoFlow> for u8 {
 pub struct TemplateAreas {
     /// `named area` containing for each template area
     #[css(skip)]
-    pub areas: Box<[NamedArea]>,
+    pub areas: crate::OwnedSlice<NamedArea>,
     /// The original CSS string value of each template area
     #[css(iterable)]
-    pub strings: Box<[Box<str>]>,
+    pub strings: crate::OwnedSlice<crate::OwnedStr>,
     /// The number of columns of the grid.
     #[css(skip)]
     pub width: u32,
@@ -493,7 +492,7 @@ pub struct TemplateAreas {
 
 impl TemplateAreas {
     /// Transform `vector` of str into `template area`
-    pub fn from_vec(strings: Vec<Box<str>>) -> Result<TemplateAreas, ()> {
+    pub fn from_vec(strings: Vec<crate::OwnedStr>) -> Result<Self, ()> {
         if strings.is_empty() {
             return Err(());
         }
@@ -539,9 +538,15 @@ impl TemplateAreas {
                     }
                     let index = areas.len();
                     areas.push(NamedArea {
-                        name: token.to_owned().into_boxed_str(),
-                        columns: column..(column + 1),
-                        rows: row..(row + 1),
+                        name: token.to_owned().into(),
+                        columns: UnsignedRange {
+                            start: column,
+                            end: column + 1,
+                        },
+                        rows: UnsignedRange {
+                            start: row,
+                            end: row + 1,
+                        },
                     });
                     assert!(area_indices.insert(token, index).is_none());
                     current_area_index = Some(index);
@@ -560,8 +565,8 @@ impl TemplateAreas {
             }
         }
         Ok(TemplateAreas {
-            areas: areas.into_boxed_slice(),
-            strings: strings.into_boxed_slice(),
+            areas: areas.into(),
+            strings: strings.into(),
             width: width,
         })
     }
@@ -573,7 +578,7 @@ impl Parse for TemplateAreas {
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         let mut strings = vec![];
-        while let Ok(string) = input.try(|i| i.expect_string().map(|s| s.as_ref().into())) {
+        while let Ok(string) = input.try(|i| i.expect_string().map(|s| s.as_ref().to_owned().into())) {
             strings.push(string);
         }
 
@@ -602,21 +607,31 @@ impl Parse for TemplateAreasArc {
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         let parsed = TemplateAreas::parse(context, input)?;
-
         Ok(TemplateAreasArc(Arc::new(parsed)))
     }
 }
 
+/// A range of rows or columns. Using this instead of std::ops::Range for FFI
+/// purposes.
+#[repr(C)]
+#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToShmem)]
+pub struct UnsignedRange {
+    /// The start of the range.
+    pub start: u32,
+    /// The end of the range.
+    pub end: u32,
+}
+
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToShmem)]
 /// Not associated with any particular grid item, but can
 /// be referenced from the grid-placement properties.
 pub struct NamedArea {
     /// Name of the `named area`
-    pub name: Box<str>,
+    pub name: crate::OwnedStr,
     /// Rows of the `named area`
-    pub rows: Range<u32>,
+    pub rows: UnsignedRange,
     /// Columns of the `named area`
-    pub columns: Range<u32>,
+    pub columns: UnsignedRange,
 }
 
 /// Tokenize the string into a list of the tokens,

From 6cb588d2b5e7ff018973f2efdf756d186f266d55 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 18 Jun 2019 22:29:58 +0000
Subject: [PATCH 22/32] style: Use Servo for the representation of grid
 template areas.

Right now we do a lot of useless string copying. In order to avoid transcoding
to utf-16 during layout, make sure to use nsCString at a few related places.

I may revisit this since we're storing other line names as atoms in some places.
So it may be better to just use atoms everywhere.

But that'd be a different patch either way.

Differential Revision: https://phabricator.services.mozilla.com/D35117
---
 .../style/gecko_bindings/sugar/refptr.rs      |   6 +-
 components/style/properties/gecko.mako.rs     | 122 ++++--------------
 .../properties/shorthands/position.mako.rs    |  31 ++---
 components/style/values/specified/position.rs |  39 ++++--
 4 files changed, 68 insertions(+), 130 deletions(-)

diff --git a/components/style/gecko_bindings/sugar/refptr.rs b/components/style/gecko_bindings/sugar/refptr.rs
index c5246733976..a141aef4e8b 100644
--- a/components/style/gecko_bindings/sugar/refptr.rs
+++ b/components/style/gecko_bindings/sugar/refptr.rs
@@ -308,11 +308,6 @@ impl_threadsafe_refcount!(
     bindings::Gecko_AddRefnsIURIArbitraryThread,
     bindings::Gecko_ReleasensIURIArbitraryThread
 );
-impl_threadsafe_refcount!(
-    structs::mozilla::css::GridTemplateAreasValue,
-    bindings::Gecko_AddRefGridTemplateAreasValueArbitraryThread,
-    bindings::Gecko_ReleaseGridTemplateAreasValueArbitraryThread
-);
 impl_threadsafe_refcount!(
     structs::SharedFontList,
     bindings::Gecko_AddRefSharedFontListArbitraryThread,
@@ -328,6 +323,7 @@ impl_threadsafe_refcount!(
 unsafe fn addref_atom(atom: *mut structs::nsAtom) {
     mem::forget(Atom::from_raw(atom));
 }
+
 #[inline]
 unsafe fn release_atom(atom: *mut structs::nsAtom) {
     let _ = Atom::from_addrefed(atom);
diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index 5a2418112c8..1105e705060 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -1083,8 +1083,8 @@ fn static_assert() {
                                   align-content justify-content align-self
                                   justify-self align-items justify-items
                                   grid-auto-rows grid-auto-columns
-                                  grid-auto-flow grid-template-areas
-                                  grid-template-rows grid-template-columns">
+                                  grid-auto-flow grid-template-rows
+                                  grid-template-columns">
     % for side in SIDES:
     <% impl_split_style_coord(side.ident, "mOffset", side.index) %>
     % endfor
@@ -1136,13 +1136,19 @@ fn static_assert() {
     pub fn set_${value.name}(&mut self, v: longhands::${value.name}::computed_value::T) {
         use crate::gecko_bindings::structs::{nsStyleGridLine_kMinLine, nsStyleGridLine_kMaxLine};
 
-        let ident = v.ident.as_ref().map_or(&[] as &[_], |ident| ident.0.as_slice());
-        self.gecko.${value.gecko}.mLineName.assign(ident);
-        self.gecko.${value.gecko}.mHasSpan = v.is_span;
+        let line = &mut self.gecko.${value.gecko};
+        let line_name = &mut line.mLineName;
+        match v.ident.as_ref() {
+            Some(i) => i.0.with_str(|s| line_name.assign(s)),
+            None => line_name.assign(""),
+        };
+        line.mHasSpan = v.is_span;
         if let Some(integer) = v.line_num {
             // clamping the integer between a range
-            self.gecko.${value.gecko}.mInteger = cmp::max(nsStyleGridLine_kMinLine,
-                cmp::min(integer, nsStyleGridLine_kMaxLine));
+            line.mInteger = cmp::max(
+                nsStyleGridLine_kMinLine,
+                cmp::min(integer, nsStyleGridLine_kMaxLine),
+            );
         }
     }
 
@@ -1205,20 +1211,20 @@ fn static_assert() {
     pub fn set_grid_template_${kind}(&mut self, v: longhands::grid_template_${kind}::computed_value::T) {
         <% self_grid = "self.gecko.mGridTemplate%s" % kind.title() %>
         use crate::gecko_bindings::structs::{nsTArray, nsStyleGridLine_kMaxLine};
-        use nsstring::nsString;
+        use nsstring::nsCString;
         use std::usize;
         use crate::values::CustomIdent;
         use crate::values::generics::grid::TrackListType::Auto;
         use crate::values::generics::grid::{GridTemplateComponent, RepeatCount};
 
         #[inline]
-        fn set_line_names(servo_names: &[CustomIdent], gecko_names: &mut nsTArray<nsString>) {
+        fn set_line_names(servo_names: &[CustomIdent], gecko_names: &mut nsTArray<nsCString>) {
             unsafe {
-                bindings::Gecko_ResizeTArrayForStrings(gecko_names, servo_names.len() as u32);
+                bindings::Gecko_ResizeTArrayForCStrings(gecko_names, servo_names.len() as u32);
             }
 
             for (servo_name, gecko_name) in servo_names.iter().zip(gecko_names.iter_mut()) {
-                gecko_name.assign(servo_name.0.as_slice());
+                servo_name.0.with_str(|s| gecko_name.assign(s))
             }
         }
 
@@ -1256,9 +1262,9 @@ fn static_assert() {
                     auto_track_size = Some(auto_repeat.track_sizes.get(0).unwrap().clone());
                 } else {
                     unsafe {
-                        bindings::Gecko_ResizeTArrayForStrings(
+                        bindings::Gecko_ResizeTArrayForCStrings(
                             &mut value.mRepeatAutoLineNameListBefore, 0);
-                        bindings::Gecko_ResizeTArrayForStrings(
+                        bindings::Gecko_ResizeTArrayForCStrings(
                             &mut value.mRepeatAutoLineNameListAfter, 0);
                     }
                 }
@@ -1332,7 +1338,7 @@ fn static_assert() {
     pub fn clone_grid_template_${kind}(&self) -> longhands::grid_template_${kind}::computed_value::T {
         <% self_grid = "self.gecko.mGridTemplate%s" % kind.title() %>
         use crate::gecko_bindings::structs::nsTArray;
-        use nsstring::nsString;
+        use nsstring::nsCString;
         use crate::values::CustomIdent;
         use crate::values::generics::grid::{GridTemplateComponent, LineNameList, RepeatCount};
         use crate::values::generics::grid::{TrackList, TrackListType, TrackListValue, TrackRepeat, TrackSize};
@@ -1343,15 +1349,15 @@ fn static_assert() {
         };
 
         #[inline]
-        fn to_boxed_customident_slice(gecko_names: &nsTArray<nsString>) -> Box<[CustomIdent]> {
+        fn to_boxed_customident_slice(gecko_names: &nsTArray<nsCString>) -> Box<[CustomIdent]> {
             let idents: Vec<CustomIdent> = gecko_names.iter().map(|gecko_name| {
-                CustomIdent(Atom::from(gecko_name.to_string()))
+                CustomIdent(Atom::from(unsafe { gecko_name.as_str_unchecked() }))
             }).collect();
             idents.into_boxed_slice()
         }
 
         #[inline]
-        fn to_line_names_vec(gecko_line_names: &nsTArray<nsTArray<nsString>>)
+        fn to_line_names_vec(gecko_line_names: &nsTArray<nsTArray<nsCString>>)
             -> Vec<Box<[CustomIdent]>> {
             gecko_line_names.iter().map(|gecko_names| {
                 to_boxed_customident_slice(gecko_names)
@@ -1415,88 +1421,6 @@ fn static_assert() {
     % endfor
 
     ${impl_simple_type_with_conversion("grid_auto_flow")}
-
-    pub fn set_grid_template_areas(&mut self, v: values::computed::position::GridTemplateAreas) {
-        use crate::gecko_bindings::bindings::Gecko_NewGridTemplateAreasValue;
-        use crate::gecko_bindings::sugar::refptr::UniqueRefPtr;
-
-        let v = match v {
-            Either::First(areas) => areas,
-            Either::Second(_) => {
-                unsafe { self.gecko.mGridTemplateAreas.clear() }
-                return;
-            },
-        };
-
-        let mut refptr = unsafe {
-            UniqueRefPtr::from_addrefed(
-                Gecko_NewGridTemplateAreasValue(v.0.areas.len() as u32, v.0.strings.len() as u32, v.0.width))
-        };
-
-        for (servo, gecko) in v.0.areas.iter().zip(refptr.mNamedAreas.iter_mut()) {
-            gecko.mName.assign_str(&*servo.name);
-            gecko.mColumnStart = servo.columns.start;
-            gecko.mColumnEnd = servo.columns.end;
-            gecko.mRowStart = servo.rows.start;
-            gecko.mRowEnd = servo.rows.end;
-        }
-
-        for (servo, gecko) in v.0.strings.iter().zip(refptr.mTemplates.iter_mut()) {
-            gecko.assign_str(&*servo);
-        }
-
-        self.gecko.mGridTemplateAreas.set_move(refptr.get())
-    }
-
-    pub fn copy_grid_template_areas_from(&mut self, other: &Self) {
-        unsafe { self.gecko.mGridTemplateAreas.set(&other.gecko.mGridTemplateAreas) }
-    }
-
-    pub fn reset_grid_template_areas(&mut self, other: &Self) {
-        self.copy_grid_template_areas_from(other)
-    }
-
-    pub fn clone_grid_template_areas(&self) -> values::computed::position::GridTemplateAreas {
-        use crate::values::None_;
-        use crate::values::specified::position::{NamedArea, TemplateAreas, TemplateAreasArc, UnsignedRange};
-
-        if self.gecko.mGridTemplateAreas.mRawPtr.is_null() {
-            return Either::Second(None_);
-        }
-
-        let gecko_grid_template_areas = self.gecko.mGridTemplateAreas.mRawPtr;
-        let areas = unsafe {
-            let vec: Vec<NamedArea> =
-                (*gecko_grid_template_areas).mNamedAreas.iter().map(|gecko_name_area| {
-                    let name = gecko_name_area.mName.to_string().into();
-                    let rows = UnsignedRange {
-                        start: gecko_name_area.mRowStart,
-                        end: gecko_name_area.mRowEnd
-                    };
-                    let columns = UnsignedRange {
-                        start: gecko_name_area.mColumnStart,
-                        end: gecko_name_area.mColumnEnd
-                    };
-                    NamedArea { name, rows, columns }
-                }).collect();
-            vec.into()
-        };
-
-        let strings = unsafe {
-            let vec: Vec<crate::OwnedStr> =
-                (*gecko_grid_template_areas).mTemplates.iter().map(|gecko_template| {
-                    gecko_template.to_string().into()
-                }).collect();
-            vec.into()
-        };
-
-        let width = unsafe {
-            (*gecko_grid_template_areas).mNColumns
-        };
-
-        Either::First(TemplateAreasArc(Arc::new(TemplateAreas { areas, strings, width })))
-    }
-
 </%self:impl_trait>
 
 <% skip_outline_longhands = " ".join("outline-style outline-width".split() +
diff --git a/components/style/properties/shorthands/position.mako.rs b/components/style/properties/shorthands/position.mako.rs
index 73d47e1bea7..ea2f34b9db8 100644
--- a/components/style/properties/shorthands/position.mako.rs
+++ b/components/style/properties/shorthands/position.mako.rs
@@ -253,18 +253,17 @@
                     products="gecko">
     use crate::parser::Parse;
     use servo_arc::Arc;
-    use crate::values::{Either, None_};
     use crate::values::generics::grid::{TrackSize, TrackList, TrackListType};
     use crate::values::generics::grid::{TrackListValue, concat_serialize_idents};
     use crate::values::specified::{GridTemplateComponent, GenericGridTemplateComponent};
     use crate::values::specified::grid::parse_line_names;
-    use crate::values::specified::position::{TemplateAreas, TemplateAreasArc};
+    use crate::values::specified::position::{GridTemplateAreas, TemplateAreas, TemplateAreasArc};
 
     /// Parsing for `<grid-template>` shorthand (also used by `grid` shorthand).
     pub fn parse_grid_template<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
-    ) -> Result<(GridTemplateComponent, GridTemplateComponent, Either<TemplateAreasArc, None_>), ParseError<'i>> {
+    ) -> Result<(GridTemplateComponent, GridTemplateComponent, GridTemplateAreas), ParseError<'i>> {
         // Other shorthand sub properties also parse the `none` keyword and this shorthand
         // should know after this keyword there is nothing to parse. Otherwise it gets
         // confused and rejects the sub properties that contains `none`.
@@ -275,13 +274,10 @@
         % for keyword, rust_type in keywords.items():
             if let Ok(x) = input.try(|i| {
                 if i.try(|i| i.expect_ident_matching("${keyword}")).is_ok() {
-                    if i.is_exhausted() {
-                        return Ok((${rust_type},
-                                   ${rust_type},
-                                   Either::Second(None_)))
-                    } else {
+                    if !i.is_exhausted() {
                         return Err(());
                     }
+                    return Ok((${rust_type}, ${rust_type}, GridTemplateAreas::None));
                 }
                 Err(())
             }) {
@@ -342,7 +338,7 @@
             };
 
             Ok((GenericGridTemplateComponent::TrackList(template_rows),
-                template_cols, Either::First(TemplateAreasArc(Arc::new(template_areas)))))
+                template_cols, GridTemplateAreas::Areas(TemplateAreasArc(Arc::new(template_areas)))))
         } else {
             let mut template_rows = GridTemplateComponent::parse(context, input)?;
             if let GenericGridTemplateComponent::TrackList(ref mut list) = template_rows {
@@ -356,7 +352,7 @@
             }
 
             input.expect_delim('/')?;
-            Ok((template_rows, GridTemplateComponent::parse(context, input)?, Either::Second(None_)))
+            Ok((template_rows, GridTemplateComponent::parse(context, input)?, GridTemplateAreas::None))
         }
     }
 
@@ -377,18 +373,18 @@
     pub fn serialize_grid_template<W>(
         template_rows: &GridTemplateComponent,
         template_columns: &GridTemplateComponent,
-        template_areas: &Either<TemplateAreasArc, None_>,
+        template_areas: &GridTemplateAreas,
         dest: &mut CssWriter<W>,
     ) -> fmt::Result
     where
         W: Write {
         match *template_areas {
-            Either::Second(_none) => {
+            GridTemplateAreas::None => {
                 template_rows.to_css(dest)?;
                 dest.write_str(" / ")?;
                 template_columns.to_css(dest)
             },
-            Either::First(ref areas) => {
+            GridTemplateAreas::Areas(ref areas) => {
                 // The length of template-area and template-rows values should be equal.
                 if areas.0.strings.len() != template_rows.track_list_len() {
                     return Ok(());
@@ -485,10 +481,9 @@
                     products="gecko">
     use crate::parser::Parse;
     use crate::properties::longhands::{grid_auto_columns, grid_auto_rows, grid_auto_flow};
-    use crate::values::{Either, None_};
     use crate::values::generics::grid::{GridTemplateComponent, TrackListType};
     use crate::values::specified::{GenericGridTemplateComponent, TrackSize};
-    use crate::values::specified::position::{AutoFlow, GridAutoFlow};
+    use crate::values::specified::position::{AutoFlow, GridAutoFlow, GridTemplateAreas};
 
     pub fn parse_value<'i, 't>(
         context: &ParserContext,
@@ -496,7 +491,7 @@
     ) -> Result<Longhands, ParseError<'i>> {
         let mut temp_rows = GridTemplateComponent::None;
         let mut temp_cols = GridTemplateComponent::None;
-        let mut temp_areas = Either::Second(None_);
+        let mut temp_areas = GridTemplateAreas::None;
         let mut auto_rows = TrackSize::default();
         let mut auto_cols = TrackSize::default();
         let mut flow = grid_auto_flow::get_initial_value();
@@ -558,7 +553,7 @@
     impl<'a> LonghandsToSerialize<'a> {
         /// Returns true if other sub properties except template-{rows,columns} are initial.
         fn is_grid_template(&self) -> bool {
-            *self.grid_template_areas == Either::Second(None_) &&
+            *self.grid_template_areas == GridTemplateAreas::None &&
             *self.grid_auto_rows == TrackSize::default() &&
             *self.grid_auto_columns == TrackSize::default() &&
             *self.grid_auto_flow == grid_auto_flow::get_initial_value()
@@ -567,7 +562,7 @@
 
     impl<'a> ToCss for LonghandsToSerialize<'a> {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
-            if *self.grid_template_areas != Either::Second(None_) ||
+            if *self.grid_template_areas != GridTemplateAreas::None ||
                (*self.grid_template_rows != GridTemplateComponent::None &&
                    *self.grid_template_columns != GridTemplateComponent::None) ||
                self.is_grid_template() {
diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs
index 9f1fa7f2ee4..dfce7218eed 100644
--- a/components/style/values/specified/position.rs
+++ b/components/style/values/specified/position.rs
@@ -15,7 +15,6 @@ use crate::values::computed::{Context, Percentage, ToComputedValue};
 use crate::values::generics::position::Position as GenericPosition;
 use crate::values::generics::position::ZIndex as GenericZIndex;
 use crate::values::specified::{AllowQuirks, Integer, LengthPercentage};
-use crate::values::{Either, None_};
 use crate::Zero;
 use cssparser::Parser;
 use selectors::parser::SelectorParseErrorKind;
@@ -477,6 +476,7 @@ impl From<GridAutoFlow> for u8 {
     ToResolvedValue,
     ToShmem,
 )]
+#[repr(C)]
 /// https://drafts.csswg.org/css-grid/#named-grid-area
 pub struct TemplateAreas {
     /// `named area` containing for each template area
@@ -599,6 +599,7 @@ impl Parse for TemplateAreas {
     ToResolvedValue,
     ToShmem,
 )]
+#[repr(transparent)]
 pub struct TemplateAreasArc(#[ignore_malloc_size_of = "Arc"] pub Arc<TemplateAreas>);
 
 impl Parse for TemplateAreasArc {
@@ -623,8 +624,9 @@ pub struct UnsignedRange {
 }
 
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToShmem)]
-/// Not associated with any particular grid item, but can
-/// be referenced from the grid-placement properties.
+#[repr(C)]
+/// Not associated with any particular grid item, but can be referenced from the
+/// grid-placement properties.
 pub struct NamedArea {
     /// Name of the `named area`
     pub name: crate::OwnedStr,
@@ -670,16 +672,37 @@ fn is_name_code_point(c: char) -> bool {
 }
 
 /// This property specifies named grid areas.
-/// The syntax of this property also provides a visualization of
-/// the structure of the grid, making the overall layout of
-/// the grid container easier to understand.
-pub type GridTemplateAreas = Either<TemplateAreasArc, None_>;
+///
+/// The syntax of this property also provides a visualization of the structure
+/// of the grid, making the overall layout of the grid container easier to
+/// understand.
+///
+/// cbindgen:derive-tagged-enum-copy-constructor=true
+#[repr(C, u8)]
+#[derive(
+    Clone,
+    Debug,
+    MallocSizeOf,
+    Parse,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToComputedValue,
+    ToCss,
+    ToResolvedValue,
+    ToShmem,
+)]
+pub enum GridTemplateAreas {
+    /// The `none` value.
+    None,
+    /// The actual value.
+    Areas(TemplateAreasArc),
+}
 
 impl GridTemplateAreas {
     #[inline]
     /// Get default value as `none`
     pub fn none() -> GridTemplateAreas {
-        Either::Second(None_)
+        GridTemplateAreas::None
     }
 }
 

From c87da27bca36af33f1ea2602d1e51dbf081f2674 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Wed, 19 Jun 2019 05:58:11 +0000
Subject: [PATCH 23/32] style: Use atoms for grid line names.

The style system already atomizes all CustomIdent values, which means that we're
just wasting memory and CPU by doing string copies all over the place.

This patch fixes it. This also simplifies further changes to use as much of the
rust data structures as possible.

I had to switch from nsTHashtable to mozilla::HashTable because the former
doesn't handle well non-default-constructible structs (like NamedLine, which
now has a StyleAtom, which is not default-constructible).

Differential Revision: https://phabricator.services.mozilla.com/D35119
---
 components/style/properties/gecko.mako.rs     | 44 ++++++++++---------
 components/style/values/specified/position.rs | 19 ++++----
 2 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index 1105e705060..fe0a1935e97 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -1137,11 +1137,12 @@ fn static_assert() {
         use crate::gecko_bindings::structs::{nsStyleGridLine_kMinLine, nsStyleGridLine_kMaxLine};
 
         let line = &mut self.gecko.${value.gecko};
-        let line_name = &mut line.mLineName;
-        match v.ident.as_ref() {
-            Some(i) => i.0.with_str(|s| line_name.assign(s)),
-            None => line_name.assign(""),
-        };
+        line.mLineName.set_move(unsafe {
+            RefPtr::from_addrefed(match v.ident {
+                Some(i) => i.0,
+                None => atom!(""),
+            }.into_addrefed())
+        });
         line.mHasSpan = v.is_span;
         if let Some(integer) = v.line_num {
             // clamping the integer between a range
@@ -1155,7 +1156,9 @@ fn static_assert() {
     pub fn copy_${value.name}_from(&mut self, other: &Self) {
         self.gecko.${value.gecko}.mHasSpan = other.gecko.${value.gecko}.mHasSpan;
         self.gecko.${value.gecko}.mInteger = other.gecko.${value.gecko}.mInteger;
-        self.gecko.${value.gecko}.mLineName.assign(&*other.gecko.${value.gecko}.mLineName);
+        unsafe {
+            self.gecko.${value.gecko}.mLineName.set(&other.gecko.${value.gecko}.mLineName);
+        }
     }
 
     pub fn reset_${value.name}(&mut self, other: &Self) {
@@ -1168,11 +1171,11 @@ fn static_assert() {
         longhands::${value.name}::computed_value::T {
             is_span: self.gecko.${value.gecko}.mHasSpan,
             ident: {
-                let name = self.gecko.${value.gecko}.mLineName.to_string();
-                if name.len() == 0 {
+                let name = unsafe { Atom::from_raw(self.gecko.${value.gecko}.mLineName.mRawPtr) };
+                if name == atom!("") {
                     None
                 } else {
-                    Some(CustomIdent(Atom::from(name)))
+                    Some(CustomIdent(name))
                 }
             },
             line_num:
@@ -1211,20 +1214,21 @@ fn static_assert() {
     pub fn set_grid_template_${kind}(&mut self, v: longhands::grid_template_${kind}::computed_value::T) {
         <% self_grid = "self.gecko.mGridTemplate%s" % kind.title() %>
         use crate::gecko_bindings::structs::{nsTArray, nsStyleGridLine_kMaxLine};
-        use nsstring::nsCString;
         use std::usize;
         use crate::values::CustomIdent;
         use crate::values::generics::grid::TrackListType::Auto;
         use crate::values::generics::grid::{GridTemplateComponent, RepeatCount};
 
         #[inline]
-        fn set_line_names(servo_names: &[CustomIdent], gecko_names: &mut nsTArray<nsCString>) {
+        fn set_line_names(servo_names: &[CustomIdent], gecko_names: &mut nsTArray<structs::RefPtr<structs::nsAtom>>) {
             unsafe {
-                bindings::Gecko_ResizeTArrayForCStrings(gecko_names, servo_names.len() as u32);
+                bindings::Gecko_ResizeAtomArray(gecko_names, servo_names.len() as u32);
             }
 
             for (servo_name, gecko_name) in servo_names.iter().zip(gecko_names.iter_mut()) {
-                servo_name.0.with_str(|s| gecko_name.assign(s))
+                gecko_name.set_move(unsafe {
+                    RefPtr::from_addrefed(servo_name.0.clone().into_addrefed())
+                });
             }
         }
 
@@ -1262,9 +1266,9 @@ fn static_assert() {
                     auto_track_size = Some(auto_repeat.track_sizes.get(0).unwrap().clone());
                 } else {
                     unsafe {
-                        bindings::Gecko_ResizeTArrayForCStrings(
+                        bindings::Gecko_ResizeAtomArray(
                             &mut value.mRepeatAutoLineNameListBefore, 0);
-                        bindings::Gecko_ResizeTArrayForCStrings(
+                        bindings::Gecko_ResizeAtomArray(
                             &mut value.mRepeatAutoLineNameListAfter, 0);
                     }
                 }
@@ -1338,7 +1342,6 @@ fn static_assert() {
     pub fn clone_grid_template_${kind}(&self) -> longhands::grid_template_${kind}::computed_value::T {
         <% self_grid = "self.gecko.mGridTemplate%s" % kind.title() %>
         use crate::gecko_bindings::structs::nsTArray;
-        use nsstring::nsCString;
         use crate::values::CustomIdent;
         use crate::values::generics::grid::{GridTemplateComponent, LineNameList, RepeatCount};
         use crate::values::generics::grid::{TrackList, TrackListType, TrackListValue, TrackRepeat, TrackSize};
@@ -1349,16 +1352,17 @@ fn static_assert() {
         };
 
         #[inline]
-        fn to_boxed_customident_slice(gecko_names: &nsTArray<nsCString>) -> Box<[CustomIdent]> {
+        fn to_boxed_customident_slice(gecko_names: &nsTArray<structs::RefPtr<structs::nsAtom>>) -> Box<[CustomIdent]> {
             let idents: Vec<CustomIdent> = gecko_names.iter().map(|gecko_name| {
-                CustomIdent(Atom::from(unsafe { gecko_name.as_str_unchecked() }))
+                CustomIdent(unsafe { Atom::from_raw(gecko_name.mRawPtr) })
             }).collect();
             idents.into_boxed_slice()
         }
 
         #[inline]
-        fn to_line_names_vec(gecko_line_names: &nsTArray<nsTArray<nsCString>>)
-            -> Vec<Box<[CustomIdent]>> {
+        fn to_line_names_vec(
+            gecko_line_names: &nsTArray<nsTArray<structs::RefPtr<structs::nsAtom>>>,
+        ) -> Vec<Box<[CustomIdent]>> {
             gecko_line_names.iter().map(|gecko_names| {
                 to_boxed_customident_slice(gecko_names)
             }).collect()
diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs
index dfce7218eed..aeffd418d27 100644
--- a/components/style/values/specified/position.rs
+++ b/components/style/values/specified/position.rs
@@ -7,7 +7,8 @@
 //!
 //! [position]: https://drafts.csswg.org/css-backgrounds-3/#position
 
-use crate::hash::FxHashMap;
+use crate::Atom;
+use crate::selector_map::PrecomputedHashMap;
 use crate::parser::{Parse, ParserContext};
 use crate::str::HTML_SPACE_CHARACTERS;
 use crate::values::computed::LengthPercentage as ComputedLengthPercentage;
@@ -500,15 +501,15 @@ impl TemplateAreas {
         let mut width = 0;
         {
             let mut row = 0u32;
-            let mut area_indices = FxHashMap::<&str, usize>::default();
+            let mut area_indices = PrecomputedHashMap::<Atom, usize>::default();
             for string in &strings {
                 let mut current_area_index: Option<usize> = None;
                 row += 1;
                 let mut column = 0u32;
                 for token in TemplateAreasTokenizer(string) {
                     column += 1;
-                    let token = if let Some(token) = token? {
-                        token
+                    let name = if let Some(token) = token? {
+                        Atom::from(token)
                     } else {
                         if let Some(index) = current_area_index.take() {
                             if areas[index].columns.end != column {
@@ -518,7 +519,7 @@ impl TemplateAreas {
                         continue;
                     };
                     if let Some(index) = current_area_index {
-                        if &*areas[index].name == token {
+                        if areas[index].name == name {
                             if areas[index].rows.start == row {
                                 areas[index].columns.end += 1;
                             }
@@ -528,7 +529,7 @@ impl TemplateAreas {
                             return Err(());
                         }
                     }
-                    if let Some(index) = area_indices.get(token).cloned() {
+                    if let Some(index) = area_indices.get(&name).cloned() {
                         if areas[index].columns.start != column || areas[index].rows.end != row {
                             return Err(());
                         }
@@ -537,8 +538,9 @@ impl TemplateAreas {
                         continue;
                     }
                     let index = areas.len();
+                    assert!(area_indices.insert(name.clone(), index).is_none());
                     areas.push(NamedArea {
-                        name: token.to_owned().into(),
+                        name,
                         columns: UnsignedRange {
                             start: column,
                             end: column + 1,
@@ -548,7 +550,6 @@ impl TemplateAreas {
                             end: row + 1,
                         },
                     });
-                    assert!(area_indices.insert(token, index).is_none());
                     current_area_index = Some(index);
                 }
                 if let Some(index) = current_area_index {
@@ -629,7 +630,7 @@ pub struct UnsignedRange {
 /// grid-placement properties.
 pub struct NamedArea {
     /// Name of the `named area`
-    pub name: crate::OwnedStr,
+    pub name: Atom,
     /// Rows of the `named area`
     pub rows: UnsignedRange,
     /// Columns of the `named area`

From 815d18922849abda1e39a89caea2f509d0d10f30 Mon Sep 17 00:00:00 2001
From: Boris Chiou <boris.chiou@gmail.com>
Date: Fri, 21 Jun 2019 21:48:54 +0000
Subject: [PATCH 24/32] style: Fix the serialization of the computed value of
 background-size.

This patch produces the following serialization:
```
    input      |  computed value
  ------------------------------
1. "auto"            "auto"
2. "auto auto"       "auto"
3. "15px auto"       "15px"
4. "15px"            "15px"
```
i.e. If the second value is 'auto', then it's omitted from our serialization,
because it's implied.

Besides, we update the wpt to address this spec issue:
https://github.com/w3c/csswg-drafts/issues/2574

Differential Revision: https://phabricator.services.mozilla.com/D35510
---
 components/style/values/generics/background.rs | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/components/style/values/generics/background.rs b/components/style/values/generics/background.rs
index b64fccb6051..d9b6624595d 100644
--- a/components/style/values/generics/background.rs
+++ b/components/style/values/generics/background.rs
@@ -6,13 +6,6 @@
 
 use crate::values::generics::length::{GenericLengthPercentageOrAuto, LengthPercentageOrAuto};
 
-fn width_and_height_are_auto<L>(
-    width: &LengthPercentageOrAuto<L>,
-    height: &LengthPercentageOrAuto<L>,
-) -> bool {
-    width.is_auto() && height.is_auto()
-}
-
 /// A generic value for the `background-size` property.
 #[derive(
     Animate,
@@ -37,10 +30,7 @@ pub enum GenericBackgroundSize<LengthPercent> {
         /// Explicit width.
         width: GenericLengthPercentageOrAuto<LengthPercent>,
         /// Explicit height.
-        /// NOTE(emilio): We should probably simplify all these in case `width`
-        /// and `height` are the same, but all other browsers agree on only
-        /// special-casing `auto`.
-        #[css(contextual_skip_if = "width_and_height_are_auto")]
+        #[css(skip_if = "GenericLengthPercentageOrAuto::is_auto")]
         height: GenericLengthPercentageOrAuto<LengthPercent>,
     },
     /// `cover`

From 6eedebe2c89d5b5449ce12dea7dfe357f26ac6aa Mon Sep 17 00:00:00 2001
From: violet <violet.bugreport@gmail.com>
Date: Sat, 22 Jun 2019 10:09:51 +0000
Subject: [PATCH 25/32] style: Invalidate style for ::selection.

This patch invalidates the style for `::selection`, which will restore the
behavior before the regression.

However, it's still not quite correct, because repaint is not triggered. Given
that `::selection` requires some major change to implement
https://github.com/w3c/csswg-drafts/issues/2474, we can address this problem
later.

Differential Revision: https://phabricator.services.mozilla.com/D35305
---
 components/style/gecko/pseudo_element.rs            |  6 ++++++
 .../style/invalidation/element/invalidator.rs       | 13 +++++++++++++
 components/style/servo/selector_parser.rs           |  6 ++++++
 3 files changed, 25 insertions(+)

diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs
index 7538b785858..dffd5acf086 100644
--- a/components/style/gecko/pseudo_element.rs
+++ b/components/style/gecko/pseudo_element.rs
@@ -116,6 +116,12 @@ impl PseudoElement {
         *self == PseudoElement::Marker
     }
 
+    /// Whether this pseudo-element is the ::selection pseudo.
+    #[inline]
+    pub fn is_selection(&self) -> bool {
+        *self == PseudoElement::Selection
+    }
+
     /// Whether this pseudo-element is ::first-letter.
     #[inline]
     pub fn is_first_letter(&self) -> bool {
diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs
index f00aa57fcff..414c81ee52b 100644
--- a/components/style/invalidation/element/invalidator.rs
+++ b/components/style/invalidation/element/invalidator.rs
@@ -805,6 +805,19 @@ where
                         if pseudo.is_marker() && self.element.marker_pseudo_element().is_none() {
                             invalidated_self = true;
                         }
+
+                        // FIXME: ::selection doesn't generate elements, so the
+                        // regular invalidation doesn't work for it. We store
+                        // the cached selection style holding off the originating
+                        // element, so we need to restyle it in order to invalidate
+                        // it. This is still not quite correct, since nothing
+                        // triggers a repaint necessarily, but matches old Gecko
+                        // behavior, and the ::selection implementation needs to
+                        // change significantly anyway to implement
+                        // https://github.com/w3c/csswg-drafts/issues/2474.
+                        if pseudo.is_selection() {
+                            invalidated_self = true;
+                        }
                     }
                 }
 
diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs
index 1ef49c87f9f..4bf3f38822a 100644
--- a/components/style/servo/selector_parser.rs
+++ b/components/style/servo/selector_parser.rs
@@ -143,6 +143,12 @@ impl PseudoElement {
         false
     }
 
+    /// Whether this pseudo-element is the ::selection pseudo.
+    #[inline]
+    pub fn is_selection(&self) -> bool {
+        *self == PseudoElement::Selection
+    }
+
     /// Whether this pseudo-element is the ::before pseudo.
     #[inline]
     pub fn is_before(&self) -> bool {

From ed2e9ce4826aa1d1d18e406e673ce4ce3fa44c51 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sun, 23 Jun 2019 13:03:00 +0200
Subject: [PATCH 26/32] Rustfmt and fix tidy on recent changes.

---
 components/selectors/builder.rs               |  9 ++---
 components/selectors/parser.rs                | 11 ++++---
 components/style/dom.rs                       |  4 +--
 components/style/gecko/conversions.rs         |  5 ++-
 components/style/gecko/selector_parser.rs     |  5 ++-
 components/style/gecko/wrapper.rs             |  5 +--
 .../style/invalidation/element/invalidator.rs |  8 ++---
 .../media_queries/media_feature_expression.rs |  3 +-
 components/style/properties/data.py           |  4 ++-
 components/style/rule_collector.rs            | 19 +++++------
 components/style/selector_map.rs              |  2 +-
 components/style/values/computed/image.rs     |  4 +--
 components/style/values/generics/image.rs     | 11 ++++++-
 components/style/values/specified/image.rs    | 33 ++++++++++++++-----
 components/style/values/specified/position.rs |  8 +++--
 .../style/values/specified/transform.rs       |  4 ++-
 16 files changed, 78 insertions(+), 57 deletions(-)

diff --git a/components/selectors/builder.rs b/components/selectors/builder.rs
index cddaf7195a1..41b83b0c40e 100644
--- a/components/selectors/builder.rs
+++ b/components/selectors/builder.rs
@@ -110,10 +110,7 @@ impl<Impl: SelectorImpl> SelectorBuilder<Impl> {
         if parsed_part {
             flags |= SelectorFlags::HAS_PART;
         }
-        self.build_with_specificity_and_flags(SpecificityAndFlags {
-            specificity,
-            flags,
-        })
+        self.build_with_specificity_and_flags(SpecificityAndFlags { specificity, flags })
     }
 
     /// Builds with an explicit SpecificityAndFlags. This is separated from build() so
@@ -208,9 +205,9 @@ bitflags! {
 pub struct SpecificityAndFlags {
     /// There are two free bits here, since we use ten bits for each specificity
     /// kind (id, class, element).
-    pub (crate) specificity: u32,
+    pub(crate) specificity: u32,
     /// There's padding after this field due to the size of the flags.
-    pub (crate) flags: SelectorFlags,
+    pub(crate) flags: SelectorFlags,
 }
 
 impl SpecificityAndFlags {
diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs
index d76defe8131..982dff793a8 100644
--- a/components/selectors/parser.rs
+++ b/components/selectors/parser.rs
@@ -751,7 +751,11 @@ impl<Impl: SelectorImpl> Selector<Impl> {
 
     /// Creates a Selector from a vec of Components, specified in parse order. Used in tests.
     #[allow(unused)]
-    pub (crate) fn from_vec(vec: Vec<Component<Impl>>, specificity: u32, flags: SelectorFlags) -> Self {
+    pub(crate) fn from_vec(
+        vec: Vec<Component<Impl>>,
+        specificity: u32,
+        flags: SelectorFlags,
+    ) -> Self {
         let mut builder = SelectorBuilder::default();
         for component in vec.into_iter() {
             if let Some(combinator) = component.as_combinator() {
@@ -760,10 +764,7 @@ impl<Impl: SelectorImpl> Selector<Impl> {
                 builder.push_simple_selector(component);
             }
         }
-        let spec = SpecificityAndFlags {
-            specificity,
-            flags,
-        };
+        let spec = SpecificityAndFlags { specificity, flags };
         Selector(builder.build_with_specificity_and_flags(spec))
     }
 
diff --git a/components/style/dom.rs b/components/style/dom.rs
index 0e68e55c47f..7d56cc543e7 100644
--- a/components/style/dom.rs
+++ b/components/style/dom.rs
@@ -349,7 +349,7 @@ pub trait TShadowRoot: Sized + Copy + Clone + PartialEq {
     /// Get the list of shadow parts for this shadow root.
     fn parts<'a>(&self) -> &[<Self::ConcreteNode as TNode>::ConcreteElement]
     where
-        Self: 'a
+        Self: 'a,
     {
         &[]
     }
@@ -534,7 +534,7 @@ pub trait TElement:
     /// Internal iterator for the part names of this element.
     fn each_part<F>(&self, _callback: F)
     where
-        F: FnMut(&Atom)
+        F: FnMut(&Atom),
     {
     }
 
diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs
index 4ce44136279..bf1b93422f0 100644
--- a/components/style/gecko/conversions.rs
+++ b/components/style/gecko/conversions.rs
@@ -99,7 +99,7 @@ impl nsStyleImage {
 
     fn set_gradient(&mut self, gradient: Box<Gradient>) {
         unsafe {
-             bindings::Gecko_SetGradientImageValue(self, Box::into_raw(gradient));
+            bindings::Gecko_SetGradientImageValue(self, Box::into_raw(gradient));
         }
     }
 
@@ -142,8 +142,7 @@ impl nsStyleImage {
                 }
             },
             nsStyleImageType::eStyleImageType_Gradient => {
-                let gradient: &Gradient =
-                    &**self.__bindgen_anon_1.mGradient.as_ref();
+                let gradient: &Gradient = &**self.__bindgen_anon_1.mGradient.as_ref();
                 Some(GenericImage::Gradient(Box::new(gradient.clone())))
             },
             nsStyleImageType::eStyleImageType_Element => {
diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs
index 659200c3786..fc27e1a8610 100644
--- a/components/style/gecko/selector_parser.rs
+++ b/components/style/gecko/selector_parser.rs
@@ -354,9 +354,8 @@ impl<'a, 'i> ::selectors::Parser<'i> for SelectorParser<'a> {
 
     #[inline]
     fn parse_part(&self) -> bool {
-        self.chrome_rules_enabled() || unsafe {
-            structs::StaticPrefs_sVarCache_layout_css_shadow_parts_enabled
-        }
+        self.chrome_rules_enabled() ||
+            unsafe { structs::StaticPrefs_sVarCache_layout_css_shadow_parts_enabled }
     }
 
     fn parse_non_ts_pseudo_class(
diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs
index ba2951f6f25..39e3678eebc 100644
--- a/components/style/gecko/wrapper.rs
+++ b/components/style/gecko/wrapper.rs
@@ -189,7 +189,7 @@ impl<'lr> TShadowRoot for GeckoShadowRoot<'lr> {
     #[inline]
     fn parts<'a>(&self) -> &[<Self::ConcreteNode as TNode>::ConcreteElement]
     where
-        Self: 'a
+        Self: 'a,
     {
         let slice: &[*const RawGeckoElement] = &*self.0.mParts;
 
@@ -1350,7 +1350,8 @@ impl<'le> TElement for GeckoElement<'le> {
 
     #[inline]
     fn has_part_attr(&self) -> bool {
-        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasPart)
+        self.as_node()
+            .get_bool_flag(nsINode_BooleanFlag::ElementHasPart)
     }
 
     // FIXME(emilio): we should probably just return a reference to the Atom.
diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs
index 414c81ee52b..2889c3e7b9d 100644
--- a/components/style/invalidation/element/invalidator.rs
+++ b/components/style/invalidation/element/invalidator.rs
@@ -78,7 +78,8 @@ pub struct DescendantInvalidationLists<'a> {
 
 impl<'a> DescendantInvalidationLists<'a> {
     fn is_empty(&self) -> bool {
-        self.dom_descendants.is_empty() && self.slotted_descendants.is_empty() &&
+        self.dom_descendants.is_empty() &&
+            self.slotted_descendants.is_empty() &&
             self.parts.is_empty()
     }
 }
@@ -179,9 +180,7 @@ impl<'a> Invalidation<'a> {
             Combinator::Child | Combinator::Descendant | Combinator::PseudoElement => {
                 InvalidationKind::Descendant(DescendantInvalidationKind::Dom)
             },
-            Combinator::Part => {
-                InvalidationKind::Descendant(DescendantInvalidationKind::Part)
-            },
+            Combinator::Part => InvalidationKind::Descendant(DescendantInvalidationKind::Part),
             Combinator::SlotAssignment => {
                 InvalidationKind::Descendant(DescendantInvalidationKind::Slotted)
             },
@@ -506,7 +505,6 @@ where
         any
     }
 
-
     fn invalidate_slotted_elements(&mut self, invalidations: &[Invalidation<'b>]) -> bool {
         if invalidations.is_empty() {
             return false;
diff --git a/components/style/media_queries/media_feature_expression.rs b/components/style/media_queries/media_feature_expression.rs
index ada30c4e895..5b5c4f4fad7 100644
--- a/components/style/media_queries/media_feature_expression.rs
+++ b/components/style/media_queries/media_feature_expression.rs
@@ -298,8 +298,7 @@ impl MediaFeatureExpression {
 
                 #[cfg(feature = "gecko")]
                 {
-                    if starts_with_ignore_ascii_case(feature_name, "-webkit-")
-                    {
+                    if starts_with_ignore_ascii_case(feature_name, "-webkit-") {
                         feature_name = &feature_name[8..];
                         requirements.insert(ParsingRequirements::WEBKIT_PREFIX);
                         if unsafe {
diff --git a/components/style/properties/data.py b/components/style/properties/data.py
index 8113c402581..13496104a04 100644
--- a/components/style/properties/data.py
+++ b/components/style/properties/data.py
@@ -165,9 +165,11 @@ def parse_property_aliases(alias_list):
             result.append((name, pref))
     return result
 
+
 def to_phys(name, logical, physical):
     return name.replace(logical, physical).replace("inset-", "")
 
+
 class Longhand(object):
     def __init__(self, style_struct, name, spec=None, animation_value_type=None, keyword=None,
                  predefined_type=None, servo_pref=None, gecko_pref=None,
@@ -247,7 +249,7 @@ class Longhand(object):
     def all_physical_mapped_properties(self):
         assert self.logical
         candidates = [s for s in LOGICAL_SIDES + LOGICAL_SIZES + LOGICAL_CORNERS
-                if s in self.name] + [s for s in LOGICAL_AXES if self.name.endswith(s)]
+                      if s in self.name] + [s for s in LOGICAL_AXES if self.name.endswith(s)]
         assert(len(candidates) == 1)
         logical_side = candidates[0]
 
diff --git a/components/style/rule_collector.rs b/components/style/rule_collector.rs
index dcd39c2b36c..f536b80b5c5 100644
--- a/components/style/rule_collector.rs
+++ b/components/style/rule_collector.rs
@@ -332,17 +332,14 @@ where
         let host = shadow.host();
         let containing_shadow = host.containing_shadow();
         let part_rules = match containing_shadow {
-            Some(shadow) => {
-                shadow
-                    .style_data()
-                    .and_then(|data| data.part_rules(self.pseudo_element))
-            },
-            None => {
-                self.stylist
-                    .cascade_data()
-                    .borrow_for_origin(Origin::Author)
-                    .part_rules(self.pseudo_element)
-            }
+            Some(shadow) => shadow
+                .style_data()
+                .and_then(|data| data.part_rules(self.pseudo_element)),
+            None => self
+                .stylist
+                .cascade_data()
+                .borrow_for_origin(Origin::Author)
+                .part_rules(self.pseudo_element),
         };
 
         // TODO(emilio): SameTreeAuthorNormal is a bit of a lie here, we may
diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs
index a1ef9d0c273..52b939e0219 100644
--- a/components/style/selector_map.rs
+++ b/components/style/selector_map.rs
@@ -258,7 +258,7 @@ impl SelectorMap<Rule> {
     }
 
     /// Adds rules in `rules` that match `element` to the `matching_rules` list.
-    pub (crate) fn get_matching_rules<E, F>(
+    pub(crate) fn get_matching_rules<E, F>(
         element: E,
         rules: &[Rule],
         matching_rules: &mut ApplicableDeclarationList,
diff --git a/components/style/values/computed/image.rs b/components/style/values/computed/image.rs
index 3012eece0e7..67b432d5ffd 100644
--- a/components/style/values/computed/image.rs
+++ b/components/style/values/computed/image.rs
@@ -66,10 +66,10 @@ impl generic::LineDirection for LineDirection {
             LineDirection::Angle(angle) => angle.radians() == PI,
             LineDirection::Vertical(VerticalPositionKeyword::Bottom) => {
                 compat_mode == GradientCompatMode::Modern
-            }
+            },
             LineDirection::Vertical(VerticalPositionKeyword::Top) => {
                 compat_mode != GradientCompatMode::Modern
-            }
+            },
             _ => false,
         }
     }
diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs
index 3f126a6e20d..c739820ae03 100644
--- a/components/style/values/generics/image.rs
+++ b/components/style/values/generics/image.rs
@@ -15,7 +15,16 @@ use style_traits::{CssWriter, ToCss};
 
 /// An <image> | <none> (for background-image, for example).
 #[derive(
-    Clone, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss, ToResolvedValue, ToShmem,
+    Clone,
+    Debug,
+    MallocSizeOf,
+    Parse,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToComputedValue,
+    ToCss,
+    ToResolvedValue,
+    ToShmem,
 )]
 pub enum GenericImageLayer<Image> {
     /// The `none` value.
diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs
index f4f4cf9afce..b0133f7a401 100644
--- a/components/style/values/specified/image.rs
+++ b/components/style/values/specified/image.rs
@@ -11,10 +11,12 @@ use crate::custom_properties::SpecifiedValue;
 use crate::parser::{Parse, ParserContext};
 use crate::stylesheets::CorsMode;
 use crate::values::generics::image::PaintWorklet;
-use crate::values::generics::image::{self as generic, Circle, GradientCompatMode, Ellipse, ShapeExtent};
+use crate::values::generics::image::{
+    self as generic, Circle, Ellipse, GradientCompatMode, ShapeExtent,
+};
 use crate::values::generics::position::Position as GenericPosition;
-use crate::values::specified::position::{Position, PositionComponent, Side};
 use crate::values::specified::position::{HorizontalPositionKeyword, VerticalPositionKeyword};
+use crate::values::specified::position::{Position, PositionComponent, Side};
 use crate::values::specified::url::SpecifiedImageUrl;
 use crate::values::specified::{Angle, Color, Length, LengthPercentage};
 use crate::values::specified::{Number, NumberOrPercentage, Percentage};
@@ -250,7 +252,12 @@ impl Parse for Gradient {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
 
-        Ok(Gradient { items, repeating, kind, compat_mode })
+        Ok(Gradient {
+            items,
+            repeating,
+            kind,
+            compat_mode,
+        })
     }
 }
 
@@ -259,7 +266,9 @@ impl Gradient {
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
-        use crate::values::specified::position::{HorizontalPositionKeyword as X, VerticalPositionKeyword as Y};
+        use crate::values::specified::position::{
+            HorizontalPositionKeyword as X, VerticalPositionKeyword as Y,
+        };
         type Point = GenericPosition<Component<X>, Component<Y>>;
 
         #[derive(Clone, Copy, Parse)]
@@ -512,7 +521,9 @@ impl GradientKind {
             d
         } else {
             match *compat_mode {
-                GradientCompatMode::Modern => LineDirection::Vertical(VerticalPositionKeyword::Bottom),
+                GradientCompatMode::Modern => {
+                    LineDirection::Vertical(VerticalPositionKeyword::Bottom)
+                },
                 _ => LineDirection::Vertical(VerticalPositionKeyword::Top),
             }
         };
@@ -563,10 +574,10 @@ impl generic::LineDirection for LineDirection {
             LineDirection::Angle(ref angle) => angle.degrees() == 180.0,
             LineDirection::Vertical(VerticalPositionKeyword::Bottom) => {
                 compat_mode == GradientCompatMode::Modern
-            }
+            },
             LineDirection::Vertical(VerticalPositionKeyword::Top) => {
                 compat_mode != GradientCompatMode::Modern
-            }
+            },
             _ => false,
         }
     }
@@ -621,7 +632,9 @@ impl LineDirection {
                 // Fall back to Modern compatibility mode in case there is a `to` keyword.
                 // According to Gecko, `-moz-linear-gradient(to ...)` should serialize like
                 // `linear-gradient(to ...)`.
-                GradientCompatMode::Moz if to_ident.is_ok() => *compat_mode = GradientCompatMode::Modern,
+                GradientCompatMode::Moz if to_ident.is_ok() => {
+                    *compat_mode = GradientCompatMode::Modern
+                },
                 // There is no `to` keyword in webkit prefixed syntax. If it's consumed,
                 // parsing should throw an error.
                 GradientCompatMode::WebKit if to_ident.is_ok() => {
@@ -741,7 +754,9 @@ impl ShapeExtent {
         compat_mode: GradientCompatMode,
     ) -> Result<Self, ParseError<'i>> {
         match Self::parse(input)? {
-            ShapeExtent::Contain | ShapeExtent::Cover if compat_mode == GradientCompatMode::Modern => {
+            ShapeExtent::Contain | ShapeExtent::Cover
+                if compat_mode == GradientCompatMode::Modern =>
+            {
                 Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
             },
             ShapeExtent::Contain => Ok(ShapeExtent::ClosestSide),
diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs
index aeffd418d27..85516790c4f 100644
--- a/components/style/values/specified/position.rs
+++ b/components/style/values/specified/position.rs
@@ -7,15 +7,15 @@
 //!
 //! [position]: https://drafts.csswg.org/css-backgrounds-3/#position
 
-use crate::Atom;
-use crate::selector_map::PrecomputedHashMap;
 use crate::parser::{Parse, ParserContext};
+use crate::selector_map::PrecomputedHashMap;
 use crate::str::HTML_SPACE_CHARACTERS;
 use crate::values::computed::LengthPercentage as ComputedLengthPercentage;
 use crate::values::computed::{Context, Percentage, ToComputedValue};
 use crate::values::generics::position::Position as GenericPosition;
 use crate::values::generics::position::ZIndex as GenericZIndex;
 use crate::values::specified::{AllowQuirks, Integer, LengthPercentage};
+use crate::Atom;
 use crate::Zero;
 use cssparser::Parser;
 use selectors::parser::SelectorParseErrorKind;
@@ -579,7 +579,9 @@ impl Parse for TemplateAreas {
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         let mut strings = vec![];
-        while let Ok(string) = input.try(|i| i.expect_string().map(|s| s.as_ref().to_owned().into())) {
+        while let Ok(string) =
+            input.try(|i| i.expect_string().map(|s| s.as_ref().to_owned().into()))
+        {
             strings.push(string);
         }
 
diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs
index d52aa69cd24..9d33fd4188f 100644
--- a/components/style/values/specified/transform.rs
+++ b/components/style/values/specified/transform.rs
@@ -9,7 +9,9 @@ use crate::values::computed::{Context, LengthPercentage as ComputedLengthPercent
 use crate::values::computed::{Percentage as ComputedPercentage, ToComputedValue};
 use crate::values::generics::transform as generic;
 use crate::values::generics::transform::{Matrix, Matrix3D};
-use crate::values::specified::position::{Side, HorizontalPositionKeyword, VerticalPositionKeyword};
+use crate::values::specified::position::{
+    HorizontalPositionKeyword, Side, VerticalPositionKeyword,
+};
 use crate::values::specified::{self, Angle, Integer, Length, LengthPercentage, Number};
 use crate::Zero;
 use cssparser::Parser;

From add08518cdaf9f75bf4d9bc2d81347824c00b154 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sun, 23 Jun 2019 13:13:21 +0200
Subject: [PATCH 27/32] Fix style system build with recent changes.

---
 components/layout/display_list/builder.rs     | 28 +++++++++++--------
 components/layout/display_list/gradient.rs    | 19 +++++++------
 components/layout_thread/dom_wrapper.rs       |  4 +++
 components/script/dom/element.rs              |  5 ++--
 .../dom/webidls/CSSStyleDeclaration.webidl    |  7 +++++
 components/style/stylist.rs                   |  5 ++--
 components/style/traversal.rs                 |  5 ++--
 7 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs
index 08fad71b4ea..8d32ec01520 100644
--- a/components/layout/display_list/builder.rs
+++ b/components/layout/display_list/builder.rs
@@ -57,10 +57,10 @@ use style::logical_geometry::{LogicalMargin, LogicalPoint, LogicalRect};
 use style::properties::{style_structs, ComputedValues};
 use style::servo::restyle_damage::ServoRestyleDamage;
 use style::values::computed::effects::SimpleShadow;
-use style::values::computed::image::Image as ComputedImage;
+use style::values::computed::image::{Image, ImageLayer};
 use style::values::computed::{Gradient, LengthOrAuto};
 use style::values::generics::background::BackgroundSize;
-use style::values::generics::image::{GradientKind, Image, PaintWorklet};
+use style::values::generics::image::{GradientKind, PaintWorklet};
 use style::values::specified::ui::CursorKind;
 use style::values::{Either, RGBA};
 use style_traits::ToCss;
@@ -726,9 +726,13 @@ impl Fragment {
         // http://www.w3.org/TR/CSS21/colors.html#background
         let background = style.get_background();
         for (i, background_image) in background.background_image.0.iter().enumerate().rev() {
+            let background_image = match *background_image {
+                ImageLayer::None => continue,
+                ImageLayer::Image(ref image) => image,
+            };
+
             match *background_image {
-                Either::First(_) => {},
-                Either::Second(Image::Gradient(ref gradient)) => {
+                Image::Gradient(ref gradient) => {
                     self.build_display_list_for_background_gradient(
                         state,
                         display_list_section,
@@ -738,7 +742,7 @@ impl Fragment {
                         i,
                     );
                 },
-                Either::Second(Image::Url(ref image_url)) => {
+                Image::Url(ref image_url) => {
                     if let Some(url) = image_url.url() {
                         let webrender_image = state.layout_context.get_webrender_image_for_url(
                             self.node,
@@ -757,7 +761,7 @@ impl Fragment {
                         }
                     }
                 },
-                Either::Second(Image::PaintWorklet(ref paint_worklet)) => {
+                Image::PaintWorklet(ref paint_worklet) => {
                     let bounding_box = self.border_box - style.logical_border_width();
                     let bounding_box_size = bounding_box.size.to_physical(style.writing_mode);
                     let background_size =
@@ -790,10 +794,10 @@ impl Fragment {
                         );
                     }
                 },
-                Either::Second(Image::Rect(_)) => {
+                Image::Rect(_) => {
                     // TODO: Implement `-moz-image-rect`
                 },
-                Either::Second(Image::Element(_)) => {
+                Image::Element(_) => {
                     // TODO: Implement `-moz-element`
                 },
             }
@@ -978,7 +982,7 @@ impl Fragment {
                     };
                     DisplayItem::Gradient(CommonDisplayItem::with_data(base, item, stops))
                 },
-                GradientKind::Radial(shape, center, _angle) => {
+                GradientKind::Radial(shape, center) => {
                     let (gradient, stops) = gradient::radial(
                         style,
                         placement.tile_size,
@@ -1115,7 +1119,7 @@ impl Fragment {
         let border_radius = border::radii(bounds, border_style_struct);
         let border_widths = border.to_physical(style.writing_mode);
 
-        if let Either::Second(ref image) = border_style_struct.border_image_source {
+        if let ImageLayer::Image(ref image) = border_style_struct.border_image_source {
             if self
                 .build_display_list_for_border_image(
                     state,
@@ -1173,7 +1177,7 @@ impl Fragment {
         style: &ComputedValues,
         base: BaseDisplayItem,
         bounds: Rect<Au>,
-        image: &ComputedImage,
+        image: &Image,
         border_width: SideOffsets2D<Au>,
     ) -> Option<()> {
         let border_style_struct = style.get_border();
@@ -1227,7 +1231,7 @@ impl Fragment {
                     stops = linear_stops;
                     NinePatchBorderSource::Gradient(wr_gradient)
                 },
-                GradientKind::Radial(shape, center, _angle) => {
+                GradientKind::Radial(shape, center) => {
                     let (wr_gradient, radial_stops) = gradient::radial(
                         style,
                         border_image_area,
diff --git a/components/layout/display_list/gradient.rs b/components/layout/display_list/gradient.rs
index 264bea7fbee..ab56b1d6c9c 100644
--- a/components/layout/display_list/gradient.rs
+++ b/components/layout/display_list/gradient.rs
@@ -9,7 +9,6 @@ use style::properties::ComputedValues;
 use style::values::computed::image::{EndingShape, LineDirection};
 use style::values::computed::{Angle, GradientItem, LengthPercentage, Percentage, Position};
 use style::values::generics::image::{Circle, ColorStop, Ellipse, ShapeExtent};
-use style::values::specified::position::{X, Y};
 use webrender_api::{ExtendMode, Gradient, GradientBuilder, GradientStop, RadialGradient};
 
 /// A helper data structure for gradients.
@@ -227,15 +226,17 @@ pub fn linear(
     direction: LineDirection,
     repeating: bool,
 ) -> (Gradient, Vec<GradientStop>) {
+    use style::values::specified::position::HorizontalPositionKeyword::*;
+    use style::values::specified::position::VerticalPositionKeyword::*;
     let angle = match direction {
         LineDirection::Angle(angle) => angle.radians(),
         LineDirection::Horizontal(x) => match x {
-            X::Left => Angle::from_degrees(270.).radians(),
-            X::Right => Angle::from_degrees(90.).radians(),
+            Left => Angle::from_degrees(270.).radians(),
+            Right => Angle::from_degrees(90.).radians(),
         },
         LineDirection::Vertical(y) => match y {
-            Y::Top => Angle::from_degrees(0.).radians(),
-            Y::Bottom => Angle::from_degrees(180.).radians(),
+            Top => Angle::from_degrees(0.).radians(),
+            Bottom => Angle::from_degrees(180.).radians(),
         },
         LineDirection::Corner(horizontal, vertical) => {
             // This the angle for one of the diagonals of the box. Our angle
@@ -243,10 +244,10 @@ pub fn linear(
             // two perpendicular angles.
             let atan = (size.height.to_f32_px() / size.width.to_f32_px()).atan();
             match (horizontal, vertical) {
-                (X::Right, Y::Bottom) => ::std::f32::consts::PI - atan,
-                (X::Left, Y::Bottom) => ::std::f32::consts::PI + atan,
-                (X::Right, Y::Top) => atan,
-                (X::Left, Y::Top) => -atan,
+                (Right, Bottom) => ::std::f32::consts::PI - atan,
+                (Left, Bottom) => ::std::f32::consts::PI + atan,
+                (Right, Top) => atan,
+                (Left, Top) => -atan,
             }
         },
     };
diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs
index a0c1a8b7a61..35463f69715 100644
--- a/components/layout_thread/dom_wrapper.rs
+++ b/components/layout_thread/dom_wrapper.rs
@@ -510,6 +510,10 @@ impl<'le> TElement for ServoLayoutElement<'le> {
         *self.element.namespace() == ns!(svg)
     }
 
+    fn has_part_attr(&self) -> bool {
+        false
+    }
+
     fn style_attribute(&self) -> Option<ArcBorrow<StyleLocked<PropertyDeclarationBlock>>> {
         unsafe {
             (*self.element.style_attribute())
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 764c7ce6051..5695e355368 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -132,8 +132,7 @@ use style::selector_parser::{
 use style::shared_lock::{Locked, SharedRwLock};
 use style::thread_state;
 use style::values::generics::NonNegative;
-use style::values::{computed, specified};
-use style::values::{CSSFloat, Either};
+use style::values::{computed, specified, CSSFloat};
 use style::CaseSensitivityExt;
 use xml5ever::serialize as xmlSerialize;
 use xml5ever::serialize::SerializeOpts as XmlSerializeOpts;
@@ -684,7 +683,7 @@ impl LayoutElementHelpers for LayoutDom<Element> {
             hints.push(from_declaration(
                 shared_lock,
                 PropertyDeclaration::BackgroundImage(background_image::SpecifiedValue(
-                    vec![Either::Second(specified::Image::for_cascade(url.into()))].into(),
+                    vec![specified::ImageLayer::Image(specified::Image::for_cascade(url.into()))].into(),
                 )),
             ));
         }
diff --git a/components/script/dom/webidls/CSSStyleDeclaration.webidl b/components/script/dom/webidls/CSSStyleDeclaration.webidl
index d64de8f3e5a..a49d3ec465a 100644
--- a/components/script/dom/webidls/CSSStyleDeclaration.webidl
+++ b/components/script/dom/webidls/CSSStyleDeclaration.webidl
@@ -237,10 +237,17 @@ partial interface CSSStyleDeclaration {
   [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString counter-reset;
 
   [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflow;
+
+  [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflowBlock;
+  [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflow-block;
+  [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflowInline;
+  [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflow-inline;
+
   [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflowX;
   [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflow-x;
   [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflowY;
   [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflow-y;
+
   [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflowWrap;
   [CEReactions, SetterThrows] attribute [TreatNullAs=EmptyString] DOMString overflow-wrap;
 
diff --git a/components/style/stylist.rs b/components/style/stylist.rs
index 33ce450151b..ed9af91af14 100644
--- a/components/style/stylist.rs
+++ b/components/style/stylist.rs
@@ -35,10 +35,9 @@ use crate::thread_state::{self, ThreadState};
 use crate::{Atom, LocalName, Namespace, WeakAtom};
 use fallible::FallibleVec;
 use hashglobe::FailedAllocationError;
+use malloc_size_of::MallocSizeOf;
 #[cfg(feature = "gecko")]
-use malloc_size_of::MallocUnconditionalShallowSizeOf;
-#[cfg(feature = "gecko")]
-use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps};
+use malloc_size_of::{MallocShallowSizeOf, MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
 use selectors::attr::{CaseSensitivity, NamespaceConstraint};
 use selectors::bloom::BloomFilter;
 use selectors::matching::VisitedHandlingMode;
diff --git a/components/style/traversal.rs b/components/style/traversal.rs
index 8bc57ada8b7..69d0e463150 100644
--- a/components/style/traversal.rs
+++ b/components/style/traversal.rs
@@ -693,8 +693,7 @@ fn notify_paint_worklet<E>(context: &StyleContext<E>, data: &ElementData)
 where
     E: TElement,
 {
-    use crate::values::generics::image::Image;
-    use crate::values::Either;
+    use crate::values::generics::image::{GenericImageLayer, Image};
     use style_traits::ToCss;
 
     // We speculatively evaluate any paint worklets during styling.
@@ -704,7 +703,7 @@ where
     if let Some(ref values) = data.styles.primary {
         for image in &values.get_background().background_image.0 {
             let (name, arguments) = match *image {
-                Either::Second(Image::PaintWorklet(ref worklet)) => {
+                GenericImageLayer::Image(Image::PaintWorklet(ref worklet)) => {
                     (&worklet.name, &worklet.arguments)
                 },
                 _ => continue,

From 36c39d53f7b240ee8af64c5a3620d34f09a5b5ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sun, 23 Jun 2019 13:25:48 +0200
Subject: [PATCH 28/32] style: Give up on test_parse_stylesheet.

It's not particularly useful.
---
 Cargo.lock                      |   1 -
 tests/unit/style/Cargo.toml     |   1 -
 tests/unit/style/lib.rs         |   1 -
 tests/unit/style/stylesheets.rs | 272 +-------------------------------
 4 files changed, 3 insertions(+), 272 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index e9f1f372c5d..6fe4a8d20d3 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4569,7 +4569,6 @@ dependencies = [
  "cssparser 0.25.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.19.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "html5ever 0.23.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "parking_lot 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rayon 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "selectors 0.21.0",
  "serde_json 1.0.13 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/tests/unit/style/Cargo.toml b/tests/unit/style/Cargo.toml
index 6b378b63ee0..b339ed48c52 100644
--- a/tests/unit/style/Cargo.toml
+++ b/tests/unit/style/Cargo.toml
@@ -15,7 +15,6 @@ app_units = "0.7"
 cssparser = "0.25"
 euclid = "0.19"
 html5ever = "0.23"
-parking_lot = "0.8"
 rayon = "1"
 serde_json = "1.0"
 selectors = {path = "../../../components/selectors"}
diff --git a/tests/unit/style/lib.rs b/tests/unit/style/lib.rs
index eea724d085e..ba1c8992e75 100644
--- a/tests/unit/style/lib.rs
+++ b/tests/unit/style/lib.rs
@@ -10,7 +10,6 @@ extern crate cssparser;
 extern crate euclid;
 #[macro_use]
 extern crate html5ever;
-extern crate parking_lot;
 extern crate rayon;
 extern crate selectors;
 extern crate serde_json;
diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs
index c6b02f37eda..d1bc0e8113f 100644
--- a/tests/unit/style/stylesheets.rs
+++ b/tests/unit/style/stylesheets.rs
@@ -2,283 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
-use cssparser::{self, SourceLocation};
-use html5ever::Namespace as NsAtom;
-use parking_lot::RwLock;
-use selectors::attr::*;
-use selectors::parser::*;
+use cssparser::SourceLocation;
 use servo_arc::Arc;
-use servo_atoms::Atom;
 use servo_config::set_pref;
 use servo_url::ServoUrl;
-use std::borrow::ToOwned;
 use std::cell::RefCell;
-use std::sync::atomic::AtomicBool;
 use style::context::QuirksMode;
 use style::error_reporting::{ContextualParseError, ParseErrorReporter};
 use style::media_queries::MediaList;
-use style::properties::longhands;
-use style::properties::{CSSWideKeyword, CustomDeclaration};
-use style::properties::{CustomDeclarationValue, Importance};
-use style::properties::{PropertyDeclaration, PropertyDeclarationBlock};
 use style::shared_lock::SharedRwLock;
-use style::stylesheets::{
-    CssRule, CssRules, NamespaceRule, StyleRule, Stylesheet, StylesheetContents,
-};
-use style::stylesheets::{Namespaces, Origin};
-use style::values::specified::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
-}
-
-#[test]
-fn test_parse_stylesheet() {
-    let css = r"
-        @namespace url(http://www.w3.org/1999/xhtml);
-        /* FIXME: only if scripting is enabled */
-        input[type=hidden i] {
-            display: block !important;
-            display: none !important;
-            display: inline;
-            --a: b !important;
-            --a: inherit !important;
-            --a: c;
-        }
-        html , body /**/ {
-            display: none;
-            display: block;
-        }
-        #d1 > .ok { background: blue; }
-        }";
-    let url = ServoUrl::parse("about::test").unwrap();
-    let lock = SharedRwLock::new();
-    let media = Arc::new(lock.wrap(MediaList::empty()));
-    let stylesheet = Stylesheet::from_str(
-        css,
-        url.clone(),
-        Origin::UserAgent,
-        media,
-        lock,
-        None,
-        None,
-        QuirksMode::NoQuirks,
-        0,
-    );
-    let mut namespaces = Namespaces::default();
-    namespaces.default = Some(ns!(html));
-    let expected = Stylesheet {
-        contents: StylesheetContents {
-            origin: Origin::UserAgent,
-            namespaces: RwLock::new(namespaces),
-            url_data: RwLock::new(url),
-            quirks_mode: QuirksMode::NoQuirks,
-            rules: CssRules::new(
-                vec![
-                    CssRule::Namespace(Arc::new(stylesheet.shared_lock.wrap(NamespaceRule {
-                        prefix: None,
-                        url: NsAtom::from("http://www.w3.org/1999/xhtml"),
-                        source_location: SourceLocation {
-                            line: 1,
-                            column: 19,
-                        },
-                    }))),
-                    CssRule::Style(Arc::new(stylesheet.shared_lock.wrap(StyleRule {
-                        selectors: SelectorList::from_vec(vec![Selector::from_vec(
-                            vec![
-                                Component::DefaultNamespace(NsAtom::from(
-                                    "http://www.w3.org/1999/xhtml",
-                                )),
-                                Component::LocalName(LocalName {
-                                    name: local_name!("input"),
-                                    lower_name: local_name!("input"),
-                                }),
-                                Component::AttributeInNoNamespace {
-                                    local_name: local_name!("type"),
-                                    operator: AttrSelectorOperator::Equal,
-                                    value: "hidden".to_owned(),
-                                    case_sensitivity: ParsedCaseSensitivity::AsciiCaseInsensitive,
-                                    never_matches: false,
-                                },
-                            ],
-                            (0 << 20) + (1 << 10) + (1 << 0),
-                        )]),
-                        block: Arc::new(stylesheet.shared_lock.wrap(block_from(vec![
-                            (
-                                PropertyDeclaration::Display(
-                                    longhands::display::SpecifiedValue::None,
-                                ),
-                                Importance::Important,
-                            ),
-                            (
-                                PropertyDeclaration::Custom(CustomDeclaration {
-                                    name: Atom::from("a"),
-                                    value: CustomDeclarationValue::CSSWideKeyword(
-                                        CSSWideKeyword::Inherit,
-                                    ),
-                                }),
-                                Importance::Important,
-                            ),
-                        ]))),
-                        source_location: SourceLocation { line: 3, column: 9 },
-                    }))),
-                    CssRule::Style(Arc::new(stylesheet.shared_lock.wrap(StyleRule {
-                        selectors: SelectorList::from_vec(vec![
-                            Selector::from_vec(
-                                vec![
-                                    Component::DefaultNamespace(NsAtom::from(
-                                        "http://www.w3.org/1999/xhtml",
-                                    )),
-                                    Component::LocalName(LocalName {
-                                        name: local_name!("html"),
-                                        lower_name: local_name!("html"),
-                                    }),
-                                ],
-                                (0 << 20) + (0 << 10) + (1 << 0),
-                            ),
-                            Selector::from_vec(
-                                vec![
-                                    Component::DefaultNamespace(NsAtom::from(
-                                        "http://www.w3.org/1999/xhtml",
-                                    )),
-                                    Component::LocalName(LocalName {
-                                        name: local_name!("body"),
-                                        lower_name: local_name!("body"),
-                                    }),
-                                ],
-                                (0 << 20) + (0 << 10) + (1 << 0),
-                            ),
-                        ]),
-                        block: Arc::new(stylesheet.shared_lock.wrap(block_from(vec![(
-                            PropertyDeclaration::Display(longhands::display::SpecifiedValue::Block),
-                            Importance::Normal,
-                        )]))),
-                        source_location: SourceLocation {
-                            line: 11,
-                            column: 9,
-                        },
-                    }))),
-                    CssRule::Style(Arc::new(stylesheet.shared_lock.wrap(StyleRule {
-                        selectors: SelectorList::from_vec(vec![Selector::from_vec(
-                            vec![
-                                Component::DefaultNamespace(NsAtom::from(
-                                    "http://www.w3.org/1999/xhtml",
-                                )),
-                                Component::ID(Atom::from("d1")),
-                                Component::Combinator(Combinator::Child),
-                                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::Numeric {
-                                        authored: Some("blue".to_owned().into_boxed_str()),
-                                        parsed: cssparser::RGBA::new(0, 0, 255, 255),
-                                    },
-                                ),
-                                Importance::Normal,
-                            ),
-                            (
-                                PropertyDeclaration::BackgroundPositionX(
-                                    longhands::background_position_x::SpecifiedValue(vec![
-                                        PositionComponent::zero(),
-                                    ].into()),
-                                ),
-                                Importance::Normal,
-                            ),
-                            (
-                                PropertyDeclaration::BackgroundPositionY(
-                                    longhands::background_position_y::SpecifiedValue(vec![
-                                        PositionComponent::zero(),
-                                    ].into()),
-                                ),
-                                Importance::Normal,
-                            ),
-                            (
-                                PropertyDeclaration::BackgroundRepeat(
-                                    longhands::background_repeat::SpecifiedValue(
-                                        vec![longhands::background_repeat::single_value
-                                                       ::get_initial_specified_value()].into(),
-                                    ),
-                                ),
-                                Importance::Normal,
-                            ),
-                            (
-                                PropertyDeclaration::BackgroundAttachment(
-                                    longhands::background_attachment::SpecifiedValue(
-                                        vec![longhands::background_attachment::single_value
-                                                       ::get_initial_specified_value()].into(),
-                                    ),
-                                ),
-                                Importance::Normal,
-                            ),
-                            (
-                                PropertyDeclaration::BackgroundImage(
-                                    longhands::background_image::SpecifiedValue(
-                                        vec![longhands::background_image::single_value
-                                                       ::get_initial_specified_value()].into(),
-                                    ),
-                                ),
-                                Importance::Normal,
-                            ),
-                            (
-                                PropertyDeclaration::BackgroundSize(
-                                    longhands::background_size::SpecifiedValue(
-                                        vec![longhands::background_size::single_value
-                                                       ::get_initial_specified_value()].into(),
-                                    ),
-                                ),
-                                Importance::Normal,
-                            ),
-                            (
-                                PropertyDeclaration::BackgroundOrigin(
-                                    longhands::background_origin::SpecifiedValue(
-                                        vec![longhands::background_origin::single_value
-                                                       ::get_initial_specified_value()].into(),
-                                    ),
-                                ),
-                                Importance::Normal,
-                            ),
-                            (
-                                PropertyDeclaration::BackgroundClip(
-                                    longhands::background_clip::SpecifiedValue(
-                                        vec![longhands::background_clip::single_value
-                                                       ::get_initial_specified_value()].into(),
-                                    ),
-                                ),
-                                Importance::Normal,
-                            ),
-                        ]))),
-                        source_location: SourceLocation {
-                            line: 15,
-                            column: 9,
-                        },
-                    }))),
-                ],
-                &stylesheet.shared_lock,
-            ),
-            source_map_url: RwLock::new(None),
-            source_url: RwLock::new(None),
-        },
-        media: Arc::new(stylesheet.shared_lock.wrap(MediaList::empty())),
-        shared_lock: stylesheet.shared_lock.clone(),
-        disabled: AtomicBool::new(false),
-    };
-
-    assert_eq!(format!("{:#?}", stylesheet), format!("{:#?}", expected));
-}
+use style::stylesheets::Stylesheet;
+use style::stylesheets::Origin;
 
 #[derive(Debug)]
 struct CSSError {

From a7e441fc15f85bcbf65c25d22e4c15b90e4640c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sun, 23 Jun 2019 14:34:45 +0200
Subject: [PATCH 29/32] style: Servo doesn't implement white-space:
 break-spaces.

---
 .../properties/longhands/inherited_text.mako.rs     | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/components/style/properties/longhands/inherited_text.mako.rs b/components/style/properties/longhands/inherited_text.mako.rs
index 3efd8123bd1..fc934d901eb 100644
--- a/components/style/properties/longhands/inherited_text.mako.rs
+++ b/components/style/properties/longhands/inherited_text.mako.rs
@@ -175,8 +175,8 @@ ${helpers.predefined_type(
 
 <%helpers:single_keyword
     name="white-space"
-    values="normal pre nowrap pre-wrap pre-line break-spaces"
-    extra_gecko_values="-moz-pre-space"
+    values="normal pre nowrap pre-wrap pre-line"
+    extra_gecko_values="break-spaces -moz-pre-space"
     gecko_enum_prefix="StyleWhiteSpace"
     needs_conversion="True"
     animation_value_type="discrete"
@@ -193,8 +193,7 @@ ${helpers.predefined_type(
                 SpecifiedValue::Pre => false,
                 SpecifiedValue::Normal |
                 SpecifiedValue::PreWrap |
-                SpecifiedValue::PreLine |
-                SpecifiedValue::BreakSpaces => true,
+                SpecifiedValue::PreLine => true,
             }
         }
 
@@ -204,8 +203,7 @@ ${helpers.predefined_type(
                 SpecifiedValue::Nowrap => false,
                 SpecifiedValue::Pre |
                 SpecifiedValue::PreWrap |
-                SpecifiedValue::PreLine |
-                SpecifiedValue::BreakSpaces => true,
+                SpecifiedValue::PreLine => true,
             }
         }
 
@@ -215,8 +213,7 @@ ${helpers.predefined_type(
                 SpecifiedValue::Nowrap |
                 SpecifiedValue::PreLine => false,
                 SpecifiedValue::Pre |
-                SpecifiedValue::PreWrap |
-                SpecifiedValue::BreakSpaces => true,
+                SpecifiedValue::PreWrap => true,
             }
         }
     }

From 15f7daf5f52bcf4c1505f03691cb0e241328f511 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sun, 23 Jun 2019 18:58:02 +0200
Subject: [PATCH 30/32] Update test expectations.

---
 tests/wpt/mozilla/tests/mozilla/calc.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/wpt/mozilla/tests/mozilla/calc.html b/tests/wpt/mozilla/tests/mozilla/calc.html
index 2408f196c00..80aa06e2ae7 100644
--- a/tests/wpt/mozilla/tests/mozilla/calc.html
+++ b/tests/wpt/mozilla/tests/mozilla/calc.html
@@ -144,7 +144,7 @@ var otherProperties = [
   ['border-spacing', 'calc(1px)', 'calc(1px)'],
   ['transform-origin', 'calc(1px + 0%)', 'calc(0% + 1px) center 0px'],
   ['perspective-origin', 'calc(1px + 0%)', 'calc(0% + 1px) center'],
-  ['background-size', 'calc(1px + 0%)', 'calc(0% + 1px) auto'],
+  ['background-size', 'calc(1px + 0%)', 'calc(0% + 1px)'],
   ['background-position', 'calc(1px + 0%) calc(2px + 0%)', 'calc(0% + 1px) calc(0% + 2px)'],
   ['border-top-left-radius', 'calc(1px + 0%)', 'calc(0% + 1px)'],
   ['border-bottom-left-radius', 'calc(1px + 0%)', 'calc(0% + 1px)'],

From dd65470dd964362c7edbe99c61303285fe06a692 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Mon, 24 Jun 2019 16:21:30 +0200
Subject: [PATCH 31/32] style: Rustfmt again.

---
 components/script/dom/element.rs | 5 ++++-
 tests/unit/style/stylesheets.rs  | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 5695e355368..1145cb0fd7e 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -683,7 +683,10 @@ impl LayoutElementHelpers for LayoutDom<Element> {
             hints.push(from_declaration(
                 shared_lock,
                 PropertyDeclaration::BackgroundImage(background_image::SpecifiedValue(
-                    vec![specified::ImageLayer::Image(specified::Image::for_cascade(url.into()))].into(),
+                    vec![specified::ImageLayer::Image(specified::Image::for_cascade(
+                        url.into(),
+                    ))]
+                    .into(),
                 )),
             ));
         }
diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs
index d1bc0e8113f..d346a42b4cd 100644
--- a/tests/unit/style/stylesheets.rs
+++ b/tests/unit/style/stylesheets.rs
@@ -11,8 +11,8 @@ use style::context::QuirksMode;
 use style::error_reporting::{ContextualParseError, ParseErrorReporter};
 use style::media_queries::MediaList;
 use style::shared_lock::SharedRwLock;
-use style::stylesheets::Stylesheet;
 use style::stylesheets::Origin;
+use style::stylesheets::Stylesheet;
 
 #[derive(Debug)]
 struct CSSError {

From d1640f43bd38c4bb3c7e57cf7af00e3a4acb5c32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Mon, 24 Jun 2019 17:36:21 +0200
Subject: [PATCH 32/32] Update WPT expectations.

---
 .../background-size-001.html.ini              | 19 ++-----------------
 .../parsing/background-size-computed.html.ini |  7 -------
 tests/wpt/mozilla/meta/MANIFEST.json          |  2 +-
 3 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/tests/wpt/metadata/css/css-backgrounds/background-size-001.html.ini b/tests/wpt/metadata/css/css-backgrounds/background-size-001.html.ini
index 071ab97fece..340eb82e0d2 100644
--- a/tests/wpt/metadata/css/css-backgrounds/background-size-001.html.ini
+++ b/tests/wpt/metadata/css/css-backgrounds/background-size-001.html.ini
@@ -1,24 +1,9 @@
 [background-size-001.html]
   type: testharness
 
-  [background-size_length_zero]
+  [background-size_length_auto]
     expected: FAIL
 
-  [background-size_length_negative_zero]
-    expected: FAIL
-
-  [background-size_length_positive_zero]
-    expected: FAIL
-
-  [background-size_length_normal]
-    expected: FAIL
-
-  [background-size_percentage_min]
-    expected: FAIL
-
-  [background-size_percentage_normal]
-    expected: FAIL
-
-  [background-size_percentage_max]
+  [background-size_percentage_auto]
     expected: FAIL
 
diff --git a/tests/wpt/metadata/css/css-backgrounds/parsing/background-size-computed.html.ini b/tests/wpt/metadata/css/css-backgrounds/parsing/background-size-computed.html.ini
index 08aa7a3f969..ede3103de7a 100644
--- a/tests/wpt/metadata/css/css-backgrounds/parsing/background-size-computed.html.ini
+++ b/tests/wpt/metadata/css/css-backgrounds/parsing/background-size-computed.html.ini
@@ -1,10 +1,3 @@
 [background-size-computed.html]
-  [Property background-size value '1px' computes to '1px']
-    expected: FAIL
-
   [Property background-size value 'auto 1px, 2% 3%, contain' computes to 'auto 1px']
     expected: FAIL
-
-  [Property background-size value '1px auto' computes to '1px']
-    expected: FAIL
-
diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json
index dd1cb8bd996..c32bb3cf8d4 100644
--- a/tests/wpt/mozilla/meta/MANIFEST.json
+++ b/tests/wpt/mozilla/meta/MANIFEST.json
@@ -18431,7 +18431,7 @@
    "testharness"
   ],
   "mozilla/calc.html": [
-   "2408f196c000a5d0f05cb35db4c8607486810351",
+   "80aa06e2ae7cd5db585873f147a21382b279b86e",
    "testharness"
   ],
   "mozilla/canvas.initial.reset.2dstate.html": [