Changeset 209065 in webkit


Ignore:
Timestamp:
Nov 29, 2016, 2:59:23 AM (8 years ago)
Author:
Antti Koivisto
Message:

Slotted nodes ignore transition
​https://bugs.webkit.org/show_bug.cgi?id=160866
<rdar://problem/29231901>

Reviewed by Sam Weinig.

Source/WebCore:

The problem is that slot (display:contents) always triggers full render tree rebuild when something
changes in the slotted subtree. This causes animation to jump to end (may be another bug).

Test: fast/shadow-dom/shadow-host-transition.html

  • style/RenderTreeUpdater.cpp:

(WebCore::RenderTreeUpdater::updateElementRenderer):
(WebCore::RenderTreeUpdater::updateBeforeOrAfterPseudoElement):

  • style/StyleChange.h:

Rearrange so the strongest ('Detach') is the highest.

  • style/StyleTreeResolver.cpp:

(WebCore::Style::TreeResolver::resolveElement):
(WebCore::Style::TreeResolver::createAnimatedElementUpdate):

If style was display:contents and stays that way, use 'Inherit' StyleChange which doesn't force render tree rebuild.
Refactor more of the functionality to createAnimatedElementUpdate.

  • style/StyleTreeResolver.h:

LayoutTests:

  • fast/shadow-dom/shadow-host-transition-expected.html: Added.
  • fast/shadow-dom/shadow-host-transition.html: Added.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r209062 r209065  
     12016-11-28  Antti Koivisto  <antti@apple.com>
     2
     3        Slotted nodes ignore transition
     4        https://bugs.webkit.org/show_bug.cgi?id=160866
     5        <rdar://problem/29231901>
     6
     7        Reviewed by Sam Weinig.
     8
     9        * fast/shadow-dom/shadow-host-transition-expected.html: Added.
     10        * fast/shadow-dom/shadow-host-transition.html: Added.
     11
    1122016-11-28  Matt Baker  <mattbaker@apple.com>
    213

  • trunk/Source/WebCore/ChangeLog

    r209064 r209065  
     12016-11-28  Antti Koivisto  <antti@apple.com>
     2
     3        Slotted nodes ignore transition
     4        https://bugs.webkit.org/show_bug.cgi?id=160866
     5        <rdar://problem/29231901>
     6
     7        Reviewed by Sam Weinig.
     8
     9        The problem is that slot (display:contents) always triggers full render tree rebuild when something
     10        changes in the slotted subtree. This causes animation to jump to end (may be another bug).
     11
     12        Test: fast/shadow-dom/shadow-host-transition.html
     13
     14        * style/RenderTreeUpdater.cpp:
     15        (WebCore::RenderTreeUpdater::updateElementRenderer):
     16        (WebCore::RenderTreeUpdater::updateBeforeOrAfterPseudoElement):
     17        * style/StyleChange.h:
     18
     19            Rearrange so the strongest ('Detach') is the highest.
     20
     21        * style/StyleTreeResolver.cpp:
     22        (WebCore::Style::TreeResolver::resolveElement):
     23        (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
     24
     25            If style was display:contents and stays that way, use 'Inherit' StyleChange which doesn't force render tree rebuild.
     26            Refactor more of the functionality to createAnimatedElementUpdate.
     27
     28        * style/StyleTreeResolver.h:
     29
    1302016-11-28  Carlos Garcia Campos  <cgarcia@igalia.com>
    231

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

    r208998 r209065  
    258258        tearDownRenderers(element, TeardownType::KeepHoverAndActive);
    259259
    260     bool hasDisplayContest = update.style && update.style->display() == CONTENTS;
    261     if (hasDisplayContest != element.hasDisplayContents()) {
    262         element.setHasDisplayContents(hasDisplayContest);
     260    bool hasDisplayContents = update.style && update.style->display() == CONTENTS;
     261    if (hasDisplayContents != element.hasDisplayContents()) {
     262        element.setHasDisplayContents(hasDisplayContents);
    263263        // Render tree position needs to be recomputed as rendering siblings may be found from the display:contents subtree.
    264264        renderTreePosition().invalidateNextSibling();
    265265    }
    266266
    267     bool shouldCreateNewRenderer = !element.renderer() && update.style && !hasDisplayContest;
     267    bool shouldCreateNewRenderer = !element.renderer() && update.style && !hasDisplayContents;
    268268    if (shouldCreateNewRenderer) {
    269269        if (element.hasCustomStyleResolveCallbacks())
    … …  
    480480    PseudoElement* pseudoElement = pseudoId == BEFORE ? current.beforePseudoElement() : current.afterPseudoElement();
    481481
    482     auto* renderer = pseudoElement ? pseudoElement->renderer() : nullptr;
    483     if (renderer)
     482    if (auto* renderer = pseudoElement ? pseudoElement->renderer() : nullptr)
    484483        renderTreePosition().invalidateNextSibling(*renderer);
    485484
    … …  
    495494    }
    496495
     496    RefPtr<PseudoElement> newPseudoElement;
     497    if (!pseudoElement) {
     498        newPseudoElement = PseudoElement::create(current, pseudoId);
     499        pseudoElement = newPseudoElement.get();
     500    }
     501
    497502    auto newStyle = RenderStyle::clonePtr(*current.renderer()->getCachedPseudoStyle(pseudoId, &current.renderer()->style()));
    498503
    499     auto elementUpdate = Style::TreeResolver::createAnimatedElementUpdate(WTFMove(newStyle), renderer, m_document);
     504    auto elementUpdate = Style::TreeResolver::createAnimatedElementUpdate(WTFMove(newStyle), *pseudoElement, Style::NoChange);
    500505
    501506    if (elementUpdate.change == Style::NoChange)
    502507        return;
    503508
    504     if (!pseudoElement) {
    505         auto newPseudoElement = PseudoElement::create(current, pseudoId);
    506         pseudoElement = newPseudoElement.ptr();
    507         InspectorInstrumentation::pseudoElementCreated(m_document.page(), newPseudoElement);
     509    if (newPseudoElement) {
     510        InspectorInstrumentation::pseudoElementCreated(m_document.page(), *newPseudoElement);
    508511        if (pseudoId == BEFORE)
    509             current.setBeforePseudoElement(WTFMove(newPseudoElement));
     512            current.setBeforePseudoElement(newPseudoElement.releaseNonNull());
    510513        else
    511             current.setAfterPseudoElement(WTFMove(newPseudoElement));
     514            current.setAfterPseudoElement(newPseudoElement.releaseNonNull());
    512515    }
    513516

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

    r208668 r209065  
    3232namespace Style {
    3333
    34 enum Change { NoChange, NoInherit, Inherit, Detach, Force };
     34enum Change { NoChange, NoInherit, Inherit, Force, Detach };
    3535
    3636Change determineChange(const RenderStyle&, const RenderStyle&);

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

    r208971 r209065  
    201201        return { };
    202202
    203     bool shouldReconstructRenderTree = element.styleValidity() >= Validity::SubtreeAndRenderersInvalid || parent().change == Detach;
    204     auto* rendererToUpdate = shouldReconstructRenderTree ? nullptr : element.renderer();
    205 
    206     auto update = createAnimatedElementUpdate(WTFMove(newStyle), rendererToUpdate, m_document);
    207 
    208     if (element.styleResolutionShouldRecompositeLayer())
    209         update.recompositeLayer = true;
     203    auto update = createAnimatedElementUpdate(WTFMove(newStyle), element, parent().change);
    210204
    211205    auto* existingStyle = element.renderStyle();
    … …  
    219213            // In practice this is rare.
    220214            scope().styleResolver.invalidateMatchedPropertiesCache();
    221             update.change = Force;
     215            update.change = std::max(update.change, Force);
    222216        }
    223217    }
    … …  
    233227            update.change = Detach;
    234228    }
    235 
    236     if (update.change != Detach && (parent().change == Force || element.styleValidity() >= Validity::SubtreeInvalid))
    237         update.change = Force;
    238229
    239230    return update;
    … …  
    254245}
    255246
    256 ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderStyle> newStyle, RenderElement* rendererToUpdate, Document& document)
    257 {
    258     ElementUpdate update;
     247ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderStyle> newStyle, Element& element, Change parentChange)
     248{
     249    auto validity = element.styleValidity();
     250    bool recompositeLayer = element.styleResolutionShouldRecompositeLayer();
     251
     252    auto makeUpdate = [&] (std::unique_ptr<RenderStyle> style, Change change) {
     253        if (validity >= Validity::SubtreeInvalid)
     254            change = std::max(change, validity == Validity::SubtreeAndRenderersInvalid ? Detach : Force);
     255        if (parentChange >= Force)
     256            change = std::max(change, parentChange);
     257        return ElementUpdate { WTFMove(style), change, recompositeLayer };
     258    };
     259
     260    auto* renderer = element.renderer();
     261
     262    bool shouldReconstruct = validity >= Validity::SubtreeAndRenderersInvalid || parentChange == Detach;
     263    if (shouldReconstruct)
     264        return makeUpdate(WTFMove(newStyle), Detach);
     265
     266    if (!renderer) {
     267        auto keepsDisplayContents = newStyle->display() == CONTENTS && element.hasDisplayContents();
     268        // Some inherited property might have changed.
     269        return makeUpdate(WTFMove(newStyle), keepsDisplayContents ? Inherit : Detach);
     270    }
    259271
    260272    std::unique_ptr<RenderStyle> animatedStyle;
    261     if (rendererToUpdate && document.frame()->animation().updateAnimations(*rendererToUpdate, *newStyle, animatedStyle))
    262         update.recompositeLayer = true;
     273    if (element.document().frame()->animation().updateAnimations(*renderer, *newStyle, animatedStyle))
     274        recompositeLayer = true;
    263275
    264276    if (animatedStyle) {
    265         update.change = determineChange(rendererToUpdate->style(), *animatedStyle);
     277        auto change = determineChange(renderer->style(), *animatedStyle);
    266278        // If animation forces render tree reconstruction pass the original style. The animation will be applied on renderer construction.
    267279        // FIXME: We should always use the animated style here.
    268         update.style = update.change == Detach ? WTFMove(newStyle) : WTFMove(animatedStyle);
    269     } else {
    270         update.change = rendererToUpdate ? determineChange(rendererToUpdate->style(), *newStyle) : Detach;
    271         update.style = WTFMove(newStyle);
    272     }
    273 
    274     return update;
     280        auto style = change == Detach ? WTFMove(newStyle) : WTFMove(animatedStyle);
     281        return makeUpdate(WTFMove(style), change);
     282    }
     283
     284    auto change = determineChange(renderer->style(), *newStyle);
     285    return makeUpdate(WTFMove(newStyle), change);
    275286}
    276287

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

    r208743 r209065  
    3333#include "StyleSharingResolver.h"
    3434#include "StyleUpdate.h"
     35#include "StyleValidity.h"
    3536#include <wtf/Function.h>
    3637#include <wtf/RefPtr.h>
    … …  
    5455    std::unique_ptr<Update> resolve(Change);
    5556
    56     static ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, RenderElement* existingRenderer, Document&);
     57    static ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, Element&, Change parentChange);
    5758
    5859private:
Note: See TracChangeset for help on using the changeset viewer.