Skip to content

GitLab

T
tangled
Commit 7c223695 authored by chrishtr@chromium.org's avatar chrishtr@chromium.org
Browse files
Options

Don't schedule invalidations when attributes changed if not needed or incorrect. 

In particular, don't do it  when attributes changed if there
is no style resolver or the style change type is subtree or
greater. In both of these cases, it will either have no
additional effect or the code is incorrect.

It could be incorrect if the element was not yet attached.
It could be not needed because if there is no style resolver,
re-making it will recalc the whole document's style. Also,
style invalidation cannot trigger a style that is greater than
subtree.

Also clear style invalidation bits unconditionally in Node::detach.

BUG=366788

Review URL: https://codereview.chromium.org/273783003

git-svn-id: svn://svn.chromium.org/blink/trunk@173665 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent ac928976
Show whitespace changes
Inline Side-by-side
Showing with 92 additions and 23 deletions
third_party/WebKit/LayoutTests/fast/css/invalidation/style-invalidation-before-attach-expected.html 0 → 100644
View file @ 7c223695
<!DOCTYPE html>
<style>
[attr] div {
color: red;
}
[attr] .baz {
color: blue;
}
</style>
<div id="a">
<div id="bar" attr="attr">
<div id="baz" class="baz">
This should be blue.
</div>
</div>
</div>
third_party/WebKit/LayoutTests/fast/css/invalidation/style-invalidation-before-attach.html 0 → 100644
View file @ 7c223695
<!DOCTYPE html>
<style>
[attr] div {
color: red;
}
[attr] .baz {
color: blue;
}
</style>
<div id="a"></div>
<script>
onload = function() {
document.body.offsetTop;
var foo = document.createElement("div");
foo.id = "foo";
foo.innerHTML = "<div id=bar><div id=baz>This should be blue.</div></div>";
// Append foo but not attach it.
a.appendChild(foo);
// Schedule invalidation on bar which sets childNeedsStyleInvalidation on foo.
bar.setAttribute("attr", "attr");
// Remove from the tree and clear all invalidation bits, but foo still has them
// since it's not in the tree.
foo.remove();
document.body.offsetTop;
// Add foo back and attach it.
a.appendChild(foo);
document.body.offsetTop;
// Schedule invalidation, but it won't get above foo since the bits are
// incorrectly set.
baz.className = "baz";
};
</script>
\ No newline at end of file
third_party/WebKit/Source/core/css/invalidation/StyleInvalidator.cpp
View file @ 7c223695
......@@ -28,6 +28,8 @@ void StyleInvalidator::invalidate(Document& document)
void StyleInvalidator::scheduleInvalidation(PassRefPtr<DescendantInvalidationSet> invalidationSet, Element& element)
{
ASSERT(element.inActiveDocument());
ASSERT(element.styleChangeType() < SubtreeStyleChange);
InvalidationList& list = ensurePendingInvalidationList(element);
// If we're already going to invalidate the whole subtree we don't need to store any new sets.
if (!list.isEmpty() && list.last()->wholeSubtreeInvalid())
......@@ -51,8 +53,6 @@ void StyleInvalidator::clearInvalidation(Node& node)
{
if (node.isElementNode() && node.needsStyleInvalidation())
m_pendingInvalidationMap.remove(toElement(&node));
node.clearChildNeedsStyleInvalidation();
node.clearNeedsStyleInvalidation();
}
void StyleInvalidator::clearPendingInvalidations()
......
third_party/WebKit/Source/core/dom/ContainerNode.cpp
View file @ 7c223695
......@@ -829,12 +829,14 @@ void ContainerNode::focusStateChanged()
if (!renderer())
return;
if (styleChangeType() < SubtreeStyleChange) {
if (renderStyle()->affectedByFocus() && renderStyle()->hasPseudoStyle(FIRST_LETTER))
setNeedsStyleRecalc(SubtreeStyleChange);
else if (isElementNode() && toElement(this)->childrenAffectedByFocus())
document().ensureStyleResolver().ensureUpdatedRuleFeatureSet().scheduleStyleInvalidationForPseudoChange(CSSSelector::PseudoFocus, *toElement(this));
else if (renderStyle()->affectedByFocus())
setNeedsStyleRecalc(LocalStyleChange);
}
if (renderer() && renderer()->style()->hasAppearance())
RenderTheme::theme().stateChanged(renderer(), FocusState);
......@@ -853,7 +855,7 @@ void ContainerNode::setFocus(bool received)
return;
// If :focus sets display: none, we lose focus but still need to recalc our style.
if (isElementNode() && toElement(this)->childrenAffectedByFocus())
if (isElementNode() && toElement(this)->childrenAffectedByFocus() && styleChangeType() < SubtreeStyleChange)
document().ensureStyleResolver().ensureUpdatedRuleFeatureSet().scheduleStyleInvalidationForPseudoChange(CSSSelector::PseudoFocus, *toElement(this));
else
setNeedsStyleRecalc(LocalStyleChange);
......@@ -868,12 +870,14 @@ void ContainerNode::setActive(bool down)
// FIXME: Why does this not need to handle the display: none transition like :hover does?
if (renderer()) {
if (styleChangeType() < SubtreeStyleChange) {
if (renderStyle()->affectedByActive() && renderStyle()->hasPseudoStyle(FIRST_LETTER))
setNeedsStyleRecalc(SubtreeStyleChange);
else if (isElementNode() && toElement(this)->childrenAffectedByActive())
document().ensureStyleResolver().ensureUpdatedRuleFeatureSet().scheduleStyleInvalidationForPseudoChange(CSSSelector::PseudoActive, *toElement(this));
else if (renderStyle()->affectedByActive())
setNeedsStyleRecalc(LocalStyleChange);
}
if (renderStyle()->hasAppearance())
RenderTheme::theme().stateChanged(renderer(), PressedState);
......@@ -891,19 +895,21 @@ void ContainerNode::setHovered(bool over)
if (!renderer()) {
if (over)
return;
if (isElementNode() && toElement(this)->childrenAffectedByHover())
if (isElementNode() && toElement(this)->childrenAffectedByHover() && styleChangeType() < SubtreeStyleChange)
document().ensureStyleResolver().ensureUpdatedRuleFeatureSet().scheduleStyleInvalidationForPseudoChange(CSSSelector::PseudoHover, *toElement(this));
else
setNeedsStyleRecalc(LocalStyleChange);
return;
}
if (styleChangeType() < SubtreeStyleChange) {
if (renderStyle()->affectedByHover() && renderStyle()->hasPseudoStyle(FIRST_LETTER))
setNeedsStyleRecalc(SubtreeStyleChange);
else if (isElementNode() && toElement(this)->childrenAffectedByHover())
document().ensureStyleResolver().ensureUpdatedRuleFeatureSet().scheduleStyleInvalidationForPseudoChange(CSSSelector::PseudoHover, *toElement(this));
else if (renderStyle()->affectedByHover())
setNeedsStyleRecalc(LocalStyleChange);
}
if (renderer()->style()->hasAppearance())
RenderTheme::theme().stateChanged(renderer(), HoverState);
......
third_party/WebKit/Source/core/dom/Element.cpp
View file @ 7c223695
......@@ -2800,7 +2800,7 @@ void Element::willModifyAttribute(const QualifiedName& name, const AtomicString&
}
if (oldValue != newValue) {
if (inActiveDocument())
if (inActiveDocument() && document().styleResolver() && styleChangeType() < SubtreeStyleChange)
document().ensureStyleResolver().ensureUpdatedRuleFeatureSet().scheduleStyleInvalidationForAttributeChange(name, *this);
if (isUpgradedCustomElement())
......
third_party/WebKit/Source/core/dom/Element.h
View file @ 7c223695
......@@ -816,6 +816,8 @@ inline bool Node::hasClass() const
inline Node::InsertionNotificationRequest Node::insertedInto(ContainerNode* insertionPoint)
{
ASSERT(!childNeedsStyleInvalidation());
ASSERT(!needsStyleInvalidation());
ASSERT(insertionPoint->inDocument() || isContainerNode());
if (insertionPoint->inDocument())
setFlag(InDocumentFlag);
......
third_party/WebKit/Source/core/dom/Node.cpp
View file @ 7c223695
......@@ -1022,6 +1022,8 @@ void Node::detach(const AttachContext& context)
if (StyleResolver* resolver = document().styleResolver())
resolver->ruleFeatureSet().styleInvalidator().clearInvalidation(*this);
clearChildNeedsStyleInvalidation();
clearNeedsStyleInvalidation();
#ifndef NDEBUG
detachingNode = 0;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment