diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index 3c6e489a797..25beffd9549 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -17,7 +17,7 @@ use dom_struct::dom_struct; use servo_arc::Arc; use servo_url::ServoUrl; use style::attr::AttrValue; -use style::properties::{DeclarationPushMode, Importance, PropertyDeclarationBlock, PropertyId, LonghandId, ShorthandId}; +use style::properties::{Importance, PropertyDeclarationBlock, PropertyId, LonghandId, ShorthandId}; use style::properties::{parse_one_declaration_into, parse_style_attribute, SourcePropertyDeclaration}; use style::selector_parser::PseudoElement; use style::shared_lock::Locked; @@ -285,8 +285,14 @@ impl CSSStyleDeclaration { let quirks_mode = window.Document().quirks_mode(); let mut declarations = SourcePropertyDeclaration::new(); let result = parse_one_declaration_into( - &mut declarations, id, &value, &self.owner.base_url(), - window.css_error_reporter(), ParsingMode::DEFAULT, quirks_mode); + &mut declarations, + id, + &value, + &self.owner.base_url(), + window.css_error_reporter(), + ParsingMode::DEFAULT, + quirks_mode, + ); // Step 6 match result { @@ -297,13 +303,17 @@ impl CSSStyleDeclaration { } } + let mut updates = Default::default(); + *changed = + pdb.prepare_for_update(&declarations, importance, &mut updates); + + if !*changed { + return Ok(()); + } + // Step 7 // Step 8 - *changed = pdb.extend( - declarations.drain(), - importance, - DeclarationPushMode::Update, - ); + pdb.update(declarations.drain(), importance, &mut updates); Ok(()) }) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 7c4812cb963..fd4765c2dff 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -418,10 +418,6 @@ where match combinator { Combinator::NextSibling | Combinator::LaterSibling => element.prev_sibling_element(), Combinator::Child | Combinator::Descendant => { - if element.blocks_ancestor_combinators() { - return None; - } - match element.parent_element() { Some(e) => return Some(e), None => {}, diff --git a/components/selectors/tree.rs b/components/selectors/tree.rs index b1c0d41caa8..ef544cc7e4c 100644 --- a/components/selectors/tree.rs +++ b/components/selectors/tree.rs @@ -129,11 +129,4 @@ pub trait Element: Sized + Clone + Debug { fn ignores_nth_child_selectors(&self) -> bool { false } - - /// Return true if we want to stop lookup ancestor of the current - /// element while matching complex selectors with descendant/child - /// combinator. - fn blocks_ancestor_combinators(&self) -> bool { - false - } } diff --git a/components/servo_arc/lib.rs b/components/servo_arc/lib.rs index e2063c79a9c..3e6cd65e85d 100644 --- a/components/servo_arc/lib.rs +++ b/components/servo_arc/lib.rs @@ -349,15 +349,10 @@ impl Arc { #[inline] pub fn is_unique(&self) -> bool { - // We can use Relaxed here, but the justification is a bit subtle. + // See the extensive discussion in [1] for why this needs to be Acquire. // - // The reason to use Acquire would be to synchronize with other threads - // that are modifying the refcount with Release, i.e. to ensure that - // their writes to memory guarded by this refcount are flushed. However, - // we know that threads only modify the contents of the Arc when they - // observe the refcount to be 1, and no other thread could observe that - // because we're holding one strong reference here. - self.inner().count.load(Relaxed) == 1 + // [1] https://github.com/servo/servo/issues/21186 + self.inner().count.load(Acquire) == 1 } } diff --git a/components/style/animation.rs b/components/style/animation.rs index d090ecb1ecb..2d421ce06a9 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -9,7 +9,7 @@ use bezier::Bezier; use context::SharedStyleContext; use dom::{OpaqueNode, TElement}; use font_metrics::FontMetricsProvider; -use properties::{self, CascadeFlags, ComputedValues, LonghandId}; +use properties::{self, CascadeMode, ComputedValues, LonghandId}; use properties::animated_properties::AnimatedProperty; use properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection; use properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState; @@ -504,9 +504,8 @@ where Some(previous_style), Some(previous_style), Some(previous_style), - /* visited_style = */ None, font_metrics_provider, - CascadeFlags::empty(), + CascadeMode::Unvisited { visited_rules: None }, context.quirks_mode(), /* rule_cache = */ None, &mut Default::default(), diff --git a/components/style/counter_style/update_predefined.py b/components/style/counter_style/update_predefined.py index 3e5be33e448..0185a8f8b42 100755 --- a/components/style/counter_style/update_predefined.py +++ b/components/style/counter_style/update_predefined.py @@ -28,5 +28,6 @@ predefined! { f.write(' "%s",\n' % name) f.write('}\n') + if __name__ == "__main__": main(os.path.join(os.path.dirname(__file__), "predefined.rs")) diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index 1e97b72ff6c..df1b873a1b7 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -10,7 +10,7 @@ use cssparser::ToCss; use gecko_bindings::structs::{self, CSSPseudoElementType}; -use properties::{CascadeFlags, ComputedValues, PropertyFlags}; +use properties::{ComputedValues, PropertyFlags}; use properties::longhands::display::computed_value::T as Display; use selector_parser::{NonTSPseudoClass, PseudoElementCascadeType, SelectorImpl}; use std::fmt; @@ -55,14 +55,12 @@ impl PseudoElement { PseudoElementCascadeType::Lazy } - /// The CascadeFlags needed to cascade this pseudo-element. + /// Whether cascading this pseudo-element makes it inherit all properties, + /// even reset ones. /// - /// This is only needed to support the broken INHERIT_ALL pseudo mode for - /// Servo. + /// This is used in Servo for anonymous boxes, though it's likely broken. #[inline] - pub fn cascade_flags(&self) -> CascadeFlags { - CascadeFlags::empty() - } + pub fn inherits_all(&self) -> bool { false } /// Whether the pseudo-element should inherit from the default computed /// values instead of from the parent element. diff --git a/components/style/gecko/regen_atoms.py b/components/style/gecko/regen_atoms.py index 36661215082..496c7be720a 100755 --- a/components/style/gecko/regen_atoms.py +++ b/components/style/gecko/regen_atoms.py @@ -21,7 +21,7 @@ PRELUDE = """ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ /* Autogenerated file created by components/style/gecko/binding_tools/regen_atoms.py, DO NOT EDIT DIRECTLY */ -"""[1:] +"""[1:] # NOQA: E501 def gnu_symbolify(source, ident): @@ -57,7 +57,7 @@ class CSSPseudoElementsAtomSource: class CSSAnonBoxesAtomSource: - PATTERN = re.compile('^(CSS_ANON_BOX|CSS_NON_INHERITING_ANON_BOX|CSS_WRAPPER_ANON_BOX)\(([^,]*),[^"]*"([^"]*)"\)', + PATTERN = re.compile('^(CSS_ANON_BOX|CSS_NON_INHERITING_ANON_BOX|CSS_WRAPPER_ANON_BOX)\(([^,]*),[^"]*"([^"]*)"\)', # NOQA: E501 re.MULTILINE) FILE = "include/nsCSSAnonBoxList.h" CLASS = "nsCSSAnonBoxes" diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index ee0c134b269..6cdcf1acd1e 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -856,13 +856,12 @@ impl<'le> GeckoElement<'le> { /// Returns true if this node is the shadow root of an use-element shadow tree. #[inline] fn is_root_of_use_element_shadow_tree(&self) -> bool { - if !self.is_root_of_anonymous_subtree() { + if !self.as_node().is_in_shadow_tree() { return false; } - match self.parent_element() { + match self.containing_shadow_host() { Some(e) => { - e.local_name() == &*local_name!("use") && - e.is_svg_element() + e.is_svg_element() && e.local_name() == &*local_name!("use") }, None => false, } @@ -882,6 +881,7 @@ impl<'le> GeckoElement<'le> { .expect("AnimationValue not found in ElementTransitions"); let property = end_value.id(); + debug_assert!(!property.is_logical()); map.insert(property, end_value.clone_arc()); } map @@ -896,6 +896,7 @@ impl<'le> GeckoElement<'le> { existing_transitions: &FnvHashMap>, ) -> bool { use values::animated::{Animate, Procedure}; + debug_assert!(!longhand_id.is_logical()); // If there is an existing transition, update only if the end value // differs. @@ -1587,25 +1588,24 @@ impl<'le> TElement for GeckoElement<'le> { fn might_need_transitions_update( &self, - old_values: Option<&ComputedValues>, - new_values: &ComputedValues, + old_style: Option<&ComputedValues>, + new_style: &ComputedValues, ) -> bool { - use properties::longhands::display::computed_value::T as Display; - - let old_values = match old_values { + let old_style = match old_style { Some(v) => v, None => return false, }; - let new_box_style = new_values.get_box(); - let transition_not_running = !self.has_css_transitions() && - new_box_style.transition_property_count() == 1 && - new_box_style.transition_combined_duration_at(0) <= 0.0f32; - let new_display_style = new_box_style.clone_display(); - let old_display_style = old_values.get_box().clone_display(); + let new_box_style = new_style.get_box(); + if !self.has_css_transitions() && !new_box_style.specifies_transitions() { + return false; + } - new_box_style.transition_property_count() > 0 && !transition_not_running && - (new_display_style != Display::None && old_display_style != Display::None) + if new_box_style.clone_display().is_none() || old_style.clone_display().is_none() { + return false; + } + + return true; } // Detect if there are any changes that require us to update transitions. @@ -1657,6 +1657,8 @@ impl<'le> TElement for GeckoElement<'le> { let transition_property: TransitionProperty = property.into(); let mut property_check_helper = |property: LonghandId| -> bool { + let property = + property.to_physical(after_change_style.writing_mode); transitions_to_keep.insert(property); self.needs_transitions_update_per_property( property, @@ -2329,14 +2331,6 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { fn ignores_nth_child_selectors(&self) -> bool { self.is_root_of_anonymous_subtree() } - - #[inline] - fn blocks_ancestor_combinators(&self) -> bool { - // If this element is the shadow root of an use-element shadow tree, - // according to the spec, we should not match rules cross the shadow - // DOM boundary. - self.is_root_of_use_element_shadow_tree() - } } /// A few helpers to help with attribute selectors and snapshotting. diff --git a/components/style/invalidation/element/element_wrapper.rs b/components/style/invalidation/element/element_wrapper.rs index 2a31153bd7a..ecbc3b94b78 100644 --- a/components/style/invalidation/element/element_wrapper.rs +++ b/components/style/invalidation/element/element_wrapper.rs @@ -367,8 +367,4 @@ where .assigned_slot() .map(|e| ElementWrapper::new(e, self.snapshot_map)) } - - fn blocks_ancestor_combinators(&self) -> bool { - self.element.blocks_ancestor_combinators() - } } diff --git a/components/style/matching.rs b/components/style/matching.rs index 05630b07fb7..6f239d361d8 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -233,18 +233,23 @@ trait PrivateMatchMethods: TElement { fn needs_animations_update( &self, context: &mut StyleContext, - old_values: Option<&ComputedValues>, - new_values: &ComputedValues, + old_style: Option<&ComputedValues>, + new_style: &ComputedValues, ) -> bool { - let new_box_style = new_values.get_box(); - let has_new_animation_style = new_box_style.specifies_animations(); + let new_box_style = new_style.get_box(); + let new_style_specifies_animations = new_box_style.specifies_animations(); - let old = match old_values { + let old_style = match old_style { Some(old) => old, - None => return has_new_animation_style, + None => return new_style_specifies_animations, }; - let old_box_style = old.get_box(); + let has_animations = self.has_css_animations(); + if !new_style_specifies_animations && !has_animations { + return false; + } + + let old_box_style = old_style.get_box(); let keyframes_could_have_changed = context .shared @@ -258,7 +263,7 @@ trait PrivateMatchMethods: TElement { // // TODO: We should check which @keyframes were added/changed/deleted and // update only animations corresponding to those @keyframes. - if keyframes_could_have_changed && (has_new_animation_style || self.has_css_animations()) { + if keyframes_could_have_changed { return true; } @@ -272,19 +277,27 @@ trait PrivateMatchMethods: TElement { // If we were display: none, we may need to trigger animations. if old_display == Display::None && new_display != Display::None { - return has_new_animation_style; + return new_style_specifies_animations; } // If we are becoming display: none, we may need to stop animations. if old_display != Display::None && new_display == Display::None { - return self.has_css_animations(); + return has_animations; + } + + // We might need to update animations if writing-mode or direction + // changed, and any of the animations contained logical properties. + // + // We may want to be more granular, but it's probably not worth it. + if new_style.writing_mode != old_style.writing_mode { + return has_animations; } false } - /// Create a SequentialTask for resolving descendants in a SMIL display property - /// animation if the display property changed from none. + /// Create a SequentialTask for resolving descendants in a SMIL display + /// property animation if the display property changed from none. #[cfg(feature = "gecko")] fn handle_display_change_for_smil_if_needed( &self, diff --git a/components/style/properties/build.py b/components/style/properties/build.py index 4f5da29246b..dd6b0d8d1bb 100644 --- a/components/style/properties/build.py +++ b/components/style/properties/build.py @@ -49,7 +49,8 @@ STYLE_STRUCT_LIST = [ def main(): - usage = "Usage: %s [ servo | gecko ] [ style-crate | geckolib