servo: Merge #19927 - Avoid stylist flushes when sheets are appended and then removed again before flusing layout (from bholley:sheet_flush_optimization); r=emilio

https://bugzilla.mozilla.org/show_bug.cgi?id=1434756

Source-Repo: https://github.com/servo/servo
Source-Revision: 0a8c58dce95da4a42afb82171055821f8aca05d9

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 41ac18396d68b2482f670a2726e2c926a900cfd6

servo/components/style/stylesheet_set.rs

@@ -18,8 +18,11 @@ struct StylesheetSetEntry<S>
 where
     S: StylesheetInDocument + PartialEq + 'static,
 {
+    /// The sheet.
     sheet: S,
-    dirty: bool,
+
+    /// Whether this sheet has been part of at least one flush.
+    committed: bool,
 }
 
 impl<S> StylesheetSetEntry<S>
@@ -27,7 +30,7 @@ where
     S: StylesheetInDocument + PartialEq + 'static,
 {
     fn new(sheet: S) -> Self {
-        Self { sheet, dirty: true }
+        Self { sheet, committed: false }
     }
 }
 
@@ -239,9 +242,9 @@ where
         loop {
             let potential_sheet = self.iter.next()?;
 
-            let dirty = mem::replace(&mut potential_sheet.dirty, false);
-            if dirty {
-                // If the sheet was dirty, we need to do a full rebuild anyway.
+            let committed = mem::replace(&mut potential_sheet.committed, true);
+            if !committed {
+                // If the sheet was uncommitted, we need to do a full rebuild anyway.
                 return Some((&potential_sheet.sheet, SheetRebuildKind::Full))
             }
 
@@ -303,18 +306,22 @@ where
     }
 
     fn remove(&mut self, sheet: &S) {
-        let old_len = self.entries.len();
-        self.entries.retain(|entry| entry.sheet != *sheet);
-        if cfg!(feature = "servo") {
+        let index = self.entries.iter().position(|entry| {
+            entry.sheet == *sheet
+        });
+        if cfg!(feature = "gecko") && index.is_none() {
             // FIXME(emilio): Make Gecko's PresShell::AddUserSheet not suck.
-            //
-            // Hopefully that's not necessary for correctness, just somewhat
-            // overkill.
-            debug_assert!(self.entries.len() != old_len, "Sheet not found?");
+            return;
         }
+        let sheet = self.entries.remove(index.unwrap());
         // Removing sheets makes us tear down the whole cascade and invalidation
-        // data.
-        self.set_data_validity_at_least(OriginValidity::FullyInvalid);
+        // data, but only if the sheet has been involved in at least one flush.
+        // Checking whether the sheet has been committed allows us to avoid
+        // rebuilding the world when sites quickly append and remove a stylesheet.
+        // See bug 1434756.
+        if sheet.committed {
+            self.set_data_validity_at_least(OriginValidity::FullyInvalid);
+        }
     }
 
     fn contains(&self, sheet: &S) -> bool {