Title: [278346] trunk
Revision
278346
Author
[email protected]
Date
2021-06-02 02:41:43 -0700 (Wed, 02 Jun 2021)

Log Message

REGRESSION(r276882): Style not invalidated correctly for media queries in shadow trees that share style
https://bugs.webkit.org/show_bug.cgi?id=226369
<rdar://problem/78684562>

Reviewed by Sam Weinig.

Source/WebCore:

Style resolvers are stateful in respect to media queries. We would only invalidate in the first shadow tree
because that evaluation flipped the state and the second evaluation would see nothing changing in media query
state.

* style/StyleScope.cpp:
(WebCore::Style::Scope::collectResolverScopes):

Add a helper to collect all scopes associated with a resolver.

(WebCore::Style::Scope::evaluateMediaQueries):

Only evaluate each resolver once, then invalidate all associated scopes if needed.

* style/StyleScope.h:

LayoutTests:

Expand the existing test to have multiple shadow trees sharing style.

* fast/shadow-dom/media-query-in-shadow-style-expected.html:
* fast/shadow-dom/media-query-in-shadow-style.html:
* fast/shadow-dom/resources/media-query-in-shadow-style-frame.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (278345 => 278346)


--- trunk/LayoutTests/ChangeLog	2021-06-02 09:07:34 UTC (rev 278345)
+++ trunk/LayoutTests/ChangeLog	2021-06-02 09:41:43 UTC (rev 278346)
@@ -1,3 +1,17 @@
+2021-06-02  Antti Koivisto  <[email protected]>
+
+        REGRESSION(r276882): Style not invalidated correctly for media queries in shadow trees that share style
+        https://bugs.webkit.org/show_bug.cgi?id=226369
+        <rdar://problem/78684562>
+
+        Reviewed by Sam Weinig.
+
+        Expand the existing test to have multiple shadow trees sharing style.
+
+        * fast/shadow-dom/media-query-in-shadow-style-expected.html:
+        * fast/shadow-dom/media-query-in-shadow-style.html:
+        * fast/shadow-dom/resources/media-query-in-shadow-style-frame.html:
+
 2021-06-02  Youenn Fablet  <[email protected]>
 
         Fix race condition in binary_wsh.py

Modified: trunk/LayoutTests/fast/shadow-dom/media-query-in-shadow-style-expected.html (278345 => 278346)


--- trunk/LayoutTests/fast/shadow-dom/media-query-in-shadow-style-expected.html	2021-06-02 09:07:34 UTC (rev 278345)
+++ trunk/LayoutTests/fast/shadow-dom/media-query-in-shadow-style-expected.html	2021-06-02 09:41:43 UTC (rev 278346)
@@ -1,4 +1,4 @@
-<iframe src="" style='color:red'>Text</div>" width=150 height=50></iframe><br>
-<iframe src="" style='color:green;background-color: lightgrey'>Text</div>" width=300 height=50></iframe><br>
-<iframe src="" style='color:red'>Text</div>" width=150 height=50></iframe><br>
-<iframe src="" style='color:green;background-color: lightgrey'>Text</div>" width=300 height=50></iframe><br>
+<iframe src="" style='color:red'>First</div><div style='color:red'>Second</div>" width=150 height=100></iframe><br>
+<iframe src="" style='color:green;background-color: lightgrey'>First</div><div style='color:green;background-color: lightgrey'>Second</div>" width=300 height=100></iframe><br>
+<iframe src="" style='color:red'>First</div><div style='color:red'>Second</div>" width=150 height=100></iframe><br>
+<iframe src="" style='color:green;background-color: lightgrey'>First</div><div style='color:green;background-color: lightgrey'>Second</div>" width=300 height=100></iframe><br>

Modified: trunk/LayoutTests/fast/shadow-dom/media-query-in-shadow-style.html (278345 => 278346)


--- trunk/LayoutTests/fast/shadow-dom/media-query-in-shadow-style.html	2021-06-02 09:07:34 UTC (rev 278345)
+++ trunk/LayoutTests/fast/shadow-dom/media-query-in-shadow-style.html	2021-06-02 09:41:43 UTC (rev 278346)
@@ -1,7 +1,7 @@
-<iframe src="" width=150 height=50></iframe><br>
-<iframe src="" width=300 height=50></iframe><br>
-<iframe src="" width=300 height=50 _onload_='resize(event.target, 150)'></iframe><br>
-<iframe src="" width=150 height=50 _onload_='resize(event.target, 300)'></iframe><br>
+<iframe src="" width=150 height=100></iframe><br>
+<iframe src="" width=300 height=100></iframe><br>
+<iframe src="" width=300 height=100 _onload_='resize(event.target, 150)'></iframe><br>
+<iframe src="" width=150 height=100 _onload_='resize(event.target, 300)'></iframe><br>
 <script>
 if (window.testRunner)
     testRunner.waitUntilDone();

Modified: trunk/LayoutTests/fast/shadow-dom/resources/media-query-in-shadow-style-frame.html (278345 => 278346)


--- trunk/LayoutTests/fast/shadow-dom/resources/media-query-in-shadow-style-frame.html	2021-06-02 09:07:34 UTC (rev 278345)
+++ trunk/LayoutTests/fast/shadow-dom/resources/media-query-in-shadow-style-frame.html	2021-06-02 09:41:43 UTC (rev 278346)
@@ -1,16 +1,19 @@
-<div id=test><span>Text</span></div>
+<div class=test><span>First</span></div>
+<div class=test><span>Second</span></div>
 <script>
-const shadow = test.attachShadow({mode: 'open'});
-shadow.innerHTML = `
-    <style>
-    @media (min-width:200px) {
-        div { color: green }
-        :host { background-color: lightgrey }
-    }
-    @media (max-width:200px) {
-        ::slotted(*) { color: red }
-    }
-    </style>
-    <div><slot></slot></div>
-`;
+for (test of document.querySelectorAll('.test')) {
+    const shadow = test.attachShadow({mode: 'open'});
+    shadow.innerHTML = `
+        <style>
+        @media (min-width:200px) {
+            div { color: green }
+            :host { background-color: lightgrey }
+        }
+        @media (max-width:200px) {
+            ::slotted(*) { color: red }
+        }
+        </style>
+        <div><slot></slot></div>
+    `;
+}
 </script>

Modified: trunk/Source/WebCore/ChangeLog (278345 => 278346)


--- trunk/Source/WebCore/ChangeLog	2021-06-02 09:07:34 UTC (rev 278345)
+++ trunk/Source/WebCore/ChangeLog	2021-06-02 09:41:43 UTC (rev 278346)
@@ -1,3 +1,26 @@
+2021-06-02  Antti Koivisto  <[email protected]>
+
+        REGRESSION(r276882): Style not invalidated correctly for media queries in shadow trees that share style
+        https://bugs.webkit.org/show_bug.cgi?id=226369
+        <rdar://problem/78684562>
+
+        Reviewed by Sam Weinig.
+
+        Style resolvers are stateful in respect to media queries. We would only invalidate in the first shadow tree
+        because that evaluation flipped the state and the second evaluation would see nothing changing in media query
+        state.
+
+        * style/StyleScope.cpp:
+        (WebCore::Style::Scope::collectResolverScopes):
+
+        Add a helper to collect all scopes associated with a resolver.
+
+        (WebCore::Style::Scope::evaluateMediaQueries):
+
+        Only evaluate each resolver once, then invalidate all associated scopes if needed.
+
+        * style/StyleScope.h:
+
 2021-05-28  Antoine Quint  <[email protected]>
 
         Hit-testing does not account for clip-path on <iframe>

Modified: trunk/Source/WebCore/style/StyleScope.cpp (278345 => 278346)


--- trunk/Source/WebCore/style/StyleScope.cpp	2021-06-02 09:07:34 UTC (rev 278345)
+++ trunk/Source/WebCore/style/StyleScope.cpp	2021-06-02 09:41:43 UTC (rev 278346)
@@ -652,33 +652,55 @@
     });
 }
 
+auto Scope::collectResolverScopes() -> ResolverScopes
+{
+    ASSERT(!m_shadowRoot);
+
+    if (!resolverIfExists())
+        return { };
+
+    ResolverScopes resolverScopes;
+
+    resolverScopes.add(makeRef(*resolverIfExists()), Vector<WeakPtr<Scope>> { makeWeakPtr(*this) });
+
+    for (auto* shadowRoot : m_document.inDocumentShadowRoots()) {
+        auto& scope = shadowRoot->styleScope();
+        auto* resolver = scope.resolverIfExists();
+        if (!resolver)
+            continue;
+        resolverScopes.add(makeRef(*resolver), Vector<WeakPtr<Scope>> { }).iterator->value.append(makeWeakPtr(scope));
+    }
+    return resolverScopes;
+}
+
 template <typename TestFunction>
 void Scope::evaluateMediaQueries(TestFunction&& testFunction)
 {
-    auto* resolver = resolverIfExists();
-    if (!resolver)
-        return;
+    bool hadChanges = false;
 
-    auto evaluationChanges = testFunction(*resolver);
-    if (evaluationChanges) {
-        switch (evaluationChanges->type) {
-        case DynamicMediaQueryEvaluationChanges::Type::InvalidateStyle: {
-            Invalidator invalidator(evaluationChanges->invalidationRuleSets);
-            invalidator.invalidateStyle(*this);
-            break;
+    auto resolverScopes = collectResolverScopes();
+    for (auto& [resolver, scopes] : resolverScopes) {
+        auto evaluationChanges = testFunction(resolver.get());
+        if (!evaluationChanges)
+            continue;
+        hadChanges = true;
+
+        for (auto& scope : scopes) {
+            switch (evaluationChanges->type) {
+            case DynamicMediaQueryEvaluationChanges::Type::InvalidateStyle: {
+                Invalidator invalidator(evaluationChanges->invalidationRuleSets);
+                invalidator.invalidateStyle(*scope);
+                break;
+            }
+            case DynamicMediaQueryEvaluationChanges::Type::ResetStyle:
+                scope->scheduleUpdate(UpdateType::ContentsOrInterpretation);
+                break;
+            }
         }
-        case DynamicMediaQueryEvaluationChanges::Type::ResetStyle:
-            scheduleUpdate(UpdateType::ContentsOrInterpretation);
-            break;
-        }
+    }
 
+    if (hadChanges)
         InspectorInstrumentation::mediaQueryResultChanged(m_document);
-    }
-
-    if (!m_shadowRoot) {
-        for (auto* descendantShadowRoot : m_document.inDocumentShadowRoots())
-            descendantShadowRoot->styleScope().evaluateMediaQueries(testFunction);
-    }
 }
 
 void Scope::didChangeActiveStyleSheetCandidates()

Modified: trunk/Source/WebCore/style/StyleScope.h (278345 => 278346)


--- trunk/Source/WebCore/style/StyleScope.h	2021-06-02 09:07:34 UTC (rev 278345)
+++ trunk/Source/WebCore/style/StyleScope.h	2021-06-02 09:41:43 UTC (rev 278346)
@@ -35,6 +35,7 @@
 #include <wtf/ListHashSet.h>
 #include <wtf/RefPtr.h>
 #include <wtf/Vector.h>
+#include <wtf/WeakPtr.h>
 #include <wtf/text/WTFString.h>
 
 namespace WebCore {
@@ -54,7 +55,7 @@
 
 class Resolver;
 
-class Scope {
+class Scope : public CanMakeWeakPtr<Scope> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     explicit Scope(Document&);
@@ -133,6 +134,8 @@
     void updateActiveStyleSheets(UpdateType);
     void scheduleUpdate(UpdateType);
 
+    using ResolverScopes = HashMap<Ref<Resolver>, Vector<WeakPtr<Scope>>>;
+    ResolverScopes collectResolverScopes();
     template <typename TestFunction> void evaluateMediaQueries(TestFunction&&);
 
     WEBCORE_EXPORT void flushPendingSelfUpdate();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to