Bug 1473180 part 2 - Add new algorithm for setting property to be used in later commit. r?emilio draft
authorXidorn Quan <me@upsuper.org>
Fri, 13 Jul 2018 21:14:15 +1000
changeset 819500 0c8cbf0f371c2598d67f92d36972534cf77c5e43
parent 819499 8831c486a9db5137251ba15c19be5d1e7673774a
child 819501 eae10aa7d84d476ec1c85c57cd2ed433c23f671f
child 820138 08a40cf9746a83fceb124dd148d02ccb0d2e4864
push id116568
push userxquan@mozilla.com
push dateWed, 18 Jul 2018 00:24:45 +0000
reviewersemilio
bugs1473180
milestone63.0a1
Bug 1473180 part 2 - Add new algorithm for setting property to be used in later commit. r?emilio MozReview-Commit-ID: CdM8hDB6rFj
servo/components/style/properties/declaration_block.rs
servo/components/style/properties/properties.mako.rs
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -6,16 +6,17 @@
 
 #![deny(missing_docs)]
 
 use context::QuirksMode;
 use cssparser::{DeclarationListParser, parse_important, ParserInput, CowRcStr};
 use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter, ParseErrorKind};
 use custom_properties::CustomPropertiesBuilder;
 use error_reporting::{ParseErrorReporter, ContextualParseError};
+use itertools::Itertools;
 use parser::ParserContext;
 use properties::animated_properties::{AnimationValue, AnimationValueMap};
 use shared_lock::Locked;
 use smallbitvec::{self, SmallBitVec};
 use smallvec::SmallVec;
 use std::fmt::{self, Write};
 use std::iter::{DoubleEndedIterator, Zip};
 use std::slice::Iter;
