From 1b8e6e65ca9df1c0f58ab2e7a042a7c29fa8316b Mon Sep 17 00:00:00 2001 From: Emily McDonough Date: Fri, 21 Apr 2023 18:54:30 +0000 Subject: [PATCH] Bug 1828534 - Separate page style getter in ServoStyleSet r=emilio Servo_ComputedValues_GetForAnonymousBox currently takes an extra argument for page-name values which is only used for page content. As more CSS page3 support is added, the methods of specifying page content will need to grow (notably for pseudo classes), and the implementation currently has a mostly separate code path for page-content. We can separate these out to simplify both the caller and the implementation. Differential Revision: https://phabricator.services.mozilla.com/D175708 --- layout/style/ServoStyleSet.cpp | 65 ++++++++++++++--------- layout/style/ServoStyleSet.h | 17 +++--- servo/ports/geckolib/glue.rs | 94 +++++++++++++++++++++------------- 3 files changed, 103 insertions(+), 73 deletions(-) diff --git a/layout/style/ServoStyleSet.cpp b/layout/style/ServoStyleSet.cpp index 989cde0b0153..5e050586c313 100644 --- a/layout/style/ServoStyleSet.cpp +++ b/layout/style/ServoStyleSet.cpp @@ -527,7 +527,7 @@ ServoStyleSet::ResolveInheritingAnonymousBoxStyle(PseudoStyleType aType, if (!style) { style = Servo_ComputedValues_GetForAnonymousBox(aParentStyle, aType, - mRawSet.get(), nullptr) + mRawSet.get()) .Consume(); MOZ_ASSERT(style); if (aParentStyle) { @@ -539,29 +539,17 @@ ServoStyleSet::ResolveInheritingAnonymousBoxStyle(PseudoStyleType aType, } already_AddRefed -ServoStyleSet::ResolveNonInheritingAnonymousBoxStyle(PseudoStyleType aType, - const nsAtom* aPageName) { +ServoStyleSet::ResolveNonInheritingAnonymousBoxStyle(PseudoStyleType aType) { + MOZ_ASSERT(aType != PseudoStyleType::pageContent, + "Use ResolvePageContentStyle for page content"); MOZ_ASSERT(PseudoStyle::IsNonInheritingAnonBox(aType)); - MOZ_ASSERT(!aPageName || aType == PseudoStyleType::pageContent, - "page name should only be specified for pageContent"); + nsCSSAnonBoxes::NonInheriting type = nsCSSAnonBoxes::NonInheritingTypeForPseudoType(aType); - - // The empty atom is used to indicate no specified page name, and is not - // usable as a page-rule selector. Changing this to null is a slight - // optimization to avoid the Servo code from doing an unnecessary hashtable - // lookup, and still use the style cache in this case. - if (aPageName == nsGkAtoms::_empty) { - aPageName = nullptr; - } - // Only use the cache if we are not doing a lookup for a named page style. - RefPtr* cache = nullptr; - if (!aPageName) { - cache = &mNonInheritingComputedStyles[type]; - if (*cache) { - RefPtr retval = *cache; - return retval.forget(); - } + RefPtr& cache = mNonInheritingComputedStyles[type]; + if (cache) { + RefPtr retval = cache; + return retval.forget(); } UpdateStylistIfNeeded(); @@ -574,13 +562,40 @@ ServoStyleSet::ResolveNonInheritingAnonymousBoxStyle(PseudoStyleType aType, "viewport needs fixup to handle blockifying it"); RefPtr computedValues = - Servo_ComputedValues_GetForAnonymousBox(nullptr, aType, mRawSet.get(), - aPageName) + Servo_ComputedValues_GetForAnonymousBox(nullptr, aType, mRawSet.get()) .Consume(); MOZ_ASSERT(computedValues); - if (cache) { - *cache = computedValues; + cache = computedValues; + return computedValues.forget(); +} + +already_AddRefed ServoStyleSet::ResolvePageContentStyle( + const nsAtom* aPageName) { + // The empty atom is used to indicate no specified page name, and is not + // usable as a page-rule selector. Changing this to null is a slight + // optimization to avoid the Servo code from doing an unnecessary hashtable + // lookup, and still use the style cache in this case. + if (aPageName == nsGkAtoms::_empty) { + aPageName = nullptr; + } + // Only use the cache if we are not doing a lookup for a named page style. + RefPtr& cache = + mNonInheritingComputedStyles[nsCSSAnonBoxes::NonInheriting::pageContent]; + if (!aPageName && cache) { + RefPtr retval = cache; + return retval.forget(); + } + + UpdateStylistIfNeeded(); + + RefPtr computedValues = + Servo_ComputedValues_GetForPageContent(mRawSet.get(), aPageName) + .Consume(); + MOZ_ASSERT(computedValues); + + if (!aPageName) { + cache = computedValues; } return computedValues.forget(); } diff --git a/layout/style/ServoStyleSet.h b/layout/style/ServoStyleSet.h index d23b628420b1..d3850522ded4 100644 --- a/layout/style/ServoStyleSet.h +++ b/layout/style/ServoStyleSet.h @@ -248,17 +248,15 @@ class ServoStyleSet { PseudoStyleType, ComputedStyle* aParentStyle); // Get a ComputedStyle for an anonymous box. The pseudo type must be - // a non-inheriting anon box. + // a non-inheriting anon box, and must not be page-content. + // See ResolvePageContentStyle for resolving page-content style. already_AddRefed ResolveNonInheritingAnonymousBoxStyle( - PseudoStyleType aType) { - return ResolveNonInheritingAnonymousBoxStyle(aType, nullptr); - } + PseudoStyleType aType); + // Get a ComputedStyle for a pageContent box with the specified page-name. + // A page name that is null or the empty atom gets the global page style. already_AddRefed ResolvePageContentStyle( - const nsAtom* aPageName) { - return ResolveNonInheritingAnonymousBoxStyle(PseudoStyleType::pageContent, - aPageName); - } + const nsAtom* aPageName); already_AddRefed ResolveXULTreePseudoStyle( dom::Element* aParentElement, nsCSSAnonBoxPseudoStaticAtom* aPseudoTag, @@ -644,9 +642,6 @@ class ServoStyleSet { nsCSSAnonBoxes::NonInheriting::_Count, RefPtr> mNonInheritingComputedStyles; - already_AddRefed ResolveNonInheritingAnonymousBoxStyle( - PseudoStyleType aType, const nsAtom* aPageName); - public: void PutCachedAnonymousContentStyles( AnonymousContentKey aKey, nsTArray>&& aStyles) { diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index c807e3744f9c..ac086ddff1ab 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -3910,9 +3910,7 @@ counter_style_descriptors! { } #[no_mangle] -pub unsafe extern "C" fn Servo_ComputedValues_GetForAnonymousBox( - parent_style_or_null: Option<&ComputedValues>, - pseudo: PseudoStyleType, +pub unsafe extern "C" fn Servo_ComputedValues_GetForPageContent( raw_data: &RawServoStyleSet, page_name: *const nsAtom, ) -> Strong { @@ -3920,47 +3918,69 @@ pub unsafe extern "C" fn Servo_ComputedValues_GetForAnonymousBox( let guard = global_style_data.shared_lock.read(); let guards = StylesheetGuards::same(&guard); let data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); - let pseudo = PseudoElement::from_pseudo_type(pseudo).unwrap(); - debug_assert!(pseudo.is_anon_box()); - // If the pseudo element is PageContent, we should append @page rules to the - // precomputed pseudo. let mut extra_declarations = vec![]; - if pseudo == PseudoElement::PageContent { - let iter = data.stylist.iter_extra_data_origins_rev(); - for (data, origin) in iter { - let level = match origin { - Origin::UserAgent => CascadeLevel::UANormal, - Origin::User => CascadeLevel::UserNormal, - Origin::Author => CascadeLevel::same_tree_author_normal(), - }; - extra_declarations.reserve(data.pages.global.len()); - let mut add_rule = |rule: &Arc>| { - extra_declarations.push(ApplicableDeclarationBlock::from_declarations( - rule.read_with(level.guard(&guards)).block.clone(), - level, - LayerOrder::root(), - )); - }; - for &(ref rule, _layer_id) in data.pages.global.iter() { - add_rule(&rule.0); - } - if !page_name.is_null() { - Atom::with(page_name, |name| { - if let Some(rules) = data.pages.named.get(name) { - // Rules are already sorted by source order. - rules.iter().for_each(|d| add_rule(&d.rule)); - } - }); - } + let iter = data.stylist.iter_extra_data_origins_rev(); + for (data, origin) in iter { + let level = match origin { + Origin::UserAgent => CascadeLevel::UANormal, + Origin::User => CascadeLevel::UserNormal, + Origin::Author => CascadeLevel::same_tree_author_normal(), + }; + extra_declarations.reserve(data.pages.global.len()); + let mut add_rule = |rule: &Arc>| { + extra_declarations.push(ApplicableDeclarationBlock::from_declarations( + rule.read_with(level.guard(&guards)).block.clone(), + level, + LayerOrder::root(), + )); + }; + for &(ref rule, _layer_id) in data.pages.global.iter() { + add_rule(&rule.0); + } + if !page_name.is_null() { + Atom::with(page_name, |name| { + if let Some(rules) = data.pages.named.get(name) { + // Rules are already sorted by source order. + rules.iter().for_each(|d| add_rule(&d.rule)); + } + }); } - } else { - debug_assert!(page_name.is_null()); } let rule_node = data.stylist - .rule_node_for_precomputed_pseudo(&guards, &pseudo, extra_declarations); + .rule_node_for_precomputed_pseudo( + &guards, + &PseudoElement::PageContent, + extra_declarations); + + data.stylist + .precomputed_values_for_pseudo_with_rule_node::( + &guards, + &PseudoElement::PageContent, + None, + rule_node, + ) + .into() +} + +#[no_mangle] +pub unsafe extern "C" fn Servo_ComputedValues_GetForAnonymousBox( + parent_style_or_null: Option<&ComputedValues>, + pseudo: PseudoStyleType, + raw_data: &RawServoStyleSet, +) -> Strong { + let pseudo = PseudoElement::from_pseudo_type(pseudo).unwrap(); + debug_assert!(pseudo.is_anon_box()); + debug_assert_ne!(pseudo, PseudoElement::PageContent); + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + let guards = StylesheetGuards::same(&guard); + let data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); + let rule_node = + data.stylist + .rule_node_for_precomputed_pseudo(&guards, &pseudo, vec![]); data.stylist .precomputed_values_for_pseudo_with_rule_node::(