From 1359c7c2184787b25e0300f2ebd8d472c4211ea3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 20 Feb 2018 11:16:12 +0100
Subject: [PATCH] style: Don't try to serialize the same shorthand over and
 over.

I looked at what were we doing in that loop, and we're doing tons of dumb stuff.

In particular, we try to serialize the "all" shorthand all the time. This patch
prevents us from trying to serialize shorthands that we've already tried to
serialize.
---
 .../style/properties/declaration_block.rs     | 25 +++++++++++++------
 .../style/properties/properties.mako.rs       | 14 +++++++++++
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs
index 278f616422c..baf68d071b0 100644
--- a/components/style/properties/declaration_block.rs
+++ b/components/style/properties/declaration_block.rs
@@ -769,7 +769,13 @@ impl PropertyDeclarationBlock {
         // Step 1 -> dest = result list
 
         // Step 2
-        let mut already_serialized_longhands = LonghandIdSet::new();
+        //
+        // NOTE(emilio): We reuse this set for both longhands and shorthands
+        // with subtly different meaning. For longhands, only longhands that
+        // have actually been serialized (either by themselves, or as part of a
+        // shorthand) appear here. For shorthands, all the shorthands that we've
+        // attempted to serialize appear here.
+        let mut already_serialized = NonCustomPropertyIdSet::new();
 
         // Step 3
         for (declaration, importance) in self.declaration_importance_iter() {
@@ -794,7 +800,7 @@ impl PropertyDeclarationBlock {
             };
 
             // Step 3.2
-            if already_serialized_longhands.contains(longhand_id) {
+            if already_serialized.contains(longhand_id.into()) {
                 continue;
             }
 
@@ -804,6 +810,12 @@ impl PropertyDeclarationBlock {
 
             // Step 3.3.2
             for &shorthand in longhand_id.shorthands() {
+                // We already attempted to serialize this shorthand before.
+                if already_serialized.contains(shorthand.into()) {
+                    continue;
+                }
+                already_serialized.insert(shorthand.into());
+
                 // Substep 2 & 3
                 let mut current_longhands = SmallVec::<[_; 10]>::new();
                 let mut important_count = 0;
@@ -814,7 +826,7 @@ impl PropertyDeclarationBlock {
                     self.declarations.iter().any(|l| {
                         match l.id() {
                             PropertyDeclarationId::Longhand(id) => {
-                                if already_serialized_longhands.contains(id) {
+                                if already_serialized.contains(id.into()) {
                                     return false;
                                 }
 
@@ -931,7 +943,7 @@ impl PropertyDeclarationBlock {
                     };
 
                     // Substep 9
-                    already_serialized_longhands.insert(longhand_id);
+                    already_serialized.insert(longhand_id.into());
                 }
 
                 // FIXME(https://github.com/w3c/csswg-drafts/issues/1774)
@@ -943,11 +955,10 @@ impl PropertyDeclarationBlock {
             }
 
             // Step 3.3.4
-            if already_serialized_longhands.contains(longhand_id) {
+            if already_serialized.contains(longhand_id.into()) {
                 continue;
             }
 
-
             // Steps 3.3.5, 3.3.6 & 3.3.7
             // Need to specify an iterator type here even though it’s unused to work around
             // "error: unable to infer enough type information about `_`;
@@ -962,7 +973,7 @@ impl PropertyDeclarationBlock {
             )?;
 
             // Step 3.3.8
-            already_serialized_longhands.insert(longhand_id);
+            already_serialized.insert(longhand_id.into());
         }
 
         // Step 4
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index c7fdf29d858..6ae1bead5ee 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -547,6 +547,20 @@ pub struct NonCustomPropertyIdSet {
 }
 
 impl NonCustomPropertyIdSet {
+    /// Creates an empty `NonCustomPropertyIdSet`.
+    pub fn new() -> Self {
+        Self {
+            storage: Default::default(),
+        }
+    }
+
+    /// Insert a non-custom-property in the set.
+    #[inline]
+    pub fn insert(&mut self, id: NonCustomPropertyId) {
+        let bit = id.0;
+        self.storage[bit / 32] |= 1 << (bit % 32);
+    }
+
     /// Return whether the given property is in the set
     #[inline]
     pub fn contains(&self, id: NonCustomPropertyId) -> bool {