From a3013fe0ae0b1153c387a3b1b09fba59f7d9854e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 22 Jul 2025 21:50:48 +0000 Subject: [PATCH] Bug 1978217 - Record use counter data inline in StylesheetContents. r=firefox-style-system-reviewers,layout-reviewers,dshin, a=dsmith No behavior change, but this is simpler. I want to reuse the use counters mechanism to fix this bug because it's a very trivial way of asking questions about the parsed data and we need to plumb it through the same places. Differential Revision: https://phabricator.services.mozilla.com/D258290 --- dom/base/nsTreeSanitizer.cpp | 2 +- layout/style/GeckoBindings.cpp | 21 +++---- layout/style/GeckoBindings.h | 3 +- layout/style/Loader.cpp | 23 +------- layout/style/StyleSheet.cpp | 57 +++++++++---------- layout/style/StyleSheet.h | 8 +-- layout/style/StyleSheetInfo.h | 3 - layout/style/test/gtest/StyloParsingBench.cpp | 14 ++--- .../style/stylesheets/stylesheet.rs | 15 +++-- servo/components/style/use_counters/mod.rs | 6 +- servo/ports/geckolib/glue.rs | 10 ++-- servo/ports/geckolib/stylesheet_loader.rs | 12 ---- 12 files changed, 66 insertions(+), 108 deletions(-) diff --git a/dom/base/nsTreeSanitizer.cpp b/dom/base/nsTreeSanitizer.cpp index 2d531f7b9c2b..1bf6378b0f00 100644 --- a/dom/base/nsTreeSanitizer.cpp +++ b/dom/base/nsTreeSanitizer.cpp @@ -1126,7 +1126,7 @@ static void SanitizeStyleSheet(const nsAString& aOriginal, css::SheetParsingMode::eAuthorSheetFeatures, extraData.get(), aDocument->GetCompatibilityMode(), /* reusable_sheets = */ nullptr, - /* use_counters = */ nullptr, StyleAllowImportRules::Yes, + StyleAllowImportRules::Yes, aSanitizationKind, &aSanitized) .Consume(); } diff --git a/layout/style/GeckoBindings.cpp b/layout/style/GeckoBindings.cpp index 53f26981b2c3..cd6b4fefb825 100644 --- a/layout/style/GeckoBindings.cpp +++ b/layout/style/GeckoBindings.cpp @@ -1338,21 +1338,18 @@ NS_IMPL_THREADSAFE_FFI_REFCOUNTING(SheetLoadDataHolder, SheetLoadDataHolder); void Gecko_StyleSheet_FinishAsyncParse( SheetLoadDataHolder* aData, - StyleStrong aSheetContents, - StyleUseCounters* aUseCounters) { - UniquePtr useCounters(aUseCounters); + StyleStrong aSheetContents) { RefPtr loadData = aData; RefPtr sheetContents = aSheetContents.Consume(); NS_DispatchToMainThreadQueue( - NS_NewRunnableFunction( - __func__, - [d = std::move(loadData), contents = std::move(sheetContents), - counters = std::move(useCounters)]() mutable { - MOZ_ASSERT(NS_IsMainThread()); - SheetLoadData* data = d->get(); - data->mSheet->FinishAsyncParse(contents.forget(), - std::move(counters)); - }), + NS_NewRunnableFunction(__func__, + [d = std::move(loadData), + contents = std::move(sheetContents)]() mutable { + MOZ_ASSERT(NS_IsMainThread()); + SheetLoadData* data = d->get(); + data->mSheet->FinishAsyncParse( + contents.forget()); + }), EventQueuePriority::RenderBlocking); } diff --git a/layout/style/GeckoBindings.h b/layout/style/GeckoBindings.h index 4cdceba041dd..3acd90628498 100644 --- a/layout/style/GeckoBindings.h +++ b/layout/style/GeckoBindings.h @@ -120,8 +120,7 @@ NS_DECL_THREADSAFE_FFI_REFCOUNTING(mozilla::css::SheetLoadDataHolder, void Gecko_StyleSheet_FinishAsyncParse( mozilla::css::SheetLoadDataHolder* data, - mozilla::StyleStrong sheet_contents, - mozilla::StyleUseCounters* use_counters); + mozilla::StyleStrong sheet_contents); mozilla::StyleSheet* Gecko_LoadStyleSheet( mozilla::css::Loader* loader, mozilla::StyleSheet* parent, diff --git a/layout/style/Loader.cpp b/layout/style/Loader.cpp index f12fb0a7e3c3..993eba086505 100644 --- a/layout/style/Loader.cpp +++ b/layout/style/Loader.cpp @@ -863,25 +863,6 @@ nsresult Loader::CheckContentPolicy(nsIPrincipal* aLoadingPrincipal, return NS_OK; } -static void RecordUseCountersIfNeeded(Document* aDoc, - const StyleSheet& aSheet) { - if (!aDoc) { - return; - } - const StyleUseCounters* docCounters = aDoc->GetStyleUseCounters(); - if (!docCounters) { - return; - } - if (aSheet.URLData()->ChromeRulesEnabled()) { - return; - } - const auto* sheetCounters = aSheet.GetStyleUseCounters(); - if (!sheetCounters) { - return; - } - Servo_UseCounters_Merge(docCounters, sheetCounters); -} - bool Loader::MaybePutIntoLoadsPerformed(SheetLoadData& aLoadData) { if (!aLoadData.mURI) { // Inline style sheet is not tracked. @@ -1574,7 +1555,7 @@ void Loader::AddPerformanceEntryForCachedSheet(SheetLoadData& aLoadData) { } void Loader::NotifyObservers(SheetLoadData& aData, nsresult aStatus) { - RecordUseCountersIfNeeded(mDocument, *aData.mSheet); + aData.mSheet->PropagateUseCountersTo(mDocument); if (MaybePutIntoLoadsPerformed(aData) && aData.mShouldEmulateNotificationsForCachedLoad) { NotifyObserversForCachedSheet(aData); @@ -2065,7 +2046,7 @@ nsresult Loader::LoadChildSheet(StyleSheet& aParentSheet, if (!isReusableSheet) { // Child sheets are not handled by NotifyObservers, and these need to be // performed here if the sheet comes from the SharedStyleSheetCache. - RecordUseCountersIfNeeded(mDocument, *data->mSheet); + data->mSheet->PropagateUseCountersTo(mDocument); if (MaybePutIntoLoadsPerformed(*data)) { NotifyObserversForCachedSheet(*data); AddPerformanceEntryForCachedSheet(*data); diff --git a/layout/style/StyleSheet.cpp b/layout/style/StyleSheet.cpp index ec600cfe6e0d..7b8ee0057716 100644 --- a/layout/style/StyleSheet.cpp +++ b/layout/style/StyleSheet.cpp @@ -522,7 +522,7 @@ void StyleSheet::DropStyleSet(ServoStyleSet* aStyleSet) { do { \ StyleSheet* current = this; \ do { \ - for (ServoStyleSet * set : current->mStyleSets) { \ + for (ServoStyleSet* set : current->mStyleSets) { \ set->function_ args_; \ } \ if (auto* docOrShadow = current->mDocumentOrShadowRoot) { \ @@ -777,14 +777,14 @@ void StyleSheet::ReplaceSync(const nsACString& aText, ErrorResult& aRv) { loader, this, /* load_data = */ nullptr, &aText, mParsingMode, URLData(), mConstructorDocument->GetCompatibilityMode(), - /* reusable_sheets = */ nullptr, - mConstructorDocument->GetStyleUseCounters(), - StyleAllowImportRules::No, StyleSanitizationKind::None, + /* reusable_sheets = */ nullptr, StyleAllowImportRules::No, + StyleSanitizationKind::None, /* sanitized_output = */ nullptr) .Consume(); // 5. Set sheet's rules to the new rules. Inner().mContents = std::move(rawContent); + PropagateUseCountersTo(mConstructorDocument); FixUpRuleListAfterContentsChangeIfNeeded(); RuleChanged(nullptr, StyleRuleChangeKind::Generic); } @@ -1225,45 +1225,48 @@ RefPtr StyleSheet::ParseSheet( ? StyleAllowImportRules::No : StyleAllowImportRules::Yes; URLExtraData* urlData = URLData(); - const bool shouldRecordCounters = - aLoader.GetDocument() && aLoader.GetDocument()->GetStyleUseCounters() && - !urlData->ChromeRulesEnabled(); - if (aLoadData->get()->mRecordErrors) { MOZ_ASSERT(NS_IsMainThread()); - UniquePtr counters; - if (shouldRecordCounters) { - counters.reset(Servo_UseCounters_Create()); - } RefPtr contents = Servo_StyleSheet_FromUTF8Bytes( &aLoader, this, aLoadData->get(), &aBytes, mParsingMode, urlData, aLoadData->get()->mCompatMode, - /* reusable_sheets = */ nullptr, counters.get(), allowImportRules, + /* reusable_sheets = */ nullptr, allowImportRules, StyleSanitizationKind::None, /* sanitized_output = */ nullptr) .Consume(); - FinishAsyncParse(contents.forget(), std::move(counters)); + FinishAsyncParse(contents.forget()); } else { Servo_StyleSheet_FromUTF8BytesAsync( aLoadData, urlData, &aBytes, mParsingMode, - aLoadData->get()->mCompatMode, shouldRecordCounters, allowImportRules); + aLoadData->get()->mCompatMode, allowImportRules); } return p; } void StyleSheet::FinishAsyncParse( - already_AddRefed aSheetContents, - UniquePtr aUseCounters) { + already_AddRefed aSheetContents) { MOZ_ASSERT(NS_IsMainThread()); MOZ_ASSERT(!mParsePromise.IsEmpty()); Inner().mContents = aSheetContents; - Inner().mUseCounters = std::move(aUseCounters); FixUpRuleListAfterContentsChangeIfNeeded(); UnblockParsePromise(); } +const StyleUseCounters* StyleSheet::UseCounters() const { + return Servo_StyleSheet_UseCounters(RawContents()); +} + +void StyleSheet::PropagateUseCountersTo(Document* aDoc) const { + if (!aDoc || URLData()->ChromeRulesEnabled()) { + return; + } + if (auto* counters = aDoc->GetStyleUseCounters()) { + Servo_UseCounters_Merge(counters, UseCounters()); + } +} + void StyleSheet::ParseSheetSync( css::Loader* aLoader, const nsACString& aBytes, css::SheetLoadData* aLoadData, @@ -1281,21 +1284,17 @@ void StyleSheet::ParseSheetSync( SetURLExtraData(); URLExtraData* urlData = URLData(); - const StyleUseCounters* useCounters = - aLoader && aLoader->GetDocument() && !urlData->ChromeRulesEnabled() - ? aLoader->GetDocument()->GetStyleUseCounters() - : nullptr; - auto allowImportRules = SelfOrAncestorIsConstructed() ? StyleAllowImportRules::No : StyleAllowImportRules::Yes; - Inner().mContents = Servo_StyleSheet_FromUTF8Bytes( - aLoader, this, aLoadData, &aBytes, mParsingMode, - urlData, compatMode, aReusableSheets, useCounters, - allowImportRules, StyleSanitizationKind::None, - /* sanitized_output = */ nullptr) - .Consume(); + Inner().mContents = + Servo_StyleSheet_FromUTF8Bytes( + aLoader, this, aLoadData, &aBytes, mParsingMode, urlData, compatMode, + aReusableSheets, allowImportRules, StyleSanitizationKind::None, + /* sanitized_output = */ nullptr) + .Consume(); + PropagateUseCountersTo(aLoader ? aLoader->GetDocument() : nullptr); } void StyleSheet::ReparseSheet(const nsACString& aInput, ErrorResult& aRv) { diff --git a/layout/style/StyleSheet.h b/layout/style/StyleSheet.h index 588904c6943b..12d1a8cc2300 100644 --- a/layout/style/StyleSheet.h +++ b/layout/style/StyleSheet.h @@ -137,8 +137,7 @@ class StyleSheet final : public nsICSSLoaderObserver, public nsWrapperCache { // Common code that needs to be called after servo finishes parsing. This is // shared between the parallel and sequential paths. - void FinishAsyncParse(already_AddRefed, - UniquePtr); + void FinishAsyncParse(already_AddRefed); // Similar to `ParseSheet`, but guarantees that // parsing will be performed synchronously. @@ -157,9 +156,8 @@ class StyleSheet final : public nsICSSLoaderObserver, public nsWrapperCache { return Inner().mContents; } - const StyleUseCounters* GetStyleUseCounters() const { - return Inner().mUseCounters.get(); - } + const StyleUseCounters* UseCounters() const; + void PropagateUseCountersTo(dom::Document*) const; URLExtraData* URLData() const { return Inner().mURLData; } diff --git a/layout/style/StyleSheetInfo.h b/layout/style/StyleSheetInfo.h index f7ba20b2b97c..8ddb35dc5048 100644 --- a/layout/style/StyleSheetInfo.h +++ b/layout/style/StyleSheetInfo.h @@ -18,7 +18,6 @@ class nsIURI; namespace mozilla { class StyleSheet; -struct StyleUseCounters; struct StyleStylesheetContents; struct URLExtraData; @@ -71,8 +70,6 @@ struct StyleSheetInfo final { RefPtr mContents; - UniquePtr mUseCounters; - // XXX We already have mSheetURI, mBaseURI, and mPrincipal. // // Can we somehow replace them with URLExtraData directly? The issue diff --git a/layout/style/test/gtest/StyloParsingBench.cpp b/layout/style/test/gtest/StyloParsingBench.cpp index 4c14ebed7c13..2986c0c113f0 100644 --- a/layout/style/test/gtest/StyloParsingBench.cpp +++ b/layout/style/test/gtest/StyloParsingBench.cpp @@ -9,7 +9,6 @@ #include "nsString.h" #include "ExampleStylesheet.h" #include "ServoBindings.h" -#include "mozilla/dom/DOMString.h" #include "mozilla/Encoding.h" #include "mozilla/Utf8.h" #include "mozilla/NullPrincipal.h" @@ -30,7 +29,7 @@ using namespace mozilla::net; # define SETPROPERTY_REPETITIONS (1000 * 1000) # define GETPROPERTY_REPETITIONS (1000 * 1000) -static void ServoParsingBench(const StyleUseCounters* aCounters) { +static void ServoParsingBench() { auto css = AsBytes(MakeStringSpan(EXAMPLE_STYLESHEET)); nsCString cssStr; cssStr.Append(css); @@ -45,8 +44,8 @@ static void ServoParsingBench(const StyleUseCounters* aCounters) { RefPtr stylesheet = Servo_StyleSheet_FromUTF8Bytes( nullptr, nullptr, nullptr, &cssStr, eAuthorSheetFeatures, data, - eCompatibility_FullStandards, nullptr, aCounters, - StyleAllowImportRules::Yes, StyleSanitizationKind::None, nullptr) + eCompatibility_FullStandards, nullptr, StyleAllowImportRules::Yes, + StyleSanitizationKind::None, nullptr) .Consume(); } } @@ -96,12 +95,7 @@ static void ServoGetPropertyValueById() { } MOZ_GTEST_BENCH(Stylo, Servo_StyleSheet_FromUTF8Bytes_Bench, - [] { ServoParsingBench(nullptr); }); - -MOZ_GTEST_BENCH(Stylo, Servo_StyleSheet_FromUTF8Bytes_Bench_UseCounters, [] { - UniquePtr counters(Servo_UseCounters_Create()); - ServoParsingBench(counters.get()); -}); + [] { ServoParsingBench(); }); MOZ_GTEST_BENCH(Stylo, Servo_DeclarationBlock_SetPropertyById_Bench, [] { ServoSetPropertyByIdBench("10px"_ns); }); diff --git a/servo/components/style/stylesheets/stylesheet.rs b/servo/components/style/stylesheets/stylesheet.rs index 64d78fae4618..48671c8546bc 100644 --- a/servo/components/style/stylesheets/stylesheet.rs +++ b/servo/components/style/stylesheets/stylesheet.rs @@ -65,6 +65,8 @@ pub struct StylesheetContents { pub source_map_url: RwLock>, /// This stylesheet's source URL. pub source_url: RwLock>, + /// The use counters of the original stylesheet. + pub use_counters: UseCounters, /// We don't want to allow construction outside of this file, to guarantee /// that all contents are created with Arc<>. @@ -82,10 +84,10 @@ impl StylesheetContents { stylesheet_loader: Option<&dyn StylesheetLoader>, error_reporter: Option<&dyn ParseErrorReporter>, quirks_mode: QuirksMode, - use_counters: Option<&UseCounters>, allow_import_rules: AllowImportRules, sanitization_data: Option<&mut SanitizationData>, ) -> Arc { + let use_counters = UseCounters::default(); let (namespaces, rules, source_map_url, source_url) = Stylesheet::parse_rules( css, &url_data, @@ -94,7 +96,7 @@ impl StylesheetContents { stylesheet_loader, error_reporter, quirks_mode, - use_counters, + Some(&use_counters), allow_import_rules, sanitization_data, ); @@ -107,6 +109,7 @@ impl StylesheetContents { quirks_mode, source_map_url: RwLock::new(source_map_url), source_url: RwLock::new(source_url), + use_counters, _forbid_construction: (), }) } @@ -137,6 +140,7 @@ impl StylesheetContents { quirks_mode, source_map_url: RwLock::new(None), source_url: RwLock::new(None), + use_counters: UseCounters::default(), _forbid_construction: (), }) } @@ -179,6 +183,7 @@ impl DeepCloneWithLock for StylesheetContents { namespaces: RwLock::new((*self.namespaces.read()).clone()), source_map_url: RwLock::new((*self.source_map_url.read()).clone()), source_url: RwLock::new((*self.source_url.read()).clone()), + use_counters: self.use_counters.clone(), _forbid_construction: (), } } @@ -405,7 +410,7 @@ impl Stylesheet { error_reporter: Option<&dyn ParseErrorReporter>, allow_import_rules: AllowImportRules, ) { - // FIXME: Consider adding use counters to Servo? + let use_counters = UseCounters::default(); let (namespaces, rules, source_map_url, source_url) = Self::parse_rules( css, &url_data, @@ -414,7 +419,7 @@ impl Stylesheet { stylesheet_loader, error_reporter, existing.contents.quirks_mode, - /* use_counters = */ None, + Some(&use_counters), allow_import_rules, /* sanitization_data = */ None, ); @@ -427,6 +432,7 @@ impl Stylesheet { *existing.contents.rules.write_with(&mut guard) = CssRules(rules); *existing.contents.source_map_url.write() = source_map_url; *existing.contents.source_url.write() = source_url; + existing.contents.use_counters.merge(&use_counters); } fn parse_rules( @@ -531,7 +537,6 @@ impl Stylesheet { stylesheet_loader, error_reporter, quirks_mode, - /* use_counters = */ None, allow_import_rules, /* sanitized_output = */ None, ); diff --git a/servo/components/style/use_counters/mod.rs b/servo/components/style/use_counters/mod.rs index 5a87372570d0..166db6c67457 100644 --- a/servo/components/style/use_counters/mod.rs +++ b/servo/components/style/use_counters/mod.rs @@ -14,14 +14,14 @@ const BITS_PER_ENTRY: usize = 64; const BITS_PER_ENTRY: usize = 32; /// One bit per each non-custom CSS property. -#[derive(Default)] +#[derive(Debug, Default, Clone)] pub struct CountedUnknownPropertyUseCounters { storage: [Cell; (property_counts::COUNTED_UNKNOWN - 1 + BITS_PER_ENTRY) / BITS_PER_ENTRY], } /// One bit per each non-custom CSS property. -#[derive(Default)] +#[derive(Debug, Default, Clone)] pub struct NonCustomPropertyUseCounters { storage: [Cell; (property_counts::NON_CUSTOM - 1 + BITS_PER_ENTRY) / BITS_PER_ENTRY], } @@ -73,7 +73,7 @@ impl NonCustomPropertyUseCounters { } /// The use-counter data related to a given document we want to store. -#[derive(Default)] +#[derive(Debug, Default, Clone)] pub struct UseCounters { /// The counters for non-custom properties that have been parsed in the /// document's stylesheets. diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 875434aa90f8..92ebcf656da4 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -1590,7 +1590,6 @@ pub extern "C" fn Servo_StyleSheet_Empty(mode: SheetParsingMode) -> Strong