@@ -34,16 +35,40 @@ pub struct AnimationRules(pub Option<Arc
 
 impl AnimationRules {
     /// Returns whether these animation rules represents an actual rule or not.
     pub fn is_empty(&self) -> bool {
         self.0.is_none() && self.1.is_none()
     }
 }
 
+/// An enum describes how a declaration should update
+/// the declaration block.
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+enum DeclarationUpdate {
+    /// The given declaration doesn't update anything.
+    None,
+    /// The given declaration is new, and should be append directly.
+    Append,
+    /// The given declaration can be updated in-place at the given position.
+    UpdateInPlace { pos: usize },
+    /// The given declaration cannot be updated in-place, and an existing
+    /// one needs to be removed at the given position.
+    AppendAndRemove { pos: usize },
+}
+
+/// A struct describes how a declaration block should be updated by
+/// a `SourcePropertyDeclaration`.
+#[derive(Default)]
+pub struct SourcePropertyDeclarationUpdate {
+    updates: SubpropertiesVec<DeclarationUpdate>,
+    new_count: usize,
+    any_removal: bool,
+}
+
 /// 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,
@@ -418,20 +443,17 @@ 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 {
-        match decl.id() {
-            PropertyDeclarationId::Longhand(id) => !self.longhands.contains(id),
-            PropertyDeclarationId::Custom(..) => false,
-        }
+        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,
@@ -574,16 +596,193 @@ impl PropertyDeclarationBlock {
         if let PropertyDeclarationId::Longhand(id) = declaration.id() {
             self.longhands.insert(id);
         }
         self.declarations.push(declaration);
         self.declarations_importance.push(importance.important());
         true
     }
 
+    /// Prepares updating this declaration block with the given
+    /// `SourcePropertyDeclaration` and importance, and returns whether
+    /// there is something to update.
+    pub fn prepare_for_update(
+        &self,
+        source_declarations: &SourcePropertyDeclaration,
+        importance: Importance,
+        updates: &mut SourcePropertyDeclarationUpdate,
+    ) -> bool {
+        debug_assert!(updates.updates.is_empty());
+        // Check whether we are updating for an all shorthand change.
+        if !matches!(source_declarations.all_shorthand, AllShorthand::NotSet) {
+            debug_assert!(source_declarations.declarations.is_empty());
+            return source_declarations.all_shorthand.declarations().any(|decl| {
+                self.is_definitely_new(&decl) ||
+                self.declarations.iter().enumerate()
+                    .find(|&(_, ref d)| d.id() == decl.id())
+                    .map_or(true, |(i, d)| {
+                        let important = self.declarations_importance[i];
+                        *d != decl || important != importance.important()
+                    })
+            });
+        }
+        // Fill `updates` with update information.
+        let mut any_update = false;
+        let new_count = &mut updates.new_count;
+        let any_removal = &mut updates.any_removal;
+        let updates = &mut updates.updates;
+        updates.extend(source_declarations.declarations.iter().map(|declaration| {
+            if self.is_definitely_new(declaration) {
+                return DeclarationUpdate::Append;
+            }
+            let longhand_id = declaration.id().as_longhand();
+            if let Some(longhand_id) = longhand_id {
+                if let Some(logical_group) = longhand_id.logical_group() {
+                    let mut needs_append = false;
+                    for (pos, decl) in self.declarations.iter().enumerate().rev() {
+                        let id = match decl.id().as_longhand() {
+                            Some(id) => id,
+                            None => continue,
+                        };
+                        if id == longhand_id {
+                            if needs_append {
+                                return DeclarationUpdate::AppendAndRemove { pos };
+                            }
+                            let important = self.declarations_importance[pos];
+                            if decl == declaration && important == importance.important() {
+                                return DeclarationUpdate::None;
+                            }
+                            return DeclarationUpdate::UpdateInPlace { pos };
+                        }
+                        if !needs_append &&
+                           id.logical_group() == Some(logical_group) &&
+                           id.is_logical() != longhand_id.is_logical() {
+                            needs_append = true;
+                        }
+                    }
+                    unreachable!("Longhand should be found in loop above");
+                }
+            }
+            self.declarations.iter().enumerate()
+                .find(|&(_, ref decl)| decl.id() == declaration.id())
+                .map_or(DeclarationUpdate::Append, |(pos, decl)| {
+                    let important = self.declarations_importance[pos];
+                    if decl == declaration && important == importance.important() {
+                        DeclarationUpdate::None
+                    } else {
+                        DeclarationUpdate::UpdateInPlace { pos }
+                    }
+                })
+        }).inspect(|update| {
+            if matches!(update, DeclarationUpdate::None) {
+                return;
+            }
+            any_update = true;
+            match update {
+                DeclarationUpdate::Append => {
+                    *new_count += 1;
+                }
+                DeclarationUpdate::AppendAndRemove { .. } => {
+                    *any_removal = true;
+                }
+                _ => {}
+            }
+        }));
+        any_update
+    }
+
+    /// Update this declaration block with the given data.
+    pub fn update(
+        &mut self,
+        drain: SourcePropertyDeclarationDrain,
+        importance: Importance,
+        updates: &mut SourcePropertyDeclarationUpdate,
+    ) {
+        let important = importance.important();
+        if !matches!(drain.all_shorthand, AllShorthand::NotSet) {
+            debug_assert!(updates.updates.is_empty());
+            for decl in drain.all_shorthand.declarations() {
+                if self.is_definitely_new(&decl) {
+                    let longhand_id = decl.id().as_longhand().unwrap();
+                    self.declarations.push(decl);
+                    self.declarations_importance.push(important);
+                    self.longhands.insert(longhand_id);
+                } else {
+                    let (idx, slot) = self.declarations.iter_mut()
+                        .enumerate().find(|&(_, ref d)| d.id() == decl.id()).unwrap();
+                    *slot = decl;
+                    self.declarations_importance.set(idx, important);
+                }
+            }
+            return;
+        }
+
+        self.declarations.reserve(updates.new_count);
+        if updates.any_removal {
+            // Prepare for removal and fixing update positions.
+            struct UpdateOrRemoval<'a> {
+                item: &'a mut DeclarationUpdate,
+                pos: usize,
+                remove: bool,
+            }
+            let mut updates_and_removals: SubpropertiesVec<UpdateOrRemoval> =
+                updates.updates.iter_mut().filter_map(|item| {
+                    let (pos, remove) = match *item {
+                        DeclarationUpdate::UpdateInPlace { pos } => (pos, false),
+                        DeclarationUpdate::AppendAndRemove { pos } => (pos, true),
+                        _ => return None,
+                    };
+                    Some(UpdateOrRemoval { item, pos, remove })
+                }).collect();
+            // Execute removals. It's important to do it in reverse index order,
+            // so that removing doesn't invalidate following positions.
+            updates_and_removals.sort_unstable_by_key(|update| update.pos);
+            updates_and_removals.iter().rev()
+                .filter(|update| update.remove)
+                .for_each(|update| {
+                    self.declarations.remove(update.pos);
+                    self.declarations_importance.remove(update.pos);
+                });
+            // Fixup pos field for updates.
+            let mut removed_count = 0;
+            for update in updates_and_removals.iter_mut() {
+                if update.remove {
+                    removed_count += 1;
+                    continue;
+                }
+                debug_assert_eq!(
+                    *update.item,
+                    DeclarationUpdate::UpdateInPlace { pos: update.pos }
+                );
+                *update.item = DeclarationUpdate::UpdateInPlace {
+                    pos: update.pos - removed_count
+                };
+            }
+        }
+        // Execute updates and appends.
+        for (decl, update) in drain.declarations.zip_eq(updates.updates.iter()) {
+            match *update {
+                DeclarationUpdate::None => {},
+                DeclarationUpdate::Append |
+                DeclarationUpdate::AppendAndRemove { .. } => {
+                    if let Some(id) = decl.id().as_longhand() {
+                        self.longhands.insert(id);
+                    }
+                    self.declarations.push(decl);
+                    self.declarations_importance.push(important);
+                }
+                DeclarationUpdate::UpdateInPlace { pos } => {
+                    self.declarations[pos] = decl;
+                    self.declarations_importance.set(pos, important);
+                }
+            }
+        }
+        updates.updates.clear();
+    }
+
     /// Returns the first declaration that would be removed by removing
     /// `property`.
     #[inline]
     pub fn first_declaration_to_remove(
         &self,
         property: &PropertyId,
     ) -> Option<usize> {
         if let Some(id) = property.longhand_id() {
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -7,16 +7,17 @@
 // Please note that valid Rust syntax may be mangled by the Mako parser.
 // For example, Vec<&Foo> will be mangled as Vec&Foo>. To work around these issues, the code
 // can be escaped. In the above example, Vec<<&Foo> or Vec< &Foo> achieves the desired result of Vec<&Foo>.
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
 #[cfg(feature = "servo")]
 use app_units::Au;
+use arrayvec::{ArrayVec, Drain as ArrayVecDrain};
 use dom::TElement;
 use custom_properties::CustomPropertiesBuilder;
 use servo_arc::{Arc, UniqueArc};
 use smallbitvec::SmallBitVec;
 use std::borrow::Cow;
 use std::{ops, ptr};
 use std::cell::RefCell;
 use std::fmt::{self, Write};
@@ -1600,16 +1601,25 @@ impl<'a> PropertyDeclarationId<'a> {
             PropertyDeclarationId::Longhand(id) => id.name().into(),
             PropertyDeclarationId::Custom(name) => {
                 let mut s = String::new();
                 write!(&mut s, "--{}", name).unwrap();
                 s.into()
             }
         }
     }
+
+    /// Returns longhand id if it is, None otherwise.
+    #[inline]
+    pub fn as_longhand(&self) -> Option<LonghandId> {
+        match *self {
+            PropertyDeclarationId::Longhand(id) => Some(id),
+            _ => None,
+        }
+    }
 }
 
 /// Servo's representation of a CSS property, that is, either a longhand, a
 /// shorthand, or a custom property.
 #[derive(Clone, Eq, PartialEq)]
 pub enum PropertyId {
     /// A longhand property.
     Longhand(LonghandId),
@@ -2199,29 +2209,28 @@ impl PropertyDeclaration {
                         }
                     })
                 }
             }
         }
     }
 }
 
-const MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL: usize =
-    ${max(len(s.sub_properties) for s in data.shorthands_except_all())};
-
-type SourcePropertyDeclarationArray =
-    [PropertyDeclaration; MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL];
+type SubpropertiesArray<T> =
+    [T; ${max(len(s.sub_properties) for s in data.shorthands_except_all())}];
+
+type SubpropertiesVec<T> = ArrayVec<SubpropertiesArray<T>>;
 
 /// A stack-allocated vector of `PropertyDeclaration`
 /// large enough to parse one CSS `key: value` declaration.
 /// (Shorthands expand to multiple `PropertyDeclaration`s.)
 pub struct SourcePropertyDeclaration {
-    declarations: ::arrayvec::ArrayVec<SourcePropertyDeclarationArray>,
-
-    /// Stored separately to keep MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL smaller.
+    declarations: SubpropertiesVec<PropertyDeclaration>,
+
+    /// Stored separately to keep SubpropertiesVec smaller.
     all_shorthand: AllShorthand,
 }
 
 impl SourcePropertyDeclaration {
     /// Create one. It’s big, try not to move it around.
     #[inline]
     pub fn new() -> Self {
         SourcePropertyDeclaration {
@@ -2251,17 +2260,17 @@ impl SourcePropertyDeclaration {
     fn push(&mut self, declaration: PropertyDeclaration) {
         let _result = self.declarations.try_push(declaration);
         debug_assert!(_result.is_ok());
     }
 }
 
 /// Return type of SourcePropertyDeclaration::drain
 pub struct SourcePropertyDeclarationDrain<'a> {
-    declarations: ::arrayvec::Drain<'a, SourcePropertyDeclarationArray>,
+    declarations: ArrayVecDrain<'a, SubpropertiesArray<PropertyDeclaration>>,
     all_shorthand: AllShorthand,
 }
 
 enum AllShorthand {
     NotSet,
     CSSWideKeyword(CSSWideKeyword),
     WithVariables(Arc<UnparsedValue>)
 }