style: Implement CSS revert keyword.

The only fishy bit is the animation stuff. In particular, there are two places
where we just mint the revert behavior:

 * When serializing web-animations keyframes (the custom properties stuff in
   declaration_block.rs). That codepath is already not sound and I wanted to
   get rid of it in bug 1501530, but what do I know.

 * When getting an animation value from a property declaration. At that point
   we no longer have the CSS rules that apply to the element to compute the
   right revert value handy. It'd also use the wrong style anyway, I think,
   given the way StyleBuilder::for_animation works.

   We _could_ probably get them out of somewhere, but it seems like a whole lot
   of code reinventing the wheel which is probably not useful, and that Blink
   and WebKit just cannot implement either since they don't have a rule tree,
   so it just doesn't seem worth the churn.

The custom properties code looks a bit different in order to minimize hash
lookups in the common case. FWIW, `revert` for custom properties doesn't seem
very useful either, but oh well.

Differential Revision: https://phabricator.services.mozilla.com/D21877

9 files changed, 119 insertions, 20 deletions

diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs
index f21929c7079..93cd3488f53 100644
--- a/components/style/custom_properties.rs
+++ b/components/style/custom_properties.rs
@@ -7,8 +7,9 @@
 //! [custom]: https://drafts.csswg.org/css-variables/
 
 use crate::hash::map::Entry;
-use crate::properties::{CSSWideKeyword, CustomDeclarationValue};
+use crate::properties::{CSSWideKeyword, CustomDeclaration, CustomDeclarationValue};
 use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, PrecomputedHasher};
+use crate::stylesheets::{Origin, PerOrigin};
 use crate::Atom;
 use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType};
 use indexmap::IndexMap;
