From f1516d228f54d2ceed40ce894b3487ef4e6fbb3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 8 Feb 2018 11:55:50 +0100 Subject: [PATCH 01/11] style: Rename StylesheetSet to DocumentStylesheetSet. MozReview-Commit-ID: 5Xl1eRLu1VF --- components/script/dom/bindings/trace.rs | 4 ++-- components/script/dom/document.rs | 10 +++++----- components/style/stylesheet_set.rs | 18 +++++++++--------- components/style/stylist.rs | 12 ++++++------ 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 97d9f062688..f4930406507 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -105,7 +105,7 @@ use style::media_queries::MediaList; use style::properties::PropertyDeclarationBlock; use style::selector_parser::{PseudoElement, Snapshot}; use style::shared_lock::{SharedRwLock as StyleSharedRwLock, Locked as StyleLocked}; -use style::stylesheet_set::StylesheetSet; +use style::stylesheet_set::DocumentStylesheetSet; use style::stylesheets::{CssRules, FontFaceRule, KeyframesRule, MediaRule, Stylesheet}; use style::stylesheets::{NamespaceRule, StyleRule, ImportRule, SupportsRule, ViewportRule}; use style::stylesheets::keyframes_rule::Keyframe; @@ -659,7 +659,7 @@ unsafe impl JSTraceable for StyleLocked { } } -unsafe impl JSTraceable for StylesheetSet +unsafe impl JSTraceable for DocumentStylesheetSet where S: JSTraceable + ::style::stylesheets::StylesheetInDocument + PartialEq + 'static, { diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 8df8ba23d9a..a24382ab80a 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -135,7 +135,7 @@ use style::media_queries::{Device, MediaList, MediaType}; use style::selector_parser::{RestyleDamage, Snapshot}; use style::shared_lock::{SharedRwLock as StyleSharedRwLock, SharedRwLockReadGuard}; use style::str::{HTML_SPACE_CHARACTERS, split_html_space_chars, str_join}; -use style::stylesheet_set::StylesheetSet; +use style::stylesheet_set::DocumentStylesheetSet; use style::stylesheets::{Stylesheet, StylesheetContents, Origin, OriginSet}; use task_source::TaskSource; use time; @@ -262,7 +262,7 @@ pub struct Document { /// Can be acquired once for accessing many objects. style_shared_lock: StyleSharedRwLock, /// List of stylesheets associated with nodes in this document. |None| if the list needs to be refreshed. - stylesheets: DomRefCell>, + stylesheets: DomRefCell>, stylesheet_list: MutNullableDom, ready_state: Cell, /// Whether the DOMContentLoaded event has already been dispatched. @@ -1466,7 +1466,7 @@ impl Document { // Mark the document element dirty so a reflow will be performed. // - // FIXME(emilio): Use the StylesheetSet invalidation stuff. + // FIXME(emilio): Use the DocumentStylesheetSet invalidation stuff. if let Some(element) = self.GetDocumentElement() { element.upcast::().dirty(NodeDamage::NodeStyleDamaged); } @@ -2264,7 +2264,7 @@ impl Document { PER_PROCESS_AUTHOR_SHARED_LOCK.clone() //StyleSharedRwLock::new() }, - stylesheets: DomRefCell::new(StylesheetSet::new()), + stylesheets: DomRefCell::new(DocumentStylesheetSet::new()), stylesheet_list: MutNullableDom::new(None), ready_state: Cell::new(ready_state), domcontentloaded_dispatched: Cell::new(domcontentloaded_dispatched), @@ -3807,7 +3807,7 @@ impl DocumentMethods for Document { self.scripts.set(None); self.anchors.set(None); self.applets.set(None); - *self.stylesheets.borrow_mut() = StylesheetSet::new(); + *self.stylesheets.borrow_mut() = DocumentStylesheetSet::new(); self.animation_frame_ident.set(0); self.animation_frame_list.borrow_mut().clear(); self.pending_restyles.borrow_mut().clear(); diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 0c3a985c26c..b2f44e1bed1 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -370,7 +370,7 @@ where /// The set of stylesheets effective for a given document. #[cfg_attr(feature = "servo", derive(MallocSizeOf))] -pub struct StylesheetSet +pub struct DocumentStylesheetSet where S: StylesheetInDocument + PartialEq + 'static, { @@ -387,13 +387,13 @@ where author_style_disabled: bool, } -impl StylesheetSet +impl DocumentStylesheetSet where S: StylesheetInDocument + PartialEq + 'static, { /// Create a new empty StylesheetSet. pub fn new() -> Self { - StylesheetSet { + Self { collections: Default::default(), invalidations: StylesheetInvalidationSet::new(), origins_dirty: OriginSet::empty(), @@ -438,7 +438,7 @@ where sheet: S, guard: &SharedRwLockReadGuard ) { - debug!("StylesheetSet::append_stylesheet"); + debug!("DocumentStylesheetSet::append_stylesheet"); self.collect_invalidations_for(device, &sheet, guard); let origin = sheet.contents(guard).origin; self.collections.borrow_mut_for_origin(&origin).append(sheet); @@ -451,7 +451,7 @@ where sheet: S, guard: &SharedRwLockReadGuard ) { - debug!("StylesheetSet::prepend_stylesheet"); + debug!("DocumentStylesheetSet::prepend_stylesheet"); self.collect_invalidations_for(device, &sheet, guard); let origin = sheet.contents(guard).origin; @@ -466,7 +466,7 @@ where before_sheet: S, guard: &SharedRwLockReadGuard, ) { - debug!("StylesheetSet::insert_stylesheet_before"); + debug!("DocumentStylesheetSet::insert_stylesheet_before"); self.collect_invalidations_for(device, &sheet, guard); let origin = sheet.contents(guard).origin; @@ -491,7 +491,7 @@ where /// Notes that the author style has been disabled for this document. pub fn set_author_style_disabled(&mut self, disabled: bool) { - debug!("StylesheetSet::set_author_style_disabled"); + debug!("DocumentStylesheetSet::set_author_style_disabled"); if self.author_style_disabled == disabled { return; } @@ -517,7 +517,7 @@ where { use std::mem; - debug!("StylesheetSet::flush"); + debug!("DocumentStylesheetSet::flush"); let had_invalidations = self.invalidations.flush(document_element, snapshots); @@ -546,7 +546,7 @@ where pub fn flush_without_invalidation(&mut self) -> OriginSet { use std::mem; - debug!("StylesheetSet::flush_without_invalidation"); + debug!("DocumentStylesheetSet::flush_without_invalidation"); self.invalidations.clear(); mem::replace(&mut self.origins_dirty, OriginSet::empty()) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 57e87819498..c32d6d207ee 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -41,7 +41,7 @@ use smallvec::SmallVec; use std::ops; use std::sync::Mutex; use style_traits::viewport::ViewportConstraints; -use stylesheet_set::{OriginValidity, SheetRebuildKind, StylesheetSet, StylesheetFlusher}; +use stylesheet_set::{OriginValidity, SheetRebuildKind, DocumentStylesheetSet, StylesheetFlusher}; #[cfg(feature = "gecko")] use stylesheets::{CounterStyleRule, FontFaceRule, FontFeatureValuesRule, PageRule}; use stylesheets::{CssRule, Origin, OriginSet, PerOrigin, PerOriginIter}; @@ -328,21 +328,21 @@ impl DocumentCascadeData { } } -/// A wrapper over a StylesheetSet that can be `Sync`, since it's only used and -/// exposed via mutable methods in the `Stylist`. +/// A wrapper over a DocumentStylesheetSet that can be `Sync`, since it's only +/// used and exposed via mutable methods in the `Stylist`. #[cfg_attr(feature = "servo", derive(MallocSizeOf))] -struct StylistStylesheetSet(StylesheetSet); +struct StylistStylesheetSet(DocumentStylesheetSet); // Read above to see why this is fine. unsafe impl Sync for StylistStylesheetSet {} impl StylistStylesheetSet { fn new() -> Self { - StylistStylesheetSet(StylesheetSet::new()) + StylistStylesheetSet(DocumentStylesheetSet::new()) } } impl ops::Deref for StylistStylesheetSet { - type Target = StylesheetSet; + type Target = DocumentStylesheetSet; fn deref(&self) -> &Self::Target { &self.0 From c88d339db277ec857869b1d5592725c45b164db4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 8 Feb 2018 12:20:02 +0100 Subject: [PATCH 02/11] style: Move the dirty bit to SheetCollection. MozReview-Commit-ID: DO9vv9vmSzF --- components/style/stylesheet_set.rs | 57 ++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index b2f44e1bed1..e7c7599311e 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -175,11 +175,14 @@ where debug_assert_eq!(origin, Origin::UserAgent); // We could iterate over `origin_sheets(origin)` to ensure state is - // consistent (that the `dirty` member of the Entry is reset to - // `false`). + // consistent (that the `committed` member of the Entry is reset to + // `true`). // // In practice it doesn't matter for correctness given our use of it // (that this is UA only). + // + // FIXME(emilio): I think it does matter and that we effectively don't + // support removing UA sheets since #19927... self.collections.borrow_for_origin(&origin).iter() } @@ -244,7 +247,8 @@ where let committed = mem::replace(&mut potential_sheet.committed, true); if !committed { - // If the sheet was uncommitted, we need to do a full rebuild anyway. + // If the sheet was uncommitted, we need to do a full rebuild + // anyway. return Some((&potential_sheet.sheet, SheetRebuildKind::Full)) } @@ -277,6 +281,11 @@ where /// which case the existing data is valid, but the origin needs to be /// rebuilt). data_validity: OriginValidity, + + /// Whether anything in the collection has changed. Note that this is + /// different from `data_validity`, in the sense that after a sheet append, + /// the data validity is still `Valid`, but we need to be marked as dirty. + dirty: bool, } impl Default for SheetCollection @@ -287,6 +296,7 @@ where Self { entries: vec![], data_validity: OriginValidity::Valid, + dirty: false, } } } @@ -321,6 +331,8 @@ where // See bug 1434756. if sheet.committed { self.set_data_validity_at_least(OriginValidity::FullyInvalid); + } else { + self.dirty = true; } } @@ -331,9 +343,13 @@ where /// Appends a given sheet into the collection. fn append(&mut self, sheet: S) { debug_assert!(!self.contains(&sheet)); - self.entries.push(StylesheetSetEntry::new(sheet)) + self.entries.push(StylesheetSetEntry::new(sheet)); // Appending sheets doesn't alter the validity of the existing data, so // we don't need to change `data_validity` here. + // + // But we need to be marked as dirty, otherwise we'll never add the new + // sheet! + self.dirty = true; } fn insert_before(&mut self, sheet: S, before_sheet: &S) { @@ -351,6 +367,7 @@ where fn set_data_validity_at_least(&mut self, validity: OriginValidity) { use std::cmp; + self.dirty = true; self.data_validity = cmp::max(validity, self.data_validity); } @@ -380,9 +397,6 @@ where /// The invalidations for stylesheets added or removed from this document. invalidations: StylesheetInvalidationSet, - /// The origins whose stylesheets have changed so far. - origins_dirty: OriginSet, - /// Has author style been disabled? author_style_disabled: bool, } @@ -396,7 +410,6 @@ where Self { collections: Default::default(), invalidations: StylesheetInvalidationSet::new(), - origins_dirty: OriginSet::empty(), author_style_disabled: false, } } @@ -426,7 +439,6 @@ where if let Some(device) = device { self.invalidations.collect_invalidations_for(device, sheet, guard); } - self.origins_dirty |= sheet.contents(guard).origin; } /// Appends a new stylesheet to the current set. @@ -497,12 +509,13 @@ where } self.author_style_disabled = disabled; self.invalidations.invalidate_fully(); - self.origins_dirty |= Origin::Author; + self.collections.borrow_mut_for_origin(&Origin::Author) + .set_data_validity_at_least(OriginValidity::FullyInvalid) } /// Returns whether the given set has changed from the last flush. pub fn has_changed(&self) -> bool { - !self.origins_dirty.is_empty() + self.collections.iter_origins().any(|(collection, _)| collection.dirty) } /// Flush the current set, unmarking it as dirty, and returns a @@ -522,12 +535,16 @@ where let had_invalidations = self.invalidations.flush(document_element, snapshots); - let origins_dirty = - mem::replace(&mut self.origins_dirty, OriginSet::empty()); - + let mut origins_dirty = OriginSet::empty(); let mut origin_data_validity = PerOrigin::::default(); - for origin in origins_dirty.iter() { - let collection = self.collections.borrow_mut_for_origin(&origin); + for (collection, origin) in self.collections.iter_mut_origins() { + let was_dirty = mem::replace(&mut collection.dirty, false); + if !was_dirty { + debug_assert_eq!(collection.data_validity, OriginValidity::Valid); + continue; + } + + origins_dirty |= origin; *origin_data_validity.borrow_mut_for_origin(&origin) = mem::replace(&mut collection.data_validity, OriginValidity::Valid); } @@ -549,7 +566,12 @@ where debug!("DocumentStylesheetSet::flush_without_invalidation"); self.invalidations.clear(); - mem::replace(&mut self.origins_dirty, OriginSet::empty()) + for (collection, origin) in self.collections.iter_mut_origins() { + collection.dirty = false; + // NOTE(emilio): I think this should also poke at the data validity + // and such, but it doesn't really matter given we don't use that + // collection for style resolution anyway. + } } /// Return an iterator over the flattened view of all the stylesheets. @@ -565,7 +587,6 @@ where /// something external may have invalidated it. pub fn force_dirty(&mut self, origins: OriginSet) { self.invalidations.invalidate_fully(); - self.origins_dirty |= origins; for origin in origins.iter() { // We don't know what happened, assume the worse. self.collections From 01915d84dbd4a3d7497c8afd2744d5200510e7c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 8 Feb 2018 12:21:27 +0100 Subject: [PATCH 03/11] style: Assert we don't try to do dumb stuff. MozReview-Commit-ID: 6DpkiwUdccD --- components/style/stylesheet_set.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index e7c7599311e..37c06f109c2 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -367,6 +367,9 @@ where fn set_data_validity_at_least(&mut self, validity: OriginValidity) { use std::cmp; + + debug_assert_ne!(validity, OriginValidity::Valid); + self.dirty = true; self.data_validity = cmp::max(validity, self.data_validity); } From bbc6857c06d4a0d4f74941ad2c2ca5926590c61c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 8 Feb 2018 12:27:31 +0100 Subject: [PATCH 04/11] style: Rename OriginValidity to DataValidity. MozReview-Commit-ID: FpsYUlWLWTt --- components/style/stylesheet_set.rs | 48 +++++++++++++++--------------- components/style/stylist.rs | 10 +++---- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 37c06f109c2..fbad32d8d9f 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -105,7 +105,7 @@ where /// The validity of the data in a given cascade origin. #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)] #[cfg_attr(feature = "servo", derive(MallocSizeOf))] -pub enum OriginValidity { +pub enum DataValidity { /// The origin is clean, all the data already there is valid, though we may /// have new sheets at the end. Valid = 0, @@ -118,9 +118,9 @@ pub enum OriginValidity { FullyInvalid = 2, } -impl Default for OriginValidity { +impl Default for DataValidity { fn default() -> Self { - OriginValidity::Valid + DataValidity::Valid } } @@ -131,7 +131,7 @@ where { origins_dirty: OriginSet, collections: &'a mut PerOrigin>, - origin_data_validity: PerOrigin, + origin_data_validity: PerOrigin, author_style_disabled: bool, had_invalidations: bool, } @@ -157,7 +157,7 @@ where S: StylesheetInDocument + PartialEq + 'static, { /// The data validity for a given origin. - pub fn origin_validity(&self, origin: Origin) -> OriginValidity { + pub fn data_validity(&self, origin: Origin) -> DataValidity { *self.origin_data_validity.borrow_for_origin(&origin) } @@ -191,11 +191,11 @@ where where 'a: 'b { - let validity = self.origin_validity(origin); + let validity = self.data_validity(origin); let origin_dirty = self.origins_dirty.contains(origin.into()); debug_assert!( - origin_dirty || validity == OriginValidity::Valid, + origin_dirty || validity == DataValidity::Valid, "origin_data_validity should be a subset of origins_dirty!" ); @@ -230,7 +230,7 @@ where S: StylesheetInDocument + PartialEq + 'static { iter: slice::IterMut<'a, StylesheetSetEntry>, - validity: OriginValidity, + validity: DataValidity, } impl<'a, S> Iterator for PerOriginFlusher<'a, S> @@ -253,9 +253,9 @@ where } let rebuild_kind = match self.validity { - OriginValidity::Valid => continue, - OriginValidity::CascadeInvalid => SheetRebuildKind::CascadeOnly, - OriginValidity::FullyInvalid => SheetRebuildKind::Full, + DataValidity::Valid => continue, + DataValidity::CascadeInvalid => SheetRebuildKind::CascadeOnly, + DataValidity::FullyInvalid => SheetRebuildKind::Full, }; return Some((&potential_sheet.sheet, rebuild_kind)); @@ -277,10 +277,10 @@ where /// The validity of the data that was already there for a given origin. /// /// Note that an origin may appear on `origins_dirty`, but still have - /// `OriginValidity::Valid`, if only sheets have been appended into it (in + /// `DataValidity::Valid`, if only sheets have been appended into it (in /// which case the existing data is valid, but the origin needs to be /// rebuilt). - data_validity: OriginValidity, + data_validity: DataValidity, /// Whether anything in the collection has changed. Note that this is /// different from `data_validity`, in the sense that after a sheet append, @@ -295,7 +295,7 @@ where fn default() -> Self { Self { entries: vec![], - data_validity: OriginValidity::Valid, + data_validity: DataValidity::Valid, dirty: false, } } @@ -330,7 +330,7 @@ where // 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); + self.set_data_validity_at_least(DataValidity::FullyInvalid); } else { self.dirty = true; } @@ -361,14 +361,14 @@ where // Inserting stylesheets somewhere but at the end changes the validity // of the cascade data, but not the invalidation data. - self.set_data_validity_at_least(OriginValidity::CascadeInvalid); + self.set_data_validity_at_least(DataValidity::CascadeInvalid); self.entries.insert(index, StylesheetSetEntry::new(sheet)); } - fn set_data_validity_at_least(&mut self, validity: OriginValidity) { + fn set_data_validity_at_least(&mut self, validity: DataValidity) { use std::cmp; - debug_assert_ne!(validity, OriginValidity::Valid); + debug_assert_ne!(validity, DataValidity::Valid); self.dirty = true; self.data_validity = cmp::max(validity, self.data_validity); @@ -378,7 +378,7 @@ where debug_assert!(!self.contains(&sheet)); // Inserting stylesheets somewhere but at the end changes the validity // of the cascade data, but not the invalidation data. - self.set_data_validity_at_least(OriginValidity::CascadeInvalid); + self.set_data_validity_at_least(DataValidity::CascadeInvalid); self.entries.insert(0, StylesheetSetEntry::new(sheet)); } @@ -513,7 +513,7 @@ where self.author_style_disabled = disabled; self.invalidations.invalidate_fully(); self.collections.borrow_mut_for_origin(&Origin::Author) - .set_data_validity_at_least(OriginValidity::FullyInvalid) + .set_data_validity_at_least(DataValidity::FullyInvalid) } /// Returns whether the given set has changed from the last flush. @@ -539,17 +539,17 @@ where self.invalidations.flush(document_element, snapshots); let mut origins_dirty = OriginSet::empty(); - let mut origin_data_validity = PerOrigin::::default(); + let mut origin_data_validity = PerOrigin::::default(); for (collection, origin) in self.collections.iter_mut_origins() { let was_dirty = mem::replace(&mut collection.dirty, false); if !was_dirty { - debug_assert_eq!(collection.data_validity, OriginValidity::Valid); + debug_assert_eq!(collection.data_validity, DataValidity::Valid); continue; } origins_dirty |= origin; *origin_data_validity.borrow_mut_for_origin(&origin) = - mem::replace(&mut collection.data_validity, OriginValidity::Valid); + mem::replace(&mut collection.data_validity, DataValidity::Valid); } StylesheetFlusher { @@ -594,7 +594,7 @@ where // We don't know what happened, assume the worse. self.collections .borrow_mut_for_origin(&origin) - .set_data_validity_at_least(OriginValidity::FullyInvalid); + .set_data_validity_at_least(DataValidity::FullyInvalid); } } } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index c32d6d207ee..42f210d0325 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -41,7 +41,7 @@ use smallvec::SmallVec; use std::ops; use std::sync::Mutex; use style_traits::viewport::ViewportConstraints; -use stylesheet_set::{OriginValidity, SheetRebuildKind, DocumentStylesheetSet, StylesheetFlusher}; +use stylesheet_set::{DataValidity, SheetRebuildKind, DocumentStylesheetSet, StylesheetFlusher}; #[cfg(feature = "gecko")] use stylesheets::{CounterStyleRule, FontFaceRule, FontFeatureValuesRule, PageRule}; use stylesheets::{CssRule, Origin, OriginSet, PerOrigin, PerOriginIter}; @@ -240,12 +240,12 @@ impl DocumentCascadeData { { debug_assert_ne!(origin, Origin::UserAgent); - let validity = flusher.origin_validity(origin); + let validity = flusher.data_validity(origin); match validity { - OriginValidity::Valid => {}, - OriginValidity::CascadeInvalid => cascade_data.clear_cascade_data(), - OriginValidity::FullyInvalid => cascade_data.clear(), + DataValidity::Valid => {}, + DataValidity::CascadeInvalid => cascade_data.clear_cascade_data(), + DataValidity::FullyInvalid => cascade_data.clear(), } let guard = guards.for_origin(origin); From 4cc5717116852ddd544306d7e486511780ec2477 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 8 Feb 2018 12:29:11 +0100 Subject: [PATCH 05/11] style: Save some work if the origin is not dirty. MozReview-Commit-ID: EGHztVLj9i3 --- components/style/stylist.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 42f210d0325..6494b4f35b6 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -240,6 +240,10 @@ impl DocumentCascadeData { { debug_assert_ne!(origin, Origin::UserAgent); + if !flusher.origin_dirty(origin) { + return Ok(()); + } + let validity = flusher.data_validity(origin); match validity { From 438251cbfc30d7f8f868815012831c4b8f759943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 8 Feb 2018 12:40:27 +0100 Subject: [PATCH 06/11] style: Make AuthorStylesEnabled an enum. Chances are we need to pass it around in a bit. Also invert the boolean because I don't want to reason about double negations, even if they're simple. MozReview-Commit-ID: KhX4lDKwDoj --- components/style/stylesheet_set.rs | 38 +++++++++++++++++------------- components/style/stylist.rs | 6 ++--- ports/geckolib/glue.rs | 9 ++++++- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index fbad32d8d9f..0277842262e 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -124,6 +124,16 @@ impl Default for DataValidity { } } +/// Whether author styles are enabled. +/// +/// This is used to support Gecko. +#[allow(missing_docs)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum AuthorStylesEnabled { + Yes, + No, +} + /// A struct to iterate over the different stylesheets to be flushed. pub struct StylesheetFlusher<'a, S> where @@ -132,7 +142,7 @@ where origins_dirty: OriginSet, collections: &'a mut PerOrigin>, origin_data_validity: PerOrigin, - author_style_disabled: bool, + author_styles_enabled: AuthorStylesEnabled, had_invalidations: bool, } @@ -199,7 +209,9 @@ where "origin_data_validity should be a subset of origins_dirty!" ); - if self.author_style_disabled && origin == Origin::Author { + if self.author_styles_enabled == AuthorStylesEnabled::No && + origin == Origin::Author + { return PerOriginFlusher { iter: [].iter_mut(), validity, @@ -400,8 +412,8 @@ where /// The invalidations for stylesheets added or removed from this document. invalidations: StylesheetInvalidationSet, - /// Has author style been disabled? - author_style_disabled: bool, + /// Whether author styles are enabled. + author_styles_enabled: AuthorStylesEnabled, } impl DocumentStylesheetSet @@ -413,7 +425,7 @@ where Self { collections: Default::default(), invalidations: StylesheetInvalidationSet::new(), - author_style_disabled: false, + author_styles_enabled: AuthorStylesEnabled::Yes, } } @@ -427,12 +439,6 @@ where self.collections.borrow_for_origin(&origin).get(index) } - /// Returns whether author styles have been disabled for the current - /// stylesheet set. - pub fn author_style_disabled(&self) -> bool { - self.author_style_disabled - } - fn collect_invalidations_for( &mut self, device: Option<&Device>, @@ -505,12 +511,12 @@ where } /// Notes that the author style has been disabled for this document. - pub fn set_author_style_disabled(&mut self, disabled: bool) { - debug!("DocumentStylesheetSet::set_author_style_disabled"); - if self.author_style_disabled == disabled { + pub fn set_author_styles_enabled(&mut self, enabled: AuthorStylesEnabled) { + debug!("DocumentStylesheetSet::set_author_styles_enabled"); + if self.author_styles_enabled == enabled { return; } - self.author_style_disabled = disabled; + self.author_styles_enabled = enabled; self.invalidations.invalidate_fully(); self.collections.borrow_mut_for_origin(&Origin::Author) .set_data_validity_at_least(DataValidity::FullyInvalid) @@ -554,7 +560,7 @@ where StylesheetFlusher { collections: &mut self.collections, - author_style_disabled: self.author_style_disabled, + author_styles_enabled: self.author_styles_enabled, had_invalidations, origins_dirty, origin_data_validity, diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 6494b4f35b6..160c70df673 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -41,7 +41,7 @@ use smallvec::SmallVec; use std::ops; use std::sync::Mutex; use style_traits::viewport::ViewportConstraints; -use stylesheet_set::{DataValidity, SheetRebuildKind, DocumentStylesheetSet, StylesheetFlusher}; +use stylesheet_set::{AuthorStylesEnabled, DataValidity, SheetRebuildKind, DocumentStylesheetSet, StylesheetFlusher}; #[cfg(feature = "gecko")] use stylesheets::{CounterStyleRule, FontFaceRule, FontFeatureValuesRule, PageRule}; use stylesheets::{CssRule, Origin, OriginSet, PerOrigin, PerOriginIter}; @@ -592,8 +592,8 @@ impl Stylist { } /// Sets whether author style is enabled or not. - pub fn set_author_style_disabled(&mut self, disabled: bool) { - self.stylesheets.set_author_style_disabled(disabled); + pub fn set_author_styles_enabled(&mut self, enabled: AuthorStylesEnabled) { + self.stylesheets.set_author_styles_enabled(enabled); } /// Returns whether we've recorded any stylesheet change so far. diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index f2b3cd531d2..94a46e3395f 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -135,6 +135,7 @@ use style::selector_parser::{PseudoElementCascadeType, SelectorImpl}; use style::shared_lock::{SharedRwLockReadGuard, StylesheetGuards, ToCssWithGuard, Locked}; use style::string_cache::{Atom, WeakAtom}; use style::style_adjuster::StyleAdjuster; +use style::stylesheet_set::AuthorStylesEnabled; use style::stylesheets::{CssRule, CssRules, CssRuleType, CssRulesHelpers, DocumentRule}; use style::stylesheets::{FontFeatureValuesRule, ImportRule, KeyframesRule, MediaRule}; use style::stylesheets::{NamespaceRule, Origin, OriginSet, PageRule, StyleRule}; @@ -1252,7 +1253,13 @@ pub extern "C" fn Servo_StyleSet_NoteStyleSheetsChanged( ) { let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); data.stylist.force_stylesheet_origins_dirty(OriginSet::from(changed_origins)); - data.stylist.set_author_style_disabled(author_style_disabled); + let enabled = + if author_style_disabled { + AuthorStylesEnabled::No + } else { + AuthorStylesEnabled::Yes + }; + data.stylist.set_author_styles_enabled(enabled); } #[no_mangle] From a254dc12a48f7b9fb154057111feaa5b93434e11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 8 Feb 2018 13:05:28 +0100 Subject: [PATCH 07/11] style: Move author-style-disabled handling to push_applicable_declarations. This will make it easier to handle it properly for Shadow DOM, though this patch doesn't do that. This also makes some method inline and infallible for convenience, since nobody checks the errors anyway. Bug: 1436798 Reviewed-by: bholley MozReview-Commit-ID: Hq3erAUs5tf --- components/style/stylesheet_set.rs | 36 ------------------------------ components/style/stylist.rs | 35 ++++++++++++++++++++--------- ports/geckolib/glue.rs | 12 +++++++--- 3 files changed, 34 insertions(+), 49 deletions(-) diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 0277842262e..eeb75e738e6 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -124,16 +124,6 @@ impl Default for DataValidity { } } -/// Whether author styles are enabled. -/// -/// This is used to support Gecko. -#[allow(missing_docs)] -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum AuthorStylesEnabled { - Yes, - No, -} - /// A struct to iterate over the different stylesheets to be flushed. pub struct StylesheetFlusher<'a, S> where @@ -142,7 +132,6 @@ where origins_dirty: OriginSet, collections: &'a mut PerOrigin>, origin_data_validity: PerOrigin, - author_styles_enabled: AuthorStylesEnabled, had_invalidations: bool, } @@ -209,14 +198,6 @@ where "origin_data_validity should be a subset of origins_dirty!" ); - if self.author_styles_enabled == AuthorStylesEnabled::No && - origin == Origin::Author - { - return PerOriginFlusher { - iter: [].iter_mut(), - validity, - } - } PerOriginFlusher { iter: self.collections.borrow_mut_for_origin(&origin).entries.iter_mut(), validity, @@ -411,9 +392,6 @@ where /// The invalidations for stylesheets added or removed from this document. invalidations: StylesheetInvalidationSet, - - /// Whether author styles are enabled. - author_styles_enabled: AuthorStylesEnabled, } impl DocumentStylesheetSet @@ -425,7 +403,6 @@ where Self { collections: Default::default(), invalidations: StylesheetInvalidationSet::new(), - author_styles_enabled: AuthorStylesEnabled::Yes, } } @@ -510,18 +487,6 @@ where self.collections.borrow_mut_for_origin(&origin).remove(&sheet) } - /// Notes that the author style has been disabled for this document. - pub fn set_author_styles_enabled(&mut self, enabled: AuthorStylesEnabled) { - debug!("DocumentStylesheetSet::set_author_styles_enabled"); - if self.author_styles_enabled == enabled { - return; - } - self.author_styles_enabled = enabled; - self.invalidations.invalidate_fully(); - self.collections.borrow_mut_for_origin(&Origin::Author) - .set_data_validity_at_least(DataValidity::FullyInvalid) - } - /// Returns whether the given set has changed from the last flush. pub fn has_changed(&self) -> bool { self.collections.iter_origins().any(|(collection, _)| collection.dirty) @@ -560,7 +525,6 @@ where StylesheetFlusher { collections: &mut self.collections, - author_styles_enabled: self.author_styles_enabled, had_invalidations, origins_dirty, origin_data_validity, diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 160c70df673..f656f77476b 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -41,7 +41,7 @@ use smallvec::SmallVec; use std::ops; use std::sync::Mutex; use style_traits::viewport::ViewportConstraints; -use stylesheet_set::{AuthorStylesEnabled, DataValidity, SheetRebuildKind, DocumentStylesheetSet, StylesheetFlusher}; +use stylesheet_set::{DataValidity, SheetRebuildKind, DocumentStylesheetSet, StylesheetFlusher}; #[cfg(feature = "gecko")] use stylesheets::{CounterStyleRule, FontFaceRule, FontFeatureValuesRule, PageRule}; use stylesheets::{CssRule, Origin, OriginSet, PerOrigin, PerOriginIter}; @@ -332,6 +332,16 @@ impl DocumentCascadeData { } } +/// Whether author styles are enabled. +/// +/// This is used to support Gecko. +#[allow(missing_docs)] +#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq)] +pub enum AuthorStylesEnabled { + Yes, + No, +} + /// A wrapper over a DocumentStylesheetSet that can be `Sync`, since it's only /// used and exposed via mutable methods in the `Stylist`. #[cfg_attr(feature = "servo", derive(MallocSizeOf))] @@ -397,6 +407,9 @@ pub struct Stylist { /// cascade level. cascade_data: DocumentCascadeData, + /// Whether author styles are enabled. + author_styles_enabled: AuthorStylesEnabled, + /// The rule tree, that stores the results of selector matching. rule_tree: RuleTree, @@ -437,6 +450,7 @@ impl Stylist { quirks_mode, stylesheets: StylistStylesheetSet::new(), cascade_data: Default::default(), + author_styles_enabled: AuthorStylesEnabled::Yes, rule_tree: RuleTree::new(), num_rebuilds: 0, } @@ -593,7 +607,7 @@ impl Stylist { /// Sets whether author style is enabled or not. pub fn set_author_styles_enabled(&mut self, enabled: AuthorStylesEnabled) { - self.stylesheets.set_author_styles_enabled(enabled); + self.author_styles_enabled = enabled; } /// Returns whether we've recorded any stylesheet change so far. @@ -1221,8 +1235,11 @@ impl Stylist { let only_default_rules = rule_inclusion == RuleInclusion::DefaultOnly; - let matches_user_and_author_rules = + let matches_user_rules = rule_hash_target.matches_user_and_author_rules(); + let matches_author_rules = + matches_user_rules && + self.author_styles_enabled == AuthorStylesEnabled::Yes; // Step 1: Normal user-agent rules. if let Some(map) = self.cascade_data.user_agent.cascade_data.normal_rules(pseudo_element) { @@ -1260,7 +1277,7 @@ impl Stylist { // rule_hash_target.matches_user_and_author_rules()) // // Which may be more what you would probably expect. - if matches_user_and_author_rules { + if matches_user_rules { // Step 3a: User normal rules. if let Some(map) = self.cascade_data.user.normal_rules(pseudo_element) { map.get_all_matching_rules( @@ -1280,7 +1297,7 @@ impl Stylist { // particular, normally document rules override ::slotted() rules, but // for !important it should be the other way around. So probably we need // to add some sort of AuthorScoped cascade level or something. - if !only_default_rules { + if matches_author_rules && !only_default_rules { // Match slotted rules in reverse order, so that the outer slotted // rules come before the inner rules (and thus have less priority). let mut slots = SmallVec::<[_; 3]>::new(); @@ -1308,9 +1325,9 @@ impl Stylist { // FIXME(emilio): It looks very wrong to match XBL / Shadow DOM rules // even for getDefaultComputedStyle! + // + // Also, this doesn't account for the author_styles_enabled stuff. let cut_off_inheritance = element.each_xbl_stylist(|stylist| { - // ServoStyleSet::CreateXBLServoStyleSet() loads XBL style sheets - // under eAuthorSheetFeatures level. if let Some(map) = stylist.cascade_data.author.normal_rules(pseudo_element) { // NOTE(emilio): This is needed because the XBL stylist may // think it has a different quirks mode than the document. @@ -1337,9 +1354,7 @@ impl Stylist { } }); - if matches_user_and_author_rules && !only_default_rules && - !cut_off_inheritance - { + if matches_author_rules && !only_default_rules && !cut_off_inheritance { // Step 3c: Author normal rules. if let Some(map) = self.cascade_data.author.normal_rules(pseudo_element) { map.get_all_matching_rules( diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 94a46e3395f..fe46a215188 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -135,7 +135,6 @@ use style::selector_parser::{PseudoElementCascadeType, SelectorImpl}; use style::shared_lock::{SharedRwLockReadGuard, StylesheetGuards, ToCssWithGuard, Locked}; use style::string_cache::{Atom, WeakAtom}; use style::style_adjuster::StyleAdjuster; -use style::stylesheet_set::AuthorStylesEnabled; use style::stylesheets::{CssRule, CssRules, CssRuleType, CssRulesHelpers, DocumentRule}; use style::stylesheets::{FontFeatureValuesRule, ImportRule, KeyframesRule, MediaRule}; use style::stylesheets::{NamespaceRule, Origin, OriginSet, PageRule, StyleRule}; @@ -143,7 +142,7 @@ use style::stylesheets::{StylesheetContents, SupportsRule}; use style::stylesheets::StylesheetLoader as StyleStylesheetLoader; use style::stylesheets::keyframes_rule::{Keyframe, KeyframeSelector, KeyframesStepValue}; use style::stylesheets::supports_rule::parse_condition_or_declaration; -use style::stylist::{add_size_of_ua_cache, RuleInclusion, Stylist}; +use style::stylist::{add_size_of_ua_cache, AuthorStylesEnabled, RuleInclusion, Stylist}; use style::thread_state; use style::timer::Timer; use style::traversal::DomTraversal; @@ -1248,11 +1247,18 @@ pub unsafe extern "C" fn Servo_StyleSet_FlushStyleSheets( #[no_mangle] pub extern "C" fn Servo_StyleSet_NoteStyleSheetsChanged( raw_data: RawServoStyleSetBorrowed, - author_style_disabled: bool, changed_origins: OriginFlags, ) { let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); data.stylist.force_stylesheet_origins_dirty(OriginSet::from(changed_origins)); +} + +#[no_mangle] +pub extern "C" fn Servo_StyleSet_SetAuthorStyleDisabled( + raw_data: RawServoStyleSetBorrowed, + author_style_disabled: bool, +) { + let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let enabled = if author_style_disabled { AuthorStylesEnabled::No From 5e8dd8a2d4011e0767159d70f6f9858eec18dbe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 8 Feb 2018 15:56:26 +0100 Subject: [PATCH 08/11] style: Move the stylesheet set methods under a macro to reuse it soon. MozReview-Commit-ID: 50Srw4Mjw18 --- components/style/stylesheet_set.rs | 161 ++++++++++++++++------------- 1 file changed, 90 insertions(+), 71 deletions(-) diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index eeb75e738e6..fcaebe5b7c6 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -394,6 +394,85 @@ where invalidations: StylesheetInvalidationSet, } +/// This macro defines methods common to DocumentStylesheetSet and +/// AuthorStylesheetSet. +/// +/// We could simplify the setup moving invalidations to SheetCollection, but +/// that would imply not sharing invalidations across origins of the same +/// documents, which is slightly annoying. +macro_rules! sheet_set_methods { + ($set_name:expr) => { + fn collect_invalidations_for( + &mut self, + device: Option<&Device>, + sheet: &S, + guard: &SharedRwLockReadGuard, + ) { + if let Some(device) = device { + self.invalidations.collect_invalidations_for(device, sheet, guard); + } + } + + /// Appends a new stylesheet to the current set. + /// + /// No device implies not computing invalidations. + pub fn append_stylesheet( + &mut self, + device: Option<&Device>, + sheet: S, + guard: &SharedRwLockReadGuard + ) { + debug!(concat!($set_name, "::append_stylesheet")); + self.collect_invalidations_for(device, &sheet, guard); + let collection = self.collection_for(&sheet, guard); + collection.append(sheet); + } + + /// Prepend a new stylesheet to the current set. + pub fn prepend_stylesheet( + &mut self, + device: Option<&Device>, + sheet: S, + guard: &SharedRwLockReadGuard + ) { + debug!(concat!($set_name, "::prepend_stylesheet")); + self.collect_invalidations_for(device, &sheet, guard); + + let collection = self.collection_for(&sheet, guard); + collection.prepend(sheet); + } + + /// Insert a given stylesheet before another stylesheet in the document. + pub fn insert_stylesheet_before( + &mut self, + device: Option<&Device>, + sheet: S, + before_sheet: S, + guard: &SharedRwLockReadGuard, + ) { + debug!(concat!($set_name, "::insert_stylesheet_before")); + self.collect_invalidations_for(device, &sheet, guard); + + let collection = self.collection_for(&sheet, guard); + collection.insert_before(sheet, &before_sheet); + } + + /// Remove a given stylesheet from the set. + pub fn remove_stylesheet( + &mut self, + device: Option<&Device>, + sheet: S, + guard: &SharedRwLockReadGuard, + ) { + debug!(concat!($set_name, "::remove_stylesheet")); + self.collect_invalidations_for(device, &sheet, guard); + + let collection = self.collection_for(&sheet, guard); + collection.remove(&sheet) + } + } +} + impl DocumentStylesheetSet where S: StylesheetInDocument + PartialEq + 'static, @@ -406,6 +485,17 @@ where } } + fn collection_for( + &mut self, + sheet: &S, + guard: &SharedRwLockReadGuard, + ) -> &mut SheetCollection { + let origin = sheet.contents(guard).origin; + self.collections.borrow_mut_for_origin(&origin) + } + + sheet_set_methods!("DocumentStylesheetSet"); + /// Returns the number of stylesheets in the set. pub fn len(&self) -> usize { self.collections.iter_origins().fold(0, |s, (item, _)| s + item.len()) @@ -416,77 +506,6 @@ where self.collections.borrow_for_origin(&origin).get(index) } - fn collect_invalidations_for( - &mut self, - device: Option<&Device>, - sheet: &S, - guard: &SharedRwLockReadGuard, - ) { - if let Some(device) = device { - self.invalidations.collect_invalidations_for(device, sheet, guard); - } - } - - /// Appends a new stylesheet to the current set. - /// - /// No device implies not computing invalidations. - pub fn append_stylesheet( - &mut self, - device: Option<&Device>, - sheet: S, - guard: &SharedRwLockReadGuard - ) { - debug!("DocumentStylesheetSet::append_stylesheet"); - self.collect_invalidations_for(device, &sheet, guard); - let origin = sheet.contents(guard).origin; - self.collections.borrow_mut_for_origin(&origin).append(sheet); - } - - /// Prepend a new stylesheet to the current set. - pub fn prepend_stylesheet( - &mut self, - device: Option<&Device>, - sheet: S, - guard: &SharedRwLockReadGuard - ) { - debug!("DocumentStylesheetSet::prepend_stylesheet"); - self.collect_invalidations_for(device, &sheet, guard); - - let origin = sheet.contents(guard).origin; - self.collections.borrow_mut_for_origin(&origin).prepend(sheet) - } - - /// Insert a given stylesheet before another stylesheet in the document. - pub fn insert_stylesheet_before( - &mut self, - device: Option<&Device>, - sheet: S, - before_sheet: S, - guard: &SharedRwLockReadGuard, - ) { - debug!("DocumentStylesheetSet::insert_stylesheet_before"); - self.collect_invalidations_for(device, &sheet, guard); - - let origin = sheet.contents(guard).origin; - self.collections - .borrow_mut_for_origin(&origin) - .insert_before(sheet, &before_sheet) - } - - /// Remove a given stylesheet from the set. - pub fn remove_stylesheet( - &mut self, - device: Option<&Device>, - sheet: S, - guard: &SharedRwLockReadGuard, - ) { - debug!("StylesheetSet::remove_stylesheet"); - self.collect_invalidations_for(device, &sheet, guard); - - let origin = sheet.contents(guard).origin; - self.collections.borrow_mut_for_origin(&origin).remove(&sheet) - } - /// Returns whether the given set has changed from the last flush. pub fn has_changed(&self) -> bool { self.collections.iter_origins().any(|(collection, _)| collection.dirty) From 7edd25890d6346e4337448b3aecd232bff31d88f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 8 Feb 2018 16:03:47 +0100 Subject: [PATCH 09/11] style: Introduce AuthorStyleSheetSet. MozReview-Commit-ID: K3ciocPJuz3 --- components/style/stylesheet_set.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index fcaebe5b7c6..6f41be45fe7 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -587,3 +587,29 @@ where } } } + +/// The set of stylesheets effective for a given XBL binding or Shadow Root. +pub struct AuthorStylesheetSet +where + S: StylesheetInDocument + PartialEq + 'static, +{ + /// The actual style sheets. + collection: SheetCollection, + /// The set of invalidations scheduled for this collection. + invalidations: StylesheetInvalidationSet, +} + +impl AuthorStylesheetSet +where + S: StylesheetInDocument + PartialEq + 'static, +{ + fn collection_for( + &mut self, + _sheet: &S, + _guard: &SharedRwLockReadGuard, + ) -> &mut SheetCollection { + &mut self.collection + } + + sheet_set_methods!("AuthorStylesheetSet"); +} From 88cc2798c4b3ebf59f16fa84707e6fcf8b54d7c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 9 Feb 2018 12:21:14 +0100 Subject: [PATCH 10/11] style: Move more stuff to the place it belongs to. In particular, `rebuild` is now done entirely by CascadeData, which simplifies more stuff. The eventual final state for this is that the data structure we use to store the XBL / Shadow DOM stuff will be something like: struct AuthorStyles { CascadeData, AuthorSheetCollection } MozReview-Commit-ID: 8TExtP58L4X --- components/style/stylesheet_set.rs | 179 ++++++++++++++--------------- components/style/stylist.rs | 108 ++++++++--------- 2 files changed, 139 insertions(+), 148 deletions(-) diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 6f41be45fe7..c170e8d6078 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -9,7 +9,7 @@ use invalidation::stylesheets::StylesheetInvalidationSet; use media_queries::Device; use selector_parser::SnapshotMap; use shared_lock::SharedRwLockReadGuard; -use std::slice; +use std::{mem, slice}; use stylesheets::{Origin, OriginSet, OriginSetIterator, PerOrigin, StylesheetInDocument}; /// Entry for a StylesheetSet. @@ -125,13 +125,11 @@ impl Default for DataValidity { } /// A struct to iterate over the different stylesheets to be flushed. -pub struct StylesheetFlusher<'a, S> +pub struct DocumentStylesheetFlusher<'a, S> where S: StylesheetInDocument + PartialEq + 'static, { - origins_dirty: OriginSet, collections: &'a mut PerOrigin>, - origin_data_validity: PerOrigin, had_invalidations: bool, } @@ -151,82 +149,59 @@ impl SheetRebuildKind { } } -impl<'a, S> StylesheetFlusher<'a, S> +impl<'a, S> DocumentStylesheetFlusher<'a, S> where S: StylesheetInDocument + PartialEq + 'static, { - /// The data validity for a given origin. - pub fn data_validity(&self, origin: Origin) -> DataValidity { - *self.origin_data_validity.borrow_for_origin(&origin) + /// Returns a flusher for `origin`. + pub fn flush_origin(&mut self, origin: Origin) -> SheetCollectionFlusher { + self.collections.borrow_mut_for_origin(&origin).flush() } - /// Whether the origin data is dirty in any way. - pub fn origin_dirty(&self, origin: Origin) -> bool { - self.origins_dirty.contains(origin.into()) - } - - /// Returns an iterator over the stylesheets of a given origin, assuming all - /// of them will be flushed. - pub fn manual_origin_sheets<'b>(&'b mut self, origin: Origin) -> StylesheetCollectionIterator<'b, S> - where - 'a: 'b - { - debug_assert_eq!(origin, Origin::UserAgent); - - // We could iterate over `origin_sheets(origin)` to ensure state is - // consistent (that the `committed` member of the Entry is reset to - // `true`). - // - // In practice it doesn't matter for correctness given our use of it - // (that this is UA only). - // - // FIXME(emilio): I think it does matter and that we effectively don't - // support removing UA sheets since #19927... - self.collections.borrow_for_origin(&origin).iter() - } - - /// Returns a flusher for the dirty origin `origin`. - pub fn origin_sheets<'b>(&'b mut self, origin: Origin) -> PerOriginFlusher<'b, S> - where - 'a: 'b - { - let validity = self.data_validity(origin); - let origin_dirty = self.origins_dirty.contains(origin.into()); - - debug_assert!( - origin_dirty || validity == DataValidity::Valid, - "origin_data_validity should be a subset of origins_dirty!" - ); - - PerOriginFlusher { - iter: self.collections.borrow_mut_for_origin(&origin).entries.iter_mut(), - validity, - } - } - - /// Returns whether running the whole flushing process would be a no-op. - pub fn nothing_to_do(&self) -> bool { - self.origins_dirty.is_empty() + /// Returns the list of stylesheets for `origin`. + /// + /// Only used for UA sheets. + pub fn origin_sheets(&mut self, origin: Origin) -> StylesheetCollectionIterator { + self.collections.borrow_mut_for_origin(&origin).iter() } /// Returns whether any DOM invalidations were processed as a result of the /// stylesheet flush. + #[inline] pub fn had_invalidations(&self) -> bool { self.had_invalidations } } -/// A flusher struct for a given origin, that takes care of returning the +/// A flusher struct for a given collection, that takes care of returning the /// appropriate stylesheets that need work. -pub struct PerOriginFlusher<'a, S> +pub struct SheetCollectionFlusher<'a, S> where S: StylesheetInDocument + PartialEq + 'static { iter: slice::IterMut<'a, StylesheetSetEntry>, validity: DataValidity, + dirty: bool, } -impl<'a, S> Iterator for PerOriginFlusher<'a, S> +impl<'a, S> SheetCollectionFlusher<'a, S> +where + S: StylesheetInDocument + PartialEq + 'static, +{ + /// Whether the collection was originally dirty. + #[inline] + pub fn dirty(&self) -> bool { + self.dirty + } + + /// What the state of the sheet data is. + #[inline] + pub fn data_validity(&self) -> DataValidity { + self.validity + } +} + +impl<'a, S> Iterator for SheetCollectionFlusher<'a, S> where S: StylesheetInDocument + PartialEq + 'static, { @@ -379,6 +354,17 @@ where fn iter(&self) -> StylesheetCollectionIterator { StylesheetCollectionIterator(self.entries.iter()) } + + fn flush(&mut self) -> SheetCollectionFlusher { + let dirty = mem::replace(&mut self.dirty, false); + let validity = mem::replace(&mut self.data_validity, DataValidity::Valid); + + SheetCollectionFlusher { + iter: self.entries.iter_mut(), + dirty, + validity, + } + } } /// The set of stylesheets effective for a given document. @@ -420,7 +406,7 @@ macro_rules! sheet_set_methods { &mut self, device: Option<&Device>, sheet: S, - guard: &SharedRwLockReadGuard + guard: &SharedRwLockReadGuard, ) { debug!(concat!($set_name, "::append_stylesheet")); self.collect_invalidations_for(device, &sheet, guard); @@ -433,7 +419,7 @@ macro_rules! sheet_set_methods { &mut self, device: Option<&Device>, sheet: S, - guard: &SharedRwLockReadGuard + guard: &SharedRwLockReadGuard, ) { debug!(concat!($set_name, "::prepend_stylesheet")); self.collect_invalidations_for(device, &sheet, guard); @@ -477,7 +463,7 @@ impl DocumentStylesheetSet where S: StylesheetInDocument + PartialEq + 'static, { - /// Create a new empty StylesheetSet. + /// Create a new empty DocumentStylesheetSet. pub fn new() -> Self { Self { collections: Default::default(), @@ -512,58 +498,41 @@ where } /// Flush the current set, unmarking it as dirty, and returns a - /// `StylesheetFlusher` in order to rebuild the stylist. - pub fn flush<'a, E>( - &'a mut self, + /// `DocumentStylesheetFlusher` in order to rebuild the stylist. + pub fn flush( + &mut self, document_element: Option, snapshots: Option<&SnapshotMap>, - ) -> StylesheetFlusher<'a, S> + ) -> DocumentStylesheetFlusher where E: TElement, { - use std::mem; - debug!("DocumentStylesheetSet::flush"); let had_invalidations = self.invalidations.flush(document_element, snapshots); - let mut origins_dirty = OriginSet::empty(); - let mut origin_data_validity = PerOrigin::::default(); - for (collection, origin) in self.collections.iter_mut_origins() { - let was_dirty = mem::replace(&mut collection.dirty, false); - if !was_dirty { - debug_assert_eq!(collection.data_validity, DataValidity::Valid); - continue; - } - - origins_dirty |= origin; - *origin_data_validity.borrow_mut_for_origin(&origin) = - mem::replace(&mut collection.data_validity, DataValidity::Valid); - } - - StylesheetFlusher { + DocumentStylesheetFlusher { collections: &mut self.collections, had_invalidations, - origins_dirty, - origin_data_validity, } } /// Flush stylesheets, but without running any of the invalidation passes. #[cfg(feature = "servo")] pub fn flush_without_invalidation(&mut self) -> OriginSet { - use std::mem; - debug!("DocumentStylesheetSet::flush_without_invalidation"); + let mut origins = OriginSet::empty(); self.invalidations.clear(); + for (collection, origin) in self.collections.iter_mut_origins() { - collection.dirty = false; - // NOTE(emilio): I think this should also poke at the data validity - // and such, but it doesn't really matter given we don't use that - // collection for style resolution anyway. + if collection.flush().dirty() { + origins |= origin; + } } + + origins } /// Return an iterator over the flattened view of all the stylesheets. @@ -599,6 +568,17 @@ where invalidations: StylesheetInvalidationSet, } +/// A struct to flush an author style sheet collection. +pub struct AuthorStylesheetFlusher<'a, S> +where + S: StylesheetInDocument + PartialEq + 'static, +{ + /// The actual flusher for the collection. + pub sheets: SheetCollectionFlusher<'a, S>, + /// Whether any sheet invalidation matched. + pub had_invalidations: bool, +} + impl AuthorStylesheetSet where S: StylesheetInDocument + PartialEq + 'static, @@ -612,4 +592,23 @@ where } sheet_set_methods!("AuthorStylesheetSet"); + + /// Flush the stylesheets for this author set. + /// + /// `host` is the root of the affected subtree, like the shadow host, for + /// example. + pub fn flush( + &mut self, + host: Option, + snapshots: Option<&SnapshotMap>, + ) -> AuthorStylesheetFlusher + where + E: TElement, + { + let had_invalidations = self.invalidations.flush(host, snapshots); + AuthorStylesheetFlusher { + sheets: self.collection.flush(), + had_invalidations, + } + } } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index f656f77476b..029adf11fb9 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -41,7 +41,8 @@ use smallvec::SmallVec; use std::ops; use std::sync::Mutex; use style_traits::viewport::ViewportConstraints; -use stylesheet_set::{DataValidity, SheetRebuildKind, DocumentStylesheetSet, StylesheetFlusher}; +use stylesheet_set::{DataValidity, SheetRebuildKind, DocumentStylesheetSet}; +use stylesheet_set::{DocumentStylesheetFlusher, SheetCollectionFlusher}; #[cfg(feature = "gecko")] use stylesheets::{CounterStyleRule, FontFaceRule, FontFeatureValuesRule, PageRule}; use stylesheets::{CssRule, Origin, OriginSet, PerOrigin, PerOriginIter}; @@ -227,46 +228,6 @@ impl DocumentCascadeData { } } - fn rebuild_origin<'a, S>( - device: &Device, - quirks_mode: QuirksMode, - flusher: &mut StylesheetFlusher<'a, S>, - guards: &StylesheetGuards, - origin: Origin, - cascade_data: &mut CascadeData, - ) -> Result<(), FailedAllocationError> - where - S: StylesheetInDocument + ToMediaListKey + PartialEq + 'static, - { - debug_assert_ne!(origin, Origin::UserAgent); - - if !flusher.origin_dirty(origin) { - return Ok(()); - } - - let validity = flusher.data_validity(origin); - - match validity { - DataValidity::Valid => {}, - DataValidity::CascadeInvalid => cascade_data.clear_cascade_data(), - DataValidity::FullyInvalid => cascade_data.clear(), - } - - let guard = guards.for_origin(origin); - for (stylesheet, rebuild_kind) in flusher.origin_sheets(origin) { - cascade_data.add_stylesheet( - device, - quirks_mode, - stylesheet, - guard, - rebuild_kind, - /* precomputed_pseudo_element_decls = */ None, - )?; - } - - Ok(()) - } - /// Rebuild the cascade data for the given document stylesheets, and /// optionally with a set of user agent stylesheets. Returns Err(..) /// to signify OOM. @@ -274,21 +235,17 @@ impl DocumentCascadeData { &mut self, device: &Device, quirks_mode: QuirksMode, - mut flusher: StylesheetFlusher<'a, S>, + mut flusher: DocumentStylesheetFlusher<'a, S>, guards: &StylesheetGuards, ) -> Result<(), FailedAllocationError> where S: StylesheetInDocument + ToMediaListKey + PartialEq + 'static, { - debug_assert!(!flusher.nothing_to_do()); - // First do UA sheets. { - if flusher.origin_dirty(Origin::UserAgent) { + if flusher.flush_origin(Origin::UserAgent).dirty() { let mut ua_cache = UA_CASCADE_DATA_CACHE.lock().unwrap(); - let origin_sheets = - flusher.manual_origin_sheets(Origin::UserAgent); - + let origin_sheets = flusher.origin_sheets(Origin::UserAgent); let ua_cascade_data = ua_cache.lookup( origin_sheets, device, @@ -302,23 +259,19 @@ impl DocumentCascadeData { } // Now do the user sheets. - Self::rebuild_origin( + self.user.rebuild( device, quirks_mode, - &mut flusher, - guards, - Origin::User, - &mut self.user, + flusher.flush_origin(Origin::User), + guards.ua_or_user, )?; // And now the author sheets. - Self::rebuild_origin( + self.author.rebuild( device, quirks_mode, - &mut flusher, - guards, - Origin::Author, - &mut self.author, + flusher.flush_origin(Origin::Author), + guards.author, )?; Ok(()) @@ -2048,6 +2001,45 @@ impl CascadeData { } } + /// Rebuild the cascade data from a given SheetCollection, incrementally if + /// possible. + fn rebuild<'a, S>( + &mut self, + device: &Device, + quirks_mode: QuirksMode, + collection: SheetCollectionFlusher, + guard: &SharedRwLockReadGuard, + ) -> Result<(), FailedAllocationError> + where + S: StylesheetInDocument + ToMediaListKey + PartialEq + 'static, + { + if !collection.dirty() { + return Ok(()); + } + + let validity = collection.data_validity(); + + match validity { + DataValidity::Valid => {}, + DataValidity::CascadeInvalid => self.clear_cascade_data(), + DataValidity::FullyInvalid => self.clear(), + } + + for (stylesheet, rebuild_kind) in collection { + self.add_stylesheet( + device, + quirks_mode, + stylesheet, + guard, + rebuild_kind, + /* precomputed_pseudo_element_decls = */ None, + )?; + } + + Ok(()) + } + + /// Returns the invalidation map. pub fn invalidation_map(&self) -> &InvalidationMap { &self.invalidation_map From f16e699db5ae9d587bf7ce06e87dbe07256c7e57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 9 Feb 2018 19:46:26 +0100 Subject: [PATCH 11/11] style: Update bindings. --- components/style/gecko/generated/bindings.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/style/gecko/generated/bindings.rs b/components/style/gecko/generated/bindings.rs index 39cf9bf9106..cad7e4054e1 100644 --- a/components/style/gecko/generated/bindings.rs +++ b/components/style/gecko/generated/bindings.rs @@ -2130,9 +2130,14 @@ extern "C" { ); } extern "C" { - pub fn Servo_StyleSet_NoteStyleSheetsChanged( + pub fn Servo_StyleSet_SetAuthorStyleDisabled( set: RawServoStyleSetBorrowed, author_style_disabled: bool, + ); +} +extern "C" { + pub fn Servo_StyleSet_NoteStyleSheetsChanged( + set: RawServoStyleSetBorrowed, changed_origins: OriginFlags, ); }