Skip to content

Commit

Permalink
Shadow DOM: Toggling class in .class ::slotted(*) does not trigger …
Browse files Browse the repository at this point in the history
…style recalc

https://bugs.webkit.org/show_bug.cgi?id=160864

Reviewed by Ryosuke Niwa.

Source/WebCore:

Also fix similar issue with ::host

Test: fast/shadow-dom/css-scoping-host-and-slotted-context-invalidation.html

* css/StyleInvalidationAnalysis.cpp:
(WebCore::StyleInvalidationAnalysis::invalidateIfNeeded):

    If we have ::slotted rules and encounter a <slot>, invalidate the slotted host children.

(WebCore::StyleInvalidationAnalysis::invalidateStyle):

    Invalidate the shadow host if we have ::host rules.

* css/StyleInvalidationAnalysis.h:
* dom/InlineStyleSheetOwner.cpp:
(WebCore::InlineStyleSheetOwner::createSheet):

    Fix a bug where it was possible to mutate stylesheets in the inline stylesheet cache.
    The included test covers this.

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

    Handle the full invalidation case.

LayoutTests:

* fast/shadow-dom/css-scoping-host-and-slotted-context-invalidation-expected.html: Added.
* fast/shadow-dom/css-scoping-host-and-slotted-context-invalidation.html: Added.


Canonical link: https://commits.webkit.org/182333@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@208610 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
anttijk committed Nov 11, 2016
1 parent 4de5e29 commit bc128ef
Show file tree
Hide file tree
Showing 8 changed files with 164 additions and 1 deletion.
11 changes: 11 additions & 0 deletions LayoutTests/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
2016-11-11 Antti Koivisto <antti@apple.com>

Shadow DOM: Toggling class in `.class ::slotted(*)` does not trigger style recalc
https://bugs.webkit.org/show_bug.cgi?id=160864

Reviewed by Ryosuke Niwa.

* fast/shadow-dom/css-scoping-host-and-slotted-context-invalidation-expected.html: Added.
* fast/shadow-dom/css-scoping-host-and-slotted-context-invalidation.html: Added.

2016-11-11 Eric Carlson <eric.carlson@apple.com>

[MediaStream] defer resolution of getUserMedia promise made in a background tab
Expand Down Expand Up @@ -1201,6 +1211,7 @@
* fast/shadow-dom/css-scoping-slot-with-id-expected.html: Added.
* fast/shadow-dom/css-scoping-slot-with-id.html: Added.


2016-11-04 Brady Eidson <beidson@apple.com>

IndexedDB 2.0: Use IDB-specific exceptions in places where the generic exceptions are currently used.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<!DOCTYPE html>
<html>
<body>
<p>Test passes if you see a single 100px by 100px green box below.</p>
<div style="width: 100px; height: 100px; background: green;"></div>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
<!DOCTYPE html>
<body>
<p>Test passes if you see a single 100px by 100px green box below.</p>
<div id="t1">
<span>FAIL</span>
</div>
<div id="t2">
<span slot="second">FAIL</span>
</div>
<div id="t3">
<span>FAIL</span>
</div>
<div id="t4">
<span>FAIL</span>
</div>
<div id="t5">
<span>FAIL</span>
</div>
<script>

function attachShadow(host)
{
const shadow = host.attachShadow({mode: 'closed'});
shadow.innerHTML = `
<style>
:host {
width: 100px;
height: 20px;
background: green;
color: red;
}
</style>
<div><slot></slot></div>
<div id="second-parent"><slot name="second"></slot></div>
`;
return shadow;
}

{
const host = document.querySelector('#t1');
const shadow = attachShadow(host);
const style = shadow.querySelector('style');
style.sheet.insertRule(".selected ::slotted(*) { color: green }");
getComputedStyle(host.querySelector('span')).backgroundColor;
const div = shadow.querySelector('div');
div.className = 'selected';
}

{
const host = document.querySelector('#t2');
const shadow = attachShadow(host);
getComputedStyle(host.querySelector('span')).backgroundColor;
const style = shadow.querySelector('style');
style.sheet.insertRule("#second-parent ::slotted(*) { color: green }");
}

{
const host = document.querySelector('#t3');
const shadow = attachShadow(host);
getComputedStyle(host.querySelector('span')).backgroundColor;
const style = document.createElement("style");
style.textContent = "div ::slotted(*) { color: green }";
shadow.appendChild(style);
}

{
const host = document.querySelector('#t4');
const shadow = attachShadow(host);
getComputedStyle(host.querySelector('span')).backgroundColor;
const style = shadow.querySelector('style');
style.sheet.insertRule(":host { color: green }", 1);
}

{
const host = document.querySelector('#t5');
const shadow = attachShadow(host);
getComputedStyle(host.querySelector('span')).backgroundColor;
const style = document.createElement("style");
style.textContent = ":host(div) { color: green }";
shadow.appendChild(style)
}

</script>
32 changes: 32 additions & 0 deletions Source/WebCore/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,35 @@
2016-11-11 Antti Koivisto <antti@apple.com>

Shadow DOM: Toggling class in `.class ::slotted(*)` does not trigger style recalc
https://bugs.webkit.org/show_bug.cgi?id=160864

Reviewed by Ryosuke Niwa.

Also fix similar issue with ::host

Test: fast/shadow-dom/css-scoping-host-and-slotted-context-invalidation.html

* css/StyleInvalidationAnalysis.cpp:
(WebCore::StyleInvalidationAnalysis::invalidateIfNeeded):

If we have ::slotted rules and encounter a <slot>, invalidate the slotted host children.

(WebCore::StyleInvalidationAnalysis::invalidateStyle):

Invalidate the shadow host if we have ::host rules.

* css/StyleInvalidationAnalysis.h:
* dom/InlineStyleSheetOwner.cpp:
(WebCore::InlineStyleSheetOwner::createSheet):

Fix a bug where it was possible to mutate stylesheets in the inline stylesheet cache.
The included test covers this.

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

Handle the full invalidation case.

2016-11-11 Brady Eidson <beidson@apple.com>

IndexedDB 2.0: "close pending flag" and firing blocked events all need fixing.
Expand Down
17 changes: 17 additions & 0 deletions Source/WebCore/css/StyleInvalidationAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "Document.h"
#include "ElementIterator.h"
#include "ElementRuleCollector.h"
#include "HTMLSlotElement.h"
#include "SelectorFilter.h"
#include "ShadowRoot.h"
#include "StyleRuleImport.h"
Expand Down Expand Up @@ -103,6 +104,17 @@ StyleInvalidationAnalysis::CheckDescendants StyleInvalidationAnalysis::invalidat
element.invalidateStyleForSubtree();
}

bool shouldCheckForSlots = !m_ruleSet.slottedPseudoElementRules().isEmpty() && !m_didInvalidateHostChildren;
if (shouldCheckForSlots && is<HTMLSlotElement>(element)) {
auto* containingShadowRoot = element.containingShadowRoot();
if (containingShadowRoot && containingShadowRoot->host()) {
for (auto& possiblySlotted : childrenOfType<Element>(*containingShadowRoot->host()))
possiblySlotted.invalidateStyle();
}
// No need to do this again.
m_didInvalidateHostChildren = true;
}

switch (element.styleValidity()) {
case Style::Validity::Valid: {
ElementRuleCollector ruleCollector(element, m_ruleSet, filter);
Expand All @@ -117,6 +129,8 @@ StyleInvalidationAnalysis::CheckDescendants StyleInvalidationAnalysis::invalidat
return CheckDescendants::Yes;
case Style::Validity::SubtreeInvalid:
case Style::Validity::SubtreeAndRenderersInvalid:
if (shouldCheckForSlots)
return CheckDescendants::Yes;
return CheckDescendants::No;
}
ASSERT_NOT_REACHED();
Expand Down Expand Up @@ -172,6 +186,9 @@ void StyleInvalidationAnalysis::invalidateStyle(ShadowRoot& shadowRoot)
{
ASSERT(!m_dirtiesAllStyle);

if (!m_ruleSet.hostPseudoClassRules().isEmpty() && shadowRoot.host())
shadowRoot.host()->invalidateStyle();

for (auto& child : childrenOfType<Element>(shadowRoot)) {
SelectorFilter filter;
invalidateStyleForTree(child, &filter);
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/css/StyleInvalidationAnalysis.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class StyleInvalidationAnalysis {
const RuleSet& m_ruleSet;
bool m_dirtiesAllStyle { false };
bool m_hasShadowPseudoElementRulesInAuthorSheet { false };
bool m_didInvalidateHostChildren { false };
};

}
Expand Down
5 changes: 4 additions & 1 deletion Source/WebCore/dom/InlineStyleSheetOwner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,15 @@ void InlineStyleSheetOwner::createSheet(Element& element, const String& text)
contents->checkLoaded();

if (cacheKey && contents->isCacheable()) {
m_sheet->contents().addedToMemoryCache();
inlineStyleSheetCache().add(*cacheKey, &m_sheet->contents());

// Prevent pathological growth.
const size_t maximumInlineStyleSheetCacheSize = 50;
if (inlineStyleSheetCache().size() > maximumInlineStyleSheetCacheSize)
if (inlineStyleSheetCache().size() > maximumInlineStyleSheetCacheSize) {
inlineStyleSheetCache().begin()->value->removedFromMemoryCache();
inlineStyleSheetCache().remove(inlineStyleSheetCache().begin());
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions Source/WebCore/style/StyleScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -407,10 +407,19 @@ void Scope::updateActiveStyleSheets(UpdateType updateType)
m_usesStyleBasedEditability = true;
}

// FIXME: Move this code somewhere else.
if (requiresFullStyleRecalc) {
if (m_shadowRoot) {
for (auto& shadowChild : childrenOfType<Element>(*m_shadowRoot))
shadowChild.invalidateStyleForSubtree();
if (m_shadowRoot->host()) {
if (!resolver().ruleSets().authorStyle().hostPseudoClassRules().isEmpty())
m_shadowRoot->host()->invalidateStyle();
if (!resolver().ruleSets().authorStyle().slottedPseudoElementRules().isEmpty()) {
for (auto& shadowChild : childrenOfType<Element>(*m_shadowRoot->host()))
shadowChild.invalidateStyle();
}
}
} else
m_document.scheduleForcedStyleRecalc();
}
Expand Down

0 comments on commit bc128ef

Please sign in to comment.