From 541977efb37a9851d3d51666dc781d4c172feced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 13 Sep 2017 17:07:13 +0200 Subject: [PATCH] style: Move ExtraStyleData into CascadeData. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It logically belongs there, and the only reason it wasn't there before we were working around it. MozReview-Commit-ID: 5a5iksHAIdN Signed-off-by: Emilio Cobos Álvarez --- components/layout_thread/lib.rs | 2 - components/style/gecko/data.rs | 12 ++---- components/style/stylist.rs | 66 ++++++++++++++++++++++++--------- ports/geckolib/glue.rs | 31 +++++++++------- 4 files changed, 69 insertions(+), 42 deletions(-) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index cd9bf1ac29b..15407b2ba38 100644 --- a/components/layout_thread/lib.rs +++ b/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), ); } diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index 603685763b7..5b5e1390f66 100644 --- a/components/style/gecko/data.rs +++ b/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 stylist::{ExtraStyleData, Stylist}; +use stylesheets::{StylesheetContents, StylesheetInDocument}; +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, } /// 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); } } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index a81252883ff..5027889a453 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -85,6 +85,10 @@ impl DocumentCascadeData { self.per_origin.iter_origins() } + fn iter_origins_rev(&self) -> PerOriginIter { + 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, ) -> 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, rebuild_kind: SheetRebuildKind, ) -> Result<(), FailedAllocationError> where @@ -345,26 +343,26 @@ impl DocumentCascadeData { } #[cfg(feature = "gecko")] CssRule::FontFace(ref rule) => { - _extra_data - .borrow_mut_for_origin(&origin) + origin_cascade_data + .extra_data .add_font_face(rule); } #[cfg(feature = "gecko")] CssRule::FontFeatureValues(ref rule) => { - _extra_data - .borrow_mut_for_origin(&origin) + origin_cascade_data + .extra_data .add_font_feature_values(rule); } #[cfg(feature = "gecko")] CssRule::CounterStyle(ref rule) => { - _extra_data - .borrow_mut_for_origin(&origin) + origin_cascade_data + .extra_data .add_counter_style(guard, rule); } #[cfg(feature = "gecko")] CssRule::Page(ref rule) => { - _extra_data - .borrow_mut_for_origin(&origin) + origin_cascade_data + .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, document_element: Option, ) -> 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>>, } +// 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.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); } } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 28a2bec6bd4..2667a46f4eb 100644 --- a/ports/geckolib/glue.rs +++ b/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 - .iter_origins() + let len: u32 = data + .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 - .iter_origins_rev() + let font_face_iter = data + .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 - .iter_origins() + data.stylist + .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 - .iter_origins() - .any(|(d, _)| !d.font_feature_values.is_empty()); + let has_rule = + data.stylist + .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 - .iter_origins_rev() - .flat_map(|(d, _)| d.font_feature_values.iter()); + let font_feature_values_iter = + data.stylist + .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 {