Bug 1473180 part 4 - Remove DeclarationPushMode::Update and related code. r?emilio draft
authorXidorn Quan <me@upsuper.org>
Mon, 16 Jul 2018 22:25:11 +1000
changeset 820139 05ce3e0531dcbe9da89d1a2ae4efe629cddc73d0
parent 820138 08a40cf9746a83fceb124dd148d02ccb0d2e4864
child 820140 4d6292b1edd2ab39dd0002f35de46a7120283dcf
push id116733
push userxquan@mozilla.com
push dateWed, 18 Jul 2018 23:58:21 +0000
reviewersemilio
bugs1473180
milestone63.0a1
Bug 1473180 part 4 - Remove DeclarationPushMode::Update and related code. r?emilio MozReview-Commit-ID: 1fI5YRa54lQ
servo/components/style/properties/declaration_block.rs
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -67,20 +67,16 @@ pub struct SourcePropertyDeclarationUpda
 /// Enum for how a given declaration should be pushed into a declaration block.
 #[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq)]
 pub enum DeclarationPushMode {
     /// Mode used when declarations were obtained from CSS parsing.
     /// If there is an existing declaration of the same property with a higher
     /// importance, the new declaration will be discarded. Otherwise, it will
     /// be appended to the end of the declaration block.
     Parsing,
-    /// In this mode, if there is an existing declaration of the same property,
-    /// the value is updated in-place. Otherwise it's appended. This is one
-    /// possible behavior of CSSOM.
-    Update,
     /// In this mode, the new declaration is always pushed to the end of the
     /// declaration block. This is another possible behavior of CSSOM.
     Append,
 }
 
 /// A declaration [importance][importance].
 ///
 /// [importance]: https://drafts.csswg.org/css-cascade/#importance
@@ -446,48 +442,20 @@ impl PropertyDeclarationBlock {
     /// Returns whether the property is definitely new for this declaration
     /// block. It returns true when the declaration is a non-custom longhand
     /// and it doesn't exist in the block, and returns false otherwise.
     #[inline]
     fn is_definitely_new(&self, decl: &PropertyDeclaration) -> bool {
         decl.id().as_longhand().map_or(false, |id| !self.longhands.contains(id))
     }
 
-    /// Returns whether calling extend with `DeclarationPushMode::Update`
-    /// will cause this declaration block to change.
-    pub fn will_change_in_update_mode(
-        &self,
-        source_declarations: &SourcePropertyDeclaration,
-        importance: Importance,
-    ) -> bool {
-        // XXX The type of parameter seems to be necessary because otherwise
-        // the compiler complains about `decl` not living long enough in the
-        // all_shorthand expression. Why?
-        let needs_update = |decl: &_| {
-            if self.is_definitely_new(decl) {
-                return true;
-            }
-            self.declarations.iter().enumerate()
-                .find(|&(_, ref slot)| slot.id() == decl.id())
-                .map_or(true, |(i, slot)| {
-                    let important = self.declarations_importance[i];
-                    *slot != *decl || important != importance.important()
-                })
-        };
-        source_declarations.declarations.iter().any(&needs_update) ||
-            source_declarations.all_shorthand.declarations().any(|decl| needs_update(&decl))
-    }
-
     /// Adds or overrides the declaration for a given property in this block.
     ///
     /// See the documentation of `push` to see what impact `source` has when the
     /// property is already there.
-    ///
-    /// When calling with `DeclarationPushMode::Update`, this should not change
-    /// anything if `will_change_in_update_mode` returns false.
     pub fn extend(
         &mut self,
         mut drain: SourcePropertyDeclarationDrain,
         importance: Importance,
         mode: DeclarationPushMode,
     ) -> bool {
         match mode {
             DeclarationPushMode::Parsing => {
@@ -565,25 +533,16 @@ impl PropertyDeclarationBlock {
 
                         if let PropertyDeclaration::Display(new_display) = declaration {
                             if display::should_ignore_parsed_value(old_display, new_display) {
                                 return false;
                             }
                         }
                     }
                 }
-                if matches!(mode, DeclarationPushMode::Update) {
-                    let important = self.declarations_importance[i];
-                    if *slot == declaration && important == importance.important() {
-                        return false;
-                    }
-                    *slot = declaration;
-                    self.declarations_importance.set(i, importance.important());
-                    return true;
-                }
 
                 index_to_remove = Some(i);
                 break;
             }
 
             if let Some(index) = index_to_remove {
                 self.declarations.remove(index);
                 self.declarations_importance.remove(index);