From 9e61c1962b3fc410395351085e33d46d9d631f01 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Thu, 12 Oct 2017 17:30:41 +0200
Subject: [PATCH] style: Unify invalidated_child with invalidated_descendants.

I think invalidated_descendants was buggy, and this fixes it.
---
 .../style/invalidation/element/collector.rs   | 26 ++++++-----------
 .../style/invalidation/element/invalidator.rs | 28 +++++--------------
 2 files changed, 16 insertions(+), 38 deletions(-)

diff --git a/components/style/invalidation/element/collector.rs b/components/style/invalidation/element/collector.rs
index f6fbff96707..271e16f323c 100644
--- a/components/style/invalidation/element/collector.rs
+++ b/components/style/invalidation/element/collector.rs
@@ -203,16 +203,20 @@ where
         }
     }
 
-    fn invalidated_child(
+    fn invalidated_descendants(
         &self,
         element: E,
-        _data: Option<&mut ElementData>,
+        data: Option<&mut ElementData>,
         child: E,
     ) {
         if child.get_data().is_none() {
             return;
         }
 
+        if data.as_ref().map_or(true, |d| d.styles.is_display_none()) {
+            return;
+        }
+
         // The child may not be a flattened tree child of the current element,
         // but may be arbitrarily deep.
         //
@@ -220,24 +224,12 @@ where
         // we need to propagate it as appropriate.
         let mut current = child.traversal_parent();
         while let Some(parent) = current.take() {
+            unsafe { parent.set_dirty_descendants() };
+            current = parent.traversal_parent();
+
             if parent == element {
                 break;
             }
-
-            unsafe { parent.set_dirty_descendants() };
-            current = parent.traversal_parent();
-        }
-    }
-
-    fn invalidated_descendants(
-        &self,
-        element: E,
-        data: Option<&mut ElementData>,
-    ) {
-        // FIXME(emilio): We probably want to walk the flattened tree here too,
-        // and remove invalidated_child instead, or something like that.
-        if data.as_ref().map_or(false, |d| !d.styles.is_display_none()) {
-            unsafe { element.set_dirty_descendants() };
         }
     }
 
diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs
index 682e7b23c6b..072577d6f44 100644
--- a/components/style/invalidation/element/invalidator.rs
+++ b/components/style/invalidation/element/invalidator.rs
@@ -54,14 +54,6 @@ where
         data: Option<&mut ElementData>,
     );
 
-    /// Executes an arbitrary action when a direct child is invalidated.
-    fn invalidated_child(
-        &self,
-        element: E,
-        data: Option<&mut ElementData>,
-        child: E,
-    );
-
     /// Executes an action when `Self` is invalidated.
     fn invalidated_self(
         &self,
@@ -74,6 +66,7 @@ where
         &self,
         element: E,
         data: Option<&mut ElementData>,
+        child: E,
     );
 }
 
@@ -388,23 +381,23 @@ where
                 sibling_invalidations,
             );
 
+        let invalidated_descendants = child_invalidator.invalidate_descendants(
+            &invalidations_for_descendants
+        );
+
         // The child may not be a flattened tree child of the current element,
         // but may be arbitrarily deep.
         //
         // Since we keep the traversal flags in terms of the flattened tree,
         // we need to propagate it as appropriate.
-        if invalidated_child {
-            self.processor.invalidated_child(
+        if invalidated_child || invalidated_descendants {
+            self.processor.invalidated_descendants(
                 self.element,
                 self.data.as_mut().map(|d| &mut **d),
                 child,
             );
         }
 
-        let invalidated_descendants = child_invalidator.invalidate_descendants(
-            &invalidations_for_descendants
-        );
-
         invalidated_child || invalidated_descendants
     }
 
@@ -517,13 +510,6 @@ where
 
         any_descendant |= self.invalidate_nac(invalidations);
 
-        if any_descendant {
-            self.processor.invalidated_descendants(
-                self.element,
-                self.data.as_mut().map(|d| &mut **d)
-            );
-        }
-
         any_descendant
     }