Ver código fonte

LibWeb: Note what's causing a style invalidation to happen

You can now build with STYLE_INVALIDATION_DEBUG and get a debug stream
of reasons why style invalidations are happening and where.

I've rewritten this code many times, so instead of throwing it away once
again, I figured we should at least have it behind a flag.
Andreas Kling 11 meses atrás
pai
commit
ddbfac38b0

+ 4 - 0
AK/Debug.h.in

@@ -230,6 +230,10 @@
 #    cmakedefine01 SYNTAX_HIGHLIGHTING_DEBUG
 #endif
 
+#ifndef STYLE_INVALIDATION_DEBUG
+#    cmakedefine01 STYLE_INVALIDATION_DEBUG
+#endif
+
 #ifndef TEXTEDITOR_DEBUG
 #    cmakedefine01 TEXTEDITOR_DEBUG
 #endif

+ 1 - 0
Meta/CMake/all_the_debug_macros.cmake

@@ -52,6 +52,7 @@ set(RESOURCE_DEBUG ON)
 set(RSA_PARSE_DEBUG ON)
 set(SHARED_QUEUE_DEBUG ON)
 set(SPAM_DEBUG ON)
+set(STYLE_INVALIDATION_DEBUG ON)
 set(SYNTAX_HIGHLIGHTING_DEBUG ON)
 set(TEXTEDITOR_DEBUG ON)
 set(TIFF_DEBUG ON)

+ 1 - 1
Userland/Libraries/LibWeb/Animations/AnimationEffect.cpp

@@ -206,7 +206,7 @@ void AnimationEffect::set_associated_animation(JS::GCPtr<Animation> value)
 {
     m_associated_animation = value;
     if (auto* target = this->target())
-        target->invalidate_style();
+        target->invalidate_style(DOM::StyleInvalidationReason::AnimationEffectSetAssociatedAnimation);
 }
 
 // https://www.w3.org/TR/web-animations-1/#animation-direction

+ 1 - 1
Userland/Libraries/LibWeb/CSS/CSSImportRule.cpp

@@ -108,7 +108,7 @@ void CSSImportRule::resource_did_load()
 
     m_document->style_computer().invalidate_rule_cache();
     m_document->style_computer().load_fonts_from_sheet(*m_style_sheet);
-    m_document->invalidate_style();
+    m_document->invalidate_style(DOM::StyleInvalidationReason::CSSImportRule);
 }
 
 }

+ 1 - 1
Userland/Libraries/LibWeb/CSS/CSSStyleRule.cpp

@@ -114,7 +114,7 @@ void CSSStyleRule::set_selector_text(StringView selector_text)
         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();
+                style_sheet_list->document_or_shadow_root().invalidate_style(DOM::StyleInvalidationReason::SetSelectorText);
             }
         }
     }

+ 2 - 2
Userland/Libraries/LibWeb/CSS/CSSStyleSheet.cpp

@@ -158,7 +158,7 @@ WebIDL::ExceptionOr<unsigned> CSSStyleSheet::insert_rule(StringView rule, unsign
 
         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();
+            m_style_sheet_list->document_or_shadow_root().invalidate_style(DOM::StyleInvalidationReason::StyleSheetInsertRule);
         }
     }
 
@@ -179,7 +179,7 @@ WebIDL::ExceptionOr<void> CSSStyleSheet::delete_rule(unsigned 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();
+            m_style_sheet_list->document_or_shadow_root().invalidate_style(DOM::StyleInvalidationReason::StyleSheetDeleteRule);
         }
     }
     return result;

+ 1 - 1
Userland/Libraries/LibWeb/CSS/StyleComputer.cpp

@@ -2870,7 +2870,7 @@ void StyleComputer::invalidate_rule_cache()
 
 void StyleComputer::did_load_font(FlyString const&)
 {
-    document().invalidate_style();
+    document().invalidate_style(DOM::StyleInvalidationReason::CSSFontLoaded);
 }
 
 Optional<FontLoader&> StyleComputer::load_font_face(ParsedFontFace const& font_face, Function<void(FontLoader const&)> on_load, Function<void()> on_fail)

+ 2 - 2
Userland/Libraries/LibWeb/CSS/StyleSheetList.cpp

@@ -109,7 +109,7 @@ void StyleSheetList::add_sheet(CSSStyleSheet& sheet)
 
     document().style_computer().invalidate_rule_cache();
     document().style_computer().load_fonts_from_sheet(sheet);
-    document_or_shadow_root().invalidate_style();
+    document_or_shadow_root().invalidate_style(DOM::StyleInvalidationReason::StyleSheetListAddSheet);
 }
 
 void StyleSheetList::remove_sheet(CSSStyleSheet& sheet)
@@ -124,7 +124,7 @@ void StyleSheetList::remove_sheet(CSSStyleSheet& sheet)
     }
 
     m_document_or_shadow_root->document().style_computer().invalidate_rule_cache();
-    document_or_shadow_root().invalidate_style();
+    document_or_shadow_root().invalidate_style(DOM::StyleInvalidationReason::StyleSheetListRemoveSheet);
 }
 
 JS::NonnullGCPtr<StyleSheetList> StyleSheetList::create(JS::NonnullGCPtr<DOM::Node> document_or_shadow_root)

+ 2 - 2
Userland/Libraries/LibWeb/DOM/AdoptedStyleSheets.cpp

@@ -32,12 +32,12 @@ JS::NonnullGCPtr<WebIDL::ObservableArray> create_adopted_style_sheets_list(Docum
 
         document.style_computer().load_fonts_from_sheet(style_sheet);
         document.style_computer().invalidate_rule_cache();
-        document.invalidate_style();
+        document.invalidate_style(DOM::StyleInvalidationReason::AdoptedStyleSheetsList);
         return {};
     });
     adopted_style_sheets->set_on_delete_an_indexed_value_callback([&document]() -> WebIDL::ExceptionOr<void> {
         document.style_computer().invalidate_rule_cache();
-        document.invalidate_style();
+        document.invalidate_style(DOM::StyleInvalidationReason::AdoptedStyleSheetsList);
         return {};
     });
 

+ 3 - 3
Userland/Libraries/LibWeb/DOM/Document.cpp

@@ -1385,9 +1385,9 @@ void Document::set_hovered_node(Node* node)
 
     auto* common_ancestor = find_common_ancestor(old_hovered_node, m_hovered_node);
     if (common_ancestor)
-        common_ancestor->invalidate_style();
+        common_ancestor->invalidate_style(StyleInvalidationReason::Hover);
     else
-        invalidate_style();
+        invalidate_style(StyleInvalidationReason::Hover);
 
     // https://w3c.github.io/uievents/#mouseout
     if (old_hovered_node && old_hovered_node != m_hovered_node) {
@@ -2716,7 +2716,7 @@ void Document::evaluate_media_rules()
 
     if (any_media_queries_changed_match_state) {
         style_computer().invalidate_rule_cache();
-        invalidate_style();
+        invalidate_style(StyleInvalidationReason::MediaQueryChangedMatchState);
         invalidate_layout();
     }
 }

+ 2 - 2
Userland/Libraries/LibWeb/DOM/Element.cpp

@@ -875,7 +875,7 @@ void Element::set_shadow_root(JS::GCPtr<ShadowRoot> shadow_root)
     m_shadow_root = move(shadow_root);
     if (m_shadow_root)
         m_shadow_root->set_host(this);
-    invalidate_style();
+    invalidate_style(StyleInvalidationReason::ElementSetShadowRoot);
 }
 
 CSS::CSSStyleDeclaration* Element::style_for_bindings()
@@ -1930,7 +1930,7 @@ void Element::invalidate_style_after_attribute_change(FlyString const& attribute
     (void)attribute_name;
 
     // FIXME: This will need to become smarter when we implement the :has() selector.
-    invalidate_style();
+    invalidate_style(StyleInvalidationReason::ElementAttributeChange);
 }
 
 // https://www.w3.org/TR/wai-aria-1.2/#tree_exclusion

+ 21 - 5
Userland/Libraries/LibWeb/DOM/Node.cpp

@@ -207,7 +207,7 @@ void Node::set_text_content(Optional<String> const& maybe_content)
     // Otherwise, do nothing.
 
     if (is_connected()) {
-        document().invalidate_style();
+        document().invalidate_style(StyleInvalidationReason::NodeSetTextContent);
         document().invalidate_layout();
     }
 
@@ -375,8 +375,24 @@ JS::GCPtr<HTML::Navigable> Node::navigable() const
     return navigable;
 }
 
-void Node::invalidate_style()
+[[maybe_unused]] static StringView to_string(StyleInvalidationReason reason)
 {
+#define __ENUMERATE_STYLE_INVALIDATION_REASON(reason) \
+    case StyleInvalidationReason::reason:             \
+        return #reason##sv;
+    switch (reason) {
+        ENUMERATE_STYLE_INVALIDATION_REASONS(__ENUMERATE_STYLE_INVALIDATION_REASON)
+    default:
+        VERIFY_NOT_REACHED();
+    }
+}
+
+void Node::invalidate_style(StyleInvalidationReason reason)
+{
+    if (!needs_style_update() && !document().needs_full_style_update()) {
+        dbgln_if(STYLE_INVALIDATION_DEBUG, "Invalidate style ({}): {}", to_string(reason), debug_description());
+    }
+
     if (is_document()) {
         auto& document = static_cast<DOM::Document&>(*this);
         document.set_needs_full_style_update(true);
@@ -598,7 +614,7 @@ void Node::insert_before(JS::NonnullGCPtr<Node> node, JS::GCPtr<Node> child, boo
         // 6. Run assign slottables for a tree with node’s root.
         assign_slottables_for_a_tree(node_to_insert->root());
 
-        node_to_insert->invalidate_style();
+        node_to_insert->invalidate_style(StyleInvalidationReason::NodeInsertBefore);
 
         // 7. For each shadow-including inclusive descendant inclusiveDescendant of node, in shadow-including tree order:
         node_to_insert->for_each_shadow_including_inclusive_descendant([&](Node& inclusive_descendant) {
@@ -639,7 +655,7 @@ void Node::insert_before(JS::NonnullGCPtr<Node> node, JS::GCPtr<Node> child, boo
 
     if (is_connected()) {
         // FIXME: This will need to become smarter when we implement the :has() selector.
-        invalidate_style();
+        invalidate_style(StyleInvalidationReason::NodeInsertBefore);
         document().invalidate_layout();
     }
 
@@ -838,7 +854,7 @@ void Node::remove(bool suppress_observers)
     if (was_connected) {
         // Since the tree structure has changed, we need to invalidate both style and layout.
         // In the future, we should find a way to only invalidate the parts that actually need it.
-        document().invalidate_style();
+        document().invalidate_style(StyleInvalidationReason::NodeRemove);
         document().invalidate_layout();
     }
 

+ 34 - 1
Userland/Libraries/LibWeb/DOM/Node.h

@@ -52,6 +52,39 @@ enum class FragmentSerializationMode {
     Outer,
 };
 
+#define ENUMERATE_STYLE_INVALIDATION_REASONS(X)     \
+    X(AdoptedStyleSheetsList)                       \
+    X(AnimationEffectSetAssociatedAnimation)        \
+    X(CSSFontLoaded)                                \
+    X(CSSImportRule)                                \
+    X(DidLoseFocus)                                 \
+    X(DidReceiveFocus)                              \
+    X(EditingInsertion)                             \
+    X(ElementAttributeChange)                       \
+    X(ElementSetShadowRoot)                         \
+    X(HTMLInputElementSetChecked)                   \
+    X(HTMLObjectElementUpdateLayoutAndChildObjects) \
+    X(HTMLSelectElementSetIsOpen)                   \
+    X(Hover)                                        \
+    X(MediaQueryChangedMatchState)                  \
+    X(NavigableSetViewportSize)                     \
+    X(NodeInsertBefore)                             \
+    X(NodeRemove)                                   \
+    X(NodeSetTextContent)                           \
+    X(Other)                                        \
+    X(SetSelectorText)                              \
+    X(SettingsChange)                               \
+    X(StyleSheetDeleteRule)                         \
+    X(StyleSheetInsertRule)                         \
+    X(StyleSheetListAddSheet)                       \
+    X(StyleSheetListRemoveSheet)
+
+enum class StyleInvalidationReason {
+#define __ENUMERATE_STYLE_INVALIDATION_REASON(reason) reason,
+    ENUMERATE_STYLE_INVALIDATION_REASONS(__ENUMERATE_STYLE_INVALIDATION_REASON)
+#undef __ENUMERATE_STYLE_INVALIDATION_REASON
+};
+
 class Node : public EventTarget {
     WEB_PLATFORM_OBJECT(Node, EventTarget);
 
@@ -227,7 +260,7 @@ public:
     bool child_needs_style_update() const { return m_child_needs_style_update; }
     void set_child_needs_style_update(bool b) { m_child_needs_style_update = b; }
 
-    void invalidate_style();
+    void invalidate_style(StyleInvalidationReason);
 
     void set_document(Badge<Document>, Document&);
 

+ 5 - 5
Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp

@@ -156,7 +156,7 @@ void HTMLInputElement::set_checked(bool checked, ChangeSource change_source)
     // so we need to invalidate the style of all siblings.
     if (parent()) {
         parent()->for_each_child([&](auto& child) {
-            child.invalidate_style();
+            child.invalidate_style(DOM::StyleInvalidationReason::HTMLInputElementSetChecked);
             return IterationDecision::Continue;
         });
     }
@@ -1142,10 +1142,10 @@ void HTMLInputElement::did_receive_focus()
 {
     if (!m_text_node)
         return;
-    m_text_node->invalidate_style();
+    m_text_node->invalidate_style(DOM::StyleInvalidationReason::DidReceiveFocus);
 
     if (m_placeholder_text_node)
-        m_placeholder_text_node->invalidate_style();
+        m_placeholder_text_node->invalidate_style(DOM::StyleInvalidationReason::DidReceiveFocus);
 
     document().set_cursor_position(DOM::Position::create(realm(), *m_text_node, 0));
 }
@@ -1153,10 +1153,10 @@ void HTMLInputElement::did_receive_focus()
 void HTMLInputElement::did_lose_focus()
 {
     if (m_text_node)
-        m_text_node->invalidate_style();
+        m_text_node->invalidate_style(DOM::StyleInvalidationReason::DidLoseFocus);
 
     if (m_placeholder_text_node)
-        m_placeholder_text_node->invalidate_style();
+        m_placeholder_text_node->invalidate_style(DOM::StyleInvalidationReason::DidLoseFocus);
 
     commit_pending_changes();
 }

+ 1 - 1
Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp

@@ -362,7 +362,7 @@ void HTMLObjectElement::update_layout_and_child_objects(Representation represent
     }
 
     m_representation = representation;
-    invalidate_style();
+    invalidate_style(DOM::StyleInvalidationReason::HTMLObjectElementUpdateLayoutAndChildObjects);
     document().invalidate_layout();
 }
 

+ 1 - 1
Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp

@@ -323,7 +323,7 @@ void HTMLSelectElement::set_is_open(bool open)
         return;
 
     m_is_open = open;
-    invalidate_style();
+    invalidate_style(DOM::StyleInvalidationReason::HTMLSelectElementSetIsOpen);
 }
 
 bool HTMLSelectElement::has_activation_behavior() const

+ 4 - 4
Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.cpp

@@ -72,10 +72,10 @@ void HTMLTextAreaElement::did_receive_focus()
 {
     if (!m_text_node)
         return;
-    m_text_node->invalidate_style();
+    m_text_node->invalidate_style(DOM::StyleInvalidationReason::DidReceiveFocus);
 
     if (m_placeholder_text_node)
-        m_placeholder_text_node->invalidate_style();
+        m_placeholder_text_node->invalidate_style(DOM::StyleInvalidationReason::DidReceiveFocus);
 
     document().set_cursor_position(DOM::Position::create(realm(), *m_text_node, 0));
 }
@@ -83,10 +83,10 @@ void HTMLTextAreaElement::did_receive_focus()
 void HTMLTextAreaElement::did_lose_focus()
 {
     if (m_text_node)
-        m_text_node->invalidate_style();
+        m_text_node->invalidate_style(DOM::StyleInvalidationReason::DidLoseFocus);
 
     if (m_placeholder_text_node)
-        m_placeholder_text_node->invalidate_style();
+        m_placeholder_text_node->invalidate_style(DOM::StyleInvalidationReason::DidLoseFocus);
 
     // The change event fires when the value is committed, if that makes sense for the control,
     // or else when the control loses focus

+ 1 - 1
Userland/Libraries/LibWeb/HTML/Navigable.cpp

@@ -2007,7 +2007,7 @@ void Navigable::set_viewport_size(CSSPixelSize size)
     m_size = size;
     if (auto document = active_document()) {
         // NOTE: Resizing the viewport changes the reference value for viewport-relative CSS lengths.
-        document->invalidate_style();
+        document->invalidate_style(DOM::StyleInvalidationReason::NavigableSetViewportSize);
         document->set_needs_layout();
     }
 

+ 1 - 1
Userland/Libraries/LibWeb/Page/EditEventHandler.cpp

@@ -116,7 +116,7 @@ void EditEventHandler::handle_insert(JS::NonnullGCPtr<DOM::Document> document, J
         } else {
             node.set_data(MUST(builder.to_string()));
         }
-        node.invalidate_style();
+        node.invalidate_style(DOM::StyleInvalidationReason::EditingInsertion);
     } else {
         auto& node = *position->node();
         auto& realm = node.realm();

+ 4 - 4
Userland/Services/WebContent/PageClient.cpp

@@ -122,28 +122,28 @@ void PageClient::set_palette_impl(Gfx::PaletteImpl& impl)
 {
     m_palette_impl = impl;
     if (auto* document = page().top_level_browsing_context().active_document())
-        document->invalidate_style();
+        document->invalidate_style(Web::DOM::StyleInvalidationReason::SettingsChange);
 }
 
 void PageClient::set_preferred_color_scheme(Web::CSS::PreferredColorScheme color_scheme)
 {
     m_preferred_color_scheme = color_scheme;
     if (auto* document = page().top_level_browsing_context().active_document())
-        document->invalidate_style();
+        document->invalidate_style(Web::DOM::StyleInvalidationReason::SettingsChange);
 }
 
 void PageClient::set_preferred_contrast(Web::CSS::PreferredContrast contrast)
 {
     m_preferred_contrast = contrast;
     if (auto* document = page().top_level_browsing_context().active_document())
-        document->invalidate_style();
+        document->invalidate_style(Web::DOM::StyleInvalidationReason::SettingsChange);
 }
 
 void PageClient::set_preferred_motion(Web::CSS::PreferredMotion motion)
 {
     m_preferred_motion = motion;
     if (auto* document = page().top_level_browsing_context().active_document())
-        document->invalidate_style();
+        document->invalidate_style(Web::DOM::StyleInvalidationReason::SettingsChange);
 }
 
 void PageClient::set_is_scripting_enabled(bool is_scripting_enabled)