From 9802d5a50f1add8ca568da90ce4b2f00da11a063 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Fri, 10 Jan 2025 20:00:43 +0300 Subject: [PATCH] LibWeb: Fix insert/delete rule invalidation for adopted style sheets Invalidaiton for adopted style sheets was broken because we had an assumption that "active" style sheet is always attached to StyleSheetList which is not true for adopted style sheets. This change addresses that by keeping track of all documents/shadow roots that own a style sheet and notifying them about invalidation instead of going through the StyleSheetList. --- Libraries/LibWeb/CSS/CSSStyleRule.cpp | 5 +- Libraries/LibWeb/CSS/CSSStyleSheet.cpp | 43 ++++++++++------- Libraries/LibWeb/CSS/CSSStyleSheet.h | 8 ++-- Libraries/LibWeb/CSS/StyleSheetList.cpp | 4 +- Libraries/LibWeb/DOM/AdoptedStyleSheets.cpp | 32 +++++++------ Libraries/LibWeb/DOM/AdoptedStyleSheets.h | 2 +- Libraries/LibWeb/DOM/ShadowRoot.cpp | 4 +- .../insert-rule-in-adopted-style-sheet.txt | 4 ++ .../insert-rule-in-adopted-style-sheet.html | 47 +++++++++++++++++++ 9 files changed, 105 insertions(+), 44 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/css/insert-rule-in-adopted-style-sheet.txt create mode 100644 Tests/LibWeb/Text/input/css/insert-rule-in-adopted-style-sheet.html diff --git a/Libraries/LibWeb/CSS/CSSStyleRule.cpp b/Libraries/LibWeb/CSS/CSSStyleRule.cpp index 618b0e56a55a8..a520a75b68e62 100644 --- a/Libraries/LibWeb/CSS/CSSStyleRule.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleRule.cpp @@ -139,10 +139,7 @@ void CSSStyleRule::set_selector_text(StringView selector_text) m_selectors = parsed_selectors.release_value(); if (auto* sheet = parent_style_sheet()) { - if (auto style_sheet_list = sheet->style_sheet_list()) { - style_sheet_list->document().style_computer().invalidate_rule_cache(); - style_sheet_list->document_or_shadow_root().invalidate_style(DOM::StyleInvalidationReason::SetSelectorText); - } + sheet->invalidate_owners(DOM::StyleInvalidationReason::SetSelectorText); } } diff --git a/Libraries/LibWeb/CSS/CSSStyleSheet.cpp b/Libraries/LibWeb/CSS/CSSStyleSheet.cpp index c4b51b88beb02..627bcc31181e5 100644 --- a/Libraries/LibWeb/CSS/CSSStyleSheet.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleSheet.cpp @@ -120,13 +120,14 @@ void CSSStyleSheet::initialize(JS::Realm& realm) void CSSStyleSheet::visit_edges(Cell::Visitor& visitor) { Base::visit_edges(visitor); - visitor.visit(m_style_sheet_list); visitor.visit(m_rules); visitor.visit(m_owner_css_rule); visitor.visit(m_default_namespace_rule); visitor.visit(m_constructor_document); visitor.visit(m_namespace_rules); visitor.visit(m_import_rules); + for (auto& document_or_shadow_root : m_owning_documents_or_shadow_roots) + visitor.visit(document_or_shadow_root); } // https://www.w3.org/TR/cssom/#dom-cssstylesheet-insertrule @@ -139,7 +140,7 @@ WebIDL::ExceptionOr CSSStyleSheet::insert_rule(StringView rule, unsign return WebIDL::NotAllowedError::create(realm(), "Can't call insert_rule() on non-modifiable stylesheets."_string); // 3. Let parsed rule be the return value of invoking parse a rule with rule. - auto context = m_style_sheet_list ? CSS::Parser::ParsingContext { m_style_sheet_list->document() } : CSS::Parser::ParsingContext { realm() }; + auto context = !m_owning_documents_or_shadow_roots.is_empty() ? Parser::ParsingContext { (*m_owning_documents_or_shadow_roots.begin())->document() } : Parser::ParsingContext { realm() }; auto parsed_rule = parse_css_rule(context, rule); // 4. If parsed rule is a syntax error, return parsed rule. @@ -157,10 +158,7 @@ WebIDL::ExceptionOr CSSStyleSheet::insert_rule(StringView rule, unsign // NOTE: The spec doesn't say where to set the parent style sheet, so we'll do it here. parsed_rule->set_parent_style_sheet(this); - if (m_style_sheet_list) { - m_style_sheet_list->document().style_computer().invalidate_rule_cache(); - m_style_sheet_list->document_or_shadow_root().invalidate_style(DOM::StyleInvalidationReason::StyleSheetInsertRule); - } + invalidate_owners(DOM::StyleInvalidationReason::StyleSheetInsertRule); } return result; @@ -178,10 +176,7 @@ WebIDL::ExceptionOr CSSStyleSheet::delete_rule(unsigned index) // 3. Remove a CSS rule in the CSS rules at index. auto result = m_rules->remove_a_css_rule(index); if (!result.is_exception()) { - if (m_style_sheet_list) { - m_style_sheet_list->document().style_computer().invalidate_rule_cache(); - m_style_sheet_list->document_or_shadow_root().invalidate_style(DOM::StyleInvalidationReason::StyleSheetDeleteRule); - } + invalidate_owners(DOM::StyleInvalidationReason::StyleSheetDeleteRule); } return result; } @@ -213,7 +208,7 @@ GC::Ref CSSStyleSheet::replace(String text) HTML::TemporaryExecutionContext execution_context { realm, HTML::TemporaryExecutionContext::CallbacksEnabled::Yes }; // 1. Let rules be the result of running parse a stylesheet’s contents from text. - auto context = m_style_sheet_list ? CSS::Parser::ParsingContext { m_style_sheet_list->document() } : CSS::Parser::ParsingContext { realm }; + auto context = !m_owning_documents_or_shadow_roots.is_empty() ? Parser::ParsingContext { (*m_owning_documents_or_shadow_roots.begin())->document() } : CSS::Parser::ParsingContext { realm }; auto* parsed_stylesheet = parse_css_stylesheet(context, text); auto& rules = parsed_stylesheet->rules(); @@ -247,7 +242,7 @@ WebIDL::ExceptionOr CSSStyleSheet::replace_sync(StringView text) return WebIDL::NotAllowedError::create(realm(), "Can't call replaceSync() on non-modifiable stylesheets"_string); // 2. Let rules be the result of running parse a stylesheet’s contents from text. - auto context = m_style_sheet_list ? CSS::Parser::ParsingContext { m_style_sheet_list->document() } : CSS::Parser::ParsingContext { realm() }; + auto context = !m_owning_documents_or_shadow_roots.is_empty() ? Parser::ParsingContext { (*m_owning_documents_or_shadow_roots.begin())->document() } : CSS::Parser::ParsingContext { realm() }; auto* parsed_stylesheet = parse_css_stylesheet(context, text); auto& rules = parsed_stylesheet->rules(); @@ -321,6 +316,25 @@ void CSSStyleSheet::for_each_effective_keyframes_at_rule(Functioninvalidate_style(reason); + document_or_shadow_root->document().style_computer().invalidate_rule_cache(); + } +} + bool CSSStyleSheet::evaluate_media_queries(HTML::Window const& window) { bool any_media_queries_changed_match_state = false; @@ -336,11 +350,6 @@ bool CSSStyleSheet::evaluate_media_queries(HTML::Window const& window) return any_media_queries_changed_match_state; } -void CSSStyleSheet::set_style_sheet_list(Badge, StyleSheetList* list) -{ - m_style_sheet_list = list; -} - Optional CSSStyleSheet::default_namespace() const { if (m_default_namespace_rule) diff --git a/Libraries/LibWeb/CSS/CSSStyleSheet.h b/Libraries/LibWeb/CSS/CSSStyleSheet.h index bdfba05d40f3f..b3c93b102b680 100644 --- a/Libraries/LibWeb/CSS/CSSStyleSheet.h +++ b/Libraries/LibWeb/CSS/CSSStyleSheet.h @@ -13,6 +13,7 @@ #include #include #include +#include #include namespace Web::CSS { @@ -62,8 +63,9 @@ class CSSStyleSheet final : public StyleSheet { bool evaluate_media_queries(HTML::Window const&); void for_each_effective_keyframes_at_rule(Function const& callback) const; - GC::Ptr style_sheet_list() const { return m_style_sheet_list; } - void set_style_sheet_list(Badge, StyleSheetList*); + void add_owning_document_or_shadow_root(DOM::Node& document_or_shadow_root); + void remove_owning_document_or_shadow_root(DOM::Node& document_or_shadow_root); + void invalidate_owners(DOM::StyleInvalidationReason); Optional default_namespace() const; GC::Ptr default_namespace_rule() const { return m_default_namespace_rule; } @@ -109,11 +111,11 @@ class CSSStyleSheet final : public StyleSheet { HashMap> m_namespace_rules; Vector> m_import_rules; - GC::Ptr m_style_sheet_list; GC::Ptr m_owner_css_rule; Optional m_base_url; GC::Ptr m_constructor_document; + HashTable> m_owning_documents_or_shadow_roots; bool m_constructed { false }; bool m_disallow_modification { false }; Optional m_did_match; diff --git a/Libraries/LibWeb/CSS/StyleSheetList.cpp b/Libraries/LibWeb/CSS/StyleSheetList.cpp index 9d76a8c839d7a..e7e204f8599b2 100644 --- a/Libraries/LibWeb/CSS/StyleSheetList.cpp +++ b/Libraries/LibWeb/CSS/StyleSheetList.cpp @@ -81,7 +81,7 @@ void StyleSheetList::create_a_css_style_sheet(String type, DOM::Element* owner_n void StyleSheetList::add_sheet(CSSStyleSheet& sheet) { - sheet.set_style_sheet_list({}, this); + sheet.add_owning_document_or_shadow_root(document_or_shadow_root()); if (m_sheets.is_empty()) { // This is the first sheet, append it to the list. @@ -114,7 +114,7 @@ void StyleSheetList::add_sheet(CSSStyleSheet& sheet) void StyleSheetList::remove_sheet(CSSStyleSheet& sheet) { - sheet.set_style_sheet_list({}, nullptr); + sheet.remove_owning_document_or_shadow_root(document_or_shadow_root()); bool did_remove = m_sheets.remove_first_matching([&](auto& entry) { return entry.ptr() == &sheet; }); VERIFY(did_remove); diff --git a/Libraries/LibWeb/DOM/AdoptedStyleSheets.cpp b/Libraries/LibWeb/DOM/AdoptedStyleSheets.cpp index db57fa2f49f57..beef222d795a1 100644 --- a/Libraries/LibWeb/DOM/AdoptedStyleSheets.cpp +++ b/Libraries/LibWeb/DOM/AdoptedStyleSheets.cpp @@ -10,11 +10,11 @@ namespace Web::DOM { -GC::Ref create_adopted_style_sheets_list(Document& document) +GC::Ref create_adopted_style_sheets_list(Node& document_or_shadow_root) { - auto adopted_style_sheets = WebIDL::ObservableArray::create(document.realm()); - adopted_style_sheets->set_on_set_an_indexed_value_callback([&document](JS::Value& value) -> WebIDL::ExceptionOr { - auto& vm = document.vm(); + auto adopted_style_sheets = WebIDL::ObservableArray::create(document_or_shadow_root.realm()); + adopted_style_sheets->set_on_set_an_indexed_value_callback([&document_or_shadow_root](JS::Value& value) -> WebIDL::ExceptionOr { + auto& vm = document_or_shadow_root.vm(); if (!value.is_object()) return vm.throw_completion(JS::ErrorType::NotAnObjectOfType, "CSSStyleSheet"); auto& object = value.as_object(); @@ -26,24 +26,26 @@ GC::Ref create_adopted_style_sheets_list(Document& docu // 1. If value’s constructed flag is not set, or its constructor document is not equal to this // DocumentOrShadowRoot's node document, throw a "NotAllowedError" DOMException. if (!style_sheet.constructed()) - return WebIDL::NotAllowedError::create(document.realm(), "StyleSheet's constructed flag is not set."_string); - if (!style_sheet.constructed() || style_sheet.constructor_document().ptr() != &document) - return WebIDL::NotAllowedError::create(document.realm(), "Sharing a StyleSheet between documents is not allowed."_string); - - document.style_computer().load_fonts_from_sheet(style_sheet); - document.style_computer().invalidate_rule_cache(); - document.invalidate_style(DOM::StyleInvalidationReason::AdoptedStyleSheetsList); + return WebIDL::NotAllowedError::create(document_or_shadow_root.realm(), "StyleSheet's constructed flag is not set."_string); + if (!style_sheet.constructed() || style_sheet.constructor_document().ptr() != &document_or_shadow_root.document()) + return WebIDL::NotAllowedError::create(document_or_shadow_root.realm(), "Sharing a StyleSheet between documents is not allowed."_string); + + style_sheet.add_owning_document_or_shadow_root(document_or_shadow_root); + document_or_shadow_root.document().style_computer().load_fonts_from_sheet(style_sheet); + document_or_shadow_root.document().style_computer().invalidate_rule_cache(); + document_or_shadow_root.invalidate_style(DOM::StyleInvalidationReason::AdoptedStyleSheetsList); return {}; }); - adopted_style_sheets->set_on_delete_an_indexed_value_callback([&document](JS::Value value) -> WebIDL::ExceptionOr { + adopted_style_sheets->set_on_delete_an_indexed_value_callback([&document_or_shadow_root](JS::Value value) -> WebIDL::ExceptionOr { VERIFY(value.is_object()); auto& object = value.as_object(); VERIFY(is(object)); auto& style_sheet = static_cast(object); - document.style_computer().unload_fonts_from_sheet(style_sheet); - document.style_computer().invalidate_rule_cache(); - document.invalidate_style(DOM::StyleInvalidationReason::AdoptedStyleSheetsList); + style_sheet.remove_owning_document_or_shadow_root(document_or_shadow_root); + document_or_shadow_root.document().style_computer().unload_fonts_from_sheet(style_sheet); + document_or_shadow_root.document().style_computer().invalidate_rule_cache(); + document_or_shadow_root.invalidate_style(DOM::StyleInvalidationReason::AdoptedStyleSheetsList); return {}; }); diff --git a/Libraries/LibWeb/DOM/AdoptedStyleSheets.h b/Libraries/LibWeb/DOM/AdoptedStyleSheets.h index 3973041a2a98c..8015eb69c53e6 100644 --- a/Libraries/LibWeb/DOM/AdoptedStyleSheets.h +++ b/Libraries/LibWeb/DOM/AdoptedStyleSheets.h @@ -11,6 +11,6 @@ namespace Web::DOM { -GC::Ref create_adopted_style_sheets_list(Document& document); +GC::Ref create_adopted_style_sheets_list(Node& document_or_shadow_root); } diff --git a/Libraries/LibWeb/DOM/ShadowRoot.cpp b/Libraries/LibWeb/DOM/ShadowRoot.cpp index 690630e5501ab..c7d3efb3e1770 100644 --- a/Libraries/LibWeb/DOM/ShadowRoot.cpp +++ b/Libraries/LibWeb/DOM/ShadowRoot.cpp @@ -141,14 +141,14 @@ void ShadowRoot::visit_edges(Visitor& visitor) GC::Ref ShadowRoot::adopted_style_sheets() const { if (!m_adopted_style_sheets) - m_adopted_style_sheets = create_adopted_style_sheets_list(const_cast(document())); + m_adopted_style_sheets = create_adopted_style_sheets_list(const_cast(*this)); return *m_adopted_style_sheets; } WebIDL::ExceptionOr ShadowRoot::set_adopted_style_sheets(JS::Value new_value) { if (!m_adopted_style_sheets) - m_adopted_style_sheets = create_adopted_style_sheets_list(const_cast(document())); + m_adopted_style_sheets = create_adopted_style_sheets_list(*this); m_adopted_style_sheets->clear(); auto iterator_record = TRY(get_iterator(vm(), new_value, JS::IteratorHint::Sync)); diff --git a/Tests/LibWeb/Text/expected/css/insert-rule-in-adopted-style-sheet.txt b/Tests/LibWeb/Text/expected/css/insert-rule-in-adopted-style-sheet.txt new file mode 100644 index 0000000000000..f86475bbe65e5 --- /dev/null +++ b/Tests/LibWeb/Text/expected/css/insert-rule-in-adopted-style-sheet.txt @@ -0,0 +1,4 @@ +color of box 1 before rule insertion: rgb(255, 0, 0) +color of box 2 before rule insertion: rgb(255, 0, 0) +color of box 1 after rule insertion: rgb(0, 128, 0) +color of box 2 after rule insertion: rgb(0, 128, 0) diff --git a/Tests/LibWeb/Text/input/css/insert-rule-in-adopted-style-sheet.html b/Tests/LibWeb/Text/input/css/insert-rule-in-adopted-style-sheet.html new file mode 100644 index 0000000000000..7e7088a5f93e1 --- /dev/null +++ b/Tests/LibWeb/Text/input/css/insert-rule-in-adopted-style-sheet.html @@ -0,0 +1,47 @@ + + + + + \ No newline at end of file