Changeset 228497 in webkit


Ignore:
Timestamp:
Feb 14, 2018, 5:27:52 PM (7 years ago)
Author:
Antti Koivisto
Message:

Do sibling invalidation on mutation
​https://bugs.webkit.org/show_bug.cgi?id=182809

Reviewed by Zalan Bujtas.

Source/WebCore:

We used to invalidate siblings for sibling combinators and nth-pseudo classes during style resolution tree walk.
This would consider any element with invalid style a reason to invalidate siblings too. However we now do
accurate invalidation on class and attribute changes and this approach ends up invalidating too much.

This patch sibling style invalidation to mutation time and removes invalidation code from style resolution tree walk.

  • dom/Element.cpp:

(WebCore::invalidateSiblingsIfNeeded):

Helper to invalidate siblings.

(WebCore::Element::invalidateStyle):
(WebCore::Element::invalidateStyleAndLayerComposition):
(WebCore::Element::invalidateStyleForSubtree):
(WebCore::Element::invalidateStyleAndRenderersForSubtree):

Invalidate siblings if needed based on affectsNextSibling/affectedByPreviousSibling bits.

(WebCore::Element::invalidateStyleInternal):
(WebCore::Element::invalidateStyleForSubtreeInternal):

Add "internal" versions that don't invalidate siblings. These are used by StyleInvalidator for accurate invalidation.

  • dom/Element.h:
  • style/StyleInvalidator.cpp:

(WebCore::Style::Invalidator::invalidateIfNeeded):
(WebCore::Style::Invalidator::invalidateStyle):

Use internal invalidation functions.

  • style/StyleTreeResolver.cpp:

(WebCore::Style::resetStyleForNonRenderedDescendants):
(WebCore::Style::TreeResolver::resolveComposedTree):

Remove sibling invalidation.

  • style/StyleTreeResolver.h:

LayoutTests:

Sibling invalidation now happens on mutation. Update the tests.

  • fast/css/indirect-adjacent-style-invalidation-1-expected.txt:
  • fast/css/indirect-adjacent-style-invalidation-1.html:
  • fast/css/indirect-adjacent-style-invalidation-2-expected.txt:
  • fast/css/indirect-adjacent-style-invalidation-2.html:
  • fast/css/indirect-adjacent-style-invalidation-3-expected.txt:
  • fast/css/indirect-adjacent-style-invalidation-3.html:
Location:
trunk
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r228486 r228497  
     12018-02-14  Antti Koivisto  <antti@apple.com>
     2
     3        Do sibling invalidation on mutation
     4        https://bugs.webkit.org/show_bug.cgi?id=182809
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        Sibling invalidation now happens on mutation. Update the tests.
     9
     10        * fast/css/indirect-adjacent-style-invalidation-1-expected.txt:
     11        * fast/css/indirect-adjacent-style-invalidation-1.html:
     12        * fast/css/indirect-adjacent-style-invalidation-2-expected.txt:
     13        * fast/css/indirect-adjacent-style-invalidation-2.html:
     14        * fast/css/indirect-adjacent-style-invalidation-3-expected.txt:
     15        * fast/css/indirect-adjacent-style-invalidation-3.html:
     16
    1172018-02-14  Daniel Bates  <dabates@apple.com>
    218

  • trunk/LayoutTests/fast/css/indirect-adjacent-style-invalidation-1-expected.txt

    r189560 r228497  
    1212PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[0]) is true
    1313PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[1]) is true
    14 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is false
    15 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is false
     14PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is true
     15PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is true
    1616PASS getComputedStyle(document.querySelectorAll("target")[0]).color is "rgb(0, 1, 2)"
    1717PASS getComputedStyle(document.querySelectorAll("target")[1]).color is "rgb(0, 1, 2)"
    … …  
    2222PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[0]) is true
    2323PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[1]) is true
    24 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is false
    25 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is false
     24PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is true
     25PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is true
    2626PASS getComputedStyle(document.querySelectorAll("target")[0]).color is "rgb(0, 0, 0)"
    2727PASS getComputedStyle(document.querySelectorAll("target")[1]).color is "rgb(0, 0, 0)"

  • trunk/LayoutTests/fast/css/indirect-adjacent-style-invalidation-1.html

    r189560 r228497  
    5050    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\".activator\")[0])");
    5151    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\".activator\")[1])");
    52     shouldBeFalse("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[0])");
    53     shouldBeFalse("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[1])");
     52    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[0])");
     53    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[1])");
    5454}
    5555

  • trunk/LayoutTests/fast/css/indirect-adjacent-style-invalidation-2-expected.txt

    r189560 r228497  
    1212PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[0]) is true
    1313PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[1]) is true
    14 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is false
    15 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is false
     14PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is true
     15PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is true
    1616PASS getComputedStyle(document.querySelectorAll("target")[0]).color is "rgb(0, 1, 2)"
    1717PASS getComputedStyle(document.querySelectorAll("target")[1]).color is "rgb(0, 1, 2)"
    … …  
    2222PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[0]) is true
    2323PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[1]) is true
    24 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is false
    25 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is false
     24PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is true
     25PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is true
    2626PASS getComputedStyle(document.querySelectorAll("target")[0]).color is "rgb(0, 0, 0)"
    2727PASS getComputedStyle(document.querySelectorAll("target")[1]).color is "rgb(0, 0, 0)"

  • trunk/LayoutTests/fast/css/indirect-adjacent-style-invalidation-2.html

    r189560 r228497  
    5050    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\".activator\")[0])");
    5151    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\".activator\")[1])");
    52     shouldBeFalse("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[0])");
    53     shouldBeFalse("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[1])");
     52    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[0])");
     53    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[1])");
    5454}
    5555

  • trunk/LayoutTests/fast/css/indirect-adjacent-style-invalidation-3-expected.txt

    r189560 r228497  
    1212PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[0]) is true
    1313PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[1]) is true
    14 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is false
    15 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is false
     14PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is true
     15PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is true
    1616PASS getComputedStyle(document.querySelectorAll("target")[0]).color is "rgb(0, 1, 2)"
    1717PASS getComputedStyle(document.querySelectorAll("target")[1]).color is "rgb(0, 1, 2)"
    … …  
    2222PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[0]) is true
    2323PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(".activator")[1]) is true
    24 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is false
    25 PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is false
     24PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[0]) is true
     25PASS window.internals.nodeNeedsStyleRecalc(document.querySelectorAll("target")[1]) is true
    2626PASS getComputedStyle(document.querySelectorAll("target")[0]).color is "rgb(0, 0, 0)"
    2727PASS getComputedStyle(document.querySelectorAll("target")[1]).color is "rgb(0, 0, 0)"

  • trunk/LayoutTests/fast/css/indirect-adjacent-style-invalidation-3.html

    r189560 r228497  
    5050    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\".activator\")[0])");
    5151    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\".activator\")[1])");
    52     shouldBeFalse("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[0])");
    53     shouldBeFalse("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[1])");
     52    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[0])");
     53    testFunction("window.internals.nodeNeedsStyleRecalc(document.querySelectorAll(\"target\")[1])");
    5454}
    5555

  • trunk/Source/WebCore/ChangeLog

    r228495 r228497  
     12018-02-14  Antti Koivisto  <antti@apple.com>
     2
     3        Do sibling invalidation on mutation
     4        https://bugs.webkit.org/show_bug.cgi?id=182809
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        We used to invalidate siblings for sibling combinators and nth-pseudo classes during style resolution tree walk.
     9        This would consider any element with invalid style a reason to invalidate siblings too. However we now do
     10        accurate invalidation on class and attribute changes and this approach ends up invalidating too much.
     11
     12        This patch sibling style invalidation to mutation time and removes invalidation code from style resolution tree walk.
     13
     14        * dom/Element.cpp:
     15        (WebCore::invalidateSiblingsIfNeeded):
     16
     17            Helper to invalidate siblings.
     18
     19        (WebCore::Element::invalidateStyle):
     20        (WebCore::Element::invalidateStyleAndLayerComposition):
     21        (WebCore::Element::invalidateStyleForSubtree):
     22        (WebCore::Element::invalidateStyleAndRenderersForSubtree):
     23
     24            Invalidate siblings if needed based on affectsNextSibling/affectedByPreviousSibling bits.
     25
     26        (WebCore::Element::invalidateStyleInternal):
     27        (WebCore::Element::invalidateStyleForSubtreeInternal):
     28
     29            Add "internal" versions that don't invalidate siblings. These are used by StyleInvalidator for accurate invalidation.
     30
     31        * dom/Element.h:
     32        * style/StyleInvalidator.cpp:
     33        (WebCore::Style::Invalidator::invalidateIfNeeded):
     34        (WebCore::Style::Invalidator::invalidateStyle):
     35
     36            Use internal invalidation functions.
     37
     38        * style/StyleTreeResolver.cpp:
     39        (WebCore::Style::resetStyleForNonRenderedDescendants):
     40        (WebCore::Style::TreeResolver::resolveComposedTree):
     41
     42            Remove sibling invalidation.
     43
     44        * style/StyleTreeResolver.h:
     45
    1462018-02-14  John Wilander  <wilander@apple.com>
    247

  • trunk/Source/WebCore/dom/Element.cpp

    r228446 r228497  
    14821482}
    14831483
     1484static void invalidateSiblingsIfNeeded(Element& element)
     1485{
     1486    if (!element.affectsNextSiblingElementStyle())
     1487        return;
     1488    auto* parent = element.parentElement();
     1489    if (parent && parent->styleValidity() >= Style::Validity::SubtreeInvalid)
     1490        return;
     1491
     1492    for (auto* sibling = element.nextElementSibling(); sibling; sibling = sibling->nextElementSibling()) {
     1493        if (sibling->styleIsAffectedByPreviousSibling())
     1494            sibling->invalidateStyleForSubtreeInternal();
     1495        if (!sibling->affectsNextSiblingElementStyle())
     1496            return;
     1497    }
     1498}
     1499
    14841500void Element::invalidateStyle()
    14851501{
    14861502    Node::invalidateStyle(Style::Validity::ElementInvalid);
     1503    invalidateSiblingsIfNeeded(*this);
    14871504}
    14881505
    … …  
    14901507{
    14911508    Node::invalidateStyle(Style::Validity::ElementInvalid, Style::InvalidationMode::RecompositeLayer);
     1509    invalidateSiblingsIfNeeded(*this);
    14921510}
    14931511
    … …  
    14951513{
    14961514    Node::invalidateStyle(Style::Validity::SubtreeInvalid);
     1515    invalidateSiblingsIfNeeded(*this);
    14971516}
    14981517
    … …  
    15001519{
    15011520    Node::invalidateStyle(Style::Validity::SubtreeAndRenderersInvalid);
     1521    invalidateSiblingsIfNeeded(*this);
     1522}
     1523
     1524void Element::invalidateStyleInternal()
     1525{
     1526    Node::invalidateStyle(Style::Validity::ElementInvalid);
     1527}
     1528
     1529void Element::invalidateStyleForSubtreeInternal()
     1530{
     1531    Node::invalidateStyle(Style::Validity::SubtreeInvalid);
    15021532}
    15031533

  • trunk/Source/WebCore/dom/Element.h

    r228446 r228497  
    546546    void invalidateStyleAndRenderersForSubtree();
    547547
     548    void invalidateStyleInternal();
     549    void invalidateStyleForSubtreeInternal();
     550
    548551    bool hasDisplayContents() const;
    549552    void storeDisplayContentsStyle(std::unique_ptr<RenderStyle>);

  • trunk/Source/WebCore/style/StyleInvalidator.cpp

    r227956 r228497  
    104104        // FIXME: This could do actual rule matching too.
    105105        if (element.shadowRoot())
    106             element.invalidateStyleForSubtree();
     106            element.invalidateStyleForSubtreeInternal();
    107107    }
    108108
    … …  
    112112        if (containingShadowRoot && containingShadowRoot->host()) {
    113113            for (auto& possiblySlotted : childrenOfType<Element>(*containingShadowRoot->host()))
    114                 possiblySlotted.invalidateStyle();
     114                possiblySlotted.invalidateStyleInternal();
    115115        }
    116116        // No need to do this again.
    … …  
    125125
    126126        if (ruleCollector.hasMatchedRules())
    127             element.invalidateStyle();
     127            element.invalidateStyleInternal();
    128128        return CheckDescendants::Yes;
    129129    }
    … …  
    194194
    195195    if (!m_ruleSet.hostPseudoClassRules().isEmpty() && shadowRoot.host())
    196         shadowRoot.host()->invalidateStyle();
     196        shadowRoot.host()->invalidateStyleInternal();
    197197
    198198    for (auto& child : childrenOfType<Element>(shadowRoot)) {

  • trunk/Source/WebCore/style/StyleTreeResolver.cpp

    r226809 r228497  
    138138static void resetStyleForNonRenderedDescendants(Element& current)
    139139{
    140     // FIXME: This is not correct with shadow trees. This should be done with ComposedTreeIterator.
    141     bool elementNeedingStyleRecalcAffectsNextSiblingElementStyle = false;
    142140    for (auto& child : childrenOfType<Element>(current)) {
    143         bool affectedByPreviousSibling = child.styleIsAffectedByPreviousSibling() && elementNeedingStyleRecalcAffectsNextSiblingElementStyle;
    144         if (child.needsStyleRecalc() || elementNeedingStyleRecalcAffectsNextSiblingElementStyle)
    145             elementNeedingStyleRecalcAffectsNextSiblingElementStyle = child.affectsNextSiblingElementStyle();
    146 
    147         if (child.needsStyleRecalc() || affectedByPreviousSibling) {
     141        if (child.needsStyleRecalc()) {
    148142            child.resetComputedStyle();
    149143            child.resetStyleRelations();
    … …  
    472466        }
    473467
    474         // FIXME: We should deal with this during style invalidation.
    475         bool affectedByPreviousSibling = element.styleIsAffectedByPreviousSibling() && parent.elementNeedingStyleRecalcAffectsNextSiblingElementStyle;
    476         if (element.needsStyleRecalc() || parent.elementNeedingStyleRecalcAffectsNextSiblingElementStyle)
    477             parent.elementNeedingStyleRecalcAffectsNextSiblingElementStyle = element.affectsNextSiblingElementStyle();
    478 
    479468        auto* style = renderOrDisplayContentsStyle(element);
    480469        auto change = NoChange;
    481470        auto descendantsToResolve = DescendantsToResolve::None;
    482471
    483         bool shouldResolve = shouldResolveElement(element, parent.descendantsToResolve) || affectedByPreviousSibling;
     472        bool shouldResolve = shouldResolveElement(element, parent.descendantsToResolve);
    484473        if (shouldResolve) {
    485474            if (!element.hasDisplayContents())
    … …  
    498487            change = elementUpdates.update.change;
    499488            descendantsToResolve = elementUpdates.descendantsToResolve;
    500 
    501             if (affectedByPreviousSibling)
    502                 descendantsToResolve = DescendantsToResolve::All;
    503489
    504490            if (elementUpdates.update.style)

  • trunk/Source/WebCore/style/StyleTreeResolver.h

    r226809 r228497  
    8080        DescendantsToResolve descendantsToResolve { DescendantsToResolve::None };
    8181        bool didPushScope { false };
    82         bool elementNeedingStyleRecalcAffectsNextSiblingElementStyle { false };
    8382
    8483        Parent(Document&);
Note: See TracChangeset for help on using the changeset viewer.