@@ -490,6 +491,7 @@ fn parse_env_function<'i, 't>(
 /// properties.
 pub struct CustomPropertiesBuilder<'a> {
     seen: PrecomputedHashSet<&'a Name>,
+    reverted: PerOrigin<PrecomputedHashSet<&'a Name>>,
     may_have_cycles: bool,
     custom_properties: Option<CustomPropertiesMap>,
     inherited: Option<&'a Arc<CustomPropertiesMap>>,
@@ -504,6 +506,7 @@ impl<'a> CustomPropertiesBuilder<'a> {
     ) -> Self {
         Self {
             seen: PrecomputedHashSet::default(),
+            reverted: Default::default(),
             may_have_cycles: false,
             custom_properties: None,
             inherited,
@@ -512,13 +515,26 @@ impl<'a> CustomPropertiesBuilder<'a> {
     }
 
     /// Cascade a given custom property declaration.
-    pub fn cascade(&mut self, name: &'a Name, specified_value: &CustomDeclarationValue) {
+    pub fn cascade(
+        &mut self,
+        declaration: &'a CustomDeclaration,
+        origin: Origin,
+    ) {
+        let CustomDeclaration {
+            ref name,
+            ref value,
+        } = *declaration;
+
+        if self.reverted.borrow_for_origin(&origin).contains(&name) {
+            return;
+        }
+
         let was_already_present = !self.seen.insert(name);
         if was_already_present {
             return;
         }
 
-        if !self.value_may_affect_style(name, specified_value) {
+        if !self.value_may_affect_style(name, value) {
             return;
         }
 
@@ -530,7 +546,7 @@ impl<'a> CustomPropertiesBuilder<'a> {
         }
 
         let map = self.custom_properties.as_mut().unwrap();
-        match *specified_value {
+        match *value {
             CustomDeclarationValue::Value(ref unparsed_value) => {
                 let has_references = !unparsed_value.references.is_empty();
                 self.may_have_cycles |= has_references;
@@ -554,6 +570,12 @@ impl<'a> CustomPropertiesBuilder<'a> {
                 map.insert(name.clone(), value);
             },
             CustomDeclarationValue::CSSWideKeyword(keyword) => match keyword {
+                CSSWideKeyword::Revert => {
+                    self.seen.remove(name);
+                    for origin in origin.following_including() {
+                        self.reverted.borrow_mut_for_origin(&origin).insert(name);
+                    }
+                },
                 CSSWideKeyword::Initial => {
                     map.remove(name);
                 },
@@ -587,10 +609,10 @@ impl<'a> CustomPropertiesBuilder<'a> {
                 // not existing in the map.
                 return false;
             },
-            (Some(existing_value), &CustomDeclarationValue::Value(ref specified_value)) => {
+            (Some(existing_value), &CustomDeclarationValue::Value(ref value)) => {
                 // Don't bother overwriting an existing inherited value with
                 // the same specified value.
-                if existing_value == specified_value {
+                if existing_value == value {
                     return false;
                 }
             },
diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs
index 4a0e07fdcac..09b055b07cd 100644
--- a/components/style/properties/cascade.rs
+++ b/components/style/properties/cascade.rs
@@ -17,6 +17,7 @@ use crate::properties::CASCADE_PROPERTY;
 use crate::rule_cache::{RuleCache, RuleCacheConditions};
 use crate::rule_tree::{CascadeLevel, StrongRuleNode};
 use crate::selector_parser::PseudoElement;
+use crate::stylesheets::{Origin, PerOrigin};
 use servo_arc::Arc;
 use crate::shared_lock::StylesheetGuards;
 use smallbitvec::SmallBitVec;
@@ -251,7 +252,7 @@ where
         for (declaration, cascade_level) in iter_declarations() {
             declarations.push((declaration, cascade_level));
             if let PropertyDeclaration::Custom(ref declaration) = *declaration {
-                builder.cascade(&declaration.name, &declaration.value);
+                builder.cascade(declaration, cascade_level.origin());
             }
         }
 
@@ -339,14 +340,8 @@ fn should_ignore_declaration_when_ignoring_document_colors(
         return false;
     }
 
-    let is_ua_or_user_rule = matches!(
-        cascade_level,
-        CascadeLevel::UANormal |
-            CascadeLevel::UserNormal |
-            CascadeLevel::UserImportant |
-            CascadeLevel::UAImportant
-    );
-
+    let is_ua_or_user_rule =
+        matches!(cascade_level.origin(), Origin::User | Origin::UserAgent);
     if is_ua_or_user_rule {
         return false;
     }
@@ -388,6 +383,7 @@ struct Cascade<'a, 'b: 'a> {
     context: &'a mut computed::Context<'b>,
     cascade_mode: CascadeMode<'a>,
     seen: LonghandIdSet,
+    reverted: PerOrigin<LonghandIdSet>,
     saved_font_size: Option<PropertyDeclaration>,
     saved_font_family: Option<PropertyDeclaration>,
 }
@@ -398,6 +394,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
             context,
             cascade_mode,
             seen: LonghandIdSet::default(),
+            reverted: Default::default(),
             saved_font_size: None,
             saved_font_family: None,
         }
@@ -488,6 +485,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
 
         for (declaration, cascade_level) in declarations {
             let declaration_id = declaration.id();
+            let origin = cascade_level.origin();
             let longhand_id = match declaration_id {
                 PropertyDeclarationId::Longhand(id) => id,
                 PropertyDeclarationId::Custom(..) => continue,
@@ -513,6 +511,10 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 continue;
             }
 
+            if self.reverted.borrow_for_origin(&origin).contains(physical_longhand_id) {
+                continue;
+            }
+
             // Only a few properties are allowed to depend on the visited state
             // of links.  When cascading visited styles, we can save time by
             // only processing these properties.
@@ -540,6 +542,15 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 }
             }
 
+            if declaration.is_revert() {
+                for origin in origin.following_including() {
+                    self.reverted
+                        .borrow_mut_for_origin(&origin)
+                        .insert(physical_longhand_id);
+                }
+                continue;
+            }
+
             self.seen.insert(physical_longhand_id);
 
             // FIXME(emilio): We should avoid generating code for logical
diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs
index d7058d99635..e4396e764a4 100644
--- a/components/style/properties/declaration_block.rs
+++ b/components/style/properties/declaration_block.rs
@@ -849,7 +849,7 @@ impl PropertyDeclarationBlock {
 
         for declaration in self.normal_declaration_iter() {
             if let PropertyDeclaration::Custom(ref declaration) = *declaration {
-                builder.cascade(&declaration.name, &declaration.value);
+                builder.cascade(declaration, Origin::Author);
             }
         }
 
diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs
index 1129a36908d..334ea5de09e 100644
--- a/components/style/properties/helpers.mako.rs
+++ b/components/style/properties/helpers.mako.rs
@@ -349,6 +349,7 @@
                                 context.builder.inherit_${property.ident}();
                             % endif
                         }
+                        CSSWideKeyword::Revert => unreachable!("Should never get here"),
                     }
                     return;
                 }
diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs
index 5669101351d..aa2d0247e82 100644
--- a/components/style/properties/helpers/animated_properties.mako.rs
+++ b/components/style/properties/helpers/animated_properties.mako.rs
@@ -452,14 +452,23 @@ impl AnimationValue {
                     % for prop in data.longhands:
                     % if prop.animatable:
                     LonghandId::${prop.camel_case} => {
+                        // FIXME(emilio, bug 1533327): I think
+                        // CSSWideKeyword::Revert handling is not fine here, but
+                        // what to do instead?
+                        //
+                        // Seems we'd need the computed value as if it was
+                        // revert, somehow. Treating it as `unset` seems fine
+                        // for now...
                         let style_struct = match declaration.keyword {
                             % if not prop.style_struct.inherited:
+                            CSSWideKeyword::Revert |
                             CSSWideKeyword::Unset |
                             % endif
                             CSSWideKeyword::Initial => {
                                 initial.get_${prop.style_struct.name_lower}()
                             },
                             % if prop.style_struct.inherited:
+                            CSSWideKeyword::Revert |
                             CSSWideKeyword::Unset |
                             % endif
                             CSSWideKeyword::Inherit => {
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index ec859a42ea7..7bc10b785c6 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -900,6 +900,8 @@ pub enum CSSWideKeyword {
     Inherit,
     /// The `unset` keyword.
     Unset,
+    /// The `revert` keyword.
+    Revert,
 }
 
 impl CSSWideKeyword {
@@ -908,6 +910,7 @@ impl CSSWideKeyword {
             CSSWideKeyword::Initial => "initial",
             CSSWideKeyword::Inherit => "inherit",
             CSSWideKeyword::Unset => "unset",
+            CSSWideKeyword::Revert => "revert",
         }
     }
 }
@@ -921,6 +924,7 @@ impl CSSWideKeyword {
                 "initial" => CSSWideKeyword::Initial,
                 "inherit" => CSSWideKeyword::Inherit,
                 "unset" => CSSWideKeyword::Unset,
+                "revert" => CSSWideKeyword::Revert,
                 _ => return Err(()),
             }
         };
@@ -2103,6 +2107,7 @@ impl PropertyDeclaration {
     }
 
     /// Returns a CSS-wide keyword if the declaration's value is one.
+    #[inline]
     pub fn get_css_wide_keyword(&self) -> Option<CSSWideKeyword> {
         match *self {
             PropertyDeclaration::CSSWideKeyword(ref declaration) => {
@@ -2112,6 +2117,12 @@ impl PropertyDeclaration {
         }
     }
 
+    /// Whether this declaration is the `revert` keyword.
+    #[inline]
+    pub fn is_revert(&self) -> bool {
+        self.get_css_wide_keyword().map_or(false, |kw| kw == CSSWideKeyword::Revert)
+    }
+
     /// Returns whether or not the property is set by a system font
     pub fn get_system(&self) -> Option<SystemFont> {
         match *self {
diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs
index a2caba6f9f7..029e3d068f9 100644
--- a/components/style/rule_tree/mod.rs
+++ b/components/style/rule_tree/mod.rs
@@ -11,7 +11,7 @@ use crate::applicable_declarations::ApplicableDeclarationList;
 use crate::gecko::selector_parser::PseudoElement;
 use crate::properties::{Importance, LonghandIdSet, PropertyDeclarationBlock};
 use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
-use crate::stylesheets::StyleRule;
+use crate::stylesheets::{StyleRule, Origin};
 use crate::thread_state;
 #[cfg(feature = "gecko")]
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
@@ -659,6 +659,27 @@ impl CascadeLevel {
         }
     }
 
+    /// Returns the cascade origin of the rule.
+    #[inline]
+    pub fn origin(&self) -> Origin {
+        match *self {
+            CascadeLevel::UAImportant |
+            CascadeLevel::UANormal => Origin::UserAgent,
+            CascadeLevel::UserImportant |
+            CascadeLevel::UserNormal => Origin::User,
+            CascadeLevel::PresHints |
+            CascadeLevel::InnerShadowNormal |
+            CascadeLevel::SameTreeAuthorNormal |
+            CascadeLevel::StyleAttributeNormal |
+            CascadeLevel::SMILOverride |
+            CascadeLevel::Animations |
+            CascadeLevel::SameTreeAuthorImportant |
+            CascadeLevel::StyleAttributeImportant |
+            CascadeLevel::InnerShadowImportant |
+            CascadeLevel::Transitions => Origin::Author,
+        }
+    }
+
     /// Returns whether this cascade level represents an animation rules.
     #[inline]
     pub fn is_animation(&self) -> bool {
diff --git a/components/style/stylesheets/origin.rs b/components/style/stylesheets/origin.rs
index 69b9b866c86..4e4c304baeb 100644
--- a/components/style/stylesheets/origin.rs
+++ b/components/style/stylesheets/origin.rs
@@ -36,6 +36,25 @@ impl Origin {
             _ => return None,
         })
     }
+
+    fn to_index(self) -> i8 {
+        match self {
+            Origin::Author => 0,
+            Origin::User => 1,
+            Origin::UserAgent => 2,
+        }
+    }
+
+    /// Returns an iterator from this origin, towards all the less specific
+    /// origins. So for `UserAgent`, it'd iterate through all origins.
+    #[inline]
+    pub fn following_including(self) -> OriginSetIterator {
+        OriginSetIterator {
+            set: OriginSet::ORIGIN_USER | OriginSet::ORIGIN_AUTHOR | OriginSet::ORIGIN_USER_AGENT,
+            cur: self.to_index(),
+            rev: true,
+        }
+    }
 }
 
 bitflags! {
@@ -57,7 +76,7 @@ impl OriginSet {
     /// See the `OriginSet` documentation for information about the order
     /// origins are iterated.
     pub fn iter(&self) -> OriginSetIterator {
-        OriginSetIterator { set: *self, cur: 0 }
+        OriginSetIterator { set: *self, cur: 0, rev: false }
     }
 }
 
@@ -79,6 +98,7 @@ impl BitOrAssign<Origin> for OriginSet {
 pub struct OriginSetIterator {
     set: OriginSet,
     cur: i8,
+    rev: bool,
 }
 
 impl Iterator for OriginSetIterator {
@@ -88,7 +108,11 @@ impl Iterator for OriginSetIterator {
         loop {
             let origin = Origin::from_index(self.cur)?;
 
-            self.cur += 1;
+            if self.rev {
+                self.cur -= 1;
+            } else {
+                self.cur += 1;
+            }
 
             if self.set.contains(origin.into()) {
                 return Some(origin);
diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs
index ccda0f290c1..e29b8923af7 100644
--- a/components/style/values/mod.rs
+++ b/components/style/values/mod.rs
@@ -160,7 +160,7 @@ impl CustomIdent {
         excluding: &[&str],
     ) -> Result<Self, ParseError<'i>> {
         let valid = match_ignore_ascii_case! { ident,
-            "initial" | "inherit" | "unset" | "default" => false,
+            "initial" | "inherit" | "unset" | "default" | "revert" => false,
             _ => true
         };
         if !valid {