From 6499cf4d280654d1b30cd7366a29d3acc53a46b3 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Wed, 9 Mar 2022 17:57:45 +0100
Subject: [PATCH] LibWeb: Always relayout document on element style change

Let's get this right before trying to make it fast. This patch removes
the code that tried to do less work when an element's style changes,
and instead simply invalidates the entire document.

Note that invalidations are still coalesced, and will not be
synchronized until update_style() and/or update_layout() is used.
---
 Userland/Libraries/LibWeb/DOM/Document.cpp |  5 +-
 Userland/Libraries/LibWeb/DOM/Document.h   |  3 +-
 Userland/Libraries/LibWeb/DOM/Element.cpp  | 65 ++--------------------
 3 files changed, 9 insertions(+), 64 deletions(-)

diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp
index 99ffbf03a9..e06f1c04f8 100644
--- a/Userland/Libraries/LibWeb/DOM/Document.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Document.cpp
@@ -546,10 +546,9 @@ void Document::force_layout()
     update_layout();
 }
 
-void Document::ensure_layout()
+void Document::invalidate_layout()
 {
-    if (m_needs_layout || !m_layout_root)
-        update_layout();
+    tear_down_layout_tree();
 }
 
 void Document::update_layout()
diff --git a/Userland/Libraries/LibWeb/DOM/Document.h b/Userland/Libraries/LibWeb/DOM/Document.h
index 9dbbe9e9ff..24d5e19b96 100644
--- a/Userland/Libraries/LibWeb/DOM/Document.h
+++ b/Userland/Libraries/LibWeb/DOM/Document.h
@@ -141,13 +141,14 @@ public:
     void set_visited_link_color(Color);
 
     void force_layout();
-    void ensure_layout();
 
     void update_style();
     void update_layout();
 
     void set_needs_layout();
 
+    void invalidate_layout();
+
     virtual bool is_child_allowed(const Node&) const override;
 
     const Layout::InitialContainingBlock* layout_node() const;
diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp
index 3379fb45cf..a103027997 100644
--- a/Userland/Libraries/LibWeb/DOM/Element.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Element.cpp
@@ -266,74 +266,19 @@ void Element::parse_attribute(const FlyString& name, const String& value)
     }
 }
 
-enum class StyleDifference {
-    None,
-    NeedsRepaint,
-    NeedsRelayout,
-};
-
-static StyleDifference compute_style_difference(CSS::StyleProperties const& old_style, CSS::StyleProperties const& new_style, Layout::NodeWithStyle const& node)
-{
-    if (old_style == new_style)
-        return StyleDifference::None;
-
-    bool needs_repaint = false;
-    bool needs_relayout = false;
-
-    if (new_style.display() != old_style.display())
-        needs_relayout = true;
-
-    if (new_style.color_or_fallback(CSS::PropertyID::Color, node, Color::Black) != old_style.color_or_fallback(CSS::PropertyID::Color, node, Color::Black))
-        needs_repaint = true;
-    else if (new_style.color_or_fallback(CSS::PropertyID::BackgroundColor, node, Color::Black) != old_style.color_or_fallback(CSS::PropertyID::BackgroundColor, node, Color::Black))
-        needs_repaint = true;
-
-    if (needs_relayout)
-        return StyleDifference::NeedsRelayout;
-    if (needs_repaint)
-        return StyleDifference::NeedsRepaint;
-    return StyleDifference::None;
-}
-
 void Element::recompute_style()
 {
     set_needs_style_update(false);
     VERIFY(parent());
     auto old_specified_css_values = m_specified_css_values;
     auto new_specified_css_values = document().style_computer().compute_style(*this);
+
+    if (old_specified_css_values == new_specified_css_values)
+        return;
+
     m_specified_css_values = new_specified_css_values;
-    if (!layout_node()) {
-        // This element doesn't have a corresponding layout node.
 
-        // If the new style is display:none, bail.
-        if (new_specified_css_values->display().is_none())
-            return;
-
-        // If we're inside a display:none ancestor or an ancestor that can't have children, bail.
-        for (auto* ancestor = parent_element(); ancestor; ancestor = ancestor->parent_element()) {
-            if (!ancestor->layout_node() || !ancestor->layout_node()->can_have_children())
-                return;
-        }
-
-        // Okay, we need a new layout subtree here.
-        Layout::TreeBuilder tree_builder;
-        (void)tree_builder.build(*this);
-        return;
-    }
-
-    auto diff = StyleDifference::NeedsRelayout;
-    if (old_specified_css_values && layout_node())
-        diff = compute_style_difference(*old_specified_css_values, *new_specified_css_values, *layout_node());
-    if (diff == StyleDifference::None)
-        return;
-    layout_node()->apply_style(*new_specified_css_values);
-    if (diff == StyleDifference::NeedsRelayout) {
-        document().set_needs_layout();
-        return;
-    }
-    if (diff == StyleDifference::NeedsRepaint) {
-        layout_node()->set_needs_display();
-    }
+    document().invalidate_layout();
 }
 
 NonnullRefPtr<CSS::StyleProperties> Element::computed_style()