From 4f4d4803263f48030cfda400410e1b2b85cb8d2d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 31 Dec 2019 18:14:35 +0000
Subject: [PATCH] style: Do not incorrectly share style across elements with
 different part names.

Do the same we do for classes for now. We could be more precise and achieve a
bit more sharing with some more effort (left a comment there), but it seems
unlikely to matter in practice (and if we did that, we'd probably want to do the
same for classes).

Differential Revision: https://phabricator.services.mozilla.com/D58453
---
 components/style/sharing/checks.rs | 15 ++++++++-
 components/style/sharing/mod.rs    | 53 ++++++++++++++++++++++++++----
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/components/style/sharing/checks.rs b/components/style/sharing/checks.rs
index 19c1c52acdc..9c860f0258f 100644
--- a/components/style/sharing/checks.rs
+++ b/components/style/sharing/checks.rs
@@ -81,7 +81,7 @@ where
     target.pres_hints() == candidate.pres_hints()
 }
 
-/// Whether a given element has the same class attribute than a given candidate.
+/// Whether a given element has the same class attribute as a given candidate.
 ///
 /// We don't try to share style across elements with different class attributes.
 pub fn have_same_class<E>(
@@ -94,6 +94,19 @@ where
     target.class_list() == candidate.class_list()
 }
 
+/// Whether a given element has the same class attribute as a given candidate.
+///
+/// We don't try to share style across elements with different part attributes.
+pub fn have_same_parts<E>(
+    target: &mut StyleSharingTarget<E>,
+    candidate: &mut StyleSharingCandidate<E>,
+) -> bool
+where
+    E: TElement,
+{
+    target.part_list() == candidate.part_list()
+}
+
 /// Whether a given element and a candidate match the same set of "revalidation"
 /// selectors.
 ///
diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs
index 9b8c1275e4b..2d36aeea7ff 100644
--- a/components/style/sharing/mod.rs
+++ b/components/style/sharing/mod.rs
@@ -124,10 +124,16 @@ impl OpaqueComputedValues {
 pub struct ValidationData {
     /// The class list of this element.
     ///
-    /// TODO(emilio): See if it's worth to sort them, or doing something else in
-    /// a similar fashion as what Boris is doing for the ID attribute.
+    /// TODO(emilio): Maybe check whether rules for these classes apply to the
+    /// element?
     class_list: Option<SmallVec<[Atom; 5]>>,
 
+    /// The part list of this element.
+    ///
+    /// TODO(emilio): Maybe check whether rules with these part names apply to
+    /// the element?
+    part_list: Option<SmallVec<[Atom; 5]>>,
+
     /// The list of presentational attributes of the element.
     pres_hints: Option<SmallVec<[ApplicableDeclarationBlock; 5]>>,
 
@@ -161,22 +167,41 @@ impl ValidationData {
         })
     }
 
+    /// Get or compute the part-list associated with this element.
+    pub fn part_list<E>(&mut self, element: E) -> &[Atom]
+    where
+        E: TElement,
+    {
+        if !element.has_part_attr() {
+            return &[]
+        }
+        self.part_list.get_or_insert_with(|| {
+            let mut list = SmallVec::<[Atom; 5]>::new();
+            element.each_part(|p| list.push(p.clone()));
+            // See below for the reasoning.
+            if !list.spilled() {
+                list.sort_unstable_by_key(|a| a.get_hash());
+            }
+            list
+        })
+    }
+
     /// Get or compute the class-list associated with this element.
     pub fn class_list<E>(&mut self, element: E) -> &[Atom]
     where
         E: TElement,
     {
         self.class_list.get_or_insert_with(|| {
-            let mut class_list = SmallVec::<[Atom; 5]>::new();
-            element.each_class(|c| class_list.push(c.clone()));
+            let mut list = SmallVec::<[Atom; 5]>::new();
+            element.each_class(|c| list.push(c.clone()));
             // Assuming there are a reasonable number of classes (we use the
             // inline capacity as "reasonable number"), sort them to so that
             // we don't mistakenly reject sharing candidates when one element
             // has "foo bar" and the other has "bar foo".
-            if !class_list.spilled() {
-                class_list.sort_by(|a, b| a.get_hash().cmp(&b.get_hash()));
+            if !list.spilled() {
+                list.sort_unstable_by_key(|a| a.get_hash());
             }
-            class_list
+            list
         })
     }
 
@@ -273,6 +298,11 @@ impl<E: TElement> StyleSharingCandidate<E> {
         self.validation_data.class_list(self.element)
     }
 
+    /// Get the part list of this candidate.
+    fn part_list(&mut self) -> &[Atom] {
+        self.validation_data.part_list(self.element)
+    }
+
     /// Get the pres hints of this candidate.
     fn pres_hints(&mut self) -> &[ApplicableDeclarationBlock] {
         self.validation_data.pres_hints(self.element)
@@ -335,6 +365,10 @@ impl<E: TElement> StyleSharingTarget<E> {
         self.validation_data.class_list(self.element)
     }
 
+    fn part_list(&mut self) -> &[Atom] {
+        self.validation_data.part_list(self.element)
+    }
+
     /// Get the pres hints of this candidate.
     fn pres_hints(&mut self) -> &[ApplicableDeclarationBlock] {
         self.validation_data.pres_hints(self.element)
@@ -772,6 +806,11 @@ impl<E: TElement> StyleSharingCache<E> {
             return None;
         }
 
+        if !checks::have_same_parts(target, candidate) {
+            trace!("Miss: Shadow parts");
+            return None;
+        }
+
         if !checks::revalidate(
             target,
             candidate,