Changeset 216761 in webkit

Timestamp:

May 12, 2017 5:31:49 AM (7 years ago)

Author:

Antti Koivisto

Message:

Updating class name of a shadow host does not update the style applied by descendants of :host()
​https://bugs.webkit.org/show_bug.cgi?id=170762
<rdar://problem/31572668>

Reviewed by Ryosuke Niwa.

Source/WebCore:

We need to invalidate shadow tree style when host classes or attributes change if it may be
affected by host rules.

Test: fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html

• css/RuleSet.cpp:

(WebCore::isHostSelectorMatchingInShadowTree):
(WebCore::RuleSet::addRule):

Check if we have :host selectors that affect shadow tree.

• css/RuleSet.h:

(WebCore::RuleSet::hasHostPseudoClassRulesMatchingInShadowTree):

• style/AttributeChangeInvalidation.cpp:

(WebCore::Style::mayBeAffectedByHostRules):
(WebCore::Style::AttributeChangeInvalidation::invalidateStyle):

Invalidate the whole subtree if there is a class change that may affect shadow tree style.

• style/ClassChangeInvalidation.cpp:

(WebCore::Style::mayBeAffectedByHostRules):
(WebCore::Style::ClassChangeInvalidation::invalidateStyle):

• style/IdChangeInvalidation.cpp:

(WebCore::Style::mayBeAffectedByHostRules):
(WebCore::Style::IdChangeInvalidation::invalidateStyle):

Same for classes and ids.
This should be refactored at some point to reduce copy-code.

LayoutTests:

• fast/shadow-dom/css-scoping-host-class-and-attribute-mutation-expected.html: Added.
• fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/shadow-dom/css-scoping-host-class-and-attribute-mutation-expected.html (added)
• LayoutTests/fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/RuleSet.cpp (modified) (2 diffs)
• Source/WebCore/css/RuleSet.h (modified) (2 diffs)
• Source/WebCore/style/AttributeChangeInvalidation.cpp (modified) (3 diffs)
• Source/WebCore/style/ClassChangeInvalidation.cpp (modified) (4 diffs)
• Source/WebCore/style/IdChangeInvalidation.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-05-12  Antti Koivisto  <antti@apple.com>
+
+        Updating class name of a shadow host does not update the style applied by descendants of :host()
+        https://bugs.webkit.org/show_bug.cgi?id=170762
+        <rdar://problem/31572668>
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/shadow-dom/css-scoping-host-class-and-attribute-mutation-expected.html: Added.
+        * fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html: Added.
+
 2017-05-12  Claudio Saavedra  <csaavedra@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-05-12  Antti Koivisto  <antti@apple.com>
+
+        Updating class name of a shadow host does not update the style applied by descendants of :host()
+        https://bugs.webkit.org/show_bug.cgi?id=170762
+        <rdar://problem/31572668>
+
+        Reviewed by Ryosuke Niwa.
+
+        We need to invalidate shadow tree style when host classes or attributes change if it may be
+        affected by host rules.
+
+        Test: fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html
+
+        * css/RuleSet.cpp:
+        (WebCore::isHostSelectorMatchingInShadowTree):
+        (WebCore::RuleSet::addRule):
+
+            Check if we have :host selectors that affect shadow tree.
+
+        * css/RuleSet.h:
+        (WebCore::RuleSet::hasHostPseudoClassRulesMatchingInShadowTree):
+        * style/AttributeChangeInvalidation.cpp:
+        (WebCore::Style::mayBeAffectedByHostRules):
+        (WebCore::Style::AttributeChangeInvalidation::invalidateStyle):
+
+            Invalidate the whole subtree if there is a class change that may affect shadow tree style.
+
+        * style/ClassChangeInvalidation.cpp:
+        (WebCore::Style::mayBeAffectedByHostRules):
+        (WebCore::Style::ClassChangeInvalidation::invalidateStyle):
+        * style/IdChangeInvalidation.cpp:
+        (WebCore::Style::mayBeAffectedByHostRules):
+        (WebCore::Style::IdChangeInvalidation::invalidateStyle):
+
+            Same for classes and ids.
+            This should be refactored at some point to reduce copy-code.
+
 2017-05-12  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebCore/css/RuleSet.cpp

@@ -195 +195 @@
 }
 
+static bool isHostSelectorMatchingInShadowTree(const CSSSelector& startSelector)
+{
+    auto* leftmostSelector = &startSelector;
+    bool hasDescendantOrChildRelation = false;
+    while (auto* previous = leftmostSelector->tagHistory()) {
+        hasDescendantOrChildRelation = leftmostSelector->hasDescendantOrChildRelation();
+        leftmostSelector = previous;
+    }
+    if (!hasDescendantOrChildRelation)
+        return false;
+
+    return leftmostSelector->match() == CSSSelector::PseudoClass && leftmostSelector->pseudoClassType() == CSSSelector::PseudoClassHost;
+}
+
 void RuleSet::addRule(StyleRule* rule, unsigned selectorIndex, AddRuleFlags addRuleFlags)
 {
@@ -309 +323 @@
         return;
     }
+
+    if (!m_hasHostPseudoClassRulesMatchingInShadowTree)
+        m_hasHostPseudoClassRulesMatchingInShadowTree = isHostSelectorMatchingInShadowTree(*ruleData.selector());
 
     if (hostPseudoClassSelector) {

trunk/Source/WebCore/css/RuleSet.h

@@ -193 +193 @@
 
     bool hasShadowPseudoElementRules() const;
+    bool hasHostPseudoClassRulesMatchingInShadowTree() const { return m_hasHostPseudoClassRulesMatchingInShadowTree; }
 
 private:
@@ -212 +213 @@
     Vector<StyleRulePage*> m_pageRules;
     unsigned m_ruleCount { 0 };
+    bool m_hasHostPseudoClassRulesMatchingInShadowTree { false };
     bool m_autoShrinkToFitEnabled { true };
     RuleFeatureSet m_features;

trunk/Source/WebCore/style/AttributeChangeInvalidation.cpp

@@ -43 +43 @@
 }
 
-static bool mayBeAffectedByHostRules(const Element& element, const QualifiedName& attributeName)
+static bool mayBeAffectedByHostRules(const Element& element, const QualifiedName& attributeName, bool& mayAffectShadowTree)
 {
+    // FIXME: More of this code should be shared between Class/Attribute/IdInvalidation.
     auto* shadowRoot = element.shadowRoot();
     if (!shadowRoot)
         return false;
     auto& shadowRuleSets = shadowRoot->styleScope().resolver().ruleSets();
-    if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty())
+    auto& authorStyle = shadowRuleSets.authorStyle();
+    if (authorStyle.hostPseudoClassRules().isEmpty() && !authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
         return false;
 
-    return mayBeAffectedByAttributeChange(shadowRuleSets, element.isHTMLElement(), attributeName);
+    if (!mayBeAffectedByAttributeChange(shadowRuleSets, element.isHTMLElement(), attributeName))
+        return false;
+
+    if (authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
+        mayAffectShadowTree = true;
+    return true;
 }
 
@@ -74 +81 @@
     auto& ruleSets = m_element.styleResolver().ruleSets();
     bool isHTML = m_element.isHTMLElement();
+    bool mayAffectShadowTree = false;
 
     bool mayAffectStyle = mayBeAffectedByAttributeChange(ruleSets, isHTML, attributeName)
-        || mayBeAffectedByHostRules(m_element, attributeName)
+        || mayBeAffectedByHostRules(m_element, attributeName, mayAffectShadowTree)
         || mayBeAffectedBySlottedRules(m_element, attributeName);
 
@@ -87 +95 @@
     }
 
-    if (m_element.shadowRoot() && ruleSets.authorStyle().hasShadowPseudoElementRules()) {
+    if (m_element.shadowRoot() && ruleSets.authorStyle().hasShadowPseudoElementRules())
+        mayAffectShadowTree = true;
+
+    if (mayAffectShadowTree) {
         m_element.invalidateStyleForSubtree();
         return;

trunk/Source/WebCore/style/ClassChangeInvalidation.cpp

@@ -87 +87 @@
 }
 
-static bool mayBeAffectedByHostRules(ShadowRoot* shadowRoot, AtomicStringImpl* changedClass)
+static bool mayBeAffectedByHostRules(ShadowRoot* shadowRoot, AtomicStringImpl* changedClass, bool& mayAffectShadowTree)
 {
+    // FIXME: More of this code should be shared between Class/Attribute/IdInvalidation.
     if (!shadowRoot)
         return false;
     auto& shadowRuleSets = shadowRoot->styleScope().resolver().ruleSets();
-    if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty())
+    auto& authorStyle = shadowRuleSets.authorStyle();
+    if (authorStyle.hostPseudoClassRules().isEmpty() && !authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
         return false;
-    return shadowRuleSets.features().classesInRules.contains(changedClass);
+
+    if (!shadowRuleSets.features().classesInRules.contains(changedClass))
+        return false;
+
+    if (authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
+        mayAffectShadowTree = true;
+    return true;
 }
 
@@ -112 +120 @@
 {
     auto changedClasses = computeClassChange(oldClasses, newClasses);
+    bool mayAffectShadowTree = false;
 
     auto& ruleSets = m_element.styleResolver().ruleSets();
@@ -120 +129 @@
     for (auto* changedClass : changedClasses) {
         bool mayAffectStyle = ruleSets.features().classesInRules.contains(changedClass)
-            || mayBeAffectedByHostRules(shadowRoot, changedClass)
+            || mayBeAffectedByHostRules(shadowRoot, changedClass, mayAffectShadowTree)
             || mayBeAffectedBySlottedRules(assignedShadowRoots, changedClass);
         if (mayAffectStyle)
@@ -129 +138 @@
         return;
 
-    if (shadowRoot && ruleSets.authorStyle().hasShadowPseudoElementRules()) {
+    if (shadowRoot && ruleSets.authorStyle().hasShadowPseudoElementRules())
+        mayAffectShadowTree = true;
+
+    if (mayAffectShadowTree) {
+        // FIXME: We should do fine-grained invalidation for shadow tree.
         m_element.invalidateStyleForSubtree();
         return;

trunk/Source/WebCore/style/IdChangeInvalidation.cpp

@@ -36 +36 @@
 namespace Style {
 
-static bool mayBeAffectedByHostRules(const Element& element, const AtomicString& changedId)
+static bool mayBeAffectedByHostRules(const Element& element, const AtomicString& changedId, bool& mayAffectShadowTree)
 {
     auto* shadowRoot = element.shadowRoot();
@@ -42 +42 @@
         return false;
     auto& shadowRuleSets = shadowRoot->styleScope().resolver().ruleSets();
-    if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty())
+    auto& authorStyle = shadowRuleSets.authorStyle();
+    if (authorStyle.hostPseudoClassRules().isEmpty() && !authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
         return false;
-    return shadowRuleSets.features().idsInRules.contains(changedId);
+
+    if (!shadowRuleSets.features().idsInRules.contains(changedId))
+        return false;
+
+    if (authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
+        mayAffectShadowTree = true;
+    return true;
 }
 
@@ -65 +72 @@
 
     auto& ruleSets = m_element.styleResolver().ruleSets();
+    bool mayAffectShadowTree = false;
 
     bool mayAffectStyle = ruleSets.features().idsInRules.contains(changedId)
-        || mayBeAffectedByHostRules(m_element, changedId)
+        || mayBeAffectedByHostRules(m_element, changedId, mayAffectShadowTree)
         || mayBeAffectedBySlottedRules(m_element, changedId);
 
@@ -73 +81 @@
         return;
 
-    if (m_element.shadowRoot() && ruleSets.authorStyle().hasShadowPseudoElementRules()) {
+    if (m_element.shadowRoot() && ruleSets.authorStyle().hasShadowPseudoElementRules())
+        mayAffectShadowTree = true;
+
+    if (mayAffectShadowTree) {
         m_element.invalidateStyleForSubtree();
         return;