From b5232c940dfcb911ba3741106017bc0a962cc45f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 2 Jun 2017 16:34:13 +0200 Subject: [PATCH 1/3] style: Move the stylesheet invalidation code to another submodule. Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1357461 MozReview-Commit-ID: DIzZXoHycZs --- components/style/invalidation/mod.rs | 293 +----------------- components/style/invalidation/stylesheets.rs | 296 +++++++++++++++++++ components/style/stylesheet_set.rs | 2 +- 3 files changed, 299 insertions(+), 292 deletions(-) create mode 100644 components/style/invalidation/stylesheets.rs diff --git a/components/style/invalidation/mod.rs b/components/style/invalidation/mod.rs index 93f0d8783ad..6107894b00d 100644 --- a/components/style/invalidation/mod.rs +++ b/components/style/invalidation/mod.rs @@ -2,295 +2,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -//! A collection of invalidations due to changes in which stylesheets affect a -//! document. +//! Different bits of code related to invalidating style. -#![deny(unsafe_code)] - -use Atom; -use data::StoredRestyleHint; -use dom::{TElement, TNode}; -use fnv::FnvHashSet; -use selector_parser::SelectorImpl; -use selectors::parser::{Component, Selector}; -use shared_lock::SharedRwLockReadGuard; -use stylesheets::{CssRule, Stylesheet}; -use stylist::Stylist; - -/// An invalidation scope represents a kind of subtree that may need to be -/// restyled. -#[derive(Debug, Hash, Eq, PartialEq)] -enum InvalidationScope { - /// All the descendants of an element with a given id. - ID(Atom), - /// All the descendants of an element with a given class name. - Class(Atom), -} - -impl InvalidationScope { - fn is_id(&self) -> bool { - matches!(*self, InvalidationScope::ID(..)) - } - - fn matches(&self, element: E) -> bool - where E: TElement, - { - match *self { - InvalidationScope::Class(ref class) => { - element.has_class(class) - } - InvalidationScope::ID(ref id) => { - match element.get_id() { - Some(element_id) => element_id == *id, - None => false, - } - } - } - } -} - -/// A set of invalidations due to stylesheet additions. -/// -/// TODO(emilio): We might be able to do the same analysis for removals and -/// media query changes too? -pub struct StylesheetInvalidationSet { - /// The style scopes we know we have to restyle so far. - invalid_scopes: FnvHashSet, - /// Whether the whole document should be invalid. - fully_invalid: bool, -} - -impl StylesheetInvalidationSet { - /// Create an empty `StylesheetInvalidationSet`. - pub fn new() -> Self { - Self { - invalid_scopes: FnvHashSet::default(), - fully_invalid: false, - } - } - - /// Mark the DOM tree styles' as fully invalid. - pub fn invalidate_fully(&mut self) { - debug!("StylesheetInvalidationSet::invalidate_fully"); - self.invalid_scopes.clear(); - self.fully_invalid = true; - } - - /// Analyze the given stylesheet, and collect invalidations from their - /// rules, in order to avoid doing a full restyle when we style the document - /// next time. - pub fn collect_invalidations_for( - &mut self, - stylist: &Stylist, - stylesheet: &Stylesheet, - guard: &SharedRwLockReadGuard) - { - debug!("StylesheetInvalidationSet::collect_invalidations_for"); - if self.fully_invalid { - debug!(" > Fully invalid already"); - return; - } - - if stylesheet.disabled() || - !stylesheet.is_effective_for_device(stylist.device(), guard) { - debug!(" > Stylesheet was not effective"); - return; // Nothing to do here. - } - - for rule in stylesheet.effective_rules(stylist.device(), guard) { - self.collect_invalidations_for_rule(rule, guard); - if self.fully_invalid { - self.invalid_scopes.clear(); - break; - } - } - - debug!(" > resulting invalidations: {:?}", self.invalid_scopes); - debug!(" > fully_invalid: {}", self.fully_invalid); - } - - /// Clears the invalidation set, invalidating elements as needed if - /// `document_element` is provided. - pub fn flush(&mut self, document_element: Option) - where E: TElement, - { - if let Some(e) = document_element { - self.process_invalidations_in_subtree(e); - } - self.invalid_scopes.clear(); - self.fully_invalid = false; - } - - /// Process style invalidations in a given subtree, that is, look for all - /// the relevant scopes in the subtree, and mark as dirty only the relevant - /// ones. - /// - /// Returns whether it invalidated at least one element's style. - #[allow(unsafe_code)] - fn process_invalidations_in_subtree(&self, element: E) -> bool - where E: TElement, - { - let mut data = match element.mutate_data() { - Some(data) => data, - None => return false, - }; - - if !data.has_styles() { - return false; - } - - if let Some(ref r) = data.get_restyle() { - if r.hint.contains_subtree() { - debug!("process_invalidations_in_subtree: {:?} was already invalid", - element); - return false; - } - } - - if self.fully_invalid { - debug!("process_invalidations_in_subtree: fully_invalid({:?})", - element); - data.ensure_restyle().hint.insert(StoredRestyleHint::subtree()); - return true; - } - - for scope in &self.invalid_scopes { - if scope.matches(element) { - debug!("process_invalidations_in_subtree: {:?} matched {:?}", - element, scope); - data.ensure_restyle().hint.insert(StoredRestyleHint::subtree()); - return true; - } - } - - - let mut any_children_invalid = false; - - for child in element.as_node().children() { - let child = match child.as_element() { - Some(e) => e, - None => continue, - }; - - any_children_invalid |= self.process_invalidations_in_subtree(child); - } - - if any_children_invalid { - debug!("Children of {:?} changed, setting dirty descendants", - element); - unsafe { element.set_dirty_descendants() } - } - - return any_children_invalid - } - - fn scan_component( - component: &Component, - scope: &mut Option) - { - match *component { - Component::Class(ref class) => { - if scope.as_ref().map_or(true, |s| !s.is_id()) { - *scope = Some(InvalidationScope::Class(class.clone())); - } - } - Component::ID(ref id) => { - if scope.is_none() { - *scope = Some(InvalidationScope::ID(id.clone())); - } - } - _ => { - // Ignore everything else, at least for now. - } - } - } - - /// Collect a style scopes for a given selector. - /// - /// We look at the outermost class or id selector to the left of an ancestor - /// combinator, in order to restyle only a given subtree. - /// - /// We prefer id scopes to class scopes, and outermost scopes to innermost - /// scopes (to reduce the amount of traversal we need to do). - fn collect_scopes(&mut self, selector: &Selector) { - debug!("StylesheetInvalidationSet::collect_scopes({:?})", selector); - - let mut scope: Option = None; - - let mut scan = true; - let mut iter = selector.inner.complex.iter(); - - loop { - for component in &mut iter { - if scan { - Self::scan_component(component, &mut scope); - } - } - match iter.next_sequence() { - None => break, - Some(combinator) => { - scan = combinator.is_ancestor(); - } - } - } - - match scope { - Some(s) => { - debug!(" > Found scope: {:?}", s); - self.invalid_scopes.insert(s); - } - None => { - debug!(" > Scope not found"); - - // If we didn't find a scope, any element could match this, so - // let's just bail out. - self.fully_invalid = true; - } - } - } - - /// Collects invalidations for a given CSS rule. - fn collect_invalidations_for_rule( - &mut self, - rule: &CssRule, - guard: &SharedRwLockReadGuard) - { - use stylesheets::CssRule::*; - debug!("StylesheetInvalidationSet::collect_invalidations_for_rule"); - debug_assert!(!self.fully_invalid, "Not worth to be here!"); - - match *rule { - Style(ref lock) => { - let style_rule = lock.read_with(guard); - for selector in &style_rule.selectors.0 { - self.collect_scopes(selector); - if self.fully_invalid { - return; - } - } - } - Document(..) | - Namespace(..) | - Import(..) | - Media(..) | - Supports(..) => { - // Do nothing, relevant nested rules are visited as part of the - // iteration. - } - FontFace(..) | - CounterStyle(..) | - Keyframes(..) | - Page(..) | - Viewport(..) => { - debug!(" > Found unsupported rule, marking the whole subtree \ - invalid."); - - // TODO(emilio): Can we do better here? - // - // At least in `@page`, we could check the relevant media, I - // guess. - self.fully_invalid = true; - } - } - } -} +pub mod stylesheets; diff --git a/components/style/invalidation/stylesheets.rs b/components/style/invalidation/stylesheets.rs new file mode 100644 index 00000000000..93f0d8783ad --- /dev/null +++ b/components/style/invalidation/stylesheets.rs @@ -0,0 +1,296 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! A collection of invalidations due to changes in which stylesheets affect a +//! document. + +#![deny(unsafe_code)] + +use Atom; +use data::StoredRestyleHint; +use dom::{TElement, TNode}; +use fnv::FnvHashSet; +use selector_parser::SelectorImpl; +use selectors::parser::{Component, Selector}; +use shared_lock::SharedRwLockReadGuard; +use stylesheets::{CssRule, Stylesheet}; +use stylist::Stylist; + +/// An invalidation scope represents a kind of subtree that may need to be +/// restyled. +#[derive(Debug, Hash, Eq, PartialEq)] +enum InvalidationScope { + /// All the descendants of an element with a given id. + ID(Atom), + /// All the descendants of an element with a given class name. + Class(Atom), +} + +impl InvalidationScope { + fn is_id(&self) -> bool { + matches!(*self, InvalidationScope::ID(..)) + } + + fn matches(&self, element: E) -> bool + where E: TElement, + { + match *self { + InvalidationScope::Class(ref class) => { + element.has_class(class) + } + InvalidationScope::ID(ref id) => { + match element.get_id() { + Some(element_id) => element_id == *id, + None => false, + } + } + } + } +} + +/// A set of invalidations due to stylesheet additions. +/// +/// TODO(emilio): We might be able to do the same analysis for removals and +/// media query changes too? +pub struct StylesheetInvalidationSet { + /// The style scopes we know we have to restyle so far. + invalid_scopes: FnvHashSet, + /// Whether the whole document should be invalid. + fully_invalid: bool, +} + +impl StylesheetInvalidationSet { + /// Create an empty `StylesheetInvalidationSet`. + pub fn new() -> Self { + Self { + invalid_scopes: FnvHashSet::default(), + fully_invalid: false, + } + } + + /// Mark the DOM tree styles' as fully invalid. + pub fn invalidate_fully(&mut self) { + debug!("StylesheetInvalidationSet::invalidate_fully"); + self.invalid_scopes.clear(); + self.fully_invalid = true; + } + + /// Analyze the given stylesheet, and collect invalidations from their + /// rules, in order to avoid doing a full restyle when we style the document + /// next time. + pub fn collect_invalidations_for( + &mut self, + stylist: &Stylist, + stylesheet: &Stylesheet, + guard: &SharedRwLockReadGuard) + { + debug!("StylesheetInvalidationSet::collect_invalidations_for"); + if self.fully_invalid { + debug!(" > Fully invalid already"); + return; + } + + if stylesheet.disabled() || + !stylesheet.is_effective_for_device(stylist.device(), guard) { + debug!(" > Stylesheet was not effective"); + return; // Nothing to do here. + } + + for rule in stylesheet.effective_rules(stylist.device(), guard) { + self.collect_invalidations_for_rule(rule, guard); + if self.fully_invalid { + self.invalid_scopes.clear(); + break; + } + } + + debug!(" > resulting invalidations: {:?}", self.invalid_scopes); + debug!(" > fully_invalid: {}", self.fully_invalid); + } + + /// Clears the invalidation set, invalidating elements as needed if + /// `document_element` is provided. + pub fn flush(&mut self, document_element: Option) + where E: TElement, + { + if let Some(e) = document_element { + self.process_invalidations_in_subtree(e); + } + self.invalid_scopes.clear(); + self.fully_invalid = false; + } + + /// Process style invalidations in a given subtree, that is, look for all + /// the relevant scopes in the subtree, and mark as dirty only the relevant + /// ones. + /// + /// Returns whether it invalidated at least one element's style. + #[allow(unsafe_code)] + fn process_invalidations_in_subtree(&self, element: E) -> bool + where E: TElement, + { + let mut data = match element.mutate_data() { + Some(data) => data, + None => return false, + }; + + if !data.has_styles() { + return false; + } + + if let Some(ref r) = data.get_restyle() { + if r.hint.contains_subtree() { + debug!("process_invalidations_in_subtree: {:?} was already invalid", + element); + return false; + } + } + + if self.fully_invalid { + debug!("process_invalidations_in_subtree: fully_invalid({:?})", + element); + data.ensure_restyle().hint.insert(StoredRestyleHint::subtree()); + return true; + } + + for scope in &self.invalid_scopes { + if scope.matches(element) { + debug!("process_invalidations_in_subtree: {:?} matched {:?}", + element, scope); + data.ensure_restyle().hint.insert(StoredRestyleHint::subtree()); + return true; + } + } + + + let mut any_children_invalid = false; + + for child in element.as_node().children() { + let child = match child.as_element() { + Some(e) => e, + None => continue, + }; + + any_children_invalid |= self.process_invalidations_in_subtree(child); + } + + if any_children_invalid { + debug!("Children of {:?} changed, setting dirty descendants", + element); + unsafe { element.set_dirty_descendants() } + } + + return any_children_invalid + } + + fn scan_component( + component: &Component, + scope: &mut Option) + { + match *component { + Component::Class(ref class) => { + if scope.as_ref().map_or(true, |s| !s.is_id()) { + *scope = Some(InvalidationScope::Class(class.clone())); + } + } + Component::ID(ref id) => { + if scope.is_none() { + *scope = Some(InvalidationScope::ID(id.clone())); + } + } + _ => { + // Ignore everything else, at least for now. + } + } + } + + /// Collect a style scopes for a given selector. + /// + /// We look at the outermost class or id selector to the left of an ancestor + /// combinator, in order to restyle only a given subtree. + /// + /// We prefer id scopes to class scopes, and outermost scopes to innermost + /// scopes (to reduce the amount of traversal we need to do). + fn collect_scopes(&mut self, selector: &Selector) { + debug!("StylesheetInvalidationSet::collect_scopes({:?})", selector); + + let mut scope: Option = None; + + let mut scan = true; + let mut iter = selector.inner.complex.iter(); + + loop { + for component in &mut iter { + if scan { + Self::scan_component(component, &mut scope); + } + } + match iter.next_sequence() { + None => break, + Some(combinator) => { + scan = combinator.is_ancestor(); + } + } + } + + match scope { + Some(s) => { + debug!(" > Found scope: {:?}", s); + self.invalid_scopes.insert(s); + } + None => { + debug!(" > Scope not found"); + + // If we didn't find a scope, any element could match this, so + // let's just bail out. + self.fully_invalid = true; + } + } + } + + /// Collects invalidations for a given CSS rule. + fn collect_invalidations_for_rule( + &mut self, + rule: &CssRule, + guard: &SharedRwLockReadGuard) + { + use stylesheets::CssRule::*; + debug!("StylesheetInvalidationSet::collect_invalidations_for_rule"); + debug_assert!(!self.fully_invalid, "Not worth to be here!"); + + match *rule { + Style(ref lock) => { + let style_rule = lock.read_with(guard); + for selector in &style_rule.selectors.0 { + self.collect_scopes(selector); + if self.fully_invalid { + return; + } + } + } + Document(..) | + Namespace(..) | + Import(..) | + Media(..) | + Supports(..) => { + // Do nothing, relevant nested rules are visited as part of the + // iteration. + } + FontFace(..) | + CounterStyle(..) | + Keyframes(..) | + Page(..) | + Viewport(..) => { + debug!(" > Found unsupported rule, marking the whole subtree \ + invalid."); + + // TODO(emilio): Can we do better here? + // + // At least in `@page`, we could check the relevant media, I + // guess. + self.fully_invalid = true; + } + } + } +} diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 2908d81bef4..5746446d2cf 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -5,7 +5,7 @@ //! A centralized set of stylesheets for a document. use dom::TElement; -use invalidation::StylesheetInvalidationSet; +use invalidation::stylesheets::StylesheetInvalidationSet; use shared_lock::SharedRwLockReadGuard; use std::slice; use stylearc::Arc; From 5c66d3c77a88281c295c20d0f1edda05186acedc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 2 Jun 2017 17:38:41 +0200 Subject: [PATCH 2/3] Cache effective media query results. This patch also makes RulesIterator not iterate over rules for which we don't process nested rules. There's nothing depending on this behavior right now afaik, and this will make us duplicate less code in following patches. Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1357461 MozReview-Commit-ID: CaMFQtAVnJF --- .../style/invalidation/media_queries.rs | 133 +++++++++++++++++ components/style/invalidation/mod.rs | 1 + components/style/stylesheet_set.rs | 5 + components/style/stylesheets.rs | 32 ++-- components/style/stylist.rs | 139 +++++++++--------- ports/geckolib/glue.rs | 19 +++ 6 files changed, 242 insertions(+), 87 deletions(-) create mode 100644 components/style/invalidation/media_queries.rs diff --git a/components/style/invalidation/media_queries.rs b/components/style/invalidation/media_queries.rs new file mode 100644 index 00000000000..46eca61a3a1 --- /dev/null +++ b/components/style/invalidation/media_queries.rs @@ -0,0 +1,133 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! Code related to the invalidation of media-query-affected rules. + +use context::QuirksMode; +use fnv::FnvHashSet; +use media_queries::Device; +use shared_lock::SharedRwLockReadGuard; +use stylesheets::{DocumentRule, ImportRule, MediaRule, SupportsRule}; +use stylesheets::{NestedRuleIterationCondition, Stylesheet}; + +/// A key for a given media query result. +/// +/// NOTE: It happens to be the case that all the media lists we care about +/// happen to have a stable address, so we can just use an opaque pointer to +/// represent them. +/// +/// Also, note that right now when a rule or stylesheet is removed, we do a full +/// style flush, so there's no need to worry about other item created with the +/// same pointer address. +/// +/// If this changes, though, we may need to remove the item from the cache if +/// present before it goes away. +#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub struct MediaListKey(usize); + +/// A trait to get a given `MediaListKey` for a given item that can hold a +/// `MediaList`. +pub trait ToMediaListKey : Sized { + /// Get a `MediaListKey` for this item. This key needs to uniquely identify + /// the item. + #[allow(unsafe_code)] + fn to_media_list_key(&self) -> MediaListKey { + use std::mem; + MediaListKey(unsafe { mem::transmute(self as *const Self) }) + } +} + +impl ToMediaListKey for Stylesheet {} +impl ToMediaListKey for ImportRule {} +impl ToMediaListKey for MediaRule {} + +/// A struct that holds the result of a media query evaluation pass for the +/// media queries that evaluated successfully. +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub struct EffectiveMediaQueryResults { + /// The set of media lists that matched last time. + set: FnvHashSet, +} + +impl EffectiveMediaQueryResults { + /// Trivially constructs an empty `EffectiveMediaQueryResults`. + pub fn new() -> Self { + Self { + set: FnvHashSet::default(), + } + } + + /// Resets the results, using an empty key. + pub fn clear(&mut self) { + self.set.clear() + } + + /// Returns whether a given item was known to be effective when the results + /// were cached. + pub fn was_effective(&self, item: &T) -> bool + where T: ToMediaListKey, + { + self.set.contains(&item.to_media_list_key()) + } + + /// Notices that an effective item has been seen, and caches it as matching. + pub fn saw_effective(&mut self, item: &T) + where T: ToMediaListKey, + { + // NOTE(emilio): We can't assert that we don't cache the same item twice + // because of stylesheet reusing... shrug. + self.set.insert(item.to_media_list_key()); + } +} + +/// A filter that filters over effective rules, but allowing all potentially +/// effective `@media` rules. +pub struct PotentiallyEffectiveMediaRules; + +impl NestedRuleIterationCondition for PotentiallyEffectiveMediaRules { + fn process_import( + _: &SharedRwLockReadGuard, + _: &Device, + _: QuirksMode, + _: &ImportRule) + -> bool + { + true + } + + fn process_media( + _: &SharedRwLockReadGuard, + _: &Device, + _: QuirksMode, + _: &MediaRule) + -> bool + { + true + } + + /// Whether we should process the nested rules in a given `@-moz-document` rule. + fn process_document( + guard: &SharedRwLockReadGuard, + device: &Device, + quirks_mode: QuirksMode, + rule: &DocumentRule) + -> bool + { + use stylesheets::EffectiveRules; + EffectiveRules::process_document(guard, device, quirks_mode, rule) + } + + /// Whether we should process the nested rules in a given `@supports` rule. + fn process_supports( + guard: &SharedRwLockReadGuard, + device: &Device, + quirks_mode: QuirksMode, + rule: &SupportsRule) + -> bool + { + use stylesheets::EffectiveRules; + EffectiveRules::process_supports(guard, device, quirks_mode, rule) + } +} diff --git a/components/style/invalidation/mod.rs b/components/style/invalidation/mod.rs index 6107894b00d..4b7f9c3cd65 100644 --- a/components/style/invalidation/mod.rs +++ b/components/style/invalidation/mod.rs @@ -4,4 +4,5 @@ //! Different bits of code related to invalidating style. +pub mod media_queries; pub mod stylesheets; diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 5746446d2cf..15bbf285a69 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -175,6 +175,11 @@ impl StylesheetSet { self.dirty = false; self.invalidations.flush(document_element); + self.iter() + } + + /// Returns an iterator over the current list of stylesheets. + pub fn iter(&self) -> StylesheetIterator { StylesheetIterator(self.entries.iter()) } diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 6347e39f0e9..23dad0d1c55 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -1048,7 +1048,6 @@ impl NestedRuleIterationCondition for AllRules { true } - /// Whether we should process the nested rules in a given `@media` rule. fn process_media( _: &SharedRwLockReadGuard, _: &Device, @@ -1059,7 +1058,6 @@ impl NestedRuleIterationCondition for AllRules { true } - /// Whether we should process the nested rules in a given `@-moz-document` rule. fn process_document( _: &SharedRwLockReadGuard, _: &Device, @@ -1070,7 +1068,6 @@ impl NestedRuleIterationCondition for AllRules { true } - /// Whether we should process the nested rules in a given `@supports` rule. fn process_supports( _: &SharedRwLockReadGuard, _: &Device, @@ -1158,36 +1155,31 @@ impl<'a, 'b, C> Iterator for RulesIterator<'a, 'b, C> sub_iter = match *rule { CssRule::Import(ref import_rule) => { let import_rule = import_rule.read_with(self.guard); - - if C::process_import(self.guard, self.device, self.quirks_mode, import_rule) { - Some(import_rule.stylesheet.rules.read_with(self.guard).0.iter()) - } else { - None + if !C::process_import(self.guard, self.device, self.quirks_mode, import_rule) { + continue; } + Some(import_rule.stylesheet.rules.read_with(self.guard).0.iter()) } CssRule::Document(ref doc_rule) => { let doc_rule = doc_rule.read_with(self.guard); - if C::process_document(self.guard, self.device, self.quirks_mode, doc_rule) { - Some(doc_rule.rules.read_with(self.guard).0.iter()) - } else { - None + if !C::process_document(self.guard, self.device, self.quirks_mode, doc_rule) { + continue; } + Some(doc_rule.rules.read_with(self.guard).0.iter()) } CssRule::Media(ref lock) => { let media_rule = lock.read_with(self.guard); - if C::process_media(self.guard, self.device, self.quirks_mode, media_rule) { - Some(media_rule.rules.read_with(self.guard).0.iter()) - } else { - None + if !C::process_media(self.guard, self.device, self.quirks_mode, media_rule) { + continue; } + Some(media_rule.rules.read_with(self.guard).0.iter()) } CssRule::Supports(ref lock) => { let supports_rule = lock.read_with(self.guard); - if C::process_supports(self.guard, self.device, self.quirks_mode, supports_rule) { - Some(supports_rule.rules.read_with(self.guard).0.iter()) - } else { - None + if !C::process_supports(self.guard, self.device, self.quirks_mode, supports_rule) { + continue; } + Some(supports_rule.rules.read_with(self.guard).0.iter()) } CssRule::Namespace(_) | CssRule::Style(_) | diff --git a/components/style/stylist.rs b/components/style/stylist.rs index eb2d43cf16c..a9087b18812 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -14,6 +14,7 @@ use error_reporting::RustLogReporter; use font_metrics::FontMetricsProvider; #[cfg(feature = "gecko")] use gecko_bindings::structs::{nsIAtom, StyleRuleInclusion}; +use invalidation::media_queries::EffectiveMediaQueryResults; use keyframes::KeyframesAnimation; use media_queries::Device; use properties::{self, CascadeFlags, ComputedValues}; @@ -39,9 +40,8 @@ use style_traits::viewport::ViewportConstraints; use stylearc::Arc; #[cfg(feature = "gecko")] use stylesheets::{CounterStyleRule, FontFaceRule}; -use stylesheets::{CssRule, DocumentRule, ImportRule, MediaRule, StyleRule, SupportsRule}; +use stylesheets::{CssRule, StyleRule}; use stylesheets::{Stylesheet, Origin, UserAgentStylesheets}; -use stylesheets::NestedRuleIterationCondition; use thread_state; use viewport::{self, MaybeNew, ViewportRule}; @@ -83,6 +83,9 @@ pub struct Stylist { /// Viewport constraints based on the current device. viewport_constraints: Option, + /// Effective media query results cached from the last rebuild. + effective_media_query_results: EffectiveMediaQueryResults, + /// If true, the quirks-mode stylesheet is applied. quirks_mode: QuirksMode, @@ -222,57 +225,6 @@ impl From for RuleInclusion { } } -/// A filter that filters over effective rules, but allowing all potentially -/// effective `@media` rules. -pub struct PotentiallyEffectiveMediaRules; - -impl NestedRuleIterationCondition for PotentiallyEffectiveMediaRules { - fn process_import( - _: &SharedRwLockReadGuard, - _: &Device, - _: QuirksMode, - _: &ImportRule) - -> bool - { - true - } - - fn process_media( - _: &SharedRwLockReadGuard, - _: &Device, - _: QuirksMode, - _: &MediaRule) - -> bool - { - true - } - - /// Whether we should process the nested rules in a given `@-moz-document` rule. - fn process_document( - guard: &SharedRwLockReadGuard, - device: &Device, - quirks_mode: QuirksMode, - rule: &DocumentRule) - -> bool - { - use stylesheets::EffectiveRules; - EffectiveRules::process_document(guard, device, quirks_mode, rule) - } - - /// Whether we should process the nested rules in a given `@supports` rule. - fn process_supports( - guard: &SharedRwLockReadGuard, - device: &Device, - quirks_mode: QuirksMode, - rule: &SupportsRule) - -> bool - { - use stylesheets::EffectiveRules; - EffectiveRules::process_supports(guard, device, quirks_mode, rule) - } -} - - impl Stylist { /// Construct a new `Stylist`, using given `Device` and `QuirksMode`. /// If more members are added here, think about whether they should @@ -285,6 +237,7 @@ impl Stylist { is_device_dirty: true, is_cleared: true, quirks_mode: quirks_mode, + effective_media_query_results: EffectiveMediaQueryResults::new(), element_map: PerPseudoElementSelectorMap::new(), pseudos_map: Default::default(), @@ -355,6 +308,7 @@ impl Stylist { self.is_cleared = true; + self.effective_media_query_results.clear(); self.viewport_constraints = None; // preserve current device self.is_device_dirty = true; @@ -482,6 +436,8 @@ impl Stylist { return; } + self.effective_media_query_results.saw_effective(stylesheet); + for rule in stylesheet.effective_rules(&self.device, guard) { match *rule { CssRule::Style(ref locked) => { @@ -515,10 +471,17 @@ impl Stylist { } self.rules_source_order += 1; } - CssRule::Import(..) => { - // effective_rules visits the inner stylesheet if + CssRule::Import(ref lock) => { + let import_rule = lock.read_with(guard); + self.effective_media_query_results.saw_effective(import_rule); + + // NOTE: effective_rules visits the inner stylesheet if // appropriate. } + CssRule::Media(ref lock) => { + let media_rule = lock.read_with(guard); + self.effective_media_query_results.saw_effective(media_rule); + } CssRule::Keyframes(ref keyframes_rule) => { let keyframes_rule = keyframes_rule.read_with(guard); debug!("Found valid keyframes rule: {:?}", *keyframes_rule); @@ -816,16 +779,50 @@ impl Stylist { device.account_for_viewport_rule(constraints); } - self.is_device_dirty |= stylesheets.iter().any(|stylesheet| { - let mq = stylesheet.media.read_with(guard); - if mq.evaluate(&self.device, self.quirks_mode) != mq.evaluate(&device, self.quirks_mode) { + self.device = device; + let features_changed = self.media_features_change_changed_style( + stylesheets.iter(), + guard + ); + self.is_device_dirty |= features_changed; + } + + /// Returns whether, given a media feature change, any previously-applicable + /// style has become non-applicable, or vice-versa. + pub fn media_features_change_changed_style<'a, I>( + &self, + stylesheets: I, + guard: &SharedRwLockReadGuard, + ) -> bool + where I: Iterator> + { + use invalidation::media_queries::PotentiallyEffectiveMediaRules; + + debug!("Stylist::media_features_change_changed_style"); + + for stylesheet in stylesheets { + let effective_now = + stylesheet.media.read_with(guard) + .evaluate(&self.device, self.quirks_mode); + + let effective_then = + self.effective_media_query_results.was_effective(&**stylesheet); + + if effective_now != effective_then { + debug!(" > Stylesheet changed -> {}, {}", + effective_then, effective_now); return true } + if !effective_now { + continue; + } + let mut iter = stylesheet.iter_rules::( &self.device, - guard); + guard + ); while let Some(rule) = iter.next() { match *rule { @@ -844,8 +841,13 @@ impl Stylist { CssRule::Import(ref lock) => { let import_rule = lock.read_with(guard); let mq = import_rule.stylesheet.media.read_with(guard); - let effective_now = mq.evaluate(&self.device, self.quirks_mode); - if effective_now != mq.evaluate(&device, self.quirks_mode) { + let effective_now = + mq.evaluate(&self.device, self.quirks_mode); + let effective_then = + self.effective_media_query_results.was_effective(import_rule); + if effective_now != effective_then { + debug!(" > @import rule changed {} -> {}", + effective_then, effective_now); return true; } @@ -856,8 +858,13 @@ impl Stylist { CssRule::Media(ref lock) => { let media_rule = lock.read_with(guard); let mq = media_rule.media_queries.read_with(guard); - let effective_now = mq.evaluate(&self.device, self.quirks_mode); - if effective_now != mq.evaluate(&device, self.quirks_mode) { + let effective_now = + mq.evaluate(&self.device, self.quirks_mode); + let effective_then = + self.effective_media_query_results.was_effective(media_rule); + if effective_now != effective_then { + debug!(" > @media rule changed {} -> {}", + effective_then, effective_now); return true; } @@ -867,11 +874,9 @@ impl Stylist { } } } + } - return false; - }); - - self.device = device; + return false; } /// Returns the viewport constraints that apply to this document because of diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 636b6a868a5..9f76b35d92d 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -747,6 +747,24 @@ pub extern "C" fn Servo_StyleSet_AppendStyleSheet(raw_data: RawServoStyleSetBorr data.clear_stylist(); } +#[no_mangle] +pub extern "C" fn Servo_StyleSet_MediumFeaturesChanged( + raw_data: RawServoStyleSetBorrowed, +) -> bool { + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + + // NOTE(emilio): We don't actually need to flush the stylist here and ensure + // it's up to date. + // + // In case it isn't we would trigger a rebuild + restyle as needed too. + let data = PerDocumentStyleData::from_ffi(raw_data).borrow(); + data.stylist.media_features_change_changed_style( + data.stylesheets.iter(), + &guard, + ) +} + #[no_mangle] pub extern "C" fn Servo_StyleSet_PrependStyleSheet(raw_data: RawServoStyleSetBorrowed, raw_sheet: RawServoStyleSheetBorrowed, @@ -1408,6 +1426,7 @@ pub extern "C" fn Servo_StyleSet_Init(pres_context: RawGeckoPresContextOwned) pub extern "C" fn Servo_StyleSet_RebuildData(raw_data: RawServoStyleSetBorrowed) { 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.reset_device(&guard); } From dd3bf6f952bdf6f548f6b04ab442f641b0e024b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 2 Jun 2017 20:34:37 +0200 Subject: [PATCH 3/3] Ensure the default computed values are up-to-date before evaluating media queries. Zoom changes can change the meaning of ems, so we can't re-evaluate media queries with the old values, because otherwise we may miss a restyle. Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1357461 MozReview-Commit-ID: HQInvR7RPqR --- components/style/gecko/data.rs | 10 ---------- components/style/gecko/media_queries.rs | 9 ++++++++- ports/geckolib/glue.rs | 14 ++++++++++++-- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index 6a4b1f8c896..3ca07ff8ae1 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -9,7 +9,6 @@ use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use dom::TElement; use fnv::FnvHashMap; use gecko::rules::{CounterStyleRule, FontFaceRule}; -use gecko::wrapper::GeckoElement; use gecko_bindings::bindings::RawServoStyleSet; use gecko_bindings::structs::RawGeckoPresContextOwned; use gecko_bindings::structs::nsIDocument; @@ -70,15 +69,6 @@ impl PerDocumentStyleData { } impl PerDocumentStyleDataImpl { - /// Reset the device state because it may have changed. - /// - /// Implies also a stylesheet flush. - pub fn reset_device(&mut self, guard: &SharedRwLockReadGuard) { - self.stylist.device_mut().reset(); - self.stylesheets.force_dirty(); - self.flush_stylesheets::(guard, None); - } - /// Recreate the style data if the stylesheets have changed. pub fn flush_stylesheets(&mut self, guard: &SharedRwLockReadGuard, diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 3cfdc7a798c..0b60a502df9 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -99,6 +99,13 @@ impl Device { self.root_font_size.store(size.0 as isize, Ordering::Relaxed) } + /// Recreates the default computed values. + pub fn reset_computed_values(&mut self) { + // NB: A following stylesheet flush will populate this if appropriate. + self.viewport_override = None; + self.default_values = ComputedValues::default_values(unsafe { &*self.pres_context }); + } + /// Recreates all the temporary state that the `Device` stores. /// /// This includes the viewport override from `@viewport` rules, and also the @@ -106,7 +113,7 @@ impl Device { pub fn reset(&mut self) { // NB: A following stylesheet flush will populate this if appropriate. self.viewport_override = None; - self.default_values = ComputedValues::default_values(unsafe { &*self.pres_context }); + self.reset_computed_values(); } /// Returns the current media type of the device. diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 9f76b35d92d..6383a2f3fba 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -758,7 +758,15 @@ pub extern "C" fn Servo_StyleSet_MediumFeaturesChanged( // it's up to date. // // In case it isn't we would trigger a rebuild + restyle as needed too. - let data = PerDocumentStyleData::from_ffi(raw_data).borrow(); + // + // We need to ensure the default computed values are up to date though, + // because those can influence the result of media query evaluation. + // + // FIXME(emilio, bug 1369984): do the computation conditionally, to do it + // less often. + let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); + + data.stylist.device_mut().reset_computed_values(); data.stylist.media_features_change_changed_style( data.stylesheets.iter(), &guard, @@ -1428,7 +1436,9 @@ pub extern "C" fn Servo_StyleSet_RebuildData(raw_data: RawServoStyleSetBorrowed) let guard = global_style_data.shared_lock.read(); let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); - data.reset_device(&guard); + data.stylist.device_mut().reset(); + data.stylesheets.force_dirty(); + data.flush_stylesheets::(&guard, None); } #[no_mangle]