From 92742f7f9098ac5bb2eb0408ca543c7bd69a71a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 29 Dec 2022 00:23:28 +0000 Subject: [PATCH] style: Avoid generic soup and extra reference count bumps This makes me a bit happier about the previous patch :) Differential Revision: https://phabricator.services.mozilla.com/D165236 --- components/selectors/context.rs | 2 +- components/selectors/parser.rs | 4 ++-- components/style/gecko/selector_parser.rs | 7 +++---- components/style/properties/gecko.mako.rs | 13 +++++++++++-- components/style/style_resolver.rs | 4 ++-- components/style/stylesheets/container_rule.rs | 16 ++++++++-------- components/style/stylist.rs | 10 +++++----- 7 files changed, 32 insertions(+), 24 deletions(-) diff --git a/components/selectors/context.rs b/components/selectors/context.rs index f595389d2f2..e29abe639a1 100644 --- a/components/selectors/context.rs +++ b/components/selectors/context.rs @@ -145,7 +145,7 @@ where pub pseudo_element_matching_fn: Option<&'a dyn Fn(&Impl::PseudoElement) -> bool>, /// Extra implementation-dependent matching data. - pub extra_data: Impl::ExtraMatchingData, + pub extra_data: Impl::ExtraMatchingData<'a>, quirks_mode: QuirksMode, needs_selector_flags: NeedsSelectorFlags, diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index ebe49171080..b4e35c31c20 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -202,7 +202,7 @@ macro_rules! with_all_bounds { /// are parameterized on SelectorImpl. See /// pub trait SelectorImpl: Clone + Debug + Sized + 'static { - type ExtraMatchingData: Sized + Default + 'static; + type ExtraMatchingData<'a>: Sized + Default; type AttrValue: $($InSelector)*; type Identifier: $($InSelector)*; type LocalName: $($InSelector)* + Borrow; @@ -2638,7 +2638,7 @@ pub mod tests { } impl SelectorImpl for DummySelectorImpl { - type ExtraMatchingData = (); + type ExtraMatchingData<'a> = std::marker::PhantomData<&'a ()>; type AttrValue = DummyAttrValue; type Identifier = DummyAtom; type LocalName = DummyAtom; diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index b71fe48f33e..5758a09a376 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -18,7 +18,6 @@ use cssparser::{CowRcStr, SourceLocation, ToCss, Token}; use dom::{DocumentState, ElementState}; use selectors::parser::SelectorParseErrorKind; use selectors::SelectorList; -use servo_arc::Arc; use std::fmt; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss as ToCss_}; @@ -236,7 +235,7 @@ pub struct SelectorImpl; /// A set of extra data to carry along with the matching context, either for /// selector-matching or invalidation. #[derive(Default)] -pub struct ExtraMatchingData { +pub struct ExtraMatchingData<'a> { /// The invalidation data to invalidate doc-state pseudo-classes correctly. pub invalidation_data: InvalidationMatchingData, @@ -246,11 +245,11 @@ pub struct ExtraMatchingData { /// The style of the originating element in order to evaluate @container /// size queries affecting pseudo-elements. - pub originating_element_style: Option>, + pub originating_element_style: Option<&'a ComputedValues>, } impl ::selectors::SelectorImpl for SelectorImpl { - type ExtraMatchingData = ExtraMatchingData; + type ExtraMatchingData<'a> = ExtraMatchingData<'a>; type AttrValue = AtomString; type Identifier = AtomIdent; type LocalName = AtomIdent; diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 93efb36f1d6..03301cc07d2 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -103,6 +103,15 @@ impl ComputedValues { ).to_outer(None) } + /// Converts the computed values to an Arc<> from a reference. + pub fn to_arc(&self) -> Arc { + // SAFETY: We're guaranteed to be allocated as an Arc<> since the + // functions above are the only ones that create ComputedValues + // instances in Gecko (and that must be the case since ComputedValues' + // member is private). + unsafe { Arc::from_raw_addrefed(self) } + } + #[inline] pub fn is_pseudo_style(&self) -> bool { self.0.mPseudoType != PseudoStyleType::NotPseudo @@ -208,8 +217,8 @@ impl ComputedValuesInner { &self, pseudo_ty, ); - // We're simulating move semantics by having C++ do a memcpy and then forgetting - // it on this end. + // We're simulating move semantics by having C++ do a memcpy and + // then forgetting it on this end. forget(self); UniqueArc::assume_init(arc).shareable() } diff --git a/components/style/style_resolver.rs b/components/style/style_resolver.rs index e2ad792eb0e..19a5338517d 100644 --- a/components/style/style_resolver.rs +++ b/components/style/style_resolver.rs @@ -507,7 +507,7 @@ where fn match_pseudo( &mut self, - originating_element_style: &Arc, + originating_element_style: &ComputedValues, pseudo_element: &PseudoElement, visited_handling: VisitedHandlingMode, ) -> Option { @@ -544,7 +544,7 @@ where NeedsSelectorFlags::Yes, ); matching_context.extra_data.originating_element_style = - Some(originating_element_style.clone()); + Some(originating_element_style); // NB: We handle animation rules for ::before and ::after when // traversing them. diff --git a/components/style/stylesheets/container_rule.rs b/components/style/stylesheets/container_rule.rs index 9b06066e1ea..3734e73c770 100644 --- a/components/style/stylesheets/container_rule.rs +++ b/components/style/stylesheets/container_rule.rs @@ -135,14 +135,14 @@ enum TraversalResult { Done(T), } -fn traverse_container( +fn traverse_container( mut e: E, - originating_element_style: Option<&S>, + originating_element_style: Option<&ComputedValues>, evaluator: F, ) -> Option<(E, R)> where E: TElement, - F: Fn(E, Option<&S>) -> TraversalResult, + F: Fn(E, Option<&ComputedValues>) -> TraversalResult, { if originating_element_style.is_some() { match evaluator(e, originating_element_style) { @@ -185,7 +185,7 @@ impl ContainerCondition { fn valid_container_info( &self, potential_container: E, - originating_element_style: Option<&Arc>, + originating_element_style: Option<&ComputedValues>, ) -> TraversalResult> where E: TElement, @@ -198,7 +198,7 @@ impl ContainerCondition { Some(d) => d, None => return TraversalResult::InProgress, }; - data.styles.primary() + &**data.styles.primary() }, }; let wm = style.writing_mode; @@ -220,7 +220,7 @@ impl ContainerCondition { } let size = potential_container.query_container_size(&box_style.clone_display()); - let style = style.clone(); + let style = style.to_arc(); TraversalResult::Done(ContainerLookupResult { element: potential_container, info: ContainerInfo { size, wm }, @@ -232,7 +232,7 @@ impl ContainerCondition { pub fn find_container( &self, e: E, - originating_element_style: Option<&Arc>, + originating_element_style: Option<&ComputedValues>, ) -> Option> where E: TElement, @@ -254,7 +254,7 @@ impl ContainerCondition { &self, device: &Device, element: E, - originating_element_style: Option<&Arc>, + originating_element_style: Option<&ComputedValues>, invalidation_flags: &mut ComputedValueFlags, ) -> KleeneValue where diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 68b01ae6538..31adb253687 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -967,7 +967,7 @@ impl Stylist { element: E, pseudo: &PseudoElement, rule_inclusion: RuleInclusion, - originating_element_style: &Arc, + originating_element_style: &ComputedValues, parent_style: &Arc, is_probe: bool, matching_fn: Option<&dyn Fn(&PseudoElement) -> bool>, @@ -1113,7 +1113,7 @@ impl Stylist { &self, guards: &StylesheetGuards, element: E, - originating_element_style: &Arc, + originating_element_style: &ComputedValues, parent_style: &Arc, pseudo: &PseudoElement, is_probe: bool, @@ -1144,7 +1144,7 @@ impl Stylist { matching_context.pseudo_element_matching_fn = matching_fn; matching_context.extra_data.originating_element_style = - Some(originating_element_style.clone()); + Some(originating_element_style); self.push_applicable_declarations( element, @@ -1176,7 +1176,7 @@ impl Stylist { ); matching_context.pseudo_element_matching_fn = matching_fn; matching_context.extra_data.originating_element_style = - Some(originating_element_style.clone()); + Some(originating_element_style); self.push_applicable_declarations( element, @@ -2398,7 +2398,7 @@ impl CascadeData { .matches( stylist.device(), element, - context.extra_data.originating_element_style.as_ref(), + context.extra_data.originating_element_style, &mut context.extra_data.cascade_input_flags, ) .to_bool(/* unknown = */ false);