From fd24782d856ebe0f393c8e798bad7a7c2ab4da81 Mon Sep 17 00:00:00 2001
From: Sam Atkins <atkinssj@serenityos.org>
Date: Thu, 17 Feb 2022 12:34:36 +0000
Subject: [PATCH] LibWeb: Only invalidate styles if a `@media` rule changes
 match status

---
 Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp | 22 +++++++++++++------
 Userland/Libraries/LibWeb/CSS/CSSRuleList.h   |  3 ++-
 .../Libraries/LibWeb/CSS/CSSStyleSheet.cpp    |  4 ++--
 Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h |  3 ++-
 Userland/Libraries/LibWeb/DOM/Document.cpp    |  9 ++++----
 5 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp b/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp
index 984ed095c4..9ceed93f82 100644
--- a/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp
+++ b/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp
@@ -103,34 +103,42 @@ void CSSRuleList::for_each_effective_style_rule(Function<void(CSSStyleRule const
     }
 }
 
-void CSSRuleList::evaluate_media_queries(DOM::Window const& window)
+bool CSSRuleList::evaluate_media_queries(DOM::Window const& window)
 {
+    bool any_media_queries_changed_match_state = false;
+
     for (auto& rule : m_rules) {
         switch (rule.type()) {
         case CSSRule::Type::Style:
             break;
         case CSSRule::Type::Import: {
             auto& import_rule = verify_cast<CSSImportRule>(rule);
-            if (import_rule.has_import_result())
-                import_rule.loaded_style_sheet()->evaluate_media_queries(window);
+            if (import_rule.has_import_result() && import_rule.loaded_style_sheet()->evaluate_media_queries(window))
+                any_media_queries_changed_match_state = true;
             break;
         }
         case CSSRule::Type::Media: {
             auto& media_rule = verify_cast<CSSMediaRule>(rule);
-            if (media_rule.evaluate(window))
-                media_rule.css_rules().evaluate_media_queries(window);
+            bool did_match = media_rule.condition_matches();
+            bool now_matches = media_rule.evaluate(window);
+            if (did_match != now_matches)
+                any_media_queries_changed_match_state = true;
+            if (now_matches && media_rule.css_rules().evaluate_media_queries(window))
+                any_media_queries_changed_match_state = true;
             break;
         }
         case CSSRule::Type::Supports: {
             auto& supports_rule = verify_cast<CSSSupportsRule>(rule);
-            if (supports_rule.condition_matches())
-                supports_rule.css_rules().evaluate_media_queries(window);
+            if (supports_rule.condition_matches() && supports_rule.css_rules().evaluate_media_queries(window))
+                any_media_queries_changed_match_state = true;
             break;
         }
         case CSSRule::Type::__Count:
             VERIFY_NOT_REACHED();
         }
     }
+
+    return any_media_queries_changed_match_state;
 }
 
 }
diff --git a/Userland/Libraries/LibWeb/CSS/CSSRuleList.h b/Userland/Libraries/LibWeb/CSS/CSSRuleList.h
index a8a76cbb4f..4c2fa5d9bd 100644
--- a/Userland/Libraries/LibWeb/CSS/CSSRuleList.h
+++ b/Userland/Libraries/LibWeb/CSS/CSSRuleList.h
@@ -52,7 +52,8 @@ public:
     DOM::ExceptionOr<unsigned> insert_a_css_rule(NonnullRefPtr<CSSRule>, u32 index);
 
     void for_each_effective_style_rule(Function<void(CSSStyleRule const&)> const& callback) const;
-    void evaluate_media_queries(DOM::Window const&);
+    // Returns whether the match state of any media queries changed after evaluation.
+    bool evaluate_media_queries(DOM::Window const&);
 
 private:
     explicit CSSRuleList(NonnullRefPtrVector<CSSRule>&&);
diff --git a/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.cpp b/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.cpp
index 783a84ae31..5aa5a03bd3 100644
--- a/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.cpp
+++ b/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.cpp
@@ -62,9 +62,9 @@ void CSSStyleSheet::for_each_effective_style_rule(Function<void(CSSStyleRule con
     m_rules->for_each_effective_style_rule(callback);
 }
 
-void CSSStyleSheet::evaluate_media_queries(DOM::Window const& window)
+bool CSSStyleSheet::evaluate_media_queries(DOM::Window const& window)
 {
-    m_rules->evaluate_media_queries(window);
+    return m_rules->evaluate_media_queries(window);
 }
 
 }
diff --git a/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h b/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h
index 6ce28b1735..30d38337e5 100644
--- a/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h
+++ b/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h
@@ -47,7 +47,8 @@ public:
     DOM::ExceptionOr<void> delete_rule(unsigned index);
 
     void for_each_effective_style_rule(Function<void(CSSStyleRule const&)> const& callback) const;
-    void evaluate_media_queries(DOM::Window const&);
+    // Returns whether the match state of any media queries changed after evaluation.
+    bool evaluate_media_queries(DOM::Window const&);
 
 private:
     explicit CSSStyleSheet(NonnullRefPtrVector<CSSRule>);
diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp
index 3a992f8d34..e08d7ff42c 100644
--- a/Userland/Libraries/LibWeb/DOM/Document.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Document.cpp
@@ -1161,13 +1161,14 @@ void Document::evaluate_media_queries_and_report_changes()
     }
 
     // Also not in the spec, but this is as good a place as any to evaluate @media rules!
+    bool any_media_queries_changed_match_state = false;
     for (auto& style_sheet : style_sheets().sheets()) {
-        style_sheet.evaluate_media_queries(window());
+        if (style_sheet.evaluate_media_queries(window()))
+            any_media_queries_changed_match_state = true;
     }
 
-    // FIXME: This invalidates too often!
-    //        We should only invalidate when one or more @media rules changes evaluation status.
-    style_computer().invalidate_rule_cache();
+    if (any_media_queries_changed_match_state)
+        style_computer().invalidate_rule_cache();
 }
 
 NonnullRefPtr<DOMImplementation> Document::implementation() const