servo: Merge #15745 - Cascade: skip duplicated properties before rather than after a virtual call (from servo:hoist)
authorSimon Sapin <simon.sapin@exyr.org>
Sun, 26 Feb 2017 23:09:44 +0100
changeset 374008 11e1a0367ea6403acca57560a27eabbeef8037fb
parent 374007 c7374c1837bc225a4171694e9ab4e907e5e66863
child 374009 873d249fb35cd7d666ca7400ef44d5706d5b1353
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
milestone54.0a1
servo: Merge #15745 - Cascade: skip duplicated properties before rather than after a virtual call (from servo:hoist) Source-Repo: https://github.com/servo/servo Source-Revision: 7c5ac06cd2339e74fdc732ab63675e0f2d6ef5e0
servo/components/script/dom/cssstyledeclaration.rs
servo/components/style/keyframes.rs
servo/components/style/properties/declaration_block.rs
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/helpers.mako.rs
servo/components/style/properties/longhand/box.mako.rs
servo/components/style/properties/longhand/font.mako.rs
servo/components/style/properties/longhand/text.mako.rs
servo/components/style/properties/properties.mako.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/script/dom/cssstyledeclaration.rs
+++ b/servo/components/script/dom/cssstyledeclaration.rs
@@ -213,17 +213,17 @@ impl CSSStyleDeclaration {
     fn get_property_value(&self, id: PropertyId) -> DOMString {
         if self.readonly {
             // Readonly style declarations are used for getComputedStyle.
             return self.get_computed_style(id);
         }
 
         let mut string = String::new();
 
-        self.owner.with_block(|ref pdb| {
+        self.owner.with_block(|pdb| {
             pdb.property_value_to_css(&id, &mut string).unwrap();
         });
 
         DOMString::from(string)
     }
 
     fn set_property(&self, id: PropertyId, value: DOMString, priority: DOMString) -> ErrorResult {
         // Step 1
@@ -276,17 +276,17 @@ impl CSSStyleDeclaration {
             Ok(())
         })
     }
 }
 
 impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-length
     fn Length(&self) -> u32 {
-        self.owner.with_block(|ref pdb| {
+        self.owner.with_block(|pdb| {
             pdb.declarations.len() as u32
         })
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-item
     fn Item(&self, index: u32) -> DOMString {
         self.IndexedGetter(index).unwrap_or_default()
     }
@@ -306,17 +306,17 @@ impl CSSStyleDeclarationMethods for CSSS
     fn GetPropertyPriority(&self, property: DOMString) -> DOMString {
         let id = if let Ok(id) = PropertyId::parse(property.into()) {
             id
         } else {
             // Unkwown property
             return DOMString::new()
         };
 
-        self.owner.with_block(|ref pdb| {
+        self.owner.with_block(|pdb| {
             if pdb.property_priority(&id).important() {
                 DOMString::from("important")
             } else {
                 // Step 4
                 DOMString::new()
             }
         })
     }
@@ -401,31 +401,31 @@ impl CSSStyleDeclarationMethods for CSSS
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat
     fn SetCssFloat(&self, value: DOMString) -> ErrorResult {
         self.SetPropertyValue(DOMString::from("float"), value)
     }
 
     // https://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
     fn IndexedGetter(&self, index: u32) -> Option<DOMString> {
-        self.owner.with_block(|ref pdb| {
+        self.owner.with_block(|pdb| {
             pdb.declarations.get(index as usize).map(|entry| {
                 let (ref declaration, importance) = *entry;
                 let mut css = declaration.to_css_string();
                 if importance.important() {
                     css += " !important";
                 }
                 DOMString::from(css)
             })
         })
     }
 
     // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext
     fn CssText(&self) -> DOMString {
-        self.owner.with_block(|ref pdb| {
+        self.owner.with_block(|pdb| {
             DOMString::from(pdb.to_css_string())
         })
     }
 
     // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext
     fn SetCssText(&self, value: DOMString) -> ErrorResult {
         let window = self.owner.window();
 
--- a/servo/components/style/keyframes.rs
+++ b/servo/components/style/keyframes.rs
@@ -7,20 +7,20 @@
 #![deny(missing_docs)]
 
 use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser};
 use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule};
 use parking_lot::RwLock;
 use parser::{ParserContext, ParserContextExtraData, log_css_error};
 use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock, PropertyId};
 use properties::{PropertyDeclarationId, LonghandId, DeclaredValue};
+use properties::LonghandIdSet;
 use properties::PropertyDeclarationParseResult;
 use properties::animated_properties::TransitionProperty;
 use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction;
-use properties::property_bit_field::PropertyBitField;
 use std::fmt;
 use std::sync::Arc;
 use style_traits::ToCss;
 use stylesheets::{MemoryHoleReporter, Stylesheet};
 
 /// A number from 0 to 1, indicating the percentage of the animation when this
 /// keyframe should run.
 #[derive(Debug, Copy, Clone, PartialEq, PartialOrd)]
@@ -243,17 +243,17 @@ pub struct KeyframesAnimation {
     pub steps: Vec<KeyframesStep>,
     /// The properties that change in this animation.
     pub properties_changed: Vec<TransitionProperty>,
 }
 
 /// Get all the animated properties in a keyframes animation.
 fn get_animated_properties(keyframes: &[Arc<RwLock<Keyframe>>]) -> Vec<TransitionProperty> {
     let mut ret = vec![];
-    let mut seen = PropertyBitField::new();
+    let mut seen = LonghandIdSet::new();
     // NB: declarations are already deduplicated, so we don't have to check for
     // it here.
     for keyframe in keyframes {
         let keyframe = keyframe.read();
         for &(ref declaration, importance) in keyframe.block.read().declarations.iter() {
             assert!(!importance.important());
 
             if let Some(property) = TransitionProperty::from_declaration(declaration) {
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -269,16 +269,49 @@ impl PropertyDeclarationBlock {
                     Some(AppendableValue::DeclarationsForShorthand(_, decls)) => {
                         shorthand.longhands_to_css(decls, dest)
                     }
                     _ => Ok(())
                 }
             }
         }
     }
+
+    /// Only keep the "winning" declaration for any given property, by importance then source order.
+    pub fn deduplicate(&mut self) {
+        let mut deduplicated = Vec::with_capacity(self.declarations.len());
+        let mut seen_normal = PropertyDeclarationIdSet::new();
+        let mut seen_important = PropertyDeclarationIdSet::new();
+
+        for (declaration, importance) in self.declarations.drain(..).rev() {
+            if importance.important() {
+                let id = declaration.id();
+                if seen_important.contains(id) {
+                    self.important_count -= 1;
+                    continue
+                }
+                if seen_normal.contains(id) {
+                    let previous_len = deduplicated.len();
+                    deduplicated.retain(|&(ref d, _)| PropertyDeclaration::id(d) != id);
+                    debug_assert_eq!(deduplicated.len(), previous_len - 1);
+                }
+                seen_important.insert(id);
+            } else {
+                let id = declaration.id();
+                if seen_normal.contains(id) ||
+                   seen_important.contains(id) {
+                    continue
+                }
+                seen_normal.insert(id)
+            }
+            deduplicated.push((declaration, importance))
+        }
+        deduplicated.reverse();
+        self.declarations = deduplicated;
+    }
 }
 
 impl ToCss for PropertyDeclarationBlock {
     // https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result
         where W: fmt::Write,
     {
         let mut is_first_serialization = true; // trailing serializations should have a prepended space
@@ -607,11 +640,11 @@ pub fn parse_property_declaration_list(c
                 log_css_error(iter.input, pos, &*message, &context);
             }
         }
     }
     let mut block = PropertyDeclarationBlock {
         declarations: iter.parser.declarations,
         important_count: important_count,
     };
-    super::deduplicate_property_declarations(&mut block);
+    block.deduplicate();
     block
 }
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -154,20 +154,16 @@ impl ComputedValues {
         self.custom_properties.as_ref().map(|x| x.clone())
     }
 
     #[allow(non_snake_case)]
     pub fn has_moz_binding(&self) -> bool {
         !self.get_box().gecko.mBinding.mRawPtr.is_null()
     }
 
-    pub fn root_font_size(&self) -> Au { self.root_font_size }
-    pub fn set_root_font_size(&mut self, s: Au) { self.root_font_size = s; }
-    pub fn set_writing_mode(&mut self, mode: WritingMode) { self.writing_mode = mode; }
-
     // FIXME(bholley): Implement this properly.
     #[inline]
     pub fn is_multicol(&self) -> bool { false }
 
     pub fn to_declaration_block(&self, property: PropertyDeclarationId) -> PropertyDeclarationBlock {
         match property {
             % for prop in data.longhands:
                 % if prop.animatable:
@@ -1242,19 +1238,16 @@ fn static_assert() {
 
     pub fn clone_font_weight(&self) -> longhands::font_weight::computed_value::T {
         debug_assert!(self.gecko.mFont.weight >= 100);
         debug_assert!(self.gecko.mFont.weight <= 900);
         debug_assert!(self.gecko.mFont.weight % 10 == 0);
         unsafe { transmute(self.gecko.mFont.weight) }
     }
 
-    // This is used for PartialEq, which we don't implement for gecko style structs.
-    pub fn compute_font_hash(&mut self) {}
-
     pub fn set_font_synthesis(&mut self, v: longhands::font_synthesis::computed_value::T) {
         use gecko_bindings::structs::{NS_FONT_SYNTHESIS_WEIGHT, NS_FONT_SYNTHESIS_STYLE};
 
         self.gecko.mFont.synthesis = 0;
         if v.weight {
             self.gecko.mFont.synthesis |= NS_FONT_SYNTHESIS_WEIGHT as u8;
         }
         if v.style {
--- a/servo/components/style/properties/helpers.mako.rs
+++ b/servo/components/style/properties/helpers.mako.rs
@@ -225,61 +225,54 @@
             use cssparser::Parser;
             use parser::{Parse, ParserContext, ParserContextExtraData};
             use properties::{CSSWideKeyword, DeclaredValue, UnparsedValue, ShorthandId};
         % endif
         use values::{Auto, Either, None_, Normal};
         use cascade_info::CascadeInfo;
         use error_reporting::ParseErrorReporter;
         use properties::longhands;
-        use properties::property_bit_field::PropertyBitField;
+        use properties::LonghandIdSet;
         use properties::{ComputedValues, PropertyDeclaration};
         use properties::style_structs;
         use std::boxed::Box as StdBox;
         use std::collections::HashMap;
         use std::sync::Arc;
         use values::computed::{Context, ToComputedValue};
         use values::{computed, specified};
         use Atom;
         ${caller.body()}
         #[allow(unused_variables)]
         pub fn cascade_property(declaration: &PropertyDeclaration,
                                 inherited_style: &ComputedValues,
                                 default_style: &Arc<ComputedValues>,
                                 context: &mut computed::Context,
-                                seen: &mut PropertyBitField,
                                 cacheable: &mut bool,
                                 cascade_info: &mut Option<<&mut CascadeInfo>,
                                 error_reporter: &mut StdBox<ParseErrorReporter + Send>) {
             let declared_value = match *declaration {
                 PropertyDeclaration::${property.camel_case}(ref declared_value) => {
                     declared_value
                 }
                 _ => panic!("entered the wrong cascade_property() implementation"),
             };
 
-            % if property.logical:
-                let wm = context.style.writing_mode;
-            % endif
-            <% maybe_wm = "wm" if property.logical else "" %>
-            <% maybe_physical = "_physical" if property.logical else "" %>
             % if not property.derived_from:
-                if seen.get${maybe_physical}_${property.ident}(${maybe_wm}) {
-                    return
-                }
-                seen.set${maybe_physical}_${property.ident}(${maybe_wm});
                 {
                     let custom_props = context.style().custom_properties();
                     ::properties::substitute_variables_${property.ident}(
                         declared_value, &custom_props,
                     |value| {
                         if let Some(ref mut cascade_info) = *cascade_info {
                             cascade_info.on_cascade_property(&declaration,
                                                              &value);
                         }
+                        % if property.logical:
+                            let wm = context.style.writing_mode;
+                        % endif
                         <% maybe_wm = ", wm" if property.logical else "" %>
                         match *value {
                             DeclaredValue::Value(ref specified_value) => {
                                 let computed = specified_value.to_computed_value(context);
                                 % if property.has_uncacheable_values:
                                 context.mutate_style().mutate_${data.current_style_struct.name_lower}()
                                                       .set_${property.ident}(computed, cacheable ${maybe_wm});
                                 % else:
@@ -316,17 +309,16 @@
                         }
                     }, error_reporter);
                 }
 
                 % if property.custom_cascade:
                     cascade_property_custom(declaration,
                                             inherited_style,
                                             context,
-                                            seen,
                                             cacheable,
                                             error_reporter);
                 % endif
             % else:
                 // Do not allow stylesheets to set derived properties.
             % endif
         }
         % if not property.derived_from:
--- a/servo/components/style/properties/longhand/box.mako.rs
+++ b/servo/components/style/properties/longhand/box.mako.rs
@@ -79,17 +79,16 @@
     }
 
     impl ComputedValueAsSpecified for SpecifiedValue {}
 
     % if product == "servo":
         fn cascade_property_custom(_declaration: &PropertyDeclaration,
                                    _inherited_style: &ComputedValues,
                                    context: &mut computed::Context,
-                                   _seen: &mut PropertyBitField,
                                    _cacheable: &mut bool,
                                    _error_reporter: &mut StdBox<ParseErrorReporter + Send>) {
             longhands::_servo_display_for_hypothetical_box::derive_from_display(context);
             longhands::_servo_text_decorations_in_effect::derive_from_display(context);
             longhands::_servo_under_display_none::derive_from_display(context);
         }
     % endif
 
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -1,18 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 <% from data import Method %>
 
 <% data.new_style_struct("Font",
-                         inherited=True,
-                         additional_methods=[Method("compute_font_hash", is_mut=True)]) %>
+                         inherited=True) %>
 <%helpers:longhand name="font-family" animatable="False" need_index="True"
                    spec="https://drafts.csswg.org/css-fonts/#propdef-font-family">
     use self::computed_value::{FontFamily, FamilyName};
     use values::HasViewportPercentage;
     use values::computed::ComputedValueAsSpecified;
     pub use self::computed_value::T as SpecifiedValue;
 
     impl ComputedValueAsSpecified for SpecifiedValue {}
--- a/servo/components/style/properties/longhand/text.mako.rs
+++ b/servo/components/style/properties/longhand/text.mako.rs
@@ -199,17 +199,16 @@
 
         if !empty { Ok(result) } else { Err(()) }
     }
 
     % if product == "servo":
         fn cascade_property_custom(_declaration: &PropertyDeclaration,
                                    _inherited_style: &ComputedValues,
                                    context: &mut computed::Context,
-                                   _seen: &mut PropertyBitField,
                                    _cacheable: &mut bool,
                                    _error_reporter: &mut StdBox<ParseErrorReporter + Send>) {
                 longhands::_servo_text_decorations_in_effect::derive_from_text_decoration(context);
         }
     % endif
 </%helpers:longhand>
 
 ${helpers.single_keyword("text-decoration-style",
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -24,27 +24,27 @@ use error_reporting::ParseErrorReporter;
 use euclid::size::Size2D;
 use computed_values;
 use font_metrics::FontMetricsProvider;
 #[cfg(feature = "gecko")] use gecko_bindings::bindings;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::{self, nsCSSPropertyID};
 #[cfg(feature = "servo")] use logical_geometry::{LogicalMargin, PhysicalSide};
 use logical_geometry::WritingMode;
 use parser::{Parse, ParserContext, ParserContextExtraData};
+use properties::animated_properties::TransitionProperty;
 #[cfg(feature = "servo")] use servo_config::prefs::PREFS;
 use servo_url::ServoUrl;
 use style_traits::ToCss;
 use stylesheets::Origin;
 #[cfg(feature = "servo")] use values::Either;
 use values::{HasViewportPercentage, computed};
 use cascade_info::CascadeInfo;
 use rule_tree::StrongRuleNode;
 #[cfg(feature = "servo")] use values::specified::BorderStyle;
 
-use self::property_bit_field::PropertyBitField;
 pub use self::declaration_block::*;
 
 #[cfg(feature = "gecko")]
 #[macro_export]
 macro_rules! property_name {
     ($s: tt) => { atom!($s) }
 }
 
@@ -173,102 +173,102 @@ pub mod shorthands {
 /// A module with all the code related to animated properties.
 ///
 /// This needs to be "included" by mako at least after all longhand modules,
 /// given they populate the global data.
 pub mod animated_properties {
     <%include file="/helpers/animated_properties.mako.rs" />
 }
 
+/// A set of longhand properties
+pub struct LonghandIdSet {
+    storage: [u32; (${len(data.longhands)} - 1 + 32) / 32]
+}
 
-// TODO(SimonSapin): Convert this to a syntax extension rather than a Mako template.
-// Maybe submit for inclusion in libstd?
-#[allow(missing_docs)]
-pub mod property_bit_field {
-    use logical_geometry::WritingMode;
-    use properties::animated_properties::TransitionProperty;
+impl LonghandIdSet {
+    /// Create an empty set
+    #[inline]
+    pub fn new() -> LonghandIdSet {
+        LonghandIdSet { storage: [0; (${len(data.longhands)} - 1 + 32) / 32] }
+    }
+
+    /// Return whether the given property is in the set
+    #[inline]
+    pub fn contains(&self, id: LonghandId) -> bool {
+        let bit = id as usize;
+        (self.storage[bit / 32] & (1 << (bit % 32))) != 0
+    }
 
-    /// A bitfield for all longhand properties, in order to quickly test whether
-    /// we've seen one of them.
-    pub struct PropertyBitField {
-        storage: [u32; (${len(data.longhands)} - 1 + 32) / 32]
+    /// Add the given property to the set
+    #[inline]
+    pub fn insert(&mut self, id: LonghandId) {
+        let bit = id as usize;
+        self.storage[bit / 32] |= 1 << (bit % 32);
+    }
+
+    /// Set the corresponding bit of TransitionProperty.
+    /// This function will panic if TransitionProperty::All is given.
+    pub fn set_transition_property_bit(&mut self, property: &TransitionProperty) {
+        match *property {
+            % for prop in data.longhands:
+                % if prop.animatable:
+                    TransitionProperty::${prop.camel_case} => self.insert(LonghandId::${prop.camel_case}),
+                % endif
+            % endfor
+            TransitionProperty::All => unreachable!("Tried to set TransitionProperty::All in a PropertyBitfield"),
+        }
     }
 
-    impl PropertyBitField {
-        /// Create a new `PropertyBitField`, with all the bits set to zero.
-        #[inline]
-        pub fn new() -> PropertyBitField {
-            PropertyBitField { storage: [0; (${len(data.longhands)} - 1 + 32) / 32] }
+    /// Return true if the corresponding bit of TransitionProperty is set.
+    /// This function will panic if TransitionProperty::All is given.
+    pub fn has_transition_property_bit(&self, property: &TransitionProperty) -> bool {
+        match *property {
+            % for prop in data.longhands:
+                % if prop.animatable:
+                    TransitionProperty::${prop.camel_case} => self.contains(LonghandId::${prop.camel_case}),
+                % endif
+            % endfor
+            TransitionProperty::All => unreachable!("Tried to get TransitionProperty::All in a PropertyBitfield"),
         }
+    }
+}
 
-        #[inline]
-        fn get(&self, bit: usize) -> bool {
-            (self.storage[bit / 32] & (1 << (bit % 32))) != 0
-        }
+/// A specialized set of PropertyDeclarationId
+pub struct PropertyDeclarationIdSet {
+    longhands: LonghandIdSet,
+
+    // FIXME: Use a HashSet instead? This Vec is usually small, so linear scan might be ok.
+    custom: Vec<::custom_properties::Name>,
+}
 
-        #[inline]
-        fn set(&mut self, bit: usize) {
-            self.storage[bit / 32] |= 1 << (bit % 32)
+impl PropertyDeclarationIdSet {
+    /// Empty set
+    pub fn new() -> Self {
+        PropertyDeclarationIdSet {
+            longhands: LonghandIdSet::new(),
+            custom: Vec::new(),
         }
-        % for i, property in enumerate(data.longhands):
-            % if not property.derived_from:
-                #[allow(non_snake_case, missing_docs)]
-                #[inline]
-                pub fn get_${property.ident}(&self) -> bool {
-                    self.get(${i})
-                }
-                #[allow(non_snake_case, missing_docs)]
-                #[inline]
-                pub fn set_${property.ident}(&mut self) {
-                    self.set(${i})
+    }
+
+    /// Returns whether the given ID is in the set
+    pub fn contains(&mut self, id: PropertyDeclarationId) -> bool {
+        match id {
+            PropertyDeclarationId::Longhand(id) => self.longhands.contains(id),
+            PropertyDeclarationId::Custom(name) => self.custom.contains(name),
+        }
+    }
+
+    /// Insert the given ID in the set
+    pub fn insert(&mut self, id: PropertyDeclarationId) {
+        match id {
+            PropertyDeclarationId::Longhand(id) => self.longhands.insert(id),
+            PropertyDeclarationId::Custom(name) => {
+                if !self.custom.contains(name) {
+                    self.custom.push(name.clone())
                 }
-            % endif
-            % if property.logical:
-                #[allow(non_snake_case, missing_docs)]
-                pub fn get_physical_${property.ident}(&self, wm: WritingMode) -> bool {
-                    <%helpers:logical_setter_helper name="${property.name}">
-                        <%def name="inner(physical_ident)">
-                            self.get_${physical_ident}()
-                        </%def>
-                    </%helpers:logical_setter_helper>
-                }
-                #[allow(non_snake_case, missing_docs)]
-                pub fn set_physical_${property.ident}(&mut self, wm: WritingMode) {
-                    <%helpers:logical_setter_helper name="${property.name}">
-                        <%def name="inner(physical_ident)">
-                            self.set_${physical_ident}()
-                        </%def>
-                    </%helpers:logical_setter_helper>
-                }
-            % endif
-        % endfor
-
-        /// Set the corresponding bit of TransitionProperty.
-        /// This function will panic if TransitionProperty::All is given.
-        pub fn set_transition_property_bit(&mut self, property: &TransitionProperty) {
-            match *property {
-                % for i, prop in enumerate(data.longhands):
-                    % if prop.animatable:
-                        TransitionProperty::${prop.camel_case} => self.set(${i}),
-                    % endif
-                % endfor
-                TransitionProperty::All => unreachable!("Tried to set TransitionProperty::All in a PropertyBitfield"),
-            }
-        }
-
-        /// Return true if the corresponding bit of TransitionProperty is set.
-        /// This function will panic if TransitionProperty::All is given.
-        pub fn has_transition_property_bit(&self, property: &TransitionProperty) -> bool {
-            match *property {
-                % for i, prop in enumerate(data.longhands):
-                    % if prop.animatable:
-                        TransitionProperty::${prop.camel_case} => self.get(${i}),
-                    % endif
-                % endfor
-                TransitionProperty::All => unreachable!("Tried to get TransitionProperty::All in a PropertyBitfield"),
             }
         }
     }
 }
 
 % for property in data.longhands:
     % if not property.derived_from:
         /// Perform CSS variable substitution if needed, and execute `f` with
@@ -362,89 +362,16 @@ pub mod property_bit_field {
                     // Invalid at computed-value time.
                     DeclaredValue::${"Inherit" if property.style_struct.inherited else "Initial"}
                 )
             );
         }
     % endif
 % endfor
 
-/// Given a property declaration block, only keep the "winning" declaration for
-/// any given property, by importance then source order.
-///
-/// The input and output are in source order
-fn deduplicate_property_declarations(block: &mut PropertyDeclarationBlock) {
-    let mut deduplicated = Vec::with_capacity(block.declarations.len());
-    let mut seen_normal = PropertyBitField::new();
-    let mut seen_important = PropertyBitField::new();
-    let mut seen_custom_normal = Vec::new();
-    let mut seen_custom_important = Vec::new();
-
-    for (declaration, importance) in block.declarations.drain(..).rev() {
-        match declaration {
-            % for property in data.longhands:
-                PropertyDeclaration::${property.camel_case}(..) => {
-                    % if not property.derived_from:
-                        if importance.important() {
-                            if seen_important.get_${property.ident}() {
-                                block.important_count -= 1;
-                                continue
-                            }
-                            if seen_normal.get_${property.ident}() {
-                                remove_one(&mut deduplicated, |d| {
-                                    matches!(d, &(PropertyDeclaration::${property.camel_case}(..), _))
-                                });
-                            }
-                            seen_important.set_${property.ident}()
-                        } else {
-                            if seen_normal.get_${property.ident}() ||
-                               seen_important.get_${property.ident}() {
-                                continue
-                            }
-                            seen_normal.set_${property.ident}()
-                        }
-                    % else:
-                        unreachable!();
-                    % endif
-                },
-            % endfor
-            PropertyDeclaration::Custom(ref name, _) => {
-                if importance.important() {
-                    if seen_custom_important.contains(name) {
-                        block.important_count -= 1;
-                        continue
-                    }
-                    if seen_custom_normal.contains(name) {
-                        remove_one(&mut deduplicated, |d| {
-                            matches!(d, &(PropertyDeclaration::Custom(ref n, _), _) if n == name)
-                        });
-                    }
-                    seen_custom_important.push(name.clone())
-                } else {
-                    if seen_custom_normal.contains(name) ||
-                       seen_custom_important.contains(name) {
-                        continue
-                    }
-                    seen_custom_normal.push(name.clone())
-                }
-            }
-        }
-        deduplicated.push((declaration, importance))
-    }
-    deduplicated.reverse();
-    block.declarations = deduplicated;
-}
-
-#[inline]
-fn remove_one<T, F: FnMut(&T) -> bool>(v: &mut Vec<T>, mut remove_this: F) {
-    let previous_len = v.len();
-    v.retain(|x| !remove_this(x));
-    debug_assert_eq!(v.len(), previous_len - 1);
-}
-
 /// An enum to represent a CSS Wide keyword.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum CSSWideKeyword {
     /// The `initial` keyword.
     InitialKeyword,
     /// The `inherit` keyword.
     InheritKeyword,
     /// The `unset` keyword.
@@ -478,16 +405,35 @@ impl LonghandId {
     /// Get the name of this longhand property.
     pub fn name(&self) -> &'static str {
         match *self {
             % for property in data.longhands:
                 LonghandId::${property.camel_case} => "${property.name}",
             % endfor
         }
     }
+
+    /// If this is a logical property, return the corresponding physical one in the given writing mode.
+    /// Otherwise, return unchanged.
+    pub fn to_physical(&self, wm: WritingMode) -> Self {
+        match *self {
+            % for property in data.longhands:
+                % if property.logical:
+                    LonghandId::${property.camel_case} => {
+                        <%helpers:logical_setter_helper name="${property.name}">
+                            <%def name="inner(physical_ident)">
+                                LonghandId::${to_camel_case(physical_ident)}
+                            </%def>
+                        </%helpers:logical_setter_helper>
+                    }
+                % endif
+            % endfor
+            _ => *self
+        }
+    }
 }
 
 /// An identifier for a given shorthand property.
 #[derive(Clone, Copy, Eq, PartialEq, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum ShorthandId {
     % for property in data.shorthands:
         /// ${property.name}
@@ -648,17 +594,17 @@ impl<T: ToCss> ToCss for DeclaredValue<T
             DeclaredValue::Inherit => dest.write_str("inherit"),
             DeclaredValue::Unset => dest.write_str("unset"),
         }
     }
 }
 
 /// An identifier for a given property declaration, which can be either a
 /// longhand or a custom property.
-#[derive(PartialEq, Clone)]
+#[derive(PartialEq, Clone, Copy)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum PropertyDeclarationId<'a> {
     /// A longhand.
     Longhand(LonghandId),
     /// A custom property declaration.
     Custom(&'a ::custom_properties::Name),
 }
 
@@ -1433,24 +1379,16 @@ impl ComputedValues {
     /// Servo for obvious reasons.
     pub fn has_moz_binding(&self) -> bool { false }
 
     /// Returns whether this style's display value is equal to contents.
     ///
     /// Since this isn't supported in Servo, this is always false for Servo.
     pub fn is_display_contents(&self) -> bool { false }
 
-    /// Get the root font size.
-    fn root_font_size(&self) -> Au { self.root_font_size }
-
-    /// Set the root font size.
-    fn set_root_font_size(&mut self, size: Au) { self.root_font_size = size }
-    /// Set the writing mode for this style.
-    pub fn set_writing_mode(&mut self, mode: WritingMode) { self.writing_mode = mode; }
-
     /// Whether the current style is multicolumn.
     #[inline]
     pub fn is_multicol(&self) -> bool {
         let style = self.get_column();
         match style.column_width {
             Either::First(_width) => true,
             Either::Second(_auto) => style.column_count.0.is_some(),
         }
@@ -1725,17 +1663,16 @@ mod lazy_static_module {
 }
 
 /// A per-longhand function that performs the CSS cascade for that longhand.
 pub type CascadePropertyFn =
     extern "Rust" fn(declaration: &PropertyDeclaration,
                      inherited_style: &ComputedValues,
                      default_style: &Arc<ComputedValues>,
                      context: &mut computed::Context,
-                     seen: &mut PropertyBitField,
                      cacheable: &mut bool,
                      cascade_info: &mut Option<<&mut CascadeInfo>,
                      error_reporter: &mut StdBox<ParseErrorReporter + Send>);
 
 /// A per-longhand array of functions to perform the CSS cascade on each of
 /// them, effectively doing virtual dispatch.
 static CASCADE_PROPERTY: [CascadePropertyFn; ${len(data.longhands)}] = [
     % for property in data.longhands:
@@ -1850,30 +1787,30 @@ pub fn apply_declarations<'a, F, I>(view
     let custom_properties =
         ::custom_properties::finish_cascade(
             custom_properties, &inherited_custom_properties);
 
     let starting_style = if !flags.contains(INHERIT_ALL) {
         ComputedValues::new(custom_properties,
                             flags.contains(SHAREABLE),
                             WritingMode::empty(),
-                            inherited_style.root_font_size(),
+                            inherited_style.root_font_size,
                             % for style_struct in data.active_style_structs():
                                 % if style_struct.inherited:
                                     inherited_style.clone_${style_struct.name_lower}(),
                                 % else:
                                     default_style.clone_${style_struct.name_lower}(),
                                 % endif
                             % endfor
                             )
     } else {
         ComputedValues::new(custom_properties,
                             flags.contains(SHAREABLE),
                             WritingMode::empty(),
-                            inherited_style.root_font_size(),
+                            inherited_style.root_font_size,
                             % for style_struct in data.active_style_structs():
                                 inherited_style.clone_${style_struct.name_lower}(),
                             % endfor
                             )
     };
 
     let mut context = computed::Context {
         is_root_element: is_root_element,
@@ -1885,17 +1822,17 @@ pub fn apply_declarations<'a, F, I>(view
     };
 
     // Set computed values, overwriting earlier declarations for the same
     // property.
     //
     // NB: The cacheable boolean is not used right now, but will be once we
     // start caching computed values in the rule nodes.
     let mut cacheable = true;
-    let mut seen = PropertyBitField::new();
+    let mut seen = LonghandIdSet::new();
 
     // Declaration blocks are stored in increasing precedence order, we want
     // them in decreasing order here.
     //
     // We could (and used to) use a pattern match here, but that bloats this
     // function to over 100K of compiled code!
     //
     // To improve i-cache behavior, we outline the individual functions and use
@@ -1935,29 +1872,35 @@ pub fn apply_declarations<'a, F, I>(view
                 % if category_to_cascade_now == "early":
                     !
                 % endif
                 is_early_property
             {
                 continue
             }
 
+            <% maybe_to_physical = ".to_physical(writing_mode)" if category_to_cascade_now != "early" else "" %>
+            let physical_longhand_id = longhand_id ${maybe_to_physical};
+            if seen.contains(physical_longhand_id) {
+                continue
+            }
+            seen.insert(physical_longhand_id);
+
             let discriminant = longhand_id as usize;
             (CASCADE_PROPERTY[discriminant])(declaration,
                                              inherited_style,
                                              default_style,
                                              &mut context,
-                                             &mut seen,
                                              &mut cacheable,
                                              &mut cascade_info,
                                              &mut error_reporter);
         }
         % if category_to_cascade_now == "early":
-            let mode = get_writing_mode(context.style.get_inheritedbox());
-            context.style.set_writing_mode(mode);
+            let writing_mode = get_writing_mode(context.style.get_inheritedbox());
+            context.style.writing_mode = writing_mode;
         % endif
     % endfor
 
     let mut style = context.style;
 
     let positioned = matches!(style.get_box().clone_position(),
         longhands::position::SpecifiedValue::absolute |
         longhands::position::SpecifiedValue::fixed);
@@ -2085,23 +2028,27 @@ pub fn apply_declarations<'a, F, I>(view
     // The initial value of outline width may be changed at computed value time.
     if style.get_outline().clone_outline_style().none_or_hidden() &&
        style.get_outline().outline_has_nonzero_width() {
         style.mutate_outline().set_outline_width(Au(0));
     }
 
     if is_root_element {
         let s = style.get_font().clone_font_size();
-        style.set_root_font_size(s);
+        style.root_font_size = s;
     }
 
-    if seen.get_font_style() || seen.get_font_weight() || seen.get_font_stretch() ||
-       seen.get_font_family() {
-        style.mutate_font().compute_font_hash();
-    }
+    % if product == "servo":
+        if seen.contains(LonghandId::FontStyle) ||
+           seen.contains(LonghandId::FontWeight) ||
+           seen.contains(LonghandId::FontStretch) ||
+           seen.contains(LonghandId::FontFamily) {
+            style.mutate_font().compute_font_hash();
+        }
+    % endif
 
     style
 }
 
 /// Modifies the style for an anonymous flow so it resets all its non-inherited
 /// style structs, and set their borders and outlines to zero.
 ///
 /// Also, it gets a new display value, which is honored except when it's
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -1333,18 +1333,18 @@ pub extern "C" fn Servo_ResolveStyleLazi
 
 #[no_mangle]
 pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeListBorrowed,
                                                   style: ServoComputedValuesBorrowed,
                                                   parent_style: ServoComputedValuesBorrowedOrNull,
                                                   pres_context: RawGeckoPresContextBorrowed,
                                                   computed_keyframes: RawGeckoComputedKeyframeValuesListBorrowedMut)
 {
+    use style::properties::LonghandIdSet;
     use style::properties::declaration_block::Importance;
-    use style::properties::property_bit_field::PropertyBitField;
     use style::values::computed::Context;
 
     let style = ComputedValues::as_arc(&style);
     let parent_style = parent_style.as_ref().map(|r| &**ComputedValues::as_arc(&r));
     let init = ComputedValues::default_values(pres_context);
 
     let context = Context {
         is_root_element: false,
@@ -1354,17 +1354,17 @@ pub extern "C" fn Servo_GetComputedKeyfr
         layout_parent_style: parent_style.unwrap_or(&init),
         style: (**style).clone(),
         font_metrics_provider: None,
     };
 
     for (index, keyframe) in keyframes.iter().enumerate() {
         let ref mut animation_values = computed_keyframes[index];
 
-        let mut seen = PropertyBitField::new();
+        let mut seen = LonghandIdSet::new();
 
         // mServoDeclarationBlock is null in the case where we have an invalid css property.
         let iter = keyframe.mPropertyValues.iter()
                                            .filter(|&property| !property.mServoDeclarationBlock.mRawPtr.is_null());
         for property in iter {
             let declarations = unsafe { &*property.mServoDeclarationBlock.mRawPtr.clone() };
             let declarations = RwLock::<PropertyDeclarationBlock>::as_arc(&declarations);
             let guard = declarations.read();
@@ -1424,17 +1424,17 @@ pub extern "C" fn Servo_AssertTreeIsClea
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_FillKeyframesForName(raw_data: RawServoStyleSetBorrowed,
                                                       name: *const nsACString,
                                                       timing_function: *const nsTimingFunction,
                                                       style: ServoComputedValuesBorrowed,
                                                       keyframes: RawGeckoKeyframeListBorrowedMut) -> bool {
     use style::gecko_bindings::structs::Keyframe;
-    use style::properties::property_bit_field::PropertyBitField;
+    use style::properties::LonghandIdSet;
 
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
     let name = unsafe { Atom::from(name.as_ref().unwrap().as_str_unchecked()) };
     let style_timing_function = unsafe { timing_function.as_ref().unwrap() };
     let style = ComputedValues::as_arc(&style);
 
     if let Some(ref animation) = data.stylist.animations().get(&name) {
        for step in &animation.steps {
@@ -1477,17 +1477,17 @@ pub extern "C" fn Servo_StyleSet_FillKey
                   // Filter out non-animatable properties.
                   let animatable =
                       guard.declarations
                            .iter()
                            .filter(|&&(ref declaration, _)| {
                                declaration.is_animatable()
                            });
 
-                  let mut seen = PropertyBitField::new();
+                  let mut seen = LonghandIdSet::new();
 
                   for (index, &(ref declaration, _)) in animatable.enumerate() {
                       unsafe {
                           let property = TransitionProperty::from_declaration(declaration).unwrap();
                           (*keyframe).mPropertyValues.set_len((index + 1) as u32);
                           (*keyframe).mPropertyValues[index].mProperty = property.into();
                           (*keyframe).mPropertyValues[index].mServoDeclarationBlock.set_arc_leaky(
                               Arc::new(RwLock::new(