Auto merge of #16798 - bzbarsky:stylist-lazier-updates, r=emilio

Allow stylo to do lazier stylist rebuilds

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix https://bugzilla.mozilla.org/show_bug.cgi?id=1361843 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because it's optimization, which should have no observable behavior difference.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16798)
<!-- Reviewable:end -->

components/style/gecko/data.rs

@@ -108,18 +108,26 @@ impl PerDocumentStyleDataImpl {
         let author_style_disabled = self.stylesheets.author_style_disabled();
         let mut stylesheets = Vec::<Arc<Stylesheet>>::new();
         self.stylesheets.flush(&mut stylesheets);
-        stylist.update(stylesheets.as_slice(),
+        stylist.clear();
-                       &StylesheetGuards::same(guard),
+        stylist.rebuild(stylesheets.as_slice(),
-                       /* ua_sheets = */ None,
+                        &StylesheetGuards::same(guard),
-                       /* stylesheets_changed = */ true,
+                        /* ua_sheets = */ None,
-                       author_style_disabled,
+                        /* stylesheets_changed = */ true,
-                       &mut extra_data);
+                        author_style_disabled,
+                        &mut extra_data);
     }
 
     /// Get the default computed values for this document.
     pub fn default_computed_values(&self) -> &Arc<ComputedValues> {
         self.stylist.device.default_computed_values_arc()
     }
+
+    /// Clear the stylist.  This will be a no-op if the stylist is
+    /// already cleared; the stylist handles that.
+    pub fn clear_stylist(&mut self) {
+        let mut stylist = Arc::get_mut(&mut self.stylist).unwrap();
+        stylist.clear();
+    }
 }
 
 unsafe impl HasFFI for PerDocumentStyleData {

components/style/gecko/generated/bindings.rs

@@ -1628,16 +1628,16 @@ extern "C" {
 extern "C" {
     pub fn Servo_StyleSet_AppendStyleSheet(set: RawServoStyleSetBorrowed,
                                            sheet: RawServoStyleSheetBorrowed,
-                                           unique_id: u32, flush: bool);
+                                           unique_id: u32);
 }
 extern "C" {
     pub fn Servo_StyleSet_PrependStyleSheet(set: RawServoStyleSetBorrowed,
                                             sheet: RawServoStyleSheetBorrowed,
-                                            unique_id: u32, flush: bool);
+                                            unique_id: u32);
 }
 extern "C" {
     pub fn Servo_StyleSet_RemoveStyleSheet(set: RawServoStyleSetBorrowed,
-                                           unique_id: u32, flush: bool);
+                                           unique_id: u32);
 }
 extern "C" {
     pub fn Servo_StyleSet_InsertStyleSheetBefore(set:
@@ -1645,8 +1645,7 @@ extern "C" {
                                                  sheet:
                                                      RawServoStyleSheetBorrowed,
                                                  unique_id: u32,
-                                                 before_unique_id: u32,
+                                                 before_unique_id: u32);
-                                                 flush: bool);
 }
 extern "C" {
     pub fn Servo_StyleSet_FlushStyleSheets(set: RawServoStyleSetBorrowed);

components/style/stylist.rs

@@ -85,6 +85,10 @@ pub struct Stylist {
     /// If true, the device has changed, and the stylist needs to be updated.
     is_device_dirty: bool,
 
+    /// If true, the stylist is in a cleared state (e.g. just-constructed, or
+    /// had clear() called on it with no following rebuild()).
+    is_cleared: bool,
+
     /// The current selector maps, after evaluating media
     /// rules against the current device.
     element_map: PerPseudoElementSelectorMap,
@@ -163,13 +167,15 @@ impl<'a> ExtraStyleData<'a> {
 }
 
 impl Stylist {
-    /// Construct a new `Stylist`, using a given `Device`.
+    /// Construct a new `Stylist`, using a given `Device`.  If more members are
+    /// added here, think about whether they should be reset in clear().
     #[inline]
     pub fn new(device: Device) -> Self {
         let mut stylist = Stylist {
             viewport_constraints: None,
             device: Arc::new(device),
             is_device_dirty: true,
+            is_cleared: true,
             quirks_mode: QuirksMode::NoQuirks,
 
             element_map: PerPseudoElementSelectorMap::new(),
@@ -219,22 +225,64 @@ impl Stylist {
         self.selectors_for_cache_revalidation.len()
     }
 
-    /// Update the stylist for the given document stylesheets, and optionally
+    /// Clear the stylist's state, effectively resetting it to more or less
+    /// the state Stylist::new creates.
+    ///
+    /// We preserve the state of the following members:
+    ///   device: Someone might have set this on us.
+    ///   quirks_mode: Again, someone might have set this on us.
+    ///   num_rebuilds: clear() followed by rebuild() should just increment this
+    ///
+    /// We don't just use struct update syntax with Stylist::new(self.device)
+    /// beause for some of our members we can clear them instead of creating new
+    /// objects.  This does cause unfortunate code duplication with
+    /// Stylist::new.
+    pub fn clear(&mut self) {
+        if self.is_cleared {
+            return
+        }
+
+        self.is_cleared = true;
+
+        self.viewport_constraints = None;
+        // preserve current device
+        self.is_device_dirty = true;
+        // preserve current quirks_mode value
+        self.element_map = PerPseudoElementSelectorMap::new();
+        self.pseudos_map = Default::default();
+        self.animations.clear(); // Or set to Default::default()?
+        self.precomputed_pseudo_element_decls = Default::default();
+        self.rules_source_order = 0;
+        // We want to keep rule_tree around across stylist rebuilds.
+        self.dependencies.clear();
+        self.selectors_for_cache_revalidation = SelectorMap::new();
+        self.num_selectors = 0;
+        self.num_declarations = 0;
+        // preserve num_rebuilds value, since it should stay across
+        // clear()/rebuild() cycles.
+    }
+
+    /// rebuild the stylist for the given document stylesheets, and optionally
     /// with a set of user agent stylesheets.
     ///
     /// This method resets all the style data each time the stylesheets change
     /// (which is indicated by the `stylesheets_changed` parameter), or the
     /// device is dirty, which means we need to re-evaluate media queries.
-    pub fn update<'a>(&mut self,
+    pub fn rebuild<'a>(&mut self,
-                      doc_stylesheets: &[Arc<Stylesheet>],
+                       doc_stylesheets: &[Arc<Stylesheet>],
-                      guards: &StylesheetGuards,
+                       guards: &StylesheetGuards,
-                      ua_stylesheets: Option<&UserAgentStylesheets>,
+                       ua_stylesheets: Option<&UserAgentStylesheets>,
-                      stylesheets_changed: bool,
+                       stylesheets_changed: bool,
-                      author_style_disabled: bool,
+                       author_style_disabled: bool,
-                      extra_data: &mut ExtraStyleData<'a>) -> bool {
+                       extra_data: &mut ExtraStyleData<'a>) -> bool {
+        debug_assert!(!self.is_cleared || self.is_device_dirty);
+
+        self.is_cleared = false;
+
         if !(self.is_device_dirty || stylesheets_changed) {
             return false;
         }
+
         self.num_rebuilds += 1;
 
         let cascaded_rule = ViewportRule {
@@ -251,21 +299,10 @@ impl Stylist {
                 .account_for_viewport_rule(constraints);
         }
 
-        self.element_map = PerPseudoElementSelectorMap::new();
-        self.pseudos_map = Default::default();
-        self.animations = Default::default();
         SelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| {
             self.pseudos_map.insert(pseudo, PerPseudoElementSelectorMap::new());
         });
 
-        self.precomputed_pseudo_element_decls = Default::default();
-        self.rules_source_order = 0;
-        self.dependencies.clear();
-        self.animations.clear();
-        self.selectors_for_cache_revalidation = SelectorMap::new();
-        self.num_selectors = 0;
-        self.num_declarations = 0;
-
         extra_data.clear_font_faces();
 
         if let Some(ua_stylesheets) = ua_stylesheets {
@@ -302,6 +339,27 @@ impl Stylist {
         true
     }
 
+    /// clear the stylist and then rebuild it.  Chances are, you want to use
+    /// either clear() or rebuild(), with the latter done lazily, instead.
+    pub fn update<'a>(&mut self,
+                      doc_stylesheets: &[Arc<Stylesheet>],
+                      guards: &StylesheetGuards,
+                      ua_stylesheets: Option<&UserAgentStylesheets>,
+                      stylesheets_changed: bool,
+                      author_style_disabled: bool,
+                      extra_data: &mut ExtraStyleData<'a>) -> bool {
+        debug_assert!(!self.is_cleared || self.is_device_dirty);
+
+        // We have to do a dirtiness check before clearing, because if
+        // we're not actually dirty we need to no-op here.
+        if !(self.is_device_dirty || stylesheets_changed) {
+            return false;
+        }
+        self.clear();
+        self.rebuild(doc_stylesheets, guards, ua_stylesheets, stylesheets_changed,
+                     author_style_disabled, extra_data)
+    }
+
     fn add_stylesheet<'a>(&mut self, stylesheet: &Stylesheet, guard: &SharedRwLockReadGuard,
                           extra_data: &mut ExtraStyleData<'a>) {
         if stylesheet.disabled() || !stylesheet.is_effective_for_device(&self.device, guard) {

ports/geckolib/glue.rs

@@ -619,60 +619,40 @@ pub extern "C" fn Servo_StyleSheet_ClearAndUpdate(stylesheet: RawServoStyleSheet
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_AppendStyleSheet(raw_data: RawServoStyleSetBorrowed,
                                                   raw_sheet: RawServoStyleSheetBorrowed,
-                                                  unique_id: u32,
+                                                  unique_id: u32) {
-                                                  flush: bool) {
-    let global_style_data = &*GLOBAL_STYLE_DATA;
-    let guard = global_style_data.shared_lock.read();
     let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
     let sheet = HasArcFFI::as_arc(&raw_sheet);
     data.stylesheets.append_stylesheet(sheet, unique_id);
-    if flush {
+    data.clear_stylist();
-        data.flush_stylesheets(&guard);
-    }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_PrependStyleSheet(raw_data: RawServoStyleSetBorrowed,
                                                    raw_sheet: RawServoStyleSheetBorrowed,
-                                                   unique_id: u32,
+                                                   unique_id: u32) {
-                                                   flush: bool) {
-    let global_style_data = &*GLOBAL_STYLE_DATA;
-    let guard = global_style_data.shared_lock.read();
     let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
     let sheet = HasArcFFI::as_arc(&raw_sheet);
     data.stylesheets.prepend_stylesheet(sheet, unique_id);
-    if flush {
+    data.clear_stylist();
-        data.flush_stylesheets(&guard);
-    }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_InsertStyleSheetBefore(raw_data: RawServoStyleSetBorrowed,
                                                         raw_sheet: RawServoStyleSheetBorrowed,
                                                         unique_id: u32,
-                                                        before_unique_id: u32,
+                                                        before_unique_id: u32) {
-                                                        flush: bool) {
-    let global_style_data = &*GLOBAL_STYLE_DATA;
-    let guard = global_style_data.shared_lock.read();
     let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
     let sheet = HasArcFFI::as_arc(&raw_sheet);
     data.stylesheets.insert_stylesheet_before(sheet, unique_id, before_unique_id);
-    if flush {
+    data.clear_stylist();
-        data.flush_stylesheets(&guard);
-    }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_RemoveStyleSheet(raw_data: RawServoStyleSetBorrowed,
-                                                  unique_id: u32,
+                                                  unique_id: u32) {
-                                                  flush: bool) {
-    let global_style_data = &*GLOBAL_STYLE_DATA;
-    let guard = global_style_data.shared_lock.read();
     let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
     data.stylesheets.remove_stylesheet(unique_id);
-    if flush {
+    data.clear_stylist();
-        data.flush_stylesheets(&guard);
-    }
 }
 
 #[no_mangle]
@@ -689,6 +669,7 @@ pub extern "C" fn Servo_StyleSet_NoteStyleSheetsChanged(raw_data: RawServoStyleS
     let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
     data.stylesheets.force_dirty();
     data.stylesheets.set_author_style_disabled(author_style_disabled);
+    data.clear_stylist();
 }
 
 #[no_mangle]