Auto merge of #18486 - emilio:extra-data-to-cascade-data, r=SimonSapin

style: Move ExtraStyleData into CascadeData.

It logically belongs there, and the only reason it wasn't there before we were
working around other stuff. Now it's needed to share UA stylesheets across documents.

<!-- 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/18486)
<!-- Reviewable:end -->

components/layout_thread/lib.rs

@@ -1198,11 +1198,9 @@ impl LayoutThread {
                 self.stylist.force_stylesheet_origins_dirty(Origin::Author.into());
             }
 
-            let mut extra_data = Default::default();
             self.stylist.flush(
                 &guards,
                 Some(ua_stylesheets),
-                &mut extra_data,
                 Some(element),
             );
         }

components/style/gecko/data.rs

@@ -12,13 +12,13 @@ use gecko_bindings::structs::{StyleSheetInfo, ServoStyleSheetInner};
 use gecko_bindings::structs::nsIDocument;
 use gecko_bindings::sugar::ownership::{HasArcFFI, HasBoxFFI, HasFFI, HasSimpleFFI};
 use invalidation::media_queries::{MediaListKey, ToMediaListKey};
-use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
+use malloc_size_of::MallocSizeOfOps;
 use media_queries::{Device, MediaList};
 use properties::ComputedValues;
 use servo_arc::Arc;
 use shared_lock::{Locked, StylesheetGuards, SharedRwLockReadGuard};
-use stylesheets::{PerOrigin, StylesheetContents, StylesheetInDocument};
+use stylesheets::{StylesheetContents, StylesheetInDocument};
-use stylist::{ExtraStyleData, Stylist};
+use stylist::Stylist;
 
 /// Little wrapper to a Gecko style sheet.
 #[derive(Debug, Eq, PartialEq)]
@@ -113,9 +113,6 @@ impl StylesheetInDocument for GeckoStyleSheet {
 pub struct PerDocumentStyleDataImpl {
     /// Rule processor.
     pub stylist: Stylist,
-
-    /// List of effective @font-face and @counter-style rules.
-    pub extra_style_data: PerOrigin<ExtraStyleData>,
 }
 
 /// The data itself is an `AtomicRefCell`, which guarantees the proper semantics
@@ -132,7 +129,6 @@ impl PerDocumentStyleData {
 
         PerDocumentStyleData(AtomicRefCell::new(PerDocumentStyleDataImpl {
             stylist: Stylist::new(device, quirks_mode.into()),
-            extra_style_data: Default::default(),
         }))
     }
 
@@ -160,7 +156,6 @@ impl PerDocumentStyleDataImpl {
         self.stylist.flush(
             &StylesheetGuards::same(guard),
             /* ua_sheets = */ None,
-            &mut self.extra_style_data,
             document_element,
         )
     }
@@ -189,7 +184,6 @@ impl PerDocumentStyleDataImpl {
     /// Measure heap usage.
     pub fn add_size_of_children(&self, ops: &mut MallocSizeOfOps, sizes: &mut ServoStyleSetSizes) {
         self.stylist.add_size_of_children(ops, sizes);
-        sizes.mStylistOther += self.extra_style_data.size_of(ops);
     }
 }
 

components/style/stylist.rs

@@ -85,6 +85,10 @@ impl DocumentCascadeData {
         self.per_origin.iter_origins()
     }
 
+    fn iter_origins_rev(&self) -> PerOriginIter<CascadeData> {
+        self.per_origin.iter_origins_rev()
+    }
+
     /// Rebuild the cascade data for the given document stylesheets, and
     /// optionally with a set of user agent stylesheets.  Returns Err(..)
     /// to signify OOM.
@@ -95,7 +99,6 @@ impl DocumentCascadeData {
         flusher: StylesheetFlusher<'a, 'b, S>,
         guards: &StylesheetGuards,
         ua_stylesheets: Option<&UserAgentStylesheets>,
-        extra_data: &mut PerOrigin<ExtraStyleData>,
     ) -> Result<(), FailedAllocationError>
     where
         'b: 'a,
@@ -114,7 +117,6 @@ impl DocumentCascadeData {
                 self.precomputed_pseudo_element_decls.clear();
             }
 
-            extra_data.borrow_mut_for_origin(&origin).clear();
             if validity == OriginValidity::CascadeInvalid {
                 cascade_data.clear_cascade_data()
             } else {
@@ -152,7 +154,6 @@ impl DocumentCascadeData {
                     quirks_mode,
                     stylesheet,
                     guards.ua_or_user,
-                    extra_data,
                     SheetRebuildKind::Full,
                 )?;
             }
@@ -181,7 +182,6 @@ impl DocumentCascadeData {
                         quirks_mode,
                         &ua_stylesheets.quirks_mode_stylesheet,
                         guards.ua_or_user,
-                        extra_data,
                         SheetRebuildKind::Full,
                     )?;
                 }
@@ -194,7 +194,6 @@ impl DocumentCascadeData {
                 quirks_mode,
                 stylesheet,
                 guards.author,
-                extra_data,
                 rebuild_kind,
             )?;
         }
@@ -209,7 +208,6 @@ impl DocumentCascadeData {
         quirks_mode: QuirksMode,
         stylesheet: &S,
         guard: &SharedRwLockReadGuard,
-        _extra_data: &mut PerOrigin<ExtraStyleData>,
         rebuild_kind: SheetRebuildKind,
     ) -> Result<(), FailedAllocationError>
     where
@@ -345,26 +343,26 @@ impl DocumentCascadeData {
                 }
                 #[cfg(feature = "gecko")]
                 CssRule::FontFace(ref rule) => {
-                    _extra_data
+                    origin_cascade_data
-                        .borrow_mut_for_origin(&origin)
+                        .extra_data
                         .add_font_face(rule);
                 }
                 #[cfg(feature = "gecko")]
                 CssRule::FontFeatureValues(ref rule) => {
-                    _extra_data
+                    origin_cascade_data
-                        .borrow_mut_for_origin(&origin)
+                        .extra_data
                         .add_font_feature_values(rule);
                 }
                 #[cfg(feature = "gecko")]
                 CssRule::CounterStyle(ref rule) => {
-                    _extra_data
+                    origin_cascade_data
-                        .borrow_mut_for_origin(&origin)
+                        .extra_data
                         .add_counter_style(guard, rule);
                 }
                 #[cfg(feature = "gecko")]
                 CssRule::Page(ref rule) => {
-                    _extra_data
+                    origin_cascade_data
-                        .borrow_mut_for_origin(&origin)
+                        .extra_data
                         .add_page(rule);
                 }
                 // We don't care about any other rule.
@@ -500,6 +498,18 @@ impl Stylist {
         }
     }
 
+    /// Iterate over the extra data in origin order.
+    #[inline]
+    pub fn iter_extra_data_origins(&self) -> ExtraStyleDataIterator {
+        ExtraStyleDataIterator(self.cascade_data.iter_origins())
+    }
+
+    /// Iterate over the extra data in reverse origin order.
+    #[inline]
+    pub fn iter_extra_data_origins_rev(&self) -> ExtraStyleDataIterator {
+        ExtraStyleDataIterator(self.cascade_data.iter_origins_rev())
+    }
+
     /// Returns the number of selectors.
     pub fn num_selectors(&self) -> usize {
         self.cascade_data.iter_origins().map(|(d, _)| d.num_selectors).sum()
@@ -548,7 +558,6 @@ impl Stylist {
         &mut self,
         guards: &StylesheetGuards,
         ua_sheets: Option<&UserAgentStylesheets>,
-        extra_data: &mut PerOrigin<ExtraStyleData>,
         document_element: Option<E>,
     ) -> bool
     where
@@ -604,7 +613,6 @@ impl Stylist {
             flusher,
             guards,
             ua_sheets,
-            extra_data,
         ).unwrap_or_else(|_| warn!("OOM in Stylist::flush"));
 
         had_invalidations
@@ -1647,7 +1655,8 @@ impl Stylist {
 
 /// This struct holds data which users of Stylist may want to extract
 /// from stylesheets which can be done at the same time as updating.
-#[derive(Default)]
+#[derive(Debug, Default)]
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct ExtraStyleData {
     /// A list of effective font-face rules and their origin.
     #[cfg(feature = "gecko")]
@@ -1666,6 +1675,11 @@ pub struct ExtraStyleData {
     pub pages: Vec<Arc<Locked<PageRule>>>,
 }
 
+// FIXME(emilio): This is kind of a lie, and relies on us not cloning
+// nsCSSFontFaceRules or nsCSSCounterStyleRules OMT (which we don't).
+#[cfg(feature = "gecko")]
+unsafe impl Sync for ExtraStyleData {}
+
 #[cfg(feature = "gecko")]
 impl ExtraStyleData {
     /// Add the given @font-face rule.
@@ -1706,6 +1720,18 @@ impl ExtraStyleData {
     }
 }
 
+/// An iterator over the different ExtraStyleData.
+pub struct ExtraStyleDataIterator<'a>(PerOriginIter<'a, CascadeData>);
+
+impl<'a> Iterator for ExtraStyleDataIterator<'a> {
+    type Item = (&'a ExtraStyleData, Origin);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next().map(|d| (&d.0.extra_data, d.1))
+    }
+}
+
+
 #[cfg(feature = "gecko")]
 impl MallocSizeOf for ExtraStyleData {
     /// Measure heap usage.
@@ -1951,6 +1977,9 @@ struct CascadeData {
     /// Effective media query results cached from the last rebuild.
     effective_media_query_results: EffectiveMediaQueryResults,
 
+    /// Extra data, like different kinds of rules, etc.
+    extra_data: ExtraStyleData,
+
     /// A monotonically increasing counter to represent the order on which a
     /// style rule appears in a stylesheet, needed to sort them by source order.
     rules_source_order: u32,
@@ -1968,6 +1997,7 @@ impl CascadeData {
             element_map: SelectorMap::new(),
             pseudos_map: PerPseudoElementMap::default(),
             animations: Default::default(),
+            extra_data: ExtraStyleData::default(),
             invalidation_map: InvalidationMap::new(),
             attribute_dependencies: NonCountingBloomFilter::new(),
             style_attribute_dependency: false,
@@ -1998,6 +2028,7 @@ impl CascadeData {
         self.element_map.clear();
         self.pseudos_map.clear();
         self.animations.clear();
+        self.extra_data.clear();
         self.rules_source_order = 0;
         self.num_selectors = 0;
         self.num_declarations = 0;
@@ -2032,6 +2063,7 @@ impl CascadeData {
         sizes.mStylistRevalidationSelectors += self.selectors_for_cache_revalidation.size_of(ops);
 
         sizes.mStylistOther += self.effective_media_query_results.size_of(ops);
+        sizes.mStylistOther += self.extra_data.size_of(ops);
     }
 }
 

ports/geckolib/glue.rs

@@ -1685,7 +1685,7 @@ pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(parent_style_or_null:
     let page_decls = match pseudo {
         PseudoElement::PageContent => {
             let mut declarations = vec![];
-            let iter = data.extra_style_data.iter_origins_rev();
+            let iter = data.stylist.iter_extra_data_origins_rev();
             for (data, origin) in iter {
                 let level = match origin {
                     Origin::UserAgent => CascadeLevel::UANormal,
@@ -3610,15 +3610,17 @@ pub extern "C" fn Servo_StyleSet_GetFontFaceRules(raw_data: RawServoStyleSetBorr
     let global_style_data = &*GLOBAL_STYLE_DATA;
     let guard = global_style_data.shared_lock.read();
 
-    let len: u32 = data.extra_style_data
+    let len: u32 = data
-        .iter_origins()
+        .stylist
+        .iter_extra_data_origins()
         .map(|(d, _)| d.font_faces.len() as u32)
         .sum();
 
     // Reversed iterator because Gecko expects rules to appear sorted
     // UserAgent first, Author last.
-    let font_face_iter = data.extra_style_data
+    let font_face_iter = data
-        .iter_origins_rev()
+        .stylist
+        .iter_extra_data_origins_rev()
         .flat_map(|(d, o)| d.font_faces.iter().zip(iter::repeat(o)));
 
     unsafe { rules.set_len(len) };
@@ -3632,12 +3634,11 @@ pub extern "C" fn Servo_StyleSet_GetFontFaceRules(raw_data: RawServoStyleSetBorr
 pub extern "C" fn Servo_StyleSet_GetCounterStyleRule(raw_data: RawServoStyleSetBorrowed,
                                                      name: *mut nsIAtom) -> *mut nsCSSCounterStyleRule {
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
-    let extra_data = &data.extra_style_data;
 
     unsafe {
         Atom::with(name, |name| {
-            extra_data
+            data.stylist
-                .iter_origins()
+                .iter_extra_data_origins()
                 .filter_map(|(d, _)| d.counter_styles.get(name))
                 .next()
         })
@@ -3656,17 +3657,19 @@ pub extern "C" fn Servo_StyleSet_BuildFontFeatureValueSet(
     let global_style_data = &*GLOBAL_STYLE_DATA;
     let guard = global_style_data.shared_lock.read();
 
-    let has_rule = data.extra_style_data
+    let has_rule =
-                  .iter_origins()
+        data.stylist
-                  .any(|(d, _)| !d.font_feature_values.is_empty());
+            .iter_extra_data_origins()
+            .any(|(d, _)| !d.font_feature_values.is_empty());
 
     if !has_rule {
       return ptr::null_mut();
     }
 
-    let font_feature_values_iter = data.extra_style_data
+    let font_feature_values_iter =
-        .iter_origins_rev()
+        data.stylist
-        .flat_map(|(d, _)| d.font_feature_values.iter());
+            .iter_extra_data_origins_rev()
+            .flat_map(|(d, _)| d.font_feature_values.iter());
 
     let set = unsafe { Gecko_ConstructFontFeatureValueSet() };
     for src in font_feature_values_iter {