From 8218b463fb0f7a28e4b3645f3566cafcd0fbfc9c Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Sun, 21 Aug 2016 00:28:48 +0200
Subject: [PATCH 01/15] Revert "Simplify CSSStyleDeclaration::GetPropertyValue"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 1637b0ba8a66839e3329a3cf552ab2d4fb3c67c6.

* As far as I know, `fn` pointers don’t necessarily inline well
* Upcoming commits are gonna change this mapping to be less trivial,
  so this would at least need a new `fn` declaration, making this
  less of a simplification.
---
 components/script/dom/cssstyledeclaration.rs | 21 +++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
index 1ab80ebf19a..101ac3ef252 100644
--- a/components/script/dom/cssstyledeclaration.rs
+++ b/components/script/dom/cssstyledeclaration.rs
@@ -14,11 +14,12 @@ use dom::element::{Element, StylePriority};
 use dom::node::{Node, NodeDamage, window_from_node};
 use dom::window::Window;
 use std::ascii::AsciiExt;
-use std::ops::Deref;
+use std::cell::Ref;
+use std::slice;
 use string_cache::Atom;
 use style::parser::ParserContextExtraData;
-use style::properties::{Shorthand, is_supported_property};
-use style::properties::{parse_one_declaration, parse_style_attribute};
+use style::properties::{PropertyDeclaration, Shorthand};
+use style::properties::{is_supported_property, parse_one_declaration, parse_style_attribute};
 use style::selector_impl::PseudoElement;
 
 // http://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
@@ -147,9 +148,19 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
                 }
             }
 
+            // Step 2.3
+            // Work around closures not being Clone
+            #[derive(Clone)]
+            struct Map<'a, 'b: 'a>(slice::Iter<'a, Ref<'b, PropertyDeclaration>>);
+            impl<'a, 'b> Iterator for Map<'a, 'b> {
+                type Item = &'a PropertyDeclaration;
+                fn next(&mut self) -> Option<Self::Item> {
+                    self.0.next().map(|r| &**r)
+                }
+            }
+
             // TODO: important is hardcoded to false because method does not implement it yet
-            let serialized_value = shorthand.serialize_shorthand_value_to_string(
-                list.iter().map(Deref::deref as fn(_) -> _), false);
+            let serialized_value = shorthand.serialize_shorthand_value_to_string(Map(list.iter()), false);
             return DOMString::from(serialized_value);
         }
 

From 24fbb26475fe2129d52f1fdb93d6cecddb48d727 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Wed, 17 Aug 2016 19:50:43 +0200
Subject: [PATCH 02/15] Add an Importance enum replacing booleans to indicate
 `!important`.

---
 components/script/dom/cssstyledeclaration.rs  | 15 ++--
 components/script/dom/element.rs              | 24 +++---
 .../style/properties/properties.mako.rs       | 73 ++++++++++++-------
 3 files changed, 64 insertions(+), 48 deletions(-)

diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
index 101ac3ef252..2ab50ba4073 100644
--- a/components/script/dom/cssstyledeclaration.rs
+++ b/components/script/dom/cssstyledeclaration.rs
@@ -10,7 +10,7 @@ use dom::bindings::inheritance::Castable;
 use dom::bindings::js::{JS, Root};
 use dom::bindings::reflector::{Reflector, reflect_dom_object};
 use dom::bindings::str::DOMString;
-use dom::element::{Element, StylePriority};
+use dom::element::Element;
 use dom::node::{Node, NodeDamage, window_from_node};
 use dom::window::Window;
 use std::ascii::AsciiExt;
@@ -18,7 +18,7 @@ use std::cell::Ref;
 use std::slice;
 use string_cache::Atom;
 use style::parser::ParserContextExtraData;
-use style::properties::{PropertyDeclaration, Shorthand};
+use style::properties::{PropertyDeclaration, Shorthand, Importance};
 use style::properties::{is_supported_property, parse_one_declaration, parse_style_attribute};
 use style::selector_impl::PseudoElement;
 
@@ -160,7 +160,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             }
 
             // TODO: important is hardcoded to false because method does not implement it yet
-            let serialized_value = shorthand.serialize_shorthand_value_to_string(Map(list.iter()), false);
+            let serialized_value = shorthand.serialize_shorthand_value_to_string(
+                Map(list.iter()), Importance::Normal);
             return DOMString::from(serialized_value);
         }
 
@@ -222,8 +223,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
         // Step 5
         let priority = match &*priority {
-            "" => StylePriority::Normal,
-            p if p.eq_ignore_ascii_case("important") => StylePriority::Important,
+            "" => Importance::Normal,
+            p if p.eq_ignore_ascii_case("important") => Importance::Important,
             _ => return Ok(()),
         };
 
@@ -265,8 +266,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
         // Step 4
         let priority = match &*priority {
-            "" => StylePriority::Normal,
-            p if p.eq_ignore_ascii_case("important") => StylePriority::Important,
+            "" => Importance::Normal,
+            p if p.eq_ignore_ascii_case("important") => Importance::Important,
             _ => return Ok(()),
         };
 
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 860818c024c..cbaaaa91edf 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -87,8 +87,8 @@ use style::attr::{AttrValue, LengthOrPercentageOrAuto};
 use style::element_state::*;
 use style::matching::{common_style_affecting_attributes, rare_style_affecting_attributes};
 use style::parser::ParserContextExtraData;
-use style::properties::DeclaredValue;
 use style::properties::longhands::{self, background_image, border_spacing, font_family, overflow_x, font_size};
+use style::properties::{DeclaredValue, Importance};
 use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, parse_style_attribute};
 use style::selector_impl::{NonTSPseudoClass, ServoSelectorImpl};
 use style::sink::Push;
@@ -660,13 +660,6 @@ impl LayoutElementHelpers for LayoutJS<Element> {
     }
 }
 
-#[derive(PartialEq, Eq, Copy, Clone, HeapSizeOf)]
-pub enum StylePriority {
-    Important,
-    Normal,
-}
-
-
 impl Element {
     pub fn html_element_in_html_document(&self) -> bool {
         self.namespace == ns!(html) && self.upcast::<Node>().is_in_html_doc()
@@ -780,11 +773,12 @@ impl Element {
 
     pub fn update_inline_style(&self,
                                declarations: Vec<PropertyDeclaration>,
-                               style_priority: StylePriority) {
-        fn update(element: &Element, mut declarations: Vec<PropertyDeclaration>, style_priority: StylePriority) {
+                               importance: Importance) {
+        fn update(element: &Element, mut declarations: Vec<PropertyDeclaration>,
+                  importance: Importance) {
             let mut inline_declarations = element.style_attribute().borrow_mut();
             if let &mut Some(ref mut existing_declarations) = &mut *inline_declarations {
-                let existing_declarations = if style_priority == StylePriority::Important {
+                let existing_declarations = if importance.important() {
                     &mut existing_declarations.important
                 } else {
                     &mut existing_declarations.normal
@@ -813,7 +807,7 @@ impl Element {
                 return;
             }
 
-            let (important, normal) = if style_priority == StylePriority::Important {
+            let (important, normal) = if importance.important() {
                 (declarations, vec![])
             } else {
                 (vec![], declarations)
@@ -825,17 +819,17 @@ impl Element {
             });
         }
 
-        update(self, declarations, style_priority);
+        update(self, declarations, importance);
         self.sync_property_with_attrs_style();
     }
 
     pub fn set_inline_style_property_priority(&self,
                                               properties: &[&str],
-                                              style_priority: StylePriority) {
+                                              importance: Importance) {
         {
             let mut inline_declarations = self.style_attribute().borrow_mut();
             if let &mut Some(ref mut declarations) = &mut *inline_declarations {
-              let (from, to) = if style_priority == StylePriority::Important {
+              let (from, to) = if importance == Importance::Important {
                   (&mut declarations.normal, &mut declarations.important)
               } else {
                   (&mut declarations.important, &mut declarations.normal)
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index aaf9ddfe88c..5db9379fc13 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -14,6 +14,8 @@ use std::ascii::AsciiExt;
 use std::boxed::Box as StdBox;
 use std::collections::HashSet;
 use std::fmt::{self, Write};
+use std::iter::{Iterator, Chain, Zip, Rev, Repeat, repeat};
+use std::slice;
 use std::sync::Arc;
 
 use app_units::Au;
@@ -261,11 +263,26 @@ mod property_bit_field {
     % endif
 % endfor
 
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub enum Importance {
+    /// Indicates a declaration without `!important`.
+    Normal,
+
+    /// Indicates a declaration with `!important`.
+    Important,
+}
+
+impl Importance {
+    pub fn important(self) -> bool {
+        match self {
+            Importance::Normal => false,
+            Importance::Important => true,
+        }
+    }
+}
 
-use std::iter::{Iterator, Chain, Zip, Rev, Repeat, repeat};
-use std::slice;
 /// Overridden declarations are skipped.
-
 // FIXME (https://github.com/servo/servo/issues/3426)
 #[derive(Debug, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
@@ -279,12 +296,12 @@ pub struct PropertyDeclarationBlock {
 impl PropertyDeclarationBlock {
     /// Provides an iterator of all declarations, with indication of !important value
     pub fn declarations(&self) -> Chain<
-        Zip<Rev<slice::Iter<PropertyDeclaration>>, Repeat<bool>>,
-        Zip<Rev<slice::Iter<PropertyDeclaration>>, Repeat<bool>>
+        Zip<Rev<slice::Iter<PropertyDeclaration>>, Repeat<Importance>>,
+        Zip<Rev<slice::Iter<PropertyDeclaration>>, Repeat<Importance>>
     > {
         // Declarations are stored in reverse order.
-        let normal = self.normal.iter().rev().zip(repeat(false));
-        let important = self.important.iter().rev().zip(repeat(true));
+        let normal = self.normal.iter().rev().zip(repeat(Importance::Normal));
+        let important = self.important.iter().rev().zip(repeat(Importance::Important));
         normal.chain(important)
     }
 }
@@ -300,7 +317,7 @@ impl ToCss for PropertyDeclarationBlock {
         let mut already_serialized = Vec::new();
 
         // Step 3
-        for (declaration, important) in self.declarations() {
+        for (declaration, importance) in self.declarations() {
             // Step 3.1
             let property = declaration.name();
 
@@ -326,11 +343,11 @@ impl ToCss for PropertyDeclarationBlock {
                     let mut current_longhands = Vec::new();
                     let mut important_count = 0;
 
-                    for &(longhand, longhand_important) in longhands.iter() {
+                    for &(longhand, longhand_importance) in longhands.iter() {
                         let longhand_name = longhand.name();
                         if properties.iter().any(|p| &longhand_name == *p) {
                             current_longhands.push(longhand);
-                            if longhand_important {
+                            if longhand_importance.important() {
                                 important_count += 1;
                             }
                         }
@@ -396,7 +413,7 @@ impl ToCss for PropertyDeclarationBlock {
                 dest,
                 &property.to_string(),
                 AppendableValue::Declaration(declaration),
-                important,
+                importance,
                 &mut is_first_serialization));
 
             // Step 3.3.8
@@ -430,7 +447,7 @@ fn handle_first_serialization<W>(dest: &mut W, is_first_serialization: &mut bool
 fn append_declaration_value<'a, W, I>
                            (dest: &mut W,
                             appendable_value: AppendableValue<'a, I>,
-                            is_important: bool)
+                            importance: Importance)
                             -> fmt::Result
                             where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
   match appendable_value {
@@ -445,7 +462,7 @@ fn append_declaration_value<'a, W, I>
        }
   }
 
-  if is_important {
+  if importance.important() {
       try!(write!(dest, " !important"));
   }
 
@@ -455,7 +472,7 @@ fn append_declaration_value<'a, W, I>
 fn append_serialization<'a, W, I>(dest: &mut W,
                                   property_name: &str,
                                   appendable_value: AppendableValue<'a, I>,
-                                  is_important: bool,
+                                  importance: Importance,
                                   is_first_serialization: &mut bool)
                                   -> fmt::Result
                                   where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
@@ -465,7 +482,7 @@ fn append_serialization<'a, W, I>(dest: &mut W,
     // Overflow does not behave like a normal shorthand. When overflow-x and overflow-y are not of equal
     // values, they no longer use the shared property name "overflow" and must be handled differently
     if shorthands::is_overflow_shorthand(&appendable_value) {
-        return append_declaration_value(dest, appendable_value, is_important);
+        return append_declaration_value(dest, appendable_value, importance);
     }
 
     try!(write!(dest, "{}:", property_name));
@@ -484,7 +501,7 @@ fn append_serialization<'a, W, I>(dest: &mut W,
          &AppendableValue::DeclarationsForShorthand(..) => try!(write!(dest, " "))
     }
 
-    try!(append_declaration_value(dest, appendable_value, is_important));
+    try!(append_declaration_value(dest, appendable_value, importance));
     write!(dest, ";")
 }
 
@@ -514,14 +531,15 @@ struct PropertyDeclarationParser<'a, 'b: 'a> {
 /// Default methods reject all at rules.
 impl<'a, 'b> AtRuleParser for PropertyDeclarationParser<'a, 'b> {
     type Prelude = ();
-    type AtRule = (Vec<PropertyDeclaration>, bool);
+    type AtRule = (Vec<PropertyDeclaration>, Importance);
 }
 
 
 impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
-    type Declaration = (Vec<PropertyDeclaration>, bool);
+    type Declaration = (Vec<PropertyDeclaration>, Importance);
 
-    fn parse_value(&self, name: &str, input: &mut Parser) -> Result<(Vec<PropertyDeclaration>, bool), ()> {
+    fn parse_value(&self, name: &str, input: &mut Parser)
+                   -> Result<(Vec<PropertyDeclaration>, Importance), ()> {
         let mut results = vec![];
         try!(input.parse_until_before(Delimiter::Bang, |input| {
             match PropertyDeclaration::parse(name, self.context, input, &mut results) {
@@ -529,8 +547,11 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
                 _ => Err(())
             }
         }));
-        let important = input.try(parse_important).is_ok();
-        Ok((results, important))
+        let importance = match input.try(parse_important) {
+            Ok(()) => Importance::Important,
+            Err(()) => Importance::Normal,
+        };
+        Ok((results, importance))
     }
 }
 
@@ -545,8 +566,8 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
     let mut iter = DeclarationListParser::new(input, parser);
     while let Some(declaration) = iter.next() {
         match declaration {
-            Ok((results, important)) => {
-                if important {
+            Ok((results, importance)) => {
+                if importance.important() {
                     important_declarations.extend(results);
                 } else {
                     normal_declarations.extend(results);
@@ -675,11 +696,11 @@ impl Shorthand {
     }
 
     /// Serializes possible shorthand value to String.
-    pub fn serialize_shorthand_value_to_string<'a, I>(self, declarations: I, is_important: bool) -> String
+    pub fn serialize_shorthand_value_to_string<'a, I>(self, declarations: I, importance: Importance) -> String
     where I: Iterator<Item=&'a PropertyDeclaration> + Clone {
         let appendable_value = self.get_shorthand_appendable_value(declarations).unwrap();
         let mut result = String::new();
-        append_declaration_value(&mut result, appendable_value, is_important).unwrap();
+        append_declaration_value(&mut result, appendable_value, importance).unwrap();
         result
     }
 
@@ -700,7 +721,7 @@ impl Shorthand {
                     dest,
                     property_name,
                     appendable_value,
-                    false,
+                    Importance::Normal,
                     is_first_serialization
                 ).and_then(|_| Ok(true))
             }

From 901aaeaf683cec1c43d02b800e7beb605ac9c8c8 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Wed, 17 Aug 2016 20:05:34 +0200
Subject: [PATCH 03/15] Remove unused Keyframe::parse method

---
 components/style/keyframes.rs | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs
index a6e667e9282..a0000c6e177 100644
--- a/components/style/keyframes.rs
+++ b/components/style/keyframes.rs
@@ -73,27 +73,6 @@ pub struct Keyframe {
     pub declarations: Arc<Vec<PropertyDeclaration>>,
 }
 
-impl Keyframe {
-    pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<Keyframe, ()> {
-        let percentages = try!(input.parse_until_before(Delimiter::CurlyBracketBlock, |input| {
-            input.parse_comma_separated(|input| KeyframePercentage::parse(input))
-        }));
-        let selector = KeyframeSelector(percentages);
-
-        try!(input.expect_curly_bracket_block());
-
-        let declarations = input.parse_nested_block(|input| {
-            Ok(parse_property_declaration_list(context, input))
-        }).unwrap();
-
-        // NB: Important declarations are explicitely ignored in the spec.
-        Ok(Keyframe {
-            selector: selector,
-            declarations: declarations.normal,
-        })
-    }
-}
-
 /// A keyframes step value. This can be a synthetised keyframes animation, that
 /// is, one autogenerated from the current computed values, or a list of
 /// declarations to apply.

From ab846ab196f2a9f891a5189c8e5e2501ead7a6a3 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Wed, 17 Aug 2016 20:49:06 +0200
Subject: [PATCH 04/15] Make parsing of keyframe declaration blocks
 spec-compliant.

Exclude animation properties.
---
 components/style/keyframes.rs                 | 51 ++++++++++++++++---
 components/style/properties/data.py           | 20 +++++++-
 .../style/properties/longhand/box.mako.rs     | 26 +++++++---
 .../style/properties/properties.mako.rs       | 25 +++++++--
 .../style/properties/shorthand/box.mako.rs    |  3 +-
 tests/unit/style/stylesheets.rs               | 11 +++-
 6 files changed, 115 insertions(+), 21 deletions(-)

diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs
index a0000c6e177..3656ccee24d 100644
--- a/components/style/keyframes.rs
+++ b/components/style/keyframes.rs
@@ -2,10 +2,12 @@
  * 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/. */
 
-use cssparser::{AtRuleParser, Delimiter, Parser, QualifiedRuleParser, RuleListParser};
+use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser};
+use cssparser::{DeclarationListParser, DeclarationParser};
 use parser::{ParserContext, log_css_error};
+use properties::PropertyDeclaration;
+use properties::PropertyDeclarationParseResult;
 use properties::animated_properties::TransitionProperty;
-use properties::{PropertyDeclaration, parse_property_declaration_list};
 use std::sync::Arc;
 
 /// A number from 1 to 100, indicating the percentage of the animation where
@@ -238,12 +240,49 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> {
 
     fn parse_block(&self, prelude: Self::Prelude, input: &mut Parser)
                    -> Result<Self::QualifiedRule, ()> {
+        let mut declarations = Vec::new();
+        let parser = KeyframeDeclarationParser {
+            context: self.context,
+        };
+        let mut iter = DeclarationListParser::new(input, parser);
+        while let Some(declaration) = iter.next() {
+            match declaration {
+                Ok(d) => declarations.extend(d),
+                Err(range) => {
+                    let pos = range.start;
+                    let message = format!("Unsupported keyframe property declaration: '{}'",
+                                          iter.input.slice(range));
+                    log_css_error(iter.input, pos, &*message, self.context);
+                }
+            }
+            // `parse_important` is not called here, `!important` is not allowed in keyframe blocks.
+        }
         Ok(Keyframe {
             selector: prelude,
-            // FIXME: needs parsing different from parse_property_declaration_list:
-            // https://drafts.csswg.org/css-animations/#keyframes
-            // Paragraph "The <declaration-list> inside of <keyframe-block> ..."
-            declarations: parse_property_declaration_list(self.context, input).normal,
+            declarations: Arc::new(declarations),
         })
     }
 }
+
+struct KeyframeDeclarationParser<'a, 'b: 'a> {
+    context: &'a ParserContext<'b>,
+}
+
+/// Default methods reject all at rules.
+impl<'a, 'b> AtRuleParser for KeyframeDeclarationParser<'a, 'b> {
+    type Prelude = ();
+    type AtRule = Vec<PropertyDeclaration>;
+}
+
+impl<'a, 'b> DeclarationParser for KeyframeDeclarationParser<'a, 'b> {
+    type Declaration = Vec<PropertyDeclaration>;
+
+    fn parse_value(&self, name: &str, input: &mut Parser) -> Result<Vec<PropertyDeclaration>, ()> {
+        let mut results = Vec::new();
+        match PropertyDeclaration::parse(name, self.context, input, &mut results, true) {
+            PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => {}
+            _ => return Err(())
+        }
+        Ok(results)
+    }
+}
diff --git a/components/style/properties/data.py b/components/style/properties/data.py
index 400b054ba74..4ae14a11177 100644
--- a/components/style/properties/data.py
+++ b/components/style/properties/data.py
@@ -65,7 +65,8 @@ class Keyword(object):
 class Longhand(object):
     def __init__(self, style_struct, name, animatable=None, derived_from=None, keyword=None,
                  predefined_type=None, custom_cascade=False, experimental=False, internal=False,
-                 need_clone=False, need_index=False, gecko_ffi_name=None, depend_on_viewport_size=False):
+                 need_clone=False, need_index=False, gecko_ffi_name=None, depend_on_viewport_size=False,
+                 allowed_in_keyframe_block=True):
         self.name = name
         self.keyword = keyword
         self.predefined_type = predefined_type
@@ -80,6 +81,13 @@ class Longhand(object):
         self.depend_on_viewport_size = depend_on_viewport_size
         self.derived_from = (derived_from or "").split()
 
+        # https://drafts.csswg.org/css-animations/#keyframes
+        # > The <declaration-list> inside of <keyframe-block> accepts any CSS property
+        # > except those defined in this specification,
+        # > but does accept the `animation-play-state` property and interprets it specially.
+        self.allowed_in_keyframe_block = allowed_in_keyframe_block \
+            and allowed_in_keyframe_block != "False"
+
         # This is done like this since just a plain bool argument seemed like
         # really random.
         if animatable is None:
@@ -98,7 +106,8 @@ class Longhand(object):
 
 
 class Shorthand(object):
-    def __init__(self, name, sub_properties, experimental=False, internal=False):
+    def __init__(self, name, sub_properties, experimental=False, internal=False,
+                 allowed_in_keyframe_block=True):
         self.name = name
         self.ident = to_rust_ident(name)
         self.camel_case = to_camel_case(self.ident)
@@ -107,6 +116,13 @@ class Shorthand(object):
         self.sub_properties = sub_properties
         self.internal = internal
 
+        # https://drafts.csswg.org/css-animations/#keyframes
+        # > The <declaration-list> inside of <keyframe-block> accepts any CSS property
+        # > except those defined in this specification,
+        # > but does accept the `animation-play-state` property and interprets it specially.
+        self.allowed_in_keyframe_block = allowed_in_keyframe_block \
+            and allowed_in_keyframe_block != "False"
+
 
 class Method(object):
     def __init__(self, name, return_type=None, arg_types=None, is_mut=False):
diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs
index 492edf3d97b..6aaa615c7de 100644
--- a/components/style/properties/longhand/box.mako.rs
+++ b/components/style/properties/longhand/box.mako.rs
@@ -636,7 +636,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
 
 <%helpers:longhand name="animation-name"
                    need_index="True"
-                   animatable="False">
+                   animatable="False",
+                   allowed_in_keyframe_block="False">
     use values::computed::ComputedValueAsSpecified;
     use values::NoViewportPercentage;
 
@@ -699,7 +700,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
 
 <%helpers:longhand name="animation-duration"
                    need_index="True"
-                   animatable="False">
+                   animatable="False",
+                   allowed_in_keyframe_block="False">
     pub use super::transition_duration::computed_value;
     pub use super::transition_duration::{get_initial_value, get_initial_single_value};
     pub use super::transition_duration::{parse, parse_one};
@@ -709,7 +711,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
 
 <%helpers:longhand name="animation-timing-function"
                    need_index="True"
-                   animatable="False">
+                   animatable="False",
+                   allowed_in_keyframe_block="False">
     pub use super::transition_timing_function::computed_value;
     pub use super::transition_timing_function::{get_initial_value, get_initial_single_value};
     pub use super::transition_timing_function::{parse, parse_one};
@@ -719,7 +722,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
 
 <%helpers:longhand name="animation-iteration-count"
                    need_index="True"
-                   animatable="False">
+                   animatable="False",
+                   allowed_in_keyframe_block="False">
     use values::computed::ComputedValueAsSpecified;
     use values::NoViewportPercentage;
 
@@ -804,22 +808,28 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
 ${helpers.keyword_list("animation-direction",
                        "normal reverse alternate alternate-reverse",
                        need_index=True,
-                       animatable=False)}
+                       animatable=False,
+                       allowed_in_keyframe_block=False)}
 
+// animation-play-state is the exception to the rule for allowed_in_keyframe_block:
+// https://drafts.csswg.org/css-animations/#keyframes
 ${helpers.keyword_list("animation-play-state",
                        "running paused",
                        need_clone=True,
                        need_index=True,
-                       animatable=False)}
+                       animatable=False,
+                       allowed_in_keyframe_block=True)}
 
 ${helpers.keyword_list("animation-fill-mode",
                        "none forwards backwards both",
                        need_index=True,
-                       animatable=False)}
+                       animatable=False,
+                       allowed_in_keyframe_block=False)}
 
 <%helpers:longhand name="animation-delay"
                    need_index="True"
-                   animatable="False">
+                   animatable="False",
+                   allowed_in_keyframe_block="False">
     pub use super::transition_duration::computed_value;
     pub use super::transition_duration::{get_initial_value, get_initial_single_value};
     pub use super::transition_duration::{parse, parse_one};
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index 5db9379fc13..d1ad214810b 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -517,7 +517,7 @@ pub fn parse_one_declaration(name: &str, input: &str, base_url: &Url, error_repo
                              -> Result<Vec<PropertyDeclaration>, ()> {
     let context = ParserContext::new_with_extra_data(Origin::Author, base_url, error_reporter, extra_data);
     let mut results = vec![];
-    match PropertyDeclaration::parse(name, &context, &mut Parser::new(input), &mut results) {
+    match PropertyDeclaration::parse(name, &context, &mut Parser::new(input), &mut results, false) {
         PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => Ok(results),
         _ => Err(())
     }
@@ -542,7 +542,7 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
                    -> Result<(Vec<PropertyDeclaration>, Importance), ()> {
         let mut results = vec![];
         try!(input.parse_until_before(Delimiter::Bang, |input| {
-            match PropertyDeclaration::parse(name, self.context, input, &mut results) {
+            match PropertyDeclaration::parse(name, self.context, input, &mut results, false) {
                 PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => Ok(()),
                 _ => Err(())
             }
@@ -832,6 +832,7 @@ pub enum PropertyDeclarationParseResult {
     UnknownProperty,
     ExperimentalProperty,
     InvalidValue,
+    AnimationPropertyInKeyframeBlock,
     ValidOrIgnoredDeclaration,
 }
 
@@ -984,8 +985,16 @@ impl PropertyDeclaration {
         }
     }
 
+    /// The `in_keyframe_block` parameter controls this:
+    ///
+    /// https://drafts.csswg.org/css-animations/#keyframes
+    /// > The <declaration-list> inside of <keyframe-block> accepts any CSS property
+    /// > except those defined in this specification,
+    /// > but does accept the `animation-play-state` property and interprets it specially.
     pub fn parse(name: &str, context: &ParserContext, input: &mut Parser,
-                 result_list: &mut Vec<PropertyDeclaration>) -> PropertyDeclarationParseResult {
+                 result_list: &mut Vec<PropertyDeclaration>,
+                 in_keyframe_block: bool)
+                 -> PropertyDeclarationParseResult {
         if let Ok(name) = ::custom_properties::parse_name(name) {
             let value = match input.try(CSSWideKeyword::parse) {
                 Ok(CSSWideKeyword::UnsetKeyword) |  // Custom properties are alawys inherited
@@ -1003,6 +1012,11 @@ impl PropertyDeclaration {
             % for property in data.longhands:
                 % if not property.derived_from:
                     "${property.name}" => {
+                        % if not property.allowed_in_keyframe_block:
+                            if in_keyframe_block {
+                                return PropertyDeclarationParseResult::AnimationPropertyInKeyframeBlock
+                            }
+                        % endif
                         % if property.internal:
                             if context.stylesheet_origin != Origin::UserAgent {
                                 return PropertyDeclarationParseResult::UnknownProperty
@@ -1028,6 +1042,11 @@ impl PropertyDeclaration {
             % endfor
             % for shorthand in data.shorthands:
                 "${shorthand.name}" => {
+                    % if not shorthand.allowed_in_keyframe_block:
+                        if in_keyframe_block {
+                            return PropertyDeclarationParseResult::AnimationPropertyInKeyframeBlock
+                        }
+                    % endif
                     % if shorthand.internal:
                         if context.stylesheet_origin != Origin::UserAgent {
                             return PropertyDeclarationParseResult::UnknownProperty
diff --git a/components/style/properties/shorthand/box.mako.rs b/components/style/properties/shorthand/box.mako.rs
index 6a502276f4e..0825a8fc314 100644
--- a/components/style/properties/shorthand/box.mako.rs
+++ b/components/style/properties/shorthand/box.mako.rs
@@ -152,7 +152,8 @@ macro_rules! try_parse_one {
                     sub_properties="animation-name animation-duration
                                     animation-timing-function animation-delay
                                     animation-iteration-count animation-direction
-                                    animation-fill-mode animation-play-state">
+                                    animation-fill-mode animation-play-state"
+                    allowed_in_keyframe_block="False">
     use properties::longhands::{animation_name, animation_duration, animation_timing_function};
     use properties::longhands::{animation_delay, animation_iteration_count, animation_direction};
     use properties::longhands::{animation_fill_mode, animation_play_state};
diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs
index 5f326ba52e5..2eb5f9f3260 100644
--- a/tests/unit/style/stylesheets.rs
+++ b/tests/unit/style/stylesheets.rs
@@ -13,6 +13,7 @@ use style::error_reporting::ParseErrorReporter;
 use style::keyframes::{Keyframe, KeyframeSelector, KeyframePercentage};
 use style::parser::ParserContextExtraData;
 use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, DeclaredValue, longhands};
+use style::properties::longhands::animation_play_state;
 use style::stylesheets::{Stylesheet, CSSRule, StyleRule, KeyframesRule, Origin};
 use style::values::specified::{LengthOrPercentageOrAuto, Percentage};
 use url::Url;
@@ -27,7 +28,12 @@ fn test_parse_stylesheet() {
         #d1 > .ok { background: blue; }
         @keyframes foo {
             from { width: 0% }
-            to { width: 100%}
+            to {
+                width: 100%;
+                width: 50% !important; /* !important not allowed here */
+                animation-name: 'foo'; /* animation properties not allowed here */
+                animation-play-state: running; /* … except animation-play-state */
+            }
         }";
     let url = Url::parse("about::test").unwrap();
     let stylesheet = Stylesheet::from_str(css, url, Origin::UserAgent,
@@ -187,6 +193,9 @@ fn test_parse_stylesheet() {
                         declarations: Arc::new(vec![
                             PropertyDeclaration::Width(DeclaredValue::Value(
                                 LengthOrPercentageOrAuto::Percentage(Percentage(1.)))),
+                            PropertyDeclaration::AnimationPlayState(DeclaredValue::Value(
+                                animation_play_state::SpecifiedValue(
+                                    vec![animation_play_state::SingleSpecifiedValue::running]))),
                         ]),
                     },
                 ]

From 31864954ed32c51a0bd6e024c0031ceb96b10e93 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Wed, 17 Aug 2016 20:49:17 +0200
Subject: [PATCH 05/15] Typo fixes

---
 components/style/properties/data.py   | 14 +++++++-------
 components/style/selector_matching.rs |  4 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/components/style/properties/data.py b/components/style/properties/data.py
index 4ae14a11177..32b37063867 100644
--- a/components/style/properties/data.py
+++ b/components/style/properties/data.py
@@ -185,15 +185,15 @@ class PropertiesData(object):
         if self.product not in products:
             return
 
-        longand = Longhand(self.current_style_struct, name, **kwargs)
-        self.current_style_struct.longhands.append(longand)
-        self.longhands.append(longand)
-        self.longhands_by_name[name] = longand
+        longhand = Longhand(self.current_style_struct, name, **kwargs)
+        self.current_style_struct.longhands.append(longhand)
+        self.longhands.append(longhand)
+        self.longhands_by_name[name] = longhand
 
-        for name in longand.derived_from:
-            self.derived_longhands.setdefault(name, []).append(longand)
+        for name in longhand.derived_from:
+            self.derived_longhands.setdefault(name, []).append(longhand)
 
-        return longand
+        return longhand
 
     def declare_shorthand(self, name, sub_properties, products="gecko servo", *args, **kwargs):
         products = products.split()
diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs
index 76e30c70d82..a793fec1ff4 100644
--- a/components/style/selector_matching.rs
+++ b/components/style/selector_matching.rs
@@ -525,10 +525,10 @@ impl Stylist {
 /// Map that contains the CSS rules for a given origin.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 struct PerOriginSelectorMap {
-    /// Rules that contains at least one property declararion with
+    /// Rules that contains at least one property declaration with
     /// normal importance.
     normal: SelectorMap<Vec<PropertyDeclaration>, TheSelectorImpl>,
-    /// Rules that contains at least one property declararion with
+    /// Rules that contains at least one property declaration with
     /// !important.
     important: SelectorMap<Vec<PropertyDeclaration>, TheSelectorImpl>,
 }

From 477cae67df2827b067ec2ef4e72ef10272d6791d Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Thu, 18 Aug 2016 17:16:51 +0200
Subject: [PATCH 06/15] Unreverse declarations in memory.

---
 components/script/dom/element.rs              |  5 ++--
 .../style/properties/properties.mako.rs       | 23 ++++++++----------
 tests/unit/style/properties/serialization.rs  |  2 --
 tests/unit/style/stylesheets.rs               | 24 +++++++++++++------
 4 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index cbaaaa91edf..f09a2dc6b34 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -788,7 +788,7 @@ impl Element {
                 // than that.
                 let existing_declarations = Arc::make_mut(existing_declarations);
 
-                while let Some(mut incoming_declaration) = declarations.pop() {
+                for mut incoming_declaration in declarations {
                     let mut replaced = false;
                     for existing_declaration in &mut *existing_declarations {
                         if existing_declaration.name() == incoming_declaration.name() {
@@ -799,8 +799,7 @@ impl Element {
                     }
 
                     if !replaced {
-                        // inserting instead of pushing since the declarations are in reverse order
-                        existing_declarations.insert(0, incoming_declaration);
+                        existing_declarations.push(incoming_declaration);
                     }
                 }
 
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index d1ad214810b..d971b1c6eab 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -14,7 +14,7 @@ use std::ascii::AsciiExt;
 use std::boxed::Box as StdBox;
 use std::collections::HashSet;
 use std::fmt::{self, Write};
-use std::iter::{Iterator, Chain, Zip, Rev, Repeat, repeat};
+use std::iter::{Iterator, Chain, Zip, Repeat, repeat};
 use std::slice;
 use std::sync::Arc;
 
@@ -296,12 +296,11 @@ pub struct PropertyDeclarationBlock {
 impl PropertyDeclarationBlock {
     /// Provides an iterator of all declarations, with indication of !important value
     pub fn declarations(&self) -> Chain<
-        Zip<Rev<slice::Iter<PropertyDeclaration>>, Repeat<Importance>>,
-        Zip<Rev<slice::Iter<PropertyDeclaration>>, Repeat<Importance>>
+        Zip<slice::Iter<PropertyDeclaration>, Repeat<Importance>>,
+        Zip<slice::Iter<PropertyDeclaration>, Repeat<Importance>>
     > {
-        // Declarations are stored in reverse order.
-        let normal = self.normal.iter().rev().zip(repeat(Importance::Normal));
-        let important = self.important.iter().rev().zip(repeat(Importance::Important));
+        let normal = self.normal.iter().zip(repeat(Importance::Normal));
+        let important = self.important.iter().zip(repeat(Importance::Important));
         normal.chain(important)
     }
 }
@@ -589,7 +588,7 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
 
 
 /// Only keep the last declaration for any given property.
-/// The input is in source order, output in reverse source order.
+/// The input and output are in source order
 fn deduplicate_property_declarations(declarations: Vec<PropertyDeclaration>)
                                      -> Vec<PropertyDeclaration> {
     let mut deduplicated = vec![];
@@ -618,6 +617,7 @@ fn deduplicate_property_declarations(declarations: Vec<PropertyDeclaration>)
         }
         deduplicated.push(declaration)
     }
+    deduplicated.reverse();
     deduplicated
 }
 
@@ -1683,8 +1683,7 @@ fn cascade_with_cached_declarations(
     // Declaration blocks are stored in increasing precedence order,
     // we want them in decreasing order here.
     for sub_list in applicable_declarations.iter().rev() {
-        // Declarations are already stored in reverse order.
-        for declaration in sub_list.declarations.iter() {
+        for declaration in sub_list.declarations.iter().rev() {
             match *declaration {
                 % for style_struct in data.active_style_structs():
                     % for property in style_struct.longhands:
@@ -1815,8 +1814,7 @@ pub fn cascade(viewport_size: Size2D<Au>,
     let mut custom_properties = None;
     let mut seen_custom = HashSet::new();
     for sub_list in applicable_declarations.iter().rev() {
-        // Declarations are already stored in reverse order.
-        for declaration in sub_list.declarations.iter() {
+        for declaration in sub_list.declarations.iter().rev() {
             match *declaration {
                 PropertyDeclaration::Custom(ref name, ref value) => {
                     ::custom_properties::cascade(
@@ -1874,8 +1872,7 @@ pub fn cascade(viewport_size: Size2D<Au>,
     ComputedValues::do_cascade_property(|cascade_property| {
         % for category_to_cascade_now in ["early", "other"]:
             for sub_list in applicable_declarations.iter().rev() {
-                // Declarations are already stored in reverse order.
-                for declaration in sub_list.declarations.iter() {
+                for declaration in sub_list.declarations.iter().rev() {
                     if let PropertyDeclaration::Custom(..) = *declaration {
                         continue
                     }
diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs
index 7ff65edd959..98ffa07f722 100644
--- a/tests/unit/style/properties/serialization.rs
+++ b/tests/unit/style/properties/serialization.rs
@@ -39,8 +39,6 @@ fn property_declaration_block_should_serialize_correctly() {
     let height = DeclaredValue::Value(LengthOrPercentageOrAuto::Length(Length::from_px(20f32)));
     important.push(PropertyDeclaration::Height(height));
 
-    normal.reverse();
-    important.reverse();
     let block = PropertyDeclarationBlock {
         normal: Arc::new(normal),
         important: Arc::new(important)
diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs
index 2eb5f9f3260..72273165b35 100644
--- a/tests/unit/style/stylesheets.rs
+++ b/tests/unit/style/stylesheets.rs
@@ -39,6 +39,16 @@ fn test_parse_stylesheet() {
     let stylesheet = Stylesheet::from_str(css, url, Origin::UserAgent,
                                           Box::new(CSSErrorReporterTest),
                                           ParserContextExtraData::default());
+    macro_rules! assert_eq {
+        ($left: expr, $right: expr) => {
+            let left = $left;
+            let right = $right;
+            if left != right {
+                panic!("{:#?} != {:#?}", left, right)
+            }
+        }
+    }
+
     assert_eq!(stylesheet, Stylesheet {
         origin: Origin::UserAgent,
         media: None,
@@ -157,13 +167,6 @@ fn test_parse_stylesheet() {
                 ],
                 declarations: PropertyDeclarationBlock {
                     normal: Arc::new(vec![
-                        PropertyDeclaration::BackgroundClip(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundOrigin(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundSize(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundImage(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundAttachment(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundRepeat(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundPosition(DeclaredValue::Initial),
                         PropertyDeclaration::BackgroundColor(DeclaredValue::Value(
                             longhands::background_color::SpecifiedValue {
                                 authored: Some("blue".to_owned()),
@@ -172,6 +175,13 @@ fn test_parse_stylesheet() {
                                 }),
                             }
                         )),
+                        PropertyDeclaration::BackgroundPosition(DeclaredValue::Initial),
+                        PropertyDeclaration::BackgroundRepeat(DeclaredValue::Initial),
+                        PropertyDeclaration::BackgroundAttachment(DeclaredValue::Initial),
+                        PropertyDeclaration::BackgroundImage(DeclaredValue::Initial),
+                        PropertyDeclaration::BackgroundSize(DeclaredValue::Initial),
+                        PropertyDeclaration::BackgroundOrigin(DeclaredValue::Initial),
+                        PropertyDeclaration::BackgroundClip(DeclaredValue::Initial),
                     ]),
                     important: Arc::new(vec![]),
                 },

From bc71e8b65b1da3959c28d820a7bc54725118bcef Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Mon, 8 Aug 2016 15:52:24 +0200
Subject: [PATCH 07/15] Add some fmt::Debug implementations

---
 components/script/dom/element.rs              | 11 ++++++++++
 components/script/layout_wrapper.rs           | 13 +++++++++++-
 .../script_layout_interface/wrapper_traits.rs |  3 ++-
 components/style/dom.rs                       |  2 +-
 .../style/properties/properties.mako.rs       | 20 ++++++++++++++++++-
 components/style/selector_matching.rs         |  3 +++
 ports/geckolib/wrapper.rs                     | 11 ++++++++++
 7 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index f09a2dc6b34..2972a9465f4 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -79,6 +79,7 @@ use std::borrow::Cow;
 use std::cell::{Cell, Ref};
 use std::convert::TryFrom;
 use std::default::Default;
+use std::fmt;
 use std::mem;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicUsize, Ordering};
@@ -115,6 +116,16 @@ pub struct Element {
     atomic_flags: AtomicElementFlags,
 }
 
+impl fmt::Debug for Element {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        try!(write!(f, "<{}", self.local_name));
+        if let Some(ref id) = *self.id_attribute.borrow() {
+            try!(write!(f, " id={}", id));
+        }
+        write!(f, ">")
+    }
+}
+
 #[derive(PartialEq, HeapSizeOf)]
 pub enum ElementCreator {
     ParserCreated,
diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs
index cbf04b48e5b..431fe55f9b0 100644
--- a/components/script/layout_wrapper.rs
+++ b/components/script/layout_wrapper.rs
@@ -49,6 +49,7 @@ use script_layout_interface::{HTMLCanvasData, LayoutNodeType, TrustedNodeAddress
 use script_layout_interface::{OpaqueStyleAndLayoutData, PartialStyleAndLayoutData};
 use selectors::matching::{DeclarationBlock, ElementFlags};
 use selectors::parser::{AttrSelector, NamespaceConstraint};
+use std::fmt;
 use std::marker::PhantomData;
 use std::mem::{transmute, transmute_copy};
 use std::sync::Arc;
@@ -405,6 +406,16 @@ pub struct ServoLayoutElement<'le> {
     chain: PhantomData<&'le ()>,
 }
 
+impl<'le> fmt::Debug for ServoLayoutElement<'le> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        try!(write!(f, "<{}", self.element.local_name()));
+        if let &Some(ref id) = unsafe { &*self.element.id_attribute() } {
+            try!(write!(f, " id={}", id));
+        }
+        write!(f, ">")
+    }
+}
+
 impl<'le> PresentationalHintsSynthetizer for ServoLayoutElement<'le> {
     fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, hints: &mut V)
         where V: Push<DeclarationBlock<Vec<PropertyDeclaration>>>
@@ -926,7 +937,7 @@ impl<ConcreteNode> Iterator for ThreadSafeLayoutNodeChildrenIterator<ConcreteNod
 
 /// A wrapper around elements that ensures layout can only
 /// ever access safe properties and cannot race on elements.
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub struct ServoThreadSafeLayoutElement<'le> {
     element: &'le Element,
 }
diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs
index 17d948cc8ca..bc64480b424 100644
--- a/components/script_layout_interface/wrapper_traits.rs
+++ b/components/script_layout_interface/wrapper_traits.rs
@@ -10,6 +10,7 @@ use gfx_traits::{ByteIndex, LayerId, LayerType};
 use msg::constellation_msg::PipelineId;
 use range::Range;
 use restyle_damage::RestyleDamage;
+use std::fmt::Debug;
 use std::sync::Arc;
 use string_cache::{Atom, Namespace};
 use style::computed_values::display;
@@ -350,7 +351,7 @@ pub trait DangerousThreadSafeLayoutNode: ThreadSafeLayoutNode {
     unsafe fn dangerous_next_sibling(&self) -> Option<Self>;
 }
 
-pub trait ThreadSafeLayoutElement: Clone + Copy + Sized +
+pub trait ThreadSafeLayoutElement: Clone + Copy + Sized + Debug +
                                    ::selectors::Element<Impl=ServoSelectorImpl> +
                                    PresentationalHintsSynthetizer {
     type ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode<ConcreteThreadSafeLayoutElement = Self>;
diff --git a/components/style/dom.rs b/components/style/dom.rs
index 184e926ef40..bb96b45e9fd 100644
--- a/components/style/dom.rs
+++ b/components/style/dom.rs
@@ -201,7 +201,7 @@ pub trait PresentationalHintsSynthetizer {
         where V: Push<DeclarationBlock<Vec<PropertyDeclaration>>>;
 }
 
-pub trait TElement : PartialEq + Sized + Copy + Clone + ElementExt + PresentationalHintsSynthetizer {
+pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + PresentationalHintsSynthetizer {
     type ConcreteNode: TNode<ConcreteElement = Self, ConcreteDocument = Self::ConcreteDocument>;
     type ConcreteDocument: TDocument<ConcreteNode = Self::ConcreteNode, ConcreteElement = Self>;
 
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index d971b1c6eab..e42f6ce09d8 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -803,7 +803,7 @@ impl<T: ToCss> ToCss for DeclaredValue<T> {
     }
 }
 
-#[derive(PartialEq, Clone, Debug)]
+#[derive(PartialEq, Clone)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum PropertyDeclaration {
     % for property in data.longhands:
@@ -867,6 +867,24 @@ impl fmt::Display for PropertyDeclarationName {
         }
     }
 }
+
+impl fmt::Debug for PropertyDeclaration {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        try!(write!(f, "{}: ", self.name()));
+        match *self {
+            % for property in data.longhands:
+                % if not property.derived_from:
+                    PropertyDeclaration::${property.camel_case}(ref value) => value.to_css(f),
+                % endif
+            % endfor
+            PropertyDeclaration::Custom(_, ref value) => value.to_css(f),
+            % if any(property.derived_from for property in data.longhands):
+                _ => Err(fmt::Error),
+            % endif
+        }
+    }
+}
+
 impl ToCss for PropertyDeclaration {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         match *self {
diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs
index a793fec1ff4..e4b1f3cdc52 100644
--- a/components/style/selector_matching.rs
+++ b/components/style/selector_matching.rs
@@ -21,6 +21,7 @@ use selectors::parser::Selector;
 use sink::Push;
 use smallvec::VecLike;
 use std::collections::HashMap;
+use std::fmt;
 use std::hash::BuildHasherDefault;
 use std::sync::Arc;
 use string_cache::Atom;
@@ -275,6 +276,7 @@ impl Stylist {
                                                   parent: &Arc<ComputedValues>)
                                                   -> Option<Arc<ComputedValues>>
         where E: Element<Impl=TheSelectorImpl> +
+              Debug +
               PresentationalHintsSynthetizer
     {
         debug_assert!(TheSelectorImpl::pseudo_element_cascade_type(pseudo).is_lazy());
@@ -343,6 +345,7 @@ impl Stylist {
                                         pseudo_element: Option<&PseudoElement>,
                                         applicable_declarations: &mut V) -> StyleRelations
         where E: Element<Impl=TheSelectorImpl> +
+                                                 fmt::Debug +
                  PresentationalHintsSynthetizer,
               V: Push<DeclarationBlock> + VecLike<DeclarationBlock>
     {
diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs
index 860173820d6..ab0d27a5d85 100644
--- a/ports/geckolib/wrapper.rs
+++ b/ports/geckolib/wrapper.rs
@@ -34,6 +34,7 @@ use selectors::matching::DeclarationBlock;
 use selectors::parser::{AttrSelector, NamespaceConstraint};
 use snapshot::GeckoElementSnapshot;
 use snapshot_helpers;
+use std::fmt;
 use std::marker::PhantomData;
 use std::ops::BitOr;
 use std::ptr;
@@ -382,6 +383,16 @@ pub struct GeckoElement<'le> {
     chain: PhantomData<&'le ()>,
 }
 
+impl<'le> fmt::Debug for GeckoElement<'le> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        try!(write!(f, "<{}", self.get_local_name()));
+        if let Some(id) = self.get_id() {
+            try!(write!(f, " id={}", id));
+        }
+        write!(f, ">")
+    }
+}
+
 impl<'le> GeckoElement<'le> {
     pub unsafe fn from_raw(el: *mut RawGeckoElement) -> GeckoElement<'le> {
         GeckoElement {

From 4062899fd8ff98e54a683f79e6d491bd916d841e Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Thu, 18 Aug 2016 18:54:36 +0200
Subject: [PATCH 08/15] Import SelectorMap back from the selectors crate.

Nobody else uses it, and I want to make breaking changes to it.
---
 components/script/dom/element.rs              |   9 +-
 components/script/layout_wrapper.rs           |   9 +-
 components/servo/Cargo.lock                   |   1 +
 components/style/Cargo.toml                   |   1 +
 components/style/animation.rs                 |   2 +-
 components/style/dom.rs                       |   6 +-
 components/style/lib.rs                       |   1 +
 .../style/properties/properties.mako.rs       |   6 +-
 components/style/selector_matching.rs         | 311 +++++++++++++++++-
 docs/components/style.md                      |   4 +-
 ports/cef/Cargo.lock                          |   1 +
 ports/geckolib/Cargo.lock                     |   1 +
 ports/geckolib/wrapper.rs                     |   4 +-
 13 files changed, 332 insertions(+), 24 deletions(-)

diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 2972a9465f4..5141c8d5adf 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -71,7 +71,7 @@ use html5ever::serialize::TraversalScope;
 use html5ever::serialize::TraversalScope::{ChildrenOnly, IncludeNode};
 use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks};
 use ref_filter_map::ref_filter_map;
-use selectors::matching::{DeclarationBlock, ElementFlags, matches};
+use selectors::matching::{ElementFlags, matches};
 use selectors::matching::{HAS_SLOW_SELECTOR, HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS};
 use selectors::parser::{AttrSelector, NamespaceConstraint, parse_author_origin_selector_list_from_str};
 use std::ascii::AsciiExt;
@@ -92,6 +92,7 @@ use style::properties::longhands::{self, background_image, border_spacing, font_
 use style::properties::{DeclaredValue, Importance};
 use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, parse_style_attribute};
 use style::selector_impl::{NonTSPseudoClass, ServoSelectorImpl};
+use style::selector_matching::DeclarationBlock;
 use style::sink::Push;
 use style::values::CSSFloat;
 use style::values::specified::{self, CSSColor, CSSRGBA, LengthOrPercentage};
@@ -291,7 +292,7 @@ pub trait LayoutElementHelpers {
 
     #[allow(unsafe_code)]
     unsafe fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, &mut V)
-        where V: Push<DeclarationBlock<Vec<PropertyDeclaration>>>;
+        where V: Push<DeclarationBlock>;
     #[allow(unsafe_code)]
     unsafe fn get_colspan(self) -> u32;
     #[allow(unsafe_code)]
@@ -324,10 +325,10 @@ impl LayoutElementHelpers for LayoutJS<Element> {
 
     #[allow(unsafe_code)]
     unsafe fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, hints: &mut V)
-        where V: Push<DeclarationBlock<Vec<PropertyDeclaration>>>
+        where V: Push<DeclarationBlock>
     {
         #[inline]
-        fn from_declaration(rule: PropertyDeclaration) -> DeclarationBlock<Vec<PropertyDeclaration>> {
+        fn from_declaration(rule: PropertyDeclaration) -> DeclarationBlock {
             DeclarationBlock::from_declarations(Arc::new(vec![rule]))
         }
 
diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs
index 431fe55f9b0..b4e54fe5bd9 100644
--- a/components/script/layout_wrapper.rs
+++ b/components/script/layout_wrapper.rs
@@ -47,7 +47,7 @@ use script_layout_interface::wrapper_traits::{DangerousThreadSafeLayoutNode, Lay
 use script_layout_interface::wrapper_traits::{ThreadSafeLayoutNode, ThreadSafeLayoutElement};
 use script_layout_interface::{HTMLCanvasData, LayoutNodeType, TrustedNodeAddress};
 use script_layout_interface::{OpaqueStyleAndLayoutData, PartialStyleAndLayoutData};
-use selectors::matching::{DeclarationBlock, ElementFlags};
+use selectors::matching::ElementFlags;
 use selectors::parser::{AttrSelector, NamespaceConstraint};
 use std::fmt;
 use std::marker::PhantomData;
@@ -60,9 +60,10 @@ use style::context::SharedStyleContext;
 use style::data::PrivateStyleData;
 use style::dom::{PresentationalHintsSynthetizer, OpaqueNode, TDocument, TElement, TNode, UnsafeNode};
 use style::element_state::*;
-use style::properties::{ComputedValues, PropertyDeclaration, PropertyDeclarationBlock};
+use style::properties::{ComputedValues, PropertyDeclarationBlock};
 use style::refcell::{Ref, RefCell, RefMut};
 use style::selector_impl::{ElementSnapshot, NonTSPseudoClass, PseudoElement, ServoSelectorImpl};
+use style::selector_matching::DeclarationBlock;
 use style::sink::Push;
 use style::str::is_whitespace;
 use url::Url;
@@ -418,7 +419,7 @@ impl<'le> fmt::Debug for ServoLayoutElement<'le> {
 
 impl<'le> PresentationalHintsSynthetizer for ServoLayoutElement<'le> {
     fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, hints: &mut V)
-        where V: Push<DeclarationBlock<Vec<PropertyDeclaration>>>
+        where V: Push<DeclarationBlock>
     {
         unsafe {
             self.element.synthesize_presentational_hints_for_legacy_attributes(hints);
@@ -1070,5 +1071,5 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> {
 
 impl<'le> PresentationalHintsSynthetizer for ServoThreadSafeLayoutElement<'le> {
     fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, _hints: &mut V)
-        where V: Push<DeclarationBlock<Vec<PropertyDeclaration>>> {}
+        where V: Push<DeclarationBlock> {}
 }
diff --git a/components/servo/Cargo.lock b/components/servo/Cargo.lock
index cff42fe802f..fef485423eb 100644
--- a/components/servo/Cargo.lock
+++ b/components/servo/Cargo.lock
@@ -2236,6 +2236,7 @@ dependencies = [
  "num-traits 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)",
  "ordered-float 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "plugins 0.0.1",
+ "quickersort 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)",
  "selectors 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml
index 76e6c8728c1..f19a0b3381b 100644
--- a/components/style/Cargo.toml
+++ b/components/style/Cargo.toml
@@ -36,6 +36,7 @@ log = "0.3.5"
 matches = "0.1"
 num-traits = "0.1.32"
 ordered-float = "0.2.2"
+quickersort = "2.0.0"
 rand = "0.3"
 rustc-serialize = "0.3"
 selectors = "0.11"
diff --git a/components/style/animation.rs b/components/style/animation.rs
index 63ddc9e8c69..2764511c7f3 100644
--- a/components/style/animation.rs
+++ b/components/style/animation.rs
@@ -16,7 +16,7 @@ use properties::longhands::animation_play_state::computed_value::AnimationPlaySt
 use properties::longhands::transition_timing_function::computed_value::StartEnd;
 use properties::longhands::transition_timing_function::computed_value::TransitionTimingFunction;
 use properties::{self, ComputedValues};
-use selectors::matching::DeclarationBlock;
+use selector_matching::DeclarationBlock;
 use std::sync::Arc;
 use std::sync::mpsc::Sender;
 use string_cache::Atom;
diff --git a/components/style/dom.rs b/components/style/dom.rs
index bb96b45e9fd..3b0940c59ef 100644
--- a/components/style/dom.rs
+++ b/components/style/dom.rs
@@ -9,11 +9,11 @@
 use context::SharedStyleContext;
 use data::PrivateStyleData;
 use element_state::ElementState;
-use properties::{ComputedValues, PropertyDeclaration, PropertyDeclarationBlock};
+use properties::{ComputedValues, PropertyDeclarationBlock};
 use refcell::{Ref, RefMut};
 use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint};
 use selector_impl::{ElementExt, PseudoElement};
-use selectors::matching::DeclarationBlock;
+use selector_matching::DeclarationBlock;
 use sink::Push;
 use std::fmt::Debug;
 use std::ops::BitOr;
@@ -198,7 +198,7 @@ pub trait TDocument : Sized + Copy + Clone {
 
 pub trait PresentationalHintsSynthetizer {
     fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, hints: &mut V)
-        where V: Push<DeclarationBlock<Vec<PropertyDeclaration>>>;
+        where V: Push<DeclarationBlock>;
 }
 
 pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + PresentationalHintsSynthetizer {
diff --git a/components/style/lib.rs b/components/style/lib.rs
index 3869b7867e9..3386f812186 100644
--- a/components/style/lib.rs
+++ b/components/style/lib.rs
@@ -58,6 +58,7 @@ extern crate log;
 extern crate matches;
 extern crate num_traits;
 extern crate ordered_float;
+extern crate quickersort;
 extern crate rand;
 extern crate rustc_serialize;
 extern crate selectors;
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index e42f6ce09d8..ff96363127b 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -31,7 +31,7 @@ use computed_values;
 #[cfg(feature = "servo")] use logical_geometry::{LogicalMargin, PhysicalSide};
 use logical_geometry::WritingMode;
 use parser::{ParserContext, ParserContextExtraData, log_css_error};
-use selectors::matching::DeclarationBlock;
+use selector_matching::DeclarationBlock;
 use stylesheets::Origin;
 use values::LocalToCss;
 use values::HasViewportPercentage;
@@ -1670,7 +1670,7 @@ mod lazy_static_module {
 #[allow(unused_mut, unused_imports)]
 fn cascade_with_cached_declarations(
         viewport_size: Size2D<Au>,
-        applicable_declarations: &[DeclarationBlock<Vec<PropertyDeclaration>>],
+        applicable_declarations: &[DeclarationBlock],
         shareable: bool,
         parent_style: &ComputedValues,
         cached_style: &ComputedValues,
@@ -1815,7 +1815,7 @@ static CASCADE_PROPERTY: [CascadePropertyFn; ${len(data.longhands)}] = [
 ///
 /// Returns the computed values and a boolean indicating whether the result is cacheable.
 pub fn cascade(viewport_size: Size2D<Au>,
-               applicable_declarations: &[DeclarationBlock<Vec<PropertyDeclaration>>],
+               applicable_declarations: &[DeclarationBlock],
                shareable: bool,
                parent_style: Option<<&ComputedValues>,
                cached_style: Option<<&ComputedValues>,
diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs
index e4b1f3cdc52..f6ea546e2e5 100644
--- a/components/style/selector_matching.rs
+++ b/components/style/selector_matching.rs
@@ -10,19 +10,22 @@ use error_reporting::StdoutErrorReporter;
 use keyframes::KeyframesAnimation;
 use media_queries::{Device, MediaType};
 use properties::{self, PropertyDeclaration, PropertyDeclarationBlock, ComputedValues};
+use quickersort::sort_by;
 use restyle_hints::{RestyleHint, DependencySet};
 use selector_impl::{ElementExt, TheSelectorImpl, PseudoElement};
 use selectors::Element;
 use selectors::bloom::BloomFilter;
-use selectors::matching::DeclarationBlock as GenericDeclarationBlock;
 use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS};
-use selectors::matching::{Rule, SelectorMap, StyleRelations};
-use selectors::parser::Selector;
+use selectors::matching::{StyleRelations, matches_compound_selector};
+use selectors::parser::{Selector, SelectorImpl, SimpleSelector, LocalName, CompoundSelector};
 use sink::Push;
 use smallvec::VecLike;
+use std::borrow::Borrow;
+use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::fmt;
 use std::hash::BuildHasherDefault;
+use std::hash::Hash;
 use std::sync::Arc;
 use string_cache::Atom;
 use style_traits::viewport::ViewportConstraints;
@@ -276,7 +279,7 @@ impl Stylist {
                                                   parent: &Arc<ComputedValues>)
                                                   -> Option<Arc<ComputedValues>>
         where E: Element<Impl=TheSelectorImpl> +
-              Debug +
+              fmt::Debug +
               PresentationalHintsSynthetizer
     {
         debug_assert!(TheSelectorImpl::pseudo_element_cascade_type(pseudo).is_lazy());
@@ -345,7 +348,7 @@ impl Stylist {
                                         pseudo_element: Option<&PseudoElement>,
                                         applicable_declarations: &mut V) -> StyleRelations
         where E: Element<Impl=TheSelectorImpl> +
-                                                 fmt::Debug +
+                 fmt::Debug +
                  PresentationalHintsSynthetizer,
               V: Push<DeclarationBlock> + VecLike<DeclarationBlock>
     {
@@ -577,3 +580,301 @@ impl PerPseudoElementSelectorMap {
         }
     }
 }
+
+/// Map element data to Rules whose last simple selector starts with them.
+///
+/// e.g.,
+/// "p > img" would go into the set of Rules corresponding to the
+/// element "img"
+/// "a .foo .bar.baz" would go into the set of Rules corresponding to
+/// the class "bar"
+///
+/// Because we match Rules right-to-left (i.e., moving up the tree
+/// from an element), we need to compare the last simple selector in the
+/// Rule with the element.
+///
+/// So, if an element has ID "id1" and classes "foo" and "bar", then all
+/// the rules it matches will have their last simple selector starting
+/// either with "#id1" or with ".foo" or with ".bar".
+///
+/// Hence, the union of the rules keyed on each of element's classes, ID,
+/// element name, etc. will contain the Rules that actually match that
+/// element.
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+pub struct SelectorMap<T, Impl: SelectorImpl> {
+    // TODO: Tune the initial capacity of the HashMap
+    id_hash: HashMap<Impl::Identifier, Vec<Rule<T, Impl>>>,
+    class_hash: HashMap<Impl::ClassName, Vec<Rule<T, Impl>>>,
+    local_name_hash: HashMap<Impl::LocalName, Vec<Rule<T, Impl>>>,
+    /// Same as local_name_hash, but keys are lower-cased.
+    /// For HTML elements in HTML documents.
+    lower_local_name_hash: HashMap<Impl::LocalName, Vec<Rule<T, Impl>>>,
+    /// Rules that don't have ID, class, or element selectors.
+    other_rules: Vec<Rule<T, Impl>>,
+    /// Whether this hash is empty.
+    empty: bool,
+}
+
+#[inline]
+fn compare<T>(a: &GenericDeclarationBlock<T>, b: &GenericDeclarationBlock<T>) -> Ordering {
+    (a.specificity, a.source_order).cmp(&(b.specificity, b.source_order))
+}
+
+impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
+    pub fn new() -> SelectorMap<T, Impl> {
+        SelectorMap {
+            id_hash: HashMap::default(),
+            class_hash: HashMap::default(),
+            local_name_hash: HashMap::default(),
+            lower_local_name_hash: HashMap::default(),
+            other_rules: vec!(),
+            empty: true,
+        }
+    }
+
+    /// Append to `rule_list` all Rules in `self` that match element.
+    ///
+    /// Extract matching rules as per element's ID, classes, tag name, etc..
+    /// Sort the Rules at the end to maintain cascading order.
+    pub fn get_all_matching_rules<E, V>(&self,
+                                        element: &E,
+                                        parent_bf: Option<&BloomFilter>,
+                                        matching_rules_list: &mut V,
+                                        relations: &mut StyleRelations)
+        where E: Element<Impl=Impl>,
+              V: VecLike<GenericDeclarationBlock<T>>
+    {
+        if self.empty {
+            return
+        }
+
+        // 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 let Some(id) = element.get_id() {
+            SelectorMap::get_matching_rules_from_hash(element,
+                                                      parent_bf,
+                                                      &self.id_hash,
+                                                      &id,
+                                                      matching_rules_list,
+                                                      relations)
+        }
+
+        element.each_class(|class| {
+            SelectorMap::get_matching_rules_from_hash(element,
+                                                      parent_bf,
+                                                      &self.class_hash,
+                                                      class,
+                                                      matching_rules_list,
+                                                      relations);
+        });
+
+        let local_name_hash = if element.is_html_element_in_html_document() {
+            &self.lower_local_name_hash
+        } else {
+            &self.local_name_hash
+        };
+        SelectorMap::get_matching_rules_from_hash(element,
+                                                  parent_bf,
+                                                  local_name_hash,
+                                                  &element.get_local_name(),
+                                                  matching_rules_list,
+                                                  relations);
+
+        SelectorMap::get_matching_rules(element,
+                                        parent_bf,
+                                        &self.other_rules,
+                                        matching_rules_list,
+                                        relations);
+
+        // Sort only the rules we just added.
+        sort_by(&mut matching_rules_list[init_len..], &compare);
+    }
+
+    /// Append to `rule_list` all universal Rules (rules with selector `*|*`) in
+    /// `self` sorted by specifity and source order.
+    pub fn get_universal_rules<V>(&self,
+                                  matching_rules_list: &mut V)
+        where V: VecLike<GenericDeclarationBlock<T>>
+    {
+        if self.empty {
+            return
+        }
+
+        let init_len = matching_rules_list.len();
+
+        for rule in self.other_rules.iter() {
+            if rule.selector.simple_selectors.is_empty() &&
+               rule.selector.next.is_none() {
+                matching_rules_list.push(rule.declarations.clone());
+            }
+        }
+
+        sort_by(&mut matching_rules_list[init_len..], &compare);
+    }
+
+    fn get_matching_rules_from_hash<E, Str, BorrowedStr: ?Sized, Vector>(
+        element: &E,
+        parent_bf: Option<&BloomFilter>,
+        hash: &HashMap<Str, Vec<Rule<T, Impl>>>,
+        key: &BorrowedStr,
+        matching_rules: &mut Vector,
+        relations: &mut StyleRelations)
+        where E: Element<Impl=Impl>,
+              Str: Borrow<BorrowedStr> + Eq + Hash,
+              BorrowedStr: Eq + Hash,
+              Vector: VecLike<GenericDeclarationBlock<T>>
+    {
+        if let Some(rules) = hash.get(key) {
+            SelectorMap::get_matching_rules(element,
+                                            parent_bf,
+                                            rules,
+                                            matching_rules,
+                                            relations)
+        }
+    }
+
+    /// Adds rules in `rules` that match `element` to the `matching_rules` list.
+    fn get_matching_rules<E, V>(element: &E,
+                                parent_bf: Option<&BloomFilter>,
+                                rules: &[Rule<T, Impl>],
+                                matching_rules: &mut V,
+                                relations: &mut StyleRelations)
+        where E: Element<Impl=Impl>,
+              V: VecLike<GenericDeclarationBlock<T>>
+    {
+        for rule in rules.iter() {
+            if matches_compound_selector(&*rule.selector,
+                                         element, parent_bf, relations) {
+                matching_rules.push(rule.declarations.clone());
+            }
+        }
+    }
+
+    /// Insert rule into the correct hash.
+    /// Order in which to try: id_hash, class_hash, local_name_hash, other_rules.
+    pub fn insert(&mut self, rule: Rule<T, Impl>) {
+        self.empty = false;
+
+        if let Some(id_name) = SelectorMap::get_id_name(&rule) {
+            find_push(&mut self.id_hash, id_name, rule);
+            return;
+        }
+
+        if let Some(class_name) = SelectorMap::get_class_name(&rule) {
+            find_push(&mut self.class_hash, class_name, rule);
+            return;
+        }
+
+        if let Some(LocalName { name, lower_name }) = SelectorMap::get_local_name(&rule) {
+            find_push(&mut self.local_name_hash, name, rule.clone());
+            find_push(&mut self.lower_local_name_hash, lower_name, rule);
+            return;
+        }
+
+        self.other_rules.push(rule);
+    }
+
+    /// Retrieve the first ID name in Rule, or None otherwise.
+    fn get_id_name(rule: &Rule<T, Impl>) -> Option<Impl::Identifier> {
+        for ss in &rule.selector.simple_selectors {
+            // TODO(pradeep): Implement case-sensitivity based on the
+            // document type and quirks mode.
+            if let SimpleSelector::ID(ref id) = *ss {
+                return Some(id.clone());
+            }
+        }
+
+        None
+    }
+
+    /// Retrieve the FIRST class name in Rule, or None otherwise.
+    fn get_class_name(rule: &Rule<T, Impl>) -> Option<Impl::ClassName> {
+        for ss in &rule.selector.simple_selectors {
+            // TODO(pradeep): Implement case-sensitivity based on the
+            // document type and quirks mode.
+            if let SimpleSelector::Class(ref class) = *ss {
+                return Some(class.clone());
+            }
+        }
+
+        None
+    }
+
+    /// Retrieve the name if it is a type selector, or None otherwise.
+    fn get_local_name(rule: &Rule<T, Impl>) -> Option<LocalName<Impl>> {
+        for ss in &rule.selector.simple_selectors {
+            if let SimpleSelector::LocalName(ref n) = *ss {
+                return Some(LocalName {
+                    name: n.name.clone(),
+                    lower_name: n.lower_name.clone(),
+                })
+            }
+        }
+
+        None
+    }
+}
+
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+pub struct Rule<T, Impl: SelectorImpl> {
+    // This is an Arc because Rule will essentially be cloned for every element
+    // that it matches. Selector contains an owned vector (through
+    // CompoundSelector) and we want to avoid the allocation.
+    pub selector: Arc<CompoundSelector<Impl>>,
+    pub declarations: GenericDeclarationBlock<T>,
+}
+
+/// A property declaration together with its precedence among rules of equal specificity so that
+/// we can sort them.
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Debug)]
+pub struct GenericDeclarationBlock<T> {
+    pub declarations: Arc<T>,
+    pub source_order: usize,
+    pub specificity: u32,
+}
+
+// FIXME(https://github.com/rust-lang/rust/issues/7671)
+// derive(Clone) requires T: Clone, even though Arc<T>: T regardless.
+impl<T> Clone for GenericDeclarationBlock<T> {
+    fn clone(&self) -> Self {
+        GenericDeclarationBlock {
+            declarations: self.declarations.clone(),
+            source_order: self.source_order,
+            specificity: self.specificity,
+        }
+    }
+}
+
+// FIXME(https://github.com/rust-lang/rust/issues/7671)
+impl<T, Impl: SelectorImpl> Clone for Rule<T, Impl> {
+    fn clone(&self) -> Rule<T, Impl> {
+        Rule {
+            selector: self.selector.clone(),
+            declarations: self.declarations.clone(),
+        }
+    }
+}
+
+impl<T> GenericDeclarationBlock<T> {
+    #[inline]
+    pub fn from_declarations(declarations: Arc<T>) -> Self {
+        GenericDeclarationBlock {
+            declarations: declarations,
+            source_order: 0,
+            specificity: 0,
+        }
+    }
+}
+
+fn find_push<T, Impl: SelectorImpl, Str: Eq + Hash>(
+    map: &mut HashMap<Str, Vec<Rule<T, Impl>>>,
+    key: Str,
+    value: Rule<T, Impl>
+) {
+    if let Some(vec) = map.get_mut(&key) {
+        vec.push(value);
+        return
+    }
+    map.insert(key, vec![value]);
+}
diff --git a/docs/components/style.md b/docs/components/style.md
index 0bf82713de1..a4a83d4e8f2 100644
--- a/docs/components/style.md
+++ b/docs/components/style.md
@@ -149,8 +149,8 @@ that you didn't find it here so it can be added :)
 [mdn-pseudo-after]: https://developer.mozilla.org/en/docs/Web/CSS/::after
 [mdn-pseudo-selection]: https://developer.mozilla.org/en/docs/Web/CSS/::selection
 [stylist]: http://doc.servo.org/style/selector_matching/struct.Stylist.html
-[selectors-selectormap]: http://doc.servo.org/selectors/matching/struct.SelectorMap.html
-[selectors-rule]: http://doc.servo.org/selectors/matching/struct.Rule.html
+[selectors-selectormap]: http://doc.servo.org/style/selector_matching/struct.SelectorMap.html
+[selectors-rule]: http://doc.servo.org/style/selector_matching/struct.Rule.html
 [per-pseudo-selectormap]: http://doc.servo.org/style/selector_matching/struct.PerPseudoElementSelectorMap.html
 [per-origin-selectormap]: http://doc.servo.org/style/selector_matching/struct.PerOriginSelectorMap.html
 [docs-pipeline]: https://github.com/servo/servo/blob/master/docs/glossary.md#pipeline
diff --git a/ports/cef/Cargo.lock b/ports/cef/Cargo.lock
index 9991d9f7905..9ff1099e997 100644
--- a/ports/cef/Cargo.lock
+++ b/ports/cef/Cargo.lock
@@ -2119,6 +2119,7 @@ dependencies = [
  "num-traits 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)",
  "ordered-float 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "plugins 0.0.1",
+ "quickersort 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)",
  "selectors 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/ports/geckolib/Cargo.lock b/ports/geckolib/Cargo.lock
index 3caf13d5c08..01001e5f8b7 100644
--- a/ports/geckolib/Cargo.lock
+++ b/ports/geckolib/Cargo.lock
@@ -365,6 +365,7 @@ dependencies = [
  "matches 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "num-traits 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)",
  "ordered-float 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "quickersort 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)",
  "selectors 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs
index ab0d27a5d85..551e7999154 100644
--- a/ports/geckolib/wrapper.rs
+++ b/ports/geckolib/wrapper.rs
@@ -30,7 +30,6 @@ use gecko_string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 use glue::GeckoDeclarationBlock;
 use libc::uintptr_t;
 use selectors::Element;
-use selectors::matching::DeclarationBlock;
 use selectors::parser::{AttrSelector, NamespaceConstraint};
 use snapshot::GeckoElementSnapshot;
 use snapshot_helpers;
@@ -50,6 +49,7 @@ use style::properties::{ComputedValues, parse_style_attribute};
 use style::properties::{PropertyDeclaration, PropertyDeclarationBlock};
 use style::refcell::{Ref, RefCell, RefMut};
 use style::selector_impl::ElementExt;
+use style::selector_matching::DeclarationBlock;
 use style::sink::Push;
 use url::Url;
 
@@ -473,7 +473,7 @@ impl<'le> PartialEq for GeckoElement<'le> {
 
 impl<'le> PresentationalHintsSynthetizer for GeckoElement<'le> {
     fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, _hints: &mut V)
-        where V: Push<DeclarationBlock<Vec<PropertyDeclaration>>>
+        where V: Push<DeclarationBlock>
     {
         // FIXME(bholley) - Need to implement this.
     }

From 35b01cd324c5a491e079f8fd2a3685f9b252b28a Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Fri, 19 Aug 2016 00:22:22 +0200
Subject: [PATCH 09/15] Sort rules by key in selector matching

---
 components/style/selector_matching.rs | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs
index f6ea546e2e5..037f6522eb9 100644
--- a/components/style/selector_matching.rs
+++ b/components/style/selector_matching.rs
@@ -21,7 +21,6 @@ use selectors::parser::{Selector, SelectorImpl, SimpleSelector, LocalName, Compo
 use sink::Push;
 use smallvec::VecLike;
 use std::borrow::Borrow;
-use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::fmt;
 use std::hash::BuildHasherDefault;
@@ -616,8 +615,8 @@ pub struct SelectorMap<T, Impl: SelectorImpl> {
 }
 
 #[inline]
-fn compare<T>(a: &GenericDeclarationBlock<T>, b: &GenericDeclarationBlock<T>) -> Ordering {
-    (a.specificity, a.source_order).cmp(&(b.specificity, b.source_order))
+fn sort_by_key<T, F: Fn(&T) -> K, K: Ord>(v: &mut [T], f: F) {
+    sort_by(v, &|a, b| f(a).cmp(&f(b)))
 }
 
 impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
@@ -687,7 +686,8 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
                                         relations);
 
         // Sort only the rules we just added.
-        sort_by(&mut matching_rules_list[init_len..], &compare);
+        sort_by_key(&mut matching_rules_list[init_len..],
+                    |rule| (rule.specificity, rule.source_order));
     }
 
     /// Append to `rule_list` all universal Rules (rules with selector `*|*`) in
@@ -709,7 +709,8 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
             }
         }
 
-        sort_by(&mut matching_rules_list[init_len..], &compare);
+        sort_by_key(&mut matching_rules_list[init_len..],
+                    |rule| (rule.specificity, rule.source_order));
     }
 
     fn get_matching_rules_from_hash<E, Str, BorrowedStr: ?Sized, Vector>(

From 577d2dc3fac727e5ea3f79db3172e93bb260e998 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Fri, 19 Aug 2016 00:42:44 +0200
Subject: [PATCH 10/15] Make SelectorMap and friends concrete

---
 components/style/selector_matching.rs | 111 ++++++++++----------------
 1 file changed, 40 insertions(+), 71 deletions(-)

diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs
index 037f6522eb9..775f81915d4 100644
--- a/components/style/selector_matching.rs
+++ b/components/style/selector_matching.rs
@@ -17,7 +17,7 @@ use selectors::Element;
 use selectors::bloom::BloomFilter;
 use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS};
 use selectors::matching::{StyleRelations, matches_compound_selector};
-use selectors::parser::{Selector, SelectorImpl, SimpleSelector, LocalName, CompoundSelector};
+use selectors::parser::{Selector, SimpleSelector, LocalName, CompoundSelector};
 use sink::Push;
 use smallvec::VecLike;
 use std::borrow::Borrow;
@@ -31,8 +31,6 @@ use style_traits::viewport::ViewportConstraints;
 use stylesheets::{CSSRule, CSSRuleIteratorExt, Origin, Stylesheet};
 use viewport::{MaybeNew, ViewportRuleCascade};
 
-pub type DeclarationBlock = GenericDeclarationBlock<Vec<PropertyDeclaration>>;
-
 /// This structure holds all the selectors and device characteristics
 /// for a given document. The selectors are converted into `Rule`s
 /// (defined in rust-selectors), and introduced in a `SelectorMap`
@@ -399,7 +397,7 @@ impl Stylist {
             relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
             Push::push(
                 applicable_declarations,
-                GenericDeclarationBlock::from_declarations(sa.normal.clone()));
+                DeclarationBlock::from_declarations(sa.normal.clone()));
         }
 
         debug!("style attr: {:?}", relations);
@@ -416,7 +414,7 @@ impl Stylist {
         if let Some(ref sa) = style_attribute {
             Push::push(
                 applicable_declarations,
-                GenericDeclarationBlock::from_declarations(sa.important.clone()));
+                DeclarationBlock::from_declarations(sa.important.clone()));
         }
 
         debug!("style attr important: {:?}", relations);
@@ -532,10 +530,10 @@ impl Stylist {
 struct PerOriginSelectorMap {
     /// Rules that contains at least one property declaration with
     /// normal importance.
-    normal: SelectorMap<Vec<PropertyDeclaration>, TheSelectorImpl>,
+    normal: SelectorMap,
     /// Rules that contains at least one property declaration with
     /// !important.
-    important: SelectorMap<Vec<PropertyDeclaration>, TheSelectorImpl>,
+    important: SelectorMap,
 }
 
 impl PerOriginSelectorMap {
@@ -600,16 +598,16 @@ impl PerPseudoElementSelectorMap {
 /// element name, etc. will contain the Rules that actually match that
 /// element.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-pub struct SelectorMap<T, Impl: SelectorImpl> {
+pub struct SelectorMap {
     // TODO: Tune the initial capacity of the HashMap
-    id_hash: HashMap<Impl::Identifier, Vec<Rule<T, Impl>>>,
-    class_hash: HashMap<Impl::ClassName, Vec<Rule<T, Impl>>>,
-    local_name_hash: HashMap<Impl::LocalName, Vec<Rule<T, Impl>>>,
+    id_hash: HashMap<Atom, Vec<Rule>>,
+    class_hash: HashMap<Atom, Vec<Rule>>,
+    local_name_hash: HashMap<Atom, Vec<Rule>>,
     /// Same as local_name_hash, but keys are lower-cased.
     /// For HTML elements in HTML documents.
-    lower_local_name_hash: HashMap<Impl::LocalName, Vec<Rule<T, Impl>>>,
+    lower_local_name_hash: HashMap<Atom, Vec<Rule>>,
     /// Rules that don't have ID, class, or element selectors.
-    other_rules: Vec<Rule<T, Impl>>,
+    other_rules: Vec<Rule>,
     /// Whether this hash is empty.
     empty: bool,
 }
@@ -619,14 +617,14 @@ fn sort_by_key<T, F: Fn(&T) -> K, K: Ord>(v: &mut [T], f: F) {
     sort_by(v, &|a, b| f(a).cmp(&f(b)))
 }
 
-impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
-    pub fn new() -> SelectorMap<T, Impl> {
+impl SelectorMap {
+    pub fn new() -> Self {
         SelectorMap {
             id_hash: HashMap::default(),
             class_hash: HashMap::default(),
             local_name_hash: HashMap::default(),
             lower_local_name_hash: HashMap::default(),
-            other_rules: vec!(),
+            other_rules: Vec::new(),
             empty: true,
         }
     }
@@ -640,8 +638,8 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
                                         parent_bf: Option<&BloomFilter>,
                                         matching_rules_list: &mut V,
                                         relations: &mut StyleRelations)
-        where E: Element<Impl=Impl>,
-              V: VecLike<GenericDeclarationBlock<T>>
+        where E: Element<Impl=TheSelectorImpl>,
+              V: VecLike<DeclarationBlock>
     {
         if self.empty {
             return
@@ -675,7 +673,7 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
         SelectorMap::get_matching_rules_from_hash(element,
                                                   parent_bf,
                                                   local_name_hash,
-                                                  &element.get_local_name(),
+                                                  element.get_local_name(),
                                                   matching_rules_list,
                                                   relations);
 
@@ -694,7 +692,7 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
     /// `self` sorted by specifity and source order.
     pub fn get_universal_rules<V>(&self,
                                   matching_rules_list: &mut V)
-        where V: VecLike<GenericDeclarationBlock<T>>
+        where V: VecLike<DeclarationBlock>
     {
         if self.empty {
             return
@@ -716,14 +714,14 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
     fn get_matching_rules_from_hash<E, Str, BorrowedStr: ?Sized, Vector>(
         element: &E,
         parent_bf: Option<&BloomFilter>,
-        hash: &HashMap<Str, Vec<Rule<T, Impl>>>,
+        hash: &HashMap<Str, Vec<Rule>>,
         key: &BorrowedStr,
         matching_rules: &mut Vector,
         relations: &mut StyleRelations)
-        where E: Element<Impl=Impl>,
+        where E: Element<Impl=TheSelectorImpl>,
               Str: Borrow<BorrowedStr> + Eq + Hash,
               BorrowedStr: Eq + Hash,
-              Vector: VecLike<GenericDeclarationBlock<T>>
+              Vector: VecLike<DeclarationBlock>
     {
         if let Some(rules) = hash.get(key) {
             SelectorMap::get_matching_rules(element,
@@ -737,11 +735,11 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
     /// Adds rules in `rules` that match `element` to the `matching_rules` list.
     fn get_matching_rules<E, V>(element: &E,
                                 parent_bf: Option<&BloomFilter>,
-                                rules: &[Rule<T, Impl>],
+                                rules: &[Rule],
                                 matching_rules: &mut V,
                                 relations: &mut StyleRelations)
-        where E: Element<Impl=Impl>,
-              V: VecLike<GenericDeclarationBlock<T>>
+        where E: Element<Impl=TheSelectorImpl>,
+              V: VecLike<DeclarationBlock>
     {
         for rule in rules.iter() {
             if matches_compound_selector(&*rule.selector,
@@ -753,7 +751,7 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
 
     /// Insert rule into the correct hash.
     /// Order in which to try: id_hash, class_hash, local_name_hash, other_rules.
-    pub fn insert(&mut self, rule: Rule<T, Impl>) {
+    pub fn insert(&mut self, rule: Rule) {
         self.empty = false;
 
         if let Some(id_name) = SelectorMap::get_id_name(&rule) {
@@ -776,7 +774,7 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
     }
 
     /// Retrieve the first ID name in Rule, or None otherwise.
-    fn get_id_name(rule: &Rule<T, Impl>) -> Option<Impl::Identifier> {
+    fn get_id_name(rule: &Rule) -> Option<Atom> {
         for ss in &rule.selector.simple_selectors {
             // TODO(pradeep): Implement case-sensitivity based on the
             // document type and quirks mode.
@@ -789,7 +787,7 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
     }
 
     /// Retrieve the FIRST class name in Rule, or None otherwise.
-    fn get_class_name(rule: &Rule<T, Impl>) -> Option<Impl::ClassName> {
+    fn get_class_name(rule: &Rule) -> Option<Atom> {
         for ss in &rule.selector.simple_selectors {
             // TODO(pradeep): Implement case-sensitivity based on the
             // document type and quirks mode.
@@ -802,7 +800,7 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
     }
 
     /// Retrieve the name if it is a type selector, or None otherwise.
-    fn get_local_name(rule: &Rule<T, Impl>) -> Option<LocalName<Impl>> {
+    fn get_local_name(rule: &Rule) -> Option<LocalName<TheSelectorImpl>> {
         for ss in &rule.selector.simple_selectors {
             if let SimpleSelector::LocalName(ref n) = *ss {
                 return Some(LocalName {
@@ -817,50 +815,29 @@ impl<T, Impl: SelectorImpl> SelectorMap<T, Impl> {
 }
 
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-pub struct Rule<T, Impl: SelectorImpl> {
+#[derive(Clone)]
+pub struct Rule {
     // This is an Arc because Rule will essentially be cloned for every element
     // that it matches. Selector contains an owned vector (through
     // CompoundSelector) and we want to avoid the allocation.
-    pub selector: Arc<CompoundSelector<Impl>>,
-    pub declarations: GenericDeclarationBlock<T>,
+    pub selector: Arc<CompoundSelector<TheSelectorImpl>>,
+    pub declarations: DeclarationBlock,
 }
 
 /// A property declaration together with its precedence among rules of equal specificity so that
 /// we can sort them.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-#[derive(Debug)]
-pub struct GenericDeclarationBlock<T> {
-    pub declarations: Arc<T>,
+#[derive(Debug, Clone)]
+pub struct DeclarationBlock {
+    pub declarations: Arc<Vec<PropertyDeclaration>>,
     pub source_order: usize,
     pub specificity: u32,
 }
 
-// FIXME(https://github.com/rust-lang/rust/issues/7671)
-// derive(Clone) requires T: Clone, even though Arc<T>: T regardless.
-impl<T> Clone for GenericDeclarationBlock<T> {
-    fn clone(&self) -> Self {
-        GenericDeclarationBlock {
-            declarations: self.declarations.clone(),
-            source_order: self.source_order,
-            specificity: self.specificity,
-        }
-    }
-}
-
-// FIXME(https://github.com/rust-lang/rust/issues/7671)
-impl<T, Impl: SelectorImpl> Clone for Rule<T, Impl> {
-    fn clone(&self) -> Rule<T, Impl> {
-        Rule {
-            selector: self.selector.clone(),
-            declarations: self.declarations.clone(),
-        }
-    }
-}
-
-impl<T> GenericDeclarationBlock<T> {
+impl DeclarationBlock {
     #[inline]
-    pub fn from_declarations(declarations: Arc<T>) -> Self {
-        GenericDeclarationBlock {
+    pub fn from_declarations(declarations: Arc<Vec<PropertyDeclaration>>) -> Self {
+        DeclarationBlock {
             declarations: declarations,
             source_order: 0,
             specificity: 0,
@@ -868,14 +845,6 @@ impl<T> GenericDeclarationBlock<T> {
     }
 }
 
-fn find_push<T, Impl: SelectorImpl, Str: Eq + Hash>(
-    map: &mut HashMap<Str, Vec<Rule<T, Impl>>>,
-    key: Str,
-    value: Rule<T, Impl>
-) {
-    if let Some(vec) = map.get_mut(&key) {
-        vec.push(value);
-        return
-    }
-    map.insert(key, vec![value]);
+fn find_push<Str: Eq + Hash>(map: &mut HashMap<Str, Vec<Rule>>, key: Str, value: Rule) {
+    map.entry(key).or_insert_with(Vec::new).push(value)
 }

From 16bbc2f26ef10ab1044d6d0c372efc58603e1fc8 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Fri, 19 Aug 2016 16:14:06 +0200
Subject: [PATCH 11/15] Merge normal and important declarations in style rules.

Have a single Vec instead of two. Fix #3426
---
 components/script/dom/cssstyledeclaration.rs  |  36 +++---
 components/script/dom/element.rs              | 103 +++++------------
 components/style/animation.rs                 |   5 +-
 components/style/keyframes.rs                 |  20 ++--
 components/style/matching.rs                  |  19 ++-
 .../style/properties/properties.mako.rs       | 108 ++++++++++--------
 components/style/selector_matching.rs         |  72 ++++++++++--
 ports/geckolib/wrapper.rs                     |   2 +-
 tests/unit/script/size_of.rs                  |   8 +-
 tests/unit/style/properties/serialization.rs  |  45 ++++----
 tests/unit/style/stylesheets.rs               |  51 +++++----
 11 files changed, 248 insertions(+), 221 deletions(-)

diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
index 2ab50ba4073..076a2283b67 100644
--- a/components/script/dom/cssstyledeclaration.rs
+++ b/components/script/dom/cssstyledeclaration.rs
@@ -92,7 +92,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     fn Length(&self) -> u32 {
         let elem = self.owner.upcast::<Element>();
         let len = match *elem.style_attribute().borrow() {
-            Some(ref declarations) => declarations.normal.len() + declarations.important.len(),
+            Some(ref declarations) => declarations.declarations.len(),
             None => 0,
         };
         len as u32
@@ -103,19 +103,15 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let index = index as usize;
         let elem = self.owner.upcast::<Element>();
         let style_attribute = elem.style_attribute().borrow();
-        let result = style_attribute.as_ref().and_then(|declarations| {
-            if index > declarations.normal.len() {
-                declarations.important
-                            .get(index - declarations.normal.len())
-                            .map(|decl| format!("{:?} !important", decl))
-            } else {
-                declarations.normal
-                            .get(index)
-                            .map(|decl| format!("{:?}", decl))
+        style_attribute.as_ref().and_then(|declarations| {
+            declarations.declarations.get(index)
+        }).map(|&(ref declaration, importance)| {
+            let mut css = declaration.to_css_string();
+            if importance.important() {
+                css += " !important";
             }
-        });
-
-        result.map_or(DOMString::new(), DOMString::from)
+            DOMString::from(css)
+        }).unwrap_or_else(DOMString::new)
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue
@@ -151,11 +147,11 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             // Step 2.3
             // Work around closures not being Clone
             #[derive(Clone)]
-            struct Map<'a, 'b: 'a>(slice::Iter<'a, Ref<'b, PropertyDeclaration>>);
+            struct Map<'a, 'b: 'a>(slice::Iter<'a, Ref<'b, (PropertyDeclaration, Importance)>>);
             impl<'a, 'b> Iterator for Map<'a, 'b> {
                 type Item = &'a PropertyDeclaration;
                 fn next(&mut self) -> Option<Self::Item> {
-                    self.0.next().map(|r| &**r)
+                    self.0.next().map(|r| &r.0)
                 }
             }
 
@@ -167,7 +163,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
         // Step 3 & 4
         match owner.get_inline_style_declaration(&property) {
-            Some(declaration) => DOMString::from(declaration.value()),
+            Some(declaration) => DOMString::from(declaration.0.value()),
             None => DOMString::new(),
         }
     }
@@ -188,8 +184,10 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             }
         // Step 3
         } else {
-            if self.owner.get_important_inline_style_declaration(&property).is_some() {
-                return DOMString::from("important");
+            if let Some(decl) = self.owner.get_inline_style_declaration(&property) {
+                if decl.1.important() {
+                    return DOMString::from("important");
+                }
             }
         }
 
@@ -366,7 +364,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         // Step 3
         let decl_block = parse_style_attribute(&value, &window.get_url(), window.css_error_reporter(),
                                                ParserContextExtraData::default());
-        *element.style_attribute().borrow_mut() = if decl_block.normal.is_empty() && decl_block.important.is_empty() {
+        *element.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() {
             None // Step 2
         } else {
             Some(decl_block)
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 5141c8d5adf..999f4aa100a 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -80,7 +80,6 @@ use std::cell::{Cell, Ref};
 use std::convert::TryFrom;
 use std::default::Default;
 use std::fmt;
-use std::mem;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicUsize, Ordering};
 use string_cache::{Atom, Namespace, QualName};
@@ -329,7 +328,9 @@ impl LayoutElementHelpers for LayoutJS<Element> {
     {
         #[inline]
         fn from_declaration(rule: PropertyDeclaration) -> DeclarationBlock {
-            DeclarationBlock::from_declarations(Arc::new(vec![rule]))
+            DeclarationBlock::from_declarations(
+                Arc::new(vec![(rule, Importance::Normal)]),
+                Importance::Normal)
         }
 
         let bgcolor = if let Some(this) = self.downcast::<HTMLBodyElement>() {
@@ -761,20 +762,11 @@ impl Element {
         fn remove(element: &Element, property: &str) {
             let mut inline_declarations = element.style_attribute.borrow_mut();
             if let &mut Some(ref mut declarations) = &mut *inline_declarations {
-                let index = declarations.normal
+                let index = declarations.declarations
                                         .iter()
-                                        .position(|decl| decl.matches(property));
+                                        .position(|&(ref decl, _)| decl.matches(property));
                 if let Some(index) = index {
-                    Arc::make_mut(&mut declarations.normal).remove(index);
-                    return;
-                }
-
-                let index = declarations.important
-                                        .iter()
-                                        .position(|decl| decl.matches(property));
-                if let Some(index) = index {
-                    Arc::make_mut(&mut declarations.important).remove(index);
-                    return;
+                    Arc::make_mut(&mut declarations.declarations).remove(index);
                 }
             }
         }
@@ -786,47 +778,29 @@ impl Element {
     pub fn update_inline_style(&self,
                                declarations: Vec<PropertyDeclaration>,
                                importance: Importance) {
-        fn update(element: &Element, mut declarations: Vec<PropertyDeclaration>,
+        fn update(element: &Element, declarations: Vec<PropertyDeclaration>,
                   importance: Importance) {
             let mut inline_declarations = element.style_attribute().borrow_mut();
             if let &mut Some(ref mut existing_declarations) = &mut *inline_declarations {
-                let existing_declarations = if importance.important() {
-                    &mut existing_declarations.important
-                } else {
-                    &mut existing_declarations.normal
-                };
-
                 // Usually, the reference count will be 1 here. But transitions could make it greater
                 // than that.
-                let existing_declarations = Arc::make_mut(existing_declarations);
+                let existing_declarations = Arc::make_mut(&mut existing_declarations.declarations);
 
-                for mut incoming_declaration in declarations {
-                    let mut replaced = false;
+                'outer: for incoming_declaration in declarations {
                     for existing_declaration in &mut *existing_declarations {
-                        if existing_declaration.name() == incoming_declaration.name() {
-                            mem::swap(existing_declaration, &mut incoming_declaration);
-                            replaced = true;
-                            break;
+                        if existing_declaration.0.name() == incoming_declaration.name() {
+                            *existing_declaration = (incoming_declaration, importance);
+                            continue 'outer;
                         }
                     }
-
-                    if !replaced {
-                        existing_declarations.push(incoming_declaration);
-                    }
+                    existing_declarations.push((incoming_declaration, importance));
                 }
 
                 return;
             }
 
-            let (important, normal) = if importance.important() {
-                (declarations, vec![])
-            } else {
-                (vec![], declarations)
-            };
-
             *inline_declarations = Some(PropertyDeclarationBlock {
-                important: Arc::new(important),
-                normal: Arc::new(normal),
+                declarations: Arc::new(declarations.into_iter().map(|d| (d, importance)).collect()),
             });
         }
 
@@ -836,30 +810,18 @@ impl Element {
 
     pub fn set_inline_style_property_priority(&self,
                                               properties: &[&str],
-                                              importance: Importance) {
+                                              new_importance: Importance) {
         {
             let mut inline_declarations = self.style_attribute().borrow_mut();
             if let &mut Some(ref mut declarations) = &mut *inline_declarations {
-              let (from, to) = if importance == Importance::Important {
-                  (&mut declarations.normal, &mut declarations.important)
-              } else {
-                  (&mut declarations.important, &mut declarations.normal)
-              };
-
-              // Usually, the reference counts of `from` and `to` will be 1 here. But transitions
-              // could make them greater than that.
-              let from = Arc::make_mut(from);
-              let to = Arc::make_mut(to);
-              let mut new_from = Vec::new();
-              for declaration in from.drain(..) {
-                  let name = declaration.name();
-                  if properties.iter().any(|p| name == **p) {
-                      to.push(declaration)
-                  } else {
-                      new_from.push(declaration)
-                  }
-              }
-              mem::replace(from, new_from);
+                // Usually, the reference counts of `from` and `to` will be 1 here. But transitions
+                // could make them greater than that.
+                let declarations = Arc::make_mut(&mut declarations.declarations);
+                for &mut (ref declaration, ref mut importance) in declarations {
+                    if properties.iter().any(|p| declaration.name() == **p) {
+                        *importance = new_importance
+                    }
+                }
             }
         }
 
@@ -868,25 +830,12 @@ impl Element {
 
     pub fn get_inline_style_declaration(&self,
                                         property: &Atom)
-                                        -> Option<Ref<PropertyDeclaration>> {
+                                        -> Option<Ref<(PropertyDeclaration, Importance)>> {
         ref_filter_map(self.style_attribute.borrow(), |inline_declarations| {
             inline_declarations.as_ref().and_then(|declarations| {
-                declarations.normal
+                declarations.declarations
                             .iter()
-                            .chain(declarations.important.iter())
-                            .find(|decl| decl.matches(&property))
-            })
-        })
-    }
-
-    pub fn get_important_inline_style_declaration(&self,
-                                                  property: &Atom)
-                                                  -> Option<Ref<PropertyDeclaration>> {
-        ref_filter_map(self.style_attribute.borrow(), |inline_declarations| {
-            inline_declarations.as_ref().and_then(|declarations| {
-                declarations.important
-                            .iter()
-                            .find(|decl| decl.matches(&property))
+                            .find(|&&(ref decl, _)| decl.matches(&property))
             })
         })
     }
diff --git a/components/style/animation.rs b/components/style/animation.rs
index 2764511c7f3..bdfd126ab15 100644
--- a/components/style/animation.rs
+++ b/components/style/animation.rs
@@ -15,7 +15,7 @@ use properties::longhands::animation_iteration_count::computed_value::AnimationI
 use properties::longhands::animation_play_state::computed_value::AnimationPlayState;
 use properties::longhands::transition_timing_function::computed_value::StartEnd;
 use properties::longhands::transition_timing_function::computed_value::TransitionTimingFunction;
-use properties::{self, ComputedValues};
+use properties::{self, ComputedValues, Importance};
 use selector_matching::DeclarationBlock;
 use std::sync::Arc;
 use std::sync::mpsc::Sender;
@@ -385,7 +385,8 @@ fn compute_style_for_animation_step(context: &SharedStyleContext,
         KeyframesStepValue::ComputedValues => style_from_cascade.clone(),
         KeyframesStepValue::Declarations(ref declarations) => {
             let declaration_block = DeclarationBlock {
-                declarations: declarations.clone(),
+                mixed_declarations: declarations.clone(),
+                importance: Importance::Normal,
                 source_order: 0,
                 specificity: ::std::u32::MAX,
             };
diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs
index 3656ccee24d..56bf4607d4d 100644
--- a/components/style/keyframes.rs
+++ b/components/style/keyframes.rs
@@ -5,9 +5,9 @@
 use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser};
 use cssparser::{DeclarationListParser, DeclarationParser};
 use parser::{ParserContext, log_css_error};
-use properties::PropertyDeclaration;
 use properties::PropertyDeclarationParseResult;
 use properties::animated_properties::TransitionProperty;
+use properties::{PropertyDeclaration, Importance};
 use std::sync::Arc;
 
 /// A number from 1 to 100, indicating the percentage of the animation where
@@ -72,7 +72,12 @@ impl KeyframeSelector {
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct Keyframe {
     pub selector: KeyframeSelector,
-    pub declarations: Arc<Vec<PropertyDeclaration>>,
+
+    /// `!important` is not allowed in keyframe declarations,
+    /// so the second value of these tuples is always `Importance::Normal`.
+    /// But including them enables `compute_style_for_animation_step` to create a `DeclarationBlock`
+    /// by cloning an `Arc<_>` (incrementing a reference count) rather than re-creating a `Vec<_>`.
+    pub declarations: Arc<Vec<(PropertyDeclaration, Importance)>>,
 }
 
 /// A keyframes step value. This can be a synthetised keyframes animation, that
@@ -82,7 +87,8 @@ pub struct Keyframe {
 #[derive(Debug, Clone, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum KeyframesStepValue {
-    Declarations(Arc<Vec<PropertyDeclaration>>),
+    /// See `Keyframe::declarations`’s docs about the presence of `Importance`.
+    Declarations(Arc<Vec<(PropertyDeclaration, Importance)>>),
     ComputedValues,
 }
 
@@ -108,7 +114,7 @@ impl KeyframesStep {
            value: KeyframesStepValue) -> Self {
         let declared_timing_function = match value {
             KeyframesStepValue::Declarations(ref declarations) => {
-                declarations.iter().any(|prop_decl| {
+                declarations.iter().any(|&(ref prop_decl, _)| {
                     match *prop_decl {
                         PropertyDeclaration::AnimationTimingFunction(..) => true,
                         _ => false,
@@ -148,8 +154,8 @@ fn get_animated_properties(keyframe: &Keyframe) -> Vec<TransitionProperty> {
     let mut ret = vec![];
     // NB: declarations are already deduplicated, so we don't have to check for
     // it here.
-    for declaration in keyframe.declarations.iter() {
-        if let Some(property) = TransitionProperty::from_declaration(&declaration) {
+    for &(ref declaration, _) in keyframe.declarations.iter() {
+        if let Some(property) = TransitionProperty::from_declaration(declaration) {
             ret.push(property);
         }
     }
@@ -247,7 +253,7 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> {
         let mut iter = DeclarationListParser::new(input, parser);
         while let Some(declaration) = iter.next() {
             match declaration {
-                Ok(d) => declarations.extend(d),
+                Ok(d) => declarations.extend(d.into_iter().map(|d| (d, Importance::Normal))),
                 Err(range) => {
                     let pos = range.start;
                     let message = format!("Unsupported keyframe property declaration: '{}'",
diff --git a/components/style/matching.rs b/components/style/matching.rs
index cb1f70602ad..ffd69ebd529 100644
--- a/components/style/matching.rs
+++ b/components/style/matching.rs
@@ -14,7 +14,7 @@ use context::{StyleContext, SharedStyleContext};
 use data::PrivateStyleData;
 use dom::{TElement, TNode, TRestyleDamage, UnsafeNode};
 use properties::longhands::display::computed_value as display;
-use properties::{ComputedValues, PropertyDeclaration, cascade};
+use properties::{ComputedValues, cascade};
 use selector_impl::{TheSelectorImpl, PseudoElement};
 use selector_matching::{DeclarationBlock, Stylist};
 use selectors::bloom::BloomFilter;
@@ -118,15 +118,11 @@ impl<'a> ApplicableDeclarationsCacheQuery<'a> {
 
 impl<'a> PartialEq for ApplicableDeclarationsCacheQuery<'a> {
     fn eq(&self, other: &ApplicableDeclarationsCacheQuery<'a>) -> bool {
-        if self.declarations.len() != other.declarations.len() {
-            return false
-        }
-        for (this, other) in self.declarations.iter().zip(other.declarations) {
-            if !arc_ptr_eq(&this.declarations, &other.declarations) {
-                return false
-            }
-        }
-        true
+        self.declarations.len() == other.declarations.len() &&
+        self.declarations.iter().zip(other.declarations).all(|(this, other)| {
+            arc_ptr_eq(&this.mixed_declarations, &other.mixed_declarations) &&
+            this.importance == other.importance
+        })
     }
 }
 impl<'a> Eq for ApplicableDeclarationsCacheQuery<'a> {}
@@ -143,8 +139,9 @@ impl<'a> Hash for ApplicableDeclarationsCacheQuery<'a> {
         for declaration in self.declarations {
             // Each declaration contians an Arc, which is a stable
             // pointer; we use that for hashing and equality.
-            let ptr = &*declaration.declarations as *const Vec<PropertyDeclaration>;
+            let ptr: *const Vec<_> = &*declaration.mixed_declarations;
             ptr.hash(state);
+            declaration.importance.hash(state);
         }
     }
 }
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index ff96363127b..45ee70ee3a1 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -14,8 +14,6 @@ use std::ascii::AsciiExt;
 use std::boxed::Box as StdBox;
 use std::collections::HashSet;
 use std::fmt::{self, Write};
-use std::iter::{Iterator, Chain, Zip, Repeat, repeat};
-use std::slice;
 use std::sync::Arc;
 
 use app_units::Au;
@@ -264,7 +262,7 @@ mod property_bit_field {
 % endfor
 
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub enum Importance {
     /// Indicates a declaration without `!important`.
     Normal,
@@ -288,21 +286,7 @@ impl Importance {
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct PropertyDeclarationBlock {
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")]
-    pub important: Arc<Vec<PropertyDeclaration>>,
-    #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")]
-    pub normal: Arc<Vec<PropertyDeclaration>>,
-}
-
-impl PropertyDeclarationBlock {
-    /// Provides an iterator of all declarations, with indication of !important value
-    pub fn declarations(&self) -> Chain<
-        Zip<slice::Iter<PropertyDeclaration>, Repeat<Importance>>,
-        Zip<slice::Iter<PropertyDeclaration>, Repeat<Importance>>
-    > {
-        let normal = self.normal.iter().zip(repeat(Importance::Normal));
-        let important = self.important.iter().zip(repeat(Importance::Important));
-        normal.chain(important)
-    }
+    pub declarations: Arc<Vec<(PropertyDeclaration, Importance)>>,
 }
 
 impl ToCss for PropertyDeclarationBlock {
@@ -316,7 +300,7 @@ impl ToCss for PropertyDeclarationBlock {
         let mut already_serialized = Vec::new();
 
         // Step 3
-        for (declaration, importance) in self.declarations() {
+        for &(ref declaration, importance) in &*self.declarations {
             // Step 3.1
             let property = declaration.name();
 
@@ -330,7 +314,7 @@ impl ToCss for PropertyDeclarationBlock {
             if !shorthands.is_empty() {
 
                 // Step 3.3.1
-                let mut longhands = self.declarations()
+                let mut longhands = self.declarations.iter()
                     .filter(|d| !already_serialized.contains(&d.0.name()))
                     .collect::<Vec<_>>();
 
@@ -342,7 +326,7 @@ impl ToCss for PropertyDeclarationBlock {
                     let mut current_longhands = Vec::new();
                     let mut important_count = 0;
 
-                    for &(longhand, longhand_importance) in longhands.iter() {
+                    for &&(ref longhand, longhand_importance) in longhands.iter() {
                         let longhand_name = longhand.name();
                         if properties.iter().any(|p| &longhand_name == *p) {
                             current_longhands.push(longhand);
@@ -386,7 +370,7 @@ impl ToCss for PropertyDeclarationBlock {
                     for current_longhand in current_longhands {
                         // Substep 9
                         already_serialized.push(current_longhand.name());
-                        let index_to_remove = longhands.iter().position(|l| l.0 == current_longhand);
+                        let index_to_remove = longhands.iter().position(|l| l.0 == *current_longhand);
                         if let Some(index) = index_to_remove {
                             // Substep 10
                             longhands.remove(index);
@@ -557,8 +541,7 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
 
 pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Parser)
                                        -> PropertyDeclarationBlock {
-    let mut important_declarations = Vec::new();
-    let mut normal_declarations = Vec::new();
+    let mut declarations = Vec::new();
     let parser = PropertyDeclarationParser {
         context: context,
     };
@@ -566,11 +549,7 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
     while let Some(declaration) = iter.next() {
         match declaration {
             Ok((results, importance)) => {
-                if importance.important() {
-                    important_declarations.extend(results);
-                } else {
-                    normal_declarations.extend(results);
-                }
+                declarations.extend(results.into_iter().map(|d| (d, importance)))
             }
             Err(range) => {
                 let pos = range.start;
@@ -581,46 +560,79 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
         }
     }
     PropertyDeclarationBlock {
-        important: Arc::new(deduplicate_property_declarations(important_declarations)),
-        normal: Arc::new(deduplicate_property_declarations(normal_declarations)),
+        declarations: Arc::new(deduplicate_property_declarations(declarations)),
     }
 }
 
 
-/// Only keep the last declaration for any given property.
+/// 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(declarations: Vec<PropertyDeclaration>)
-                                     -> Vec<PropertyDeclaration> {
-    let mut deduplicated = vec![];
-    let mut seen = PropertyBitField::new();
-    let mut seen_custom = Vec::new();
-    for declaration in declarations.into_iter().rev() {
+fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Importance)>)
+                                     -> Vec<(PropertyDeclaration, Importance)> {
+    let mut deduplicated = Vec::new();
+    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 declarations.into_iter().rev() {
         match declaration {
             % for property in data.longhands:
                 PropertyDeclaration::${property.camel_case}(..) => {
                     % if not property.derived_from:
-                        if seen.get_${property.ident}() {
-                            continue
+                        if importance.important() {
+                            if seen_important.get_${property.ident}() {
+                                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}()
                         }
-                        seen.set_${property.ident}()
                     % else:
                         unreachable!();
                     % endif
                 },
             % endfor
             PropertyDeclaration::Custom(ref name, _) => {
-                if seen_custom.contains(name) {
-                    continue
+                if importance.important() {
+                    if seen_custom_important.contains(name) {
+                        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())
                 }
-                seen_custom.push(name.clone())
             }
         }
-        deduplicated.push(declaration)
+        deduplicated.push((declaration, importance))
     }
     deduplicated.reverse();
     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);
+}
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum CSSWideKeyword {
@@ -1701,7 +1713,7 @@ fn cascade_with_cached_declarations(
     // Declaration blocks are stored in increasing precedence order,
     // we want them in decreasing order here.
     for sub_list in applicable_declarations.iter().rev() {
-        for declaration in sub_list.declarations.iter().rev() {
+        for declaration in sub_list.iter().rev() {
             match *declaration {
                 % for style_struct in data.active_style_structs():
                     % for property in style_struct.longhands:
@@ -1832,7 +1844,7 @@ pub fn cascade(viewport_size: Size2D<Au>,
     let mut custom_properties = None;
     let mut seen_custom = HashSet::new();
     for sub_list in applicable_declarations.iter().rev() {
-        for declaration in sub_list.declarations.iter().rev() {
+        for declaration in sub_list.iter().rev() {
             match *declaration {
                 PropertyDeclaration::Custom(ref name, ref value) => {
                     ::custom_properties::cascade(
@@ -1890,7 +1902,7 @@ pub fn cascade(viewport_size: Size2D<Au>,
     ComputedValues::do_cascade_property(|cascade_property| {
         % for category_to_cascade_now in ["early", "other"]:
             for sub_list in applicable_declarations.iter().rev() {
-                for declaration in sub_list.declarations.iter().rev() {
+                for declaration in sub_list.iter().rev() {
                     if let PropertyDeclaration::Custom(..) = *declaration {
                         continue
                     }
diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs
index 775f81915d4..c8f6dc089f1 100644
--- a/components/style/selector_matching.rs
+++ b/components/style/selector_matching.rs
@@ -9,7 +9,7 @@ use element_state::*;
 use error_reporting::StdoutErrorReporter;
 use keyframes::KeyframesAnimation;
 use media_queries::{Device, MediaType};
-use properties::{self, PropertyDeclaration, PropertyDeclarationBlock, ComputedValues};
+use properties::{self, PropertyDeclaration, PropertyDeclarationBlock, ComputedValues, Importance};
 use quickersort::sort_by;
 use restyle_hints::{RestyleHint, DependencySet};
 use selector_impl::{ElementExt, TheSelectorImpl, PseudoElement};
@@ -25,6 +25,7 @@ use std::collections::HashMap;
 use std::fmt;
 use std::hash::BuildHasherDefault;
 use std::hash::Hash;
+use std::slice;
 use std::sync::Arc;
 use string_cache::Atom;
 use style_traits::viewport::ViewportConstraints;
@@ -163,8 +164,8 @@ impl Stylist {
         // Take apart the StyleRule into individual Rules and insert
         // them into the SelectorMap of that priority.
         macro_rules! append(
-            ($style_rule: ident, $priority: ident) => {
-                if !$style_rule.declarations.$priority.is_empty() {
+            ($style_rule: ident, $priority: ident, $importance: expr) => {
+                if !$style_rule.declarations.declarations.is_empty() {
                     for selector in &$style_rule.selectors {
                         let map = if let Some(ref pseudo) = selector.pseudo_element {
                             self.pseudos_map
@@ -179,7 +180,8 @@ impl Stylist {
                             selector: selector.complex_selector.clone(),
                             declarations: DeclarationBlock {
                                 specificity: selector.specificity,
-                                declarations: $style_rule.declarations.$priority.clone(),
+                                mixed_declarations: $style_rule.declarations.declarations.clone(),
+                                importance: $importance,
                                 source_order: rules_source_order,
                             },
                         });
@@ -191,8 +193,8 @@ impl Stylist {
         for rule in stylesheet.effective_rules(&self.device) {
             match *rule {
                 CSSRule::Style(ref style_rule) => {
-                    append!(style_rule, normal);
-                    append!(style_rule, important);
+                    append!(style_rule, normal, Importance::Normal);
+                    append!(style_rule, important, Importance::Important);
                     rules_source_order += 1;
 
                     for selector in &style_rule.selectors {
@@ -397,7 +399,9 @@ impl Stylist {
             relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
             Push::push(
                 applicable_declarations,
-                DeclarationBlock::from_declarations(sa.normal.clone()));
+                DeclarationBlock::from_declarations(
+                    sa.declarations.clone(),
+                    Importance::Normal));
         }
 
         debug!("style attr: {:?}", relations);
@@ -414,7 +418,9 @@ impl Stylist {
         if let Some(ref sa) = style_attribute {
             Push::push(
                 applicable_declarations,
-                DeclarationBlock::from_declarations(sa.important.clone()));
+                DeclarationBlock::from_declarations(
+                    sa.declarations.clone(),
+                    Importance::Important));
         }
 
         debug!("style attr important: {:?}", relations);
@@ -829,20 +835,64 @@ pub struct Rule {
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[derive(Debug, Clone)]
 pub struct DeclarationBlock {
-    pub declarations: Arc<Vec<PropertyDeclaration>>,
+    /// Contains declarations of either importance, but only those of self.importance are relevant.
+    /// Use DeclarationBlock::iter
+    pub mixed_declarations: Arc<Vec<(PropertyDeclaration, Importance)>>,
+    pub importance: Importance,
     pub source_order: usize,
     pub specificity: u32,
 }
 
 impl DeclarationBlock {
     #[inline]
-    pub fn from_declarations(declarations: Arc<Vec<PropertyDeclaration>>) -> Self {
+    pub fn from_declarations(declarations: Arc<Vec<(PropertyDeclaration, Importance)>>,
+                             importance: Importance)
+                             -> Self {
         DeclarationBlock {
-            declarations: declarations,
+            mixed_declarations: declarations,
+            importance: importance,
             source_order: 0,
             specificity: 0,
         }
     }
+
+    pub fn iter(&self) -> DeclarationBlockIter {
+        DeclarationBlockIter {
+            iter: self.mixed_declarations.iter(),
+            importance: self.importance,
+        }
+    }
+}
+
+pub struct DeclarationBlockIter<'a> {
+    iter: slice::Iter<'a, (PropertyDeclaration, Importance)>,
+    importance: Importance,
+}
+
+impl<'a> Iterator for DeclarationBlockIter<'a> {
+    type Item = &'a PropertyDeclaration;
+
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        while let Some(&(ref declaration, importance)) = self.iter.next() {
+            if importance == self.importance {
+                return Some(declaration)
+            }
+        }
+        None
+    }
+}
+
+impl<'a> DoubleEndedIterator for DeclarationBlockIter<'a> {
+    #[inline]
+    fn next_back(&mut self) -> Option<Self::Item> {
+        while let Some(&(ref declaration, importance)) = self.iter.next_back() {
+            if importance == self.importance {
+                return Some(declaration)
+            }
+        }
+        None
+    }
 }
 
 fn find_push<Str: Eq + Hash>(map: &mut HashMap<Str, Vec<Rule>>, key: Str, value: Rule) {
diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs
index 551e7999154..6e4f137cfdc 100644
--- a/ports/geckolib/wrapper.rs
+++ b/ports/geckolib/wrapper.rs
@@ -45,8 +45,8 @@ use style::element_state::ElementState;
 use style::error_reporting::StdoutErrorReporter;
 use style::gecko_selector_impl::{GeckoSelectorImpl, NonTSPseudoClass, PseudoElement};
 use style::parser::ParserContextExtraData;
+use style::properties::PropertyDeclarationBlock;
 use style::properties::{ComputedValues, parse_style_attribute};
-use style::properties::{PropertyDeclaration, PropertyDeclarationBlock};
 use style::refcell::{Ref, RefCell, RefMut};
 use style::selector_impl::ElementExt;
 use style::selector_matching::DeclarationBlock;
diff --git a/tests/unit/script/size_of.rs b/tests/unit/script/size_of.rs
index b30304d5dac..472beb888c3 100644
--- a/tests/unit/script/size_of.rs
+++ b/tests/unit/script/size_of.rs
@@ -40,10 +40,10 @@ macro_rules! sizeof_checker (
 // Update the sizes here
 sizeof_checker!(size_event_target, EventTarget, 40);
 sizeof_checker!(size_node, Node, 160);
-sizeof_checker!(size_element, Element, 336);
-sizeof_checker!(size_htmlelement, HTMLElement, 352);
-sizeof_checker!(size_div, HTMLDivElement, 352);
-sizeof_checker!(size_span, HTMLSpanElement, 352);
+sizeof_checker!(size_element, Element, 328);
+sizeof_checker!(size_htmlelement, HTMLElement, 344);
+sizeof_checker!(size_div, HTMLDivElement, 344);
+sizeof_checker!(size_span, HTMLSpanElement, 344);
 sizeof_checker!(size_text, Text, 192);
 sizeof_checker!(size_characterdata, CharacterData, 192);
 sizeof_checker!(size_servothreadsafelayoutnode, ServoThreadSafeLayoutNode, 16);
diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs
index 98ffa07f722..2f2b5d39127 100644
--- a/tests/unit/style/properties/serialization.rs
+++ b/tests/unit/style/properties/serialization.rs
@@ -5,7 +5,7 @@
 pub use cssparser::ToCss;
 pub use std::sync::Arc;
 pub use style::computed_values::display::T::inline_block;
-pub use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarationBlock};
+pub use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarationBlock, Importance};
 pub use style::values::specified::{BorderStyle, CSSColor, Length};
 pub use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent};
 pub use style::properties::longhands::outline_color::computed_value::T as ComputedColor;
@@ -18,37 +18,41 @@ fn property_declaration_block_should_serialize_correctly() {
     use style::properties::longhands::overflow_x::computed_value::T as OverflowXValue;
     use style::properties::longhands::overflow_y::computed_value::T as OverflowYContainer;
 
-    let mut normal = Vec::new();
-    let mut important = Vec::new();
+    let declarations = vec![
+        (PropertyDeclaration::Width(
+            DeclaredValue::Value(LengthOrPercentageOrAuto::Length(Length::from_px(70f32)))),
+         Importance::Normal),
 
-    let length = DeclaredValue::Value(LengthOrPercentageOrAuto::Length(Length::from_px(70f32)));
-    normal.push(PropertyDeclaration::Width(length));
+        (PropertyDeclaration::MinHeight(
+            DeclaredValue::Value(LengthOrPercentage::Length(Length::from_px(20f32)))),
+         Importance::Normal),
 
-    let min_height = DeclaredValue::Value(LengthOrPercentage::Length(Length::from_px(20f32)));
-    normal.push(PropertyDeclaration::MinHeight(min_height));
+        (PropertyDeclaration::Height(
+            DeclaredValue::Value(LengthOrPercentageOrAuto::Length(Length::from_px(20f32)))),
+         Importance::Important),
 
-    let value = DeclaredValue::Value(inline_block);
-    normal.push(PropertyDeclaration::Display(value));
+        (PropertyDeclaration::Display(
+            DeclaredValue::Value(inline_block)),
+         Importance::Normal),
 
-    let overflow_x = DeclaredValue::Value(OverflowXValue::auto);
-    normal.push(PropertyDeclaration::OverflowX(overflow_x));
+        (PropertyDeclaration::OverflowX(
+            DeclaredValue::Value(OverflowXValue::auto)),
+         Importance::Normal),
 
-    let overflow_y = DeclaredValue::Value(OverflowYContainer(OverflowXValue::auto));
-    normal.push(PropertyDeclaration::OverflowY(overflow_y));
-
-    let height = DeclaredValue::Value(LengthOrPercentageOrAuto::Length(Length::from_px(20f32)));
-    important.push(PropertyDeclaration::Height(height));
+        (PropertyDeclaration::OverflowY(
+            DeclaredValue::Value(OverflowYContainer(OverflowXValue::auto))),
+         Importance::Normal),
+    ];
 
     let block = PropertyDeclarationBlock {
-        normal: Arc::new(normal),
-        important: Arc::new(important)
+        declarations: Arc::new(declarations),
     };
 
     let css_string = block.to_css_string();
 
     assert_eq!(
         css_string,
-        "width: 70px; min-height: 20px; display: inline-block; overflow: auto; height: 20px !important;"
+        "width: 70px; min-height: 20px; height: 20px !important; display: inline-block; overflow: auto;"
     );
 }
 
@@ -57,8 +61,7 @@ mod shorthand_serialization {
 
     pub fn shorthand_properties_to_string(properties: Vec<PropertyDeclaration>) -> String {
         let block = PropertyDeclarationBlock {
-            normal: Arc::new(properties),
-            important: Arc::new(Vec::new())
+            declarations: Arc::new(properties.into_iter().map(|d| (d, Importance::Normal)).collect()),
         };
 
         block.to_css_string()
diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs
index 72273165b35..27d35c513cf 100644
--- a/tests/unit/style/stylesheets.rs
+++ b/tests/unit/style/stylesheets.rs
@@ -13,6 +13,7 @@ use style::error_reporting::ParseErrorReporter;
 use style::keyframes::{Keyframe, KeyframeSelector, KeyframePercentage};
 use style::parser::ParserContextExtraData;
 use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, DeclaredValue, longhands};
+use style::properties::Importance;
 use style::properties::longhands::animation_play_state;
 use style::stylesheets::{Stylesheet, CSSRule, StyleRule, KeyframesRule, Origin};
 use style::values::specified::{LengthOrPercentageOrAuto, Percentage};
@@ -87,10 +88,10 @@ fn test_parse_stylesheet() {
                     },
                 ],
                 declarations: PropertyDeclarationBlock {
-                    normal: Arc::new(vec![]),
-                    important: Arc::new(vec![
-                        PropertyDeclaration::Display(DeclaredValue::Value(
+                    declarations: Arc::new(vec![
+                        (PropertyDeclaration::Display(DeclaredValue::Value(
                             longhands::display::SpecifiedValue::none)),
+                         Importance::Important),
                     ]),
                 },
             }),
@@ -132,11 +133,11 @@ fn test_parse_stylesheet() {
                     },
                 ],
                 declarations: PropertyDeclarationBlock {
-                    normal: Arc::new(vec![
-                        PropertyDeclaration::Display(DeclaredValue::Value(
+                    declarations: Arc::new(vec![
+                        (PropertyDeclaration::Display(DeclaredValue::Value(
                             longhands::display::SpecifiedValue::block)),
+                         Importance::Normal),
                     ]),
-                    important: Arc::new(vec![]),
                 },
             }),
             CSSRule::Style(StyleRule {
@@ -166,24 +167,31 @@ fn test_parse_stylesheet() {
                     },
                 ],
                 declarations: PropertyDeclarationBlock {
-                    normal: Arc::new(vec![
-                        PropertyDeclaration::BackgroundColor(DeclaredValue::Value(
+                    declarations: Arc::new(vec![
+                        (PropertyDeclaration::BackgroundColor(DeclaredValue::Value(
                             longhands::background_color::SpecifiedValue {
                                 authored: Some("blue".to_owned()),
                                 parsed: cssparser::Color::RGBA(cssparser::RGBA {
                                     red: 0., green: 0., blue: 1., alpha: 1.
                                 }),
                             }
-                        )),
-                        PropertyDeclaration::BackgroundPosition(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundRepeat(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundAttachment(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundImage(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundSize(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundOrigin(DeclaredValue::Initial),
-                        PropertyDeclaration::BackgroundClip(DeclaredValue::Initial),
+                         )),
+                         Importance::Normal),
+                        (PropertyDeclaration::BackgroundPosition(DeclaredValue::Initial),
+                         Importance::Normal),
+                        (PropertyDeclaration::BackgroundRepeat(DeclaredValue::Initial),
+                         Importance::Normal),
+                        (PropertyDeclaration::BackgroundAttachment(DeclaredValue::Initial),
+                         Importance::Normal),
+                        (PropertyDeclaration::BackgroundImage(DeclaredValue::Initial),
+                         Importance::Normal),
+                        (PropertyDeclaration::BackgroundSize(DeclaredValue::Initial),
+                         Importance::Normal),
+                        (PropertyDeclaration::BackgroundOrigin(DeclaredValue::Initial),
+                         Importance::Normal),
+                        (PropertyDeclaration::BackgroundClip(DeclaredValue::Initial),
+                         Importance::Normal),
                     ]),
-                    important: Arc::new(vec![]),
                 },
             }),
             CSSRule::Keyframes(KeyframesRule {
@@ -193,19 +201,22 @@ fn test_parse_stylesheet() {
                         selector: KeyframeSelector::new_for_unit_testing(
                                       vec![KeyframePercentage::new(0.)]),
                         declarations: Arc::new(vec![
-                            PropertyDeclaration::Width(DeclaredValue::Value(
+                            (PropertyDeclaration::Width(DeclaredValue::Value(
                                 LengthOrPercentageOrAuto::Percentage(Percentage(0.)))),
+                             Importance::Normal),
                         ]),
                     },
                     Keyframe {
                         selector: KeyframeSelector::new_for_unit_testing(
                                       vec![KeyframePercentage::new(1.)]),
                         declarations: Arc::new(vec![
-                            PropertyDeclaration::Width(DeclaredValue::Value(
+                            (PropertyDeclaration::Width(DeclaredValue::Value(
                                 LengthOrPercentageOrAuto::Percentage(Percentage(1.)))),
-                            PropertyDeclaration::AnimationPlayState(DeclaredValue::Value(
+                             Importance::Normal),
+                            (PropertyDeclaration::AnimationPlayState(DeclaredValue::Value(
                                 animation_play_state::SpecifiedValue(
                                     vec![animation_play_state::SingleSpecifiedValue::running]))),
+                             Importance::Normal),
                         ]),
                     },
                 ]

From a175c9981ed8d767b9e8b18fda9bcbdc5f500740 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Fri, 19 Aug 2016 17:13:20 +0200
Subject: [PATCH 12/15] Only cascade at a priority level rules that have
 declarations of that priority.

---
 components/script/dom/element.rs              | 27 +++++----
 .../style/properties/properties.mako.rs       | 55 +++++++++++++++++--
 components/style/selector_matching.rs         | 35 +++++++-----
 tests/unit/script/size_of.rs                  |  8 +--
 tests/unit/style/properties/serialization.rs  |  4 ++
 tests/unit/style/stylesheets.rs               |  6 ++
 6 files changed, 99 insertions(+), 36 deletions(-)

diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 999f4aa100a..f6f8661689c 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -767,6 +767,7 @@ impl Element {
                                         .position(|&(ref decl, _)| decl.matches(property));
                 if let Some(index) = index {
                     Arc::make_mut(&mut declarations.declarations).remove(index);
+                    declarations.recalc_any();
                 }
             }
         }
@@ -781,26 +782,30 @@ impl Element {
         fn update(element: &Element, declarations: Vec<PropertyDeclaration>,
                   importance: Importance) {
             let mut inline_declarations = element.style_attribute().borrow_mut();
-            if let &mut Some(ref mut existing_declarations) = &mut *inline_declarations {
-                // Usually, the reference count will be 1 here. But transitions could make it greater
-                // than that.
-                let existing_declarations = Arc::make_mut(&mut existing_declarations.declarations);
+            if let &mut Some(ref mut declaration_block) = &mut *inline_declarations {
+                {
+                    // Usually, the reference count will be 1 here. But transitions could make it greater
+                    // than that.
+                    let existing_declarations = Arc::make_mut(&mut declaration_block.declarations);
 
-                'outer: for incoming_declaration in declarations {
-                    for existing_declaration in &mut *existing_declarations {
-                        if existing_declaration.0.name() == incoming_declaration.name() {
-                            *existing_declaration = (incoming_declaration, importance);
-                            continue 'outer;
+                    'outer: for incoming_declaration in declarations {
+                        for existing_declaration in &mut *existing_declarations {
+                            if existing_declaration.0.name() == incoming_declaration.name() {
+                                *existing_declaration = (incoming_declaration, importance);
+                                continue 'outer;
+                            }
                         }
+                        existing_declarations.push((incoming_declaration, importance));
                     }
-                    existing_declarations.push((incoming_declaration, importance));
                 }
-
+                declaration_block.recalc_any();
                 return;
             }
 
             *inline_declarations = Some(PropertyDeclarationBlock {
                 declarations: Arc::new(declarations.into_iter().map(|d| (d, importance)).collect()),
+                any_important: importance.important(),
+                any_normal: !importance.important(),
             });
         }
 
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index 45ee70ee3a1..539e03253c4 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -287,6 +287,28 @@ impl Importance {
 pub struct PropertyDeclarationBlock {
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")]
     pub declarations: Arc<Vec<(PropertyDeclaration, Importance)>>,
+
+    /// Whether `self.declaration` contains at least one declaration with `Importance::Normal`
+    pub any_normal: bool,
+
+    /// Whether `self.declaration` contains at least one declaration with `Importance::Important`
+    pub any_important: bool,
+}
+
+impl PropertyDeclarationBlock {
+    pub fn recalc_any(&mut self) {
+        let mut any_normal = false;
+        let mut any_important = false;
+        for &(_, importance) in &*self.declarations {
+            if importance.important() {
+                any_important = true
+            } else {
+                any_normal = true
+            }
+        }
+        self.any_normal = any_normal;
+        self.any_important = any_important;
+    }
 }
 
 impl ToCss for PropertyDeclarationBlock {
@@ -542,6 +564,8 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
 pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Parser)
                                        -> PropertyDeclarationBlock {
     let mut declarations = Vec::new();
+    let mut any_normal = false;
+    let mut any_important = false;
     let parser = PropertyDeclarationParser {
         context: context,
     };
@@ -549,6 +573,11 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
     while let Some(declaration) = iter.next() {
         match declaration {
             Ok((results, importance)) => {
+                if importance.important() {
+                    any_important = true
+                } else {
+                    any_normal = true
+                }
                 declarations.extend(results.into_iter().map(|d| (d, importance)))
             }
             Err(range) => {
@@ -559,16 +588,24 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
             }
         }
     }
-    PropertyDeclarationBlock {
-        declarations: Arc::new(deduplicate_property_declarations(declarations)),
+    let (declarations, removed_any) = deduplicate_property_declarations(declarations);
+    let mut block = PropertyDeclarationBlock {
+        declarations: Arc::new(declarations),
+        any_normal: any_normal,
+        any_important: any_important,
+    };
+    if removed_any {
+        block.recalc_any();
     }
+    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(declarations: Vec<(PropertyDeclaration, Importance)>)
-                                     -> Vec<(PropertyDeclaration, Importance)> {
+                                     -> (Vec<(PropertyDeclaration, Importance)>, bool) {
+    let mut removed_any = false;
     let mut deduplicated = Vec::new();
     let mut seen_normal = PropertyBitField::new();
     let mut seen_important = PropertyBitField::new();
@@ -581,17 +618,20 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp
                     % if not property.derived_from:
                         if importance.important() {
                             if seen_important.get_${property.ident}() {
+                                removed_any = true;
                                 continue
                             }
                             if seen_normal.get_${property.ident}() {
                                 remove_one(&mut deduplicated, |d| {
                                     matches!(d, &(PropertyDeclaration::${property.camel_case}(..), _))
-                                })
+                                });
+                                removed_any = true;
                             }
                             seen_important.set_${property.ident}()
                         } else {
                             if seen_normal.get_${property.ident}() ||
                                seen_important.get_${property.ident}() {
+                                removed_any = true;
                                 continue
                             }
                             seen_normal.set_${property.ident}()
@@ -604,17 +644,20 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp
             PropertyDeclaration::Custom(ref name, _) => {
                 if importance.important() {
                     if seen_custom_important.contains(name) {
+                        removed_any = true;
                         continue
                     }
                     if seen_custom_normal.contains(name) {
                         remove_one(&mut deduplicated, |d| {
                             matches!(d, &(PropertyDeclaration::Custom(ref n, _), _) if n == name)
-                        })
+                        });
+                        removed_any = true;
                     }
                     seen_custom_important.push(name.clone())
                 } else {
                     if seen_custom_normal.contains(name) ||
                        seen_custom_important.contains(name) {
+                        removed_any = true;
                         continue
                     }
                     seen_custom_normal.push(name.clone())
@@ -624,7 +667,7 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp
         deduplicated.push((declaration, importance))
     }
     deduplicated.reverse();
-    deduplicated
+    (deduplicated, removed_any)
 }
 
 #[inline]
diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs
index c8f6dc089f1..3c6c1fe89f5 100644
--- a/components/style/selector_matching.rs
+++ b/components/style/selector_matching.rs
@@ -164,8 +164,8 @@ impl Stylist {
         // Take apart the StyleRule into individual Rules and insert
         // them into the SelectorMap of that priority.
         macro_rules! append(
-            ($style_rule: ident, $priority: ident, $importance: expr) => {
-                if !$style_rule.declarations.declarations.is_empty() {
+            ($style_rule: ident, $priority: ident, $importance: expr, $any: ident) => {
+                if $style_rule.declarations.$any {
                     for selector in &$style_rule.selectors {
                         let map = if let Some(ref pseudo) = selector.pseudo_element {
                             self.pseudos_map
@@ -193,8 +193,8 @@ impl Stylist {
         for rule in stylesheet.effective_rules(&self.device) {
             match *rule {
                 CSSRule::Style(ref style_rule) => {
-                    append!(style_rule, normal, Importance::Normal);
-                    append!(style_rule, important, Importance::Important);
+                    append!(style_rule, normal, Importance::Normal, any_normal);
+                    append!(style_rule, important, Importance::Important, any_important);
                     rules_source_order += 1;
 
                     for selector in &style_rule.selectors {
@@ -396,12 +396,14 @@ impl Stylist {
 
         // Step 4: Normal style attributes.
         if let Some(ref sa)  = style_attribute {
-            relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
-            Push::push(
-                applicable_declarations,
-                DeclarationBlock::from_declarations(
-                    sa.declarations.clone(),
-                    Importance::Normal));
+            if sa.any_normal {
+                relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
+                Push::push(
+                    applicable_declarations,
+                    DeclarationBlock::from_declarations(
+                        sa.declarations.clone(),
+                        Importance::Normal));
+            }
         }
 
         debug!("style attr: {:?}", relations);
@@ -416,11 +418,14 @@ impl Stylist {
 
         // Step 6: `!important` style attributes.
         if let Some(ref sa) = style_attribute {
-            Push::push(
-                applicable_declarations,
-                DeclarationBlock::from_declarations(
-                    sa.declarations.clone(),
-                    Importance::Important));
+            if sa.any_important {
+                relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
+                Push::push(
+                    applicable_declarations,
+                    DeclarationBlock::from_declarations(
+                        sa.declarations.clone(),
+                        Importance::Important));
+            }
         }
 
         debug!("style attr important: {:?}", relations);
diff --git a/tests/unit/script/size_of.rs b/tests/unit/script/size_of.rs
index 472beb888c3..b30304d5dac 100644
--- a/tests/unit/script/size_of.rs
+++ b/tests/unit/script/size_of.rs
@@ -40,10 +40,10 @@ macro_rules! sizeof_checker (
 // Update the sizes here
 sizeof_checker!(size_event_target, EventTarget, 40);
 sizeof_checker!(size_node, Node, 160);
-sizeof_checker!(size_element, Element, 328);
-sizeof_checker!(size_htmlelement, HTMLElement, 344);
-sizeof_checker!(size_div, HTMLDivElement, 344);
-sizeof_checker!(size_span, HTMLSpanElement, 344);
+sizeof_checker!(size_element, Element, 336);
+sizeof_checker!(size_htmlelement, HTMLElement, 352);
+sizeof_checker!(size_div, HTMLDivElement, 352);
+sizeof_checker!(size_span, HTMLSpanElement, 352);
 sizeof_checker!(size_text, Text, 192);
 sizeof_checker!(size_characterdata, CharacterData, 192);
 sizeof_checker!(size_servothreadsafelayoutnode, ServoThreadSafeLayoutNode, 16);
diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs
index 2f2b5d39127..d22043b237b 100644
--- a/tests/unit/style/properties/serialization.rs
+++ b/tests/unit/style/properties/serialization.rs
@@ -46,6 +46,8 @@ fn property_declaration_block_should_serialize_correctly() {
 
     let block = PropertyDeclarationBlock {
         declarations: Arc::new(declarations),
+        any_important: true,
+        any_normal: true,
     };
 
     let css_string = block.to_css_string();
@@ -62,6 +64,8 @@ mod shorthand_serialization {
     pub fn shorthand_properties_to_string(properties: Vec<PropertyDeclaration>) -> String {
         let block = PropertyDeclarationBlock {
             declarations: Arc::new(properties.into_iter().map(|d| (d, Importance::Normal)).collect()),
+            any_important: false,
+            any_normal: true,
         };
 
         block.to_css_string()
diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs
index 27d35c513cf..28b795d440e 100644
--- a/tests/unit/style/stylesheets.rs
+++ b/tests/unit/style/stylesheets.rs
@@ -93,6 +93,8 @@ fn test_parse_stylesheet() {
                             longhands::display::SpecifiedValue::none)),
                          Importance::Important),
                     ]),
+                    any_normal: false,
+                    any_important: true,
                 },
             }),
             CSSRule::Style(StyleRule {
@@ -138,6 +140,8 @@ fn test_parse_stylesheet() {
                             longhands::display::SpecifiedValue::block)),
                          Importance::Normal),
                     ]),
+                    any_normal: true,
+                    any_important: false,
                 },
             }),
             CSSRule::Style(StyleRule {
@@ -192,6 +196,8 @@ fn test_parse_stylesheet() {
                         (PropertyDeclaration::BackgroundClip(DeclaredValue::Initial),
                          Importance::Normal),
                     ]),
+                    any_normal: true,
+                    any_important: false,
                 },
             }),
             CSSRule::Keyframes(KeyframesRule {

From dec28a13ea7b5c6681e65fa231b6a31fbdd0f56a Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Sat, 20 Aug 2016 14:36:43 +0200
Subject: [PATCH 13/15] Use FNV hash in SelectorMap

---
 components/style/selector_matching.rs | 56 +++++++++++++--------------
 1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs
index 3c6c1fe89f5..573de8b4575 100644
--- a/components/style/selector_matching.rs
+++ b/components/style/selector_matching.rs
@@ -16,8 +16,8 @@ use selector_impl::{ElementExt, TheSelectorImpl, PseudoElement};
 use selectors::Element;
 use selectors::bloom::BloomFilter;
 use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS};
-use selectors::matching::{StyleRelations, matches_compound_selector};
-use selectors::parser::{Selector, SimpleSelector, LocalName, CompoundSelector};
+use selectors::matching::{StyleRelations, matches_complex_selector};
+use selectors::parser::{Selector, SimpleSelector, LocalName, ComplexSelector};
 use sink::Push;
 use smallvec::VecLike;
 use std::borrow::Borrow;
@@ -32,6 +32,8 @@ use style_traits::viewport::ViewportConstraints;
 use stylesheets::{CSSRule, CSSRuleIteratorExt, Origin, Stylesheet};
 use viewport::{MaybeNew, ViewportRuleCascade};
 
+pub type FnvHashMap<K, V> = HashMap<K, V, BuildHasherDefault<::fnv::FnvHasher>>;
+
 /// This structure holds all the selectors and device characteristics
 /// for a given document. The selectors are converted into `Rule`s
 /// (defined in rust-selectors), and introduced in a `SelectorMap`
@@ -61,19 +63,15 @@ pub struct Stylist {
 
     /// The selector maps corresponding to a given pseudo-element
     /// (depending on the implementation)
-    pseudos_map: HashMap<PseudoElement,
-                         PerPseudoElementSelectorMap,
-                         BuildHasherDefault<::fnv::FnvHasher>>,
+    pseudos_map: FnvHashMap<PseudoElement, PerPseudoElementSelectorMap>,
 
     /// A map with all the animations indexed by name.
-    animations: HashMap<Atom, KeyframesAnimation>,
+    animations: FnvHashMap<Atom, KeyframesAnimation>,
 
     /// Applicable declarations for a given non-eagerly cascaded pseudo-element.
     /// These are eagerly computed once, and then used to resolve the new
     /// computed values on the fly on layout.
-    precomputed_pseudo_element_decls: HashMap<PseudoElement,
-                                              Vec<DeclarationBlock>,
-                                              BuildHasherDefault<::fnv::FnvHasher>>,
+    precomputed_pseudo_element_decls: FnvHashMap<PseudoElement, Vec<DeclarationBlock>>,
 
     rules_source_order: usize,
 
@@ -98,9 +96,9 @@ impl Stylist {
             quirks_mode: false,
 
             element_map: PerPseudoElementSelectorMap::new(),
-            pseudos_map: HashMap::with_hasher(Default::default()),
-            animations: HashMap::with_hasher(Default::default()),
-            precomputed_pseudo_element_decls: HashMap::with_hasher(Default::default()),
+            pseudos_map: Default::default(),
+            animations: Default::default(),
+            precomputed_pseudo_element_decls: Default::default(),
             rules_source_order: 0,
             state_deps: DependencySet::new(),
 
@@ -124,13 +122,13 @@ impl Stylist {
         }
 
         self.element_map = PerPseudoElementSelectorMap::new();
-        self.pseudos_map = HashMap::with_hasher(Default::default());
-        self.animations = HashMap::with_hasher(Default::default());
+        self.pseudos_map = Default::default();
+        self.animations = Default::default();
         TheSelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| {
             self.pseudos_map.insert(pseudo, PerPseudoElementSelectorMap::new());
         });
 
-        self.precomputed_pseudo_element_decls = HashMap::with_hasher(Default::default());
+        self.precomputed_pseudo_element_decls = Default::default();
         self.rules_source_order = 0;
         self.state_deps.clear();
 
@@ -456,7 +454,7 @@ impl Stylist {
     }
 
     #[inline]
-    pub fn animations(&self) -> &HashMap<Atom, KeyframesAnimation> {
+    pub fn animations(&self) -> &FnvHashMap<Atom, KeyframesAnimation> {
         &self.animations
     }
 
@@ -611,12 +609,12 @@ impl PerPseudoElementSelectorMap {
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct SelectorMap {
     // TODO: Tune the initial capacity of the HashMap
-    id_hash: HashMap<Atom, Vec<Rule>>,
-    class_hash: HashMap<Atom, Vec<Rule>>,
-    local_name_hash: HashMap<Atom, Vec<Rule>>,
+    id_hash: FnvHashMap<Atom, Vec<Rule>>,
+    class_hash: FnvHashMap<Atom, Vec<Rule>>,
+    local_name_hash: FnvHashMap<Atom, Vec<Rule>>,
     /// Same as local_name_hash, but keys are lower-cased.
     /// For HTML elements in HTML documents.
-    lower_local_name_hash: HashMap<Atom, Vec<Rule>>,
+    lower_local_name_hash: FnvHashMap<Atom, Vec<Rule>>,
     /// Rules that don't have ID, class, or element selectors.
     other_rules: Vec<Rule>,
     /// Whether this hash is empty.
@@ -712,7 +710,7 @@ impl SelectorMap {
         let init_len = matching_rules_list.len();
 
         for rule in self.other_rules.iter() {
-            if rule.selector.simple_selectors.is_empty() &&
+            if rule.selector.compound_selector.is_empty() &&
                rule.selector.next.is_none() {
                 matching_rules_list.push(rule.declarations.clone());
             }
@@ -725,7 +723,7 @@ impl SelectorMap {
     fn get_matching_rules_from_hash<E, Str, BorrowedStr: ?Sized, Vector>(
         element: &E,
         parent_bf: Option<&BloomFilter>,
-        hash: &HashMap<Str, Vec<Rule>>,
+        hash: &FnvHashMap<Str, Vec<Rule>>,
         key: &BorrowedStr,
         matching_rules: &mut Vector,
         relations: &mut StyleRelations)
@@ -753,7 +751,7 @@ impl SelectorMap {
               V: VecLike<DeclarationBlock>
     {
         for rule in rules.iter() {
-            if matches_compound_selector(&*rule.selector,
+            if matches_complex_selector(&*rule.selector,
                                          element, parent_bf, relations) {
                 matching_rules.push(rule.declarations.clone());
             }
@@ -786,7 +784,7 @@ impl SelectorMap {
 
     /// Retrieve the first ID name in Rule, or None otherwise.
     fn get_id_name(rule: &Rule) -> Option<Atom> {
-        for ss in &rule.selector.simple_selectors {
+        for ss in &rule.selector.compound_selector {
             // TODO(pradeep): Implement case-sensitivity based on the
             // document type and quirks mode.
             if let SimpleSelector::ID(ref id) = *ss {
@@ -799,7 +797,7 @@ impl SelectorMap {
 
     /// Retrieve the FIRST class name in Rule, or None otherwise.
     fn get_class_name(rule: &Rule) -> Option<Atom> {
-        for ss in &rule.selector.simple_selectors {
+        for ss in &rule.selector.compound_selector {
             // TODO(pradeep): Implement case-sensitivity based on the
             // document type and quirks mode.
             if let SimpleSelector::Class(ref class) = *ss {
@@ -812,7 +810,7 @@ impl SelectorMap {
 
     /// Retrieve the name if it is a type selector, or None otherwise.
     fn get_local_name(rule: &Rule) -> Option<LocalName<TheSelectorImpl>> {
-        for ss in &rule.selector.simple_selectors {
+        for ss in &rule.selector.compound_selector {
             if let SimpleSelector::LocalName(ref n) = *ss {
                 return Some(LocalName {
                     name: n.name.clone(),
@@ -830,8 +828,8 @@ impl SelectorMap {
 pub struct Rule {
     // This is an Arc because Rule will essentially be cloned for every element
     // that it matches. Selector contains an owned vector (through
-    // CompoundSelector) and we want to avoid the allocation.
-    pub selector: Arc<CompoundSelector<TheSelectorImpl>>,
+    // ComplexSelector) and we want to avoid the allocation.
+    pub selector: Arc<ComplexSelector<TheSelectorImpl>>,
     pub declarations: DeclarationBlock,
 }
 
@@ -900,6 +898,6 @@ impl<'a> DoubleEndedIterator for DeclarationBlockIter<'a> {
     }
 }
 
-fn find_push<Str: Eq + Hash>(map: &mut HashMap<Str, Vec<Rule>>, key: Str, value: Rule) {
+fn find_push<Str: Eq + Hash>(map: &mut FnvHashMap<Str, Vec<Rule>>, key: Str, value: Rule) {
     map.entry(key).or_insert_with(Vec::new).push(value)
 }

From 5e4bdac2bd431c8acef79daeac621ffc84971735 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Sun, 21 Aug 2016 00:24:23 +0200
Subject: [PATCH 14/15] Import SelectorMap unit tests from the selectors crate.

---
 components/style/selector_matching.rs |  18 ++---
 tests/unit/style/lib.rs               |   1 +
 tests/unit/style/selector_matching.rs | 103 ++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 9 deletions(-)
 create mode 100644 tests/unit/style/selector_matching.rs

diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs
index 573de8b4575..d04aa322924 100644
--- a/components/style/selector_matching.rs
+++ b/components/style/selector_matching.rs
@@ -609,16 +609,16 @@ impl PerPseudoElementSelectorMap {
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct SelectorMap {
     // TODO: Tune the initial capacity of the HashMap
-    id_hash: FnvHashMap<Atom, Vec<Rule>>,
-    class_hash: FnvHashMap<Atom, Vec<Rule>>,
-    local_name_hash: FnvHashMap<Atom, Vec<Rule>>,
+    pub id_hash: FnvHashMap<Atom, Vec<Rule>>,
+    pub class_hash: FnvHashMap<Atom, Vec<Rule>>,
+    pub local_name_hash: FnvHashMap<Atom, Vec<Rule>>,
     /// Same as local_name_hash, but keys are lower-cased.
     /// For HTML elements in HTML documents.
-    lower_local_name_hash: FnvHashMap<Atom, Vec<Rule>>,
+    pub lower_local_name_hash: FnvHashMap<Atom, Vec<Rule>>,
     /// Rules that don't have ID, class, or element selectors.
-    other_rules: Vec<Rule>,
+    pub other_rules: Vec<Rule>,
     /// Whether this hash is empty.
-    empty: bool,
+    pub empty: bool,
 }
 
 #[inline]
@@ -783,7 +783,7 @@ impl SelectorMap {
     }
 
     /// Retrieve the first ID name in Rule, or None otherwise.
-    fn get_id_name(rule: &Rule) -> Option<Atom> {
+    pub fn get_id_name(rule: &Rule) -> Option<Atom> {
         for ss in &rule.selector.compound_selector {
             // TODO(pradeep): Implement case-sensitivity based on the
             // document type and quirks mode.
@@ -796,7 +796,7 @@ impl SelectorMap {
     }
 
     /// Retrieve the FIRST class name in Rule, or None otherwise.
-    fn get_class_name(rule: &Rule) -> Option<Atom> {
+    pub fn get_class_name(rule: &Rule) -> Option<Atom> {
         for ss in &rule.selector.compound_selector {
             // TODO(pradeep): Implement case-sensitivity based on the
             // document type and quirks mode.
@@ -809,7 +809,7 @@ impl SelectorMap {
     }
 
     /// Retrieve the name if it is a type selector, or None otherwise.
-    fn get_local_name(rule: &Rule) -> Option<LocalName<TheSelectorImpl>> {
+    pub fn get_local_name(rule: &Rule) -> Option<LocalName<TheSelectorImpl>> {
         for ss in &rule.selector.compound_selector {
             if let SimpleSelector::LocalName(ref n) = *ss {
                 return Some(LocalName {
diff --git a/tests/unit/style/lib.rs b/tests/unit/style/lib.rs
index 56111373e96..d394e69e8c6 100644
--- a/tests/unit/style/lib.rs
+++ b/tests/unit/style/lib.rs
@@ -23,6 +23,7 @@ mod logical_geometry;
 mod media_queries;
 mod parsing;
 mod properties;
+mod selector_matching;
 mod str;
 mod stylesheets;
 mod value;
diff --git a/tests/unit/style/selector_matching.rs b/tests/unit/style/selector_matching.rs
new file mode 100644
index 00000000000..22e11288c48
--- /dev/null
+++ b/tests/unit/style/selector_matching.rs
@@ -0,0 +1,103 @@
+/* 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/. */
+
+use cssparser::Parser;
+use selectors::parser::{LocalName, ParserContext, parse_selector_list};
+use std::sync::Arc;
+use string_cache::Atom;
+use style::properties::Importance;
+use style::selector_matching::{DeclarationBlock, Rule, SelectorMap};
+
+/// Helper method to get some Rules from selector strings.
+/// Each sublist of the result contains the Rules for one StyleRule.
+fn get_mock_rules(css_selectors: &[&str]) -> Vec<Vec<Rule>> {
+    css_selectors.iter().enumerate().map(|(i, selectors)| {
+        let context = ParserContext::new();
+        parse_selector_list(&context, &mut Parser::new(*selectors))
+        .unwrap().into_iter().map(|s| {
+            Rule {
+                selector: s.complex_selector.clone(),
+                declarations: DeclarationBlock {
+                    mixed_declarations: Arc::new(Vec::new()),
+                    importance: Importance::Normal,
+                    specificity: s.specificity,
+                    source_order: i,
+                }
+            }
+        }).collect()
+    }).collect()
+}
+
+fn get_mock_map(selectors: &[&str]) -> SelectorMap {
+    let mut map = SelectorMap::new();
+    let selector_rules = get_mock_rules(selectors);
+
+    for rules in selector_rules.into_iter() {
+        for rule in rules.into_iter() {
+            map.insert(rule)
+        }
+    }
+
+    map
+}
+
+#[test]
+fn test_rule_ordering_same_specificity() {
+    let rules_list = get_mock_rules(&["a.intro", "img.sidebar"]);
+    let a = &rules_list[0][0].declarations;
+    let b = &rules_list[1][0].declarations;
+    assert!((a.specificity, a.source_order) < ((b.specificity, b.source_order)),
+            "The rule that comes later should win.");
+}
+
+
+#[test]
+fn test_get_id_name() {
+    let rules_list = get_mock_rules(&[".intro", "#top"]);
+    assert_eq!(SelectorMap::get_id_name(&rules_list[0][0]), None);
+    assert_eq!(SelectorMap::get_id_name(&rules_list[1][0]), Some(Atom::from("top")));
+}
+
+#[test]
+fn test_get_class_name() {
+    let rules_list = get_mock_rules(&[".intro.foo", "#top"]);
+    assert_eq!(SelectorMap::get_class_name(&rules_list[0][0]), Some(Atom::from("intro")));
+    assert_eq!(SelectorMap::get_class_name(&rules_list[1][0]), None);
+}
+
+#[test]
+fn test_get_local_name() {
+    let rules_list = get_mock_rules(&["img.foo", "#top", "IMG", "ImG"]);
+    let check = |i: usize, names: Option<(&str, &str)>| {
+        assert!(SelectorMap::get_local_name(&rules_list[i][0])
+                == names.map(|(name, lower_name)| LocalName {
+                        name: Atom::from(name),
+                        lower_name: Atom::from(lower_name) }))
+    };
+    check(0, Some(("img", "img")));
+    check(1, None);
+    check(2, Some(("IMG", "img")));
+    check(3, Some(("ImG", "img")));
+}
+
+#[test]
+fn test_insert() {
+    let rules_list = get_mock_rules(&[".intro.foo", "#top"]);
+    let mut selector_map = SelectorMap::new();
+    selector_map.insert(rules_list[1][0].clone());
+    assert_eq!(1, selector_map.id_hash.get(&atom!("top")).unwrap()[0].declarations.source_order);
+    selector_map.insert(rules_list[0][0].clone());
+    assert_eq!(0, selector_map.class_hash.get(&Atom::from("intro")).unwrap()[0].declarations.source_order);
+    assert!(selector_map.class_hash.get(&Atom::from("foo")).is_none());
+}
+
+#[test]
+fn test_get_universal_rules() {
+    let map = get_mock_map(&["*|*", "#foo > *|*", ".klass", "#id"]);
+    let mut decls = vec![];
+
+    map.get_universal_rules(&mut decls);
+
+    assert_eq!(decls.len(), 1);
+}

From f9150af9367c9aff7184010b0656fd2881eae7cb Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Sun, 21 Aug 2016 03:20:29 +0200
Subject: [PATCH 15/15] Keep track of the number of important and normal
 declarations in a block
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

… instead of the presence (`bool` flags) of each.

This allows removing `recalc_any` which iterated over the `Vec`.
---
 components/script/dom/element.rs              | 61 +++++++++++++++---
 .../style/properties/properties.mako.rs       | 64 +++++++------------
 components/style/selector_matching.rs         | 12 ++--
 tests/unit/style/properties/serialization.rs  | 12 ++--
 tests/unit/style/stylesheets.rs               | 28 +++++---
 5 files changed, 107 insertions(+), 70 deletions(-)

diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index f6f8661689c..26ad3eaee40 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -762,12 +762,21 @@ impl Element {
         fn remove(element: &Element, property: &str) {
             let mut inline_declarations = element.style_attribute.borrow_mut();
             if let &mut Some(ref mut declarations) = &mut *inline_declarations {
-                let index = declarations.declarations
-                                        .iter()
-                                        .position(|&(ref decl, _)| decl.matches(property));
+                let mut importance = None;
+                let index = declarations.declarations.iter().position(|&(ref decl, i)| {
+                    let matching = decl.matches(property);
+                    if matching {
+                        importance = Some(i)
+                    }
+                    matching
+                });
                 if let Some(index) = index {
                     Arc::make_mut(&mut declarations.declarations).remove(index);
-                    declarations.recalc_any();
+                    if importance.unwrap().important() {
+                        declarations.important_count -= 1;
+                    } else {
+                        declarations.normal_count -= 1;
+                    }
                 }
             }
         }
@@ -791,21 +800,42 @@ impl Element {
                     'outer: for incoming_declaration in declarations {
                         for existing_declaration in &mut *existing_declarations {
                             if existing_declaration.0.name() == incoming_declaration.name() {
+                                match (existing_declaration.1, importance) {
+                                    (Importance::Normal, Importance::Important) => {
+                                        declaration_block.normal_count -= 1;
+                                        declaration_block.important_count += 1;
+                                    }
+                                    (Importance::Important, Importance::Normal) => {
+                                        declaration_block.normal_count += 1;
+                                        declaration_block.important_count -= 1;
+                                    }
+                                    _ => {}
+                                }
                                 *existing_declaration = (incoming_declaration, importance);
                                 continue 'outer;
                             }
                         }
                         existing_declarations.push((incoming_declaration, importance));
+                        if importance.important() {
+                            declaration_block.important_count += 1;
+                        } else {
+                            declaration_block.normal_count += 1;
+                        }
                     }
                 }
-                declaration_block.recalc_any();
                 return;
             }
 
+            let (normal_count, important_count) = if importance.important() {
+                (0, declarations.len() as u32)
+            } else {
+                (declarations.len() as u32, 0)
+            };
+
             *inline_declarations = Some(PropertyDeclarationBlock {
                 declarations: Arc::new(declarations.into_iter().map(|d| (d, importance)).collect()),
-                any_important: importance.important(),
-                any_normal: !importance.important(),
+                normal_count: normal_count,
+                important_count: important_count,
             });
         }
 
@@ -818,13 +848,24 @@ impl Element {
                                               new_importance: Importance) {
         {
             let mut inline_declarations = self.style_attribute().borrow_mut();
-            if let &mut Some(ref mut declarations) = &mut *inline_declarations {
+            if let &mut Some(ref mut block) = &mut *inline_declarations {
                 // Usually, the reference counts of `from` and `to` will be 1 here. But transitions
                 // could make them greater than that.
-                let declarations = Arc::make_mut(&mut declarations.declarations);
+                let declarations = Arc::make_mut(&mut block.declarations);
                 for &mut (ref declaration, ref mut importance) in declarations {
                     if properties.iter().any(|p| declaration.name() == **p) {
-                        *importance = new_importance
+                        match (*importance, new_importance) {
+                            (Importance::Normal, Importance::Important) => {
+                                block.normal_count -= 1;
+                                block.important_count += 1;
+                            }
+                            (Importance::Important, Importance::Normal) => {
+                                block.normal_count += 1;
+                                block.important_count -= 1;
+                            }
+                            _ => {}
+                        }
+                        *importance = new_importance;
                     }
                 }
             }
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index 539e03253c4..b0c92710a06 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -288,27 +288,11 @@ pub struct PropertyDeclarationBlock {
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")]
     pub declarations: Arc<Vec<(PropertyDeclaration, Importance)>>,
 
-    /// Whether `self.declaration` contains at least one declaration with `Importance::Normal`
-    pub any_normal: bool,
+    /// The number of entries in `self.declaration` with `Importance::Normal`
+    pub normal_count: u32,
 
-    /// Whether `self.declaration` contains at least one declaration with `Importance::Important`
-    pub any_important: bool,
-}
-
-impl PropertyDeclarationBlock {
-    pub fn recalc_any(&mut self) {
-        let mut any_normal = false;
-        let mut any_important = false;
-        for &(_, importance) in &*self.declarations {
-            if importance.important() {
-                any_important = true
-            } else {
-                any_normal = true
-            }
-        }
-        self.any_normal = any_normal;
-        self.any_important = any_important;
-    }
+    /// The number of entries in `self.declaration` with `Importance::Important`
+    pub important_count: u32,
 }
 
 impl ToCss for PropertyDeclarationBlock {
@@ -564,8 +548,8 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
 pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Parser)
                                        -> PropertyDeclarationBlock {
     let mut declarations = Vec::new();
-    let mut any_normal = false;
-    let mut any_important = false;
+    let mut normal_count = 0;
+    let mut important_count = 0;
     let parser = PropertyDeclarationParser {
         context: context,
     };
@@ -574,9 +558,9 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
         match declaration {
             Ok((results, importance)) => {
                 if importance.important() {
-                    any_important = true
+                    important_count += results.len() as u32;
                 } else {
-                    any_normal = true
+                    normal_count += results.len() as u32;
                 }
                 declarations.extend(results.into_iter().map(|d| (d, importance)))
             }
@@ -588,50 +572,46 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
             }
         }
     }
-    let (declarations, removed_any) = deduplicate_property_declarations(declarations);
     let mut block = PropertyDeclarationBlock {
         declarations: Arc::new(declarations),
-        any_normal: any_normal,
-        any_important: any_important,
+        normal_count: normal_count,
+        important_count: important_count,
     };
-    if removed_any {
-        block.recalc_any();
-    }
+    deduplicate_property_declarations(&mut block);
     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(declarations: Vec<(PropertyDeclaration, Importance)>)
-                                     -> (Vec<(PropertyDeclaration, Importance)>, bool) {
-    let mut removed_any = false;
+fn deduplicate_property_declarations(block: &mut PropertyDeclarationBlock) {
     let mut deduplicated = Vec::new();
     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 declarations.into_iter().rev() {
+
+    let declarations = Arc::get_mut(&mut block.declarations).unwrap();
+    for (declaration, importance) in 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}() {
-                                removed_any = true;
+                                block.important_count -= 1;
                                 continue
                             }
                             if seen_normal.get_${property.ident}() {
                                 remove_one(&mut deduplicated, |d| {
                                     matches!(d, &(PropertyDeclaration::${property.camel_case}(..), _))
                                 });
-                                removed_any = true;
+                                block.normal_count -= 1;
                             }
                             seen_important.set_${property.ident}()
                         } else {
                             if seen_normal.get_${property.ident}() ||
                                seen_important.get_${property.ident}() {
-                                removed_any = true;
+                                block.normal_count -= 1;
                                 continue
                             }
                             seen_normal.set_${property.ident}()
@@ -644,20 +624,20 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp
             PropertyDeclaration::Custom(ref name, _) => {
                 if importance.important() {
                     if seen_custom_important.contains(name) {
-                        removed_any = true;
+                        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)
                         });
-                        removed_any = true;
+                        block.normal_count -= 1;
                     }
                     seen_custom_important.push(name.clone())
                 } else {
                     if seen_custom_normal.contains(name) ||
                        seen_custom_important.contains(name) {
-                        removed_any = true;
+                        block.normal_count -= 1;
                         continue
                     }
                     seen_custom_normal.push(name.clone())
@@ -667,7 +647,7 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp
         deduplicated.push((declaration, importance))
     }
     deduplicated.reverse();
-    (deduplicated, removed_any)
+    *declarations = deduplicated;
 }
 
 #[inline]
diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs
index d04aa322924..781f2298c4a 100644
--- a/components/style/selector_matching.rs
+++ b/components/style/selector_matching.rs
@@ -162,8 +162,8 @@ impl Stylist {
         // Take apart the StyleRule into individual Rules and insert
         // them into the SelectorMap of that priority.
         macro_rules! append(
-            ($style_rule: ident, $priority: ident, $importance: expr, $any: ident) => {
-                if $style_rule.declarations.$any {
+            ($style_rule: ident, $priority: ident, $importance: expr, $count: ident) => {
+                if $style_rule.declarations.$count > 0 {
                     for selector in &$style_rule.selectors {
                         let map = if let Some(ref pseudo) = selector.pseudo_element {
                             self.pseudos_map
@@ -191,8 +191,8 @@ impl Stylist {
         for rule in stylesheet.effective_rules(&self.device) {
             match *rule {
                 CSSRule::Style(ref style_rule) => {
-                    append!(style_rule, normal, Importance::Normal, any_normal);
-                    append!(style_rule, important, Importance::Important, any_important);
+                    append!(style_rule, normal, Importance::Normal, normal_count);
+                    append!(style_rule, important, Importance::Important, important_count);
                     rules_source_order += 1;
 
                     for selector in &style_rule.selectors {
@@ -394,7 +394,7 @@ impl Stylist {
 
         // Step 4: Normal style attributes.
         if let Some(ref sa)  = style_attribute {
-            if sa.any_normal {
+            if sa.normal_count > 0 {
                 relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
                 Push::push(
                     applicable_declarations,
@@ -416,7 +416,7 @@ impl Stylist {
 
         // Step 6: `!important` style attributes.
         if let Some(ref sa) = style_attribute {
-            if sa.any_important {
+            if sa.important_count > 0 {
                 relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
                 Push::push(
                     applicable_declarations,
diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs
index d22043b237b..f47b5303a0f 100644
--- a/tests/unit/style/properties/serialization.rs
+++ b/tests/unit/style/properties/serialization.rs
@@ -46,8 +46,10 @@ fn property_declaration_block_should_serialize_correctly() {
 
     let block = PropertyDeclarationBlock {
         declarations: Arc::new(declarations),
-        any_important: true,
-        any_normal: true,
+
+        // Incorrect, but not used here:
+        normal_count: 0,
+        important_count: 0,
     };
 
     let css_string = block.to_css_string();
@@ -64,8 +66,10 @@ mod shorthand_serialization {
     pub fn shorthand_properties_to_string(properties: Vec<PropertyDeclaration>) -> String {
         let block = PropertyDeclarationBlock {
             declarations: Arc::new(properties.into_iter().map(|d| (d, Importance::Normal)).collect()),
-            any_important: false,
-            any_normal: true,
+
+            // Incorrect, but not used here:
+            normal_count: 0,
+            important_count: 0,
         };
 
         block.to_css_string()
diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs
index 28b795d440e..a73f43b51ba 100644
--- a/tests/unit/style/stylesheets.rs
+++ b/tests/unit/style/stylesheets.rs
@@ -24,8 +24,18 @@ 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: none !important; }
-        html , body /**/ { display: block; }
+        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; }
         @keyframes foo {
             from { width: 0% }
@@ -92,9 +102,11 @@ fn test_parse_stylesheet() {
                         (PropertyDeclaration::Display(DeclaredValue::Value(
                             longhands::display::SpecifiedValue::none)),
                          Importance::Important),
+                        (PropertyDeclaration::Custom(Atom::from("a"), DeclaredValue::Inherit),
+                         Importance::Important),
                     ]),
-                    any_normal: false,
-                    any_important: true,
+                    normal_count: 0,
+                    important_count: 2,
                 },
             }),
             CSSRule::Style(StyleRule {
@@ -140,8 +152,8 @@ fn test_parse_stylesheet() {
                             longhands::display::SpecifiedValue::block)),
                          Importance::Normal),
                     ]),
-                    any_normal: true,
-                    any_important: false,
+                    normal_count: 1,
+                    important_count: 0,
                 },
             }),
             CSSRule::Style(StyleRule {
@@ -196,8 +208,8 @@ fn test_parse_stylesheet() {
                         (PropertyDeclaration::BackgroundClip(DeclaredValue::Initial),
                          Importance::Normal),
                     ]),
-                    any_normal: true,
-                    any_important: false,
+                    normal_count: 8,
+                    important_count: 0,
                 },
             }),
             CSSRule::Keyframes(KeyframesRule {