Changeset 209065 in webkit

Timestamp:

Nov 29, 2016 2:59:23 AM (7 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:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/shadow-dom/shadow-host-transition-expected.html (added)
• LayoutTests/fast/shadow-dom/shadow-host-transition.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/style/RenderTreeUpdater.cpp (modified) (3 diffs)
• Source/WebCore/style/StyleChange.h (modified) (1 diff)
• Source/WebCore/style/StyleTreeResolver.cpp (modified) (4 diffs)
• Source/WebCore/style/StyleTreeResolver.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-11-28  Antti Koivisto  <antti@apple.com>
+
+        Slotted nodes ignore transition
+        https://bugs.webkit.org/show_bug.cgi?id=160866
+        <rdar://problem/29231901>
+
+        Reviewed by Sam Weinig.
+
+        * fast/shadow-dom/shadow-host-transition-expected.html: Added.
+        * fast/shadow-dom/shadow-host-transition.html: Added.
+
 2016-11-28  Matt Baker  <mattbaker@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-11-28  Antti Koivisto  <antti@apple.com>
+
+        Slotted nodes ignore transition
+        https://bugs.webkit.org/show_bug.cgi?id=160866
+        <rdar://problem/29231901>
+
+        Reviewed by Sam Weinig.
+
+        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:
+
 2016-11-28  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebCore/style/RenderTreeUpdater.cpp

@@ -258 +258 @@
         tearDownRenderers(element, TeardownType::KeepHoverAndActive);
 
-    bool hasDisplayContest = update.style && update.style->display() == CONTENTS;
-    if (hasDisplayContest != element.hasDisplayContents()) {
-        element.setHasDisplayContents(hasDisplayContest);
+    bool hasDisplayContents = update.style && update.style->display() == CONTENTS;
+    if (hasDisplayContents != element.hasDisplayContents()) {
+        element.setHasDisplayContents(hasDisplayContents);
         // Render tree position needs to be recomputed as rendering siblings may be found from the display:contents subtree.
         renderTreePosition().invalidateNextSibling();
     }
 
-    bool shouldCreateNewRenderer = !element.renderer() && update.style && !hasDisplayContest;
+    bool shouldCreateNewRenderer = !element.renderer() && update.style && !hasDisplayContents;
     if (shouldCreateNewRenderer) {
         if (element.hasCustomStyleResolveCallbacks())
@@ -480 +480 @@
     PseudoElement* pseudoElement = pseudoId == BEFORE ? current.beforePseudoElement() : current.afterPseudoElement();
 
-    auto* renderer = pseudoElement ? pseudoElement->renderer() : nullptr;
-    if (renderer)
+    if (auto* renderer = pseudoElement ? pseudoElement->renderer() : nullptr)
         renderTreePosition().invalidateNextSibling(*renderer);
 
@@ -495 +494 @@
     }
 
+    RefPtr<PseudoElement> newPseudoElement;
+    if (!pseudoElement) {
+        newPseudoElement = PseudoElement::create(current, pseudoId);
+        pseudoElement = newPseudoElement.get();
+    }
+
     auto newStyle = RenderStyle::clonePtr(*current.renderer()->getCachedPseudoStyle(pseudoId, &current.renderer()->style()));
 
-    auto elementUpdate = Style::TreeResolver::createAnimatedElementUpdate(WTFMove(newStyle), renderer, m_document);
+    auto elementUpdate = Style::TreeResolver::createAnimatedElementUpdate(WTFMove(newStyle), *pseudoElement, Style::NoChange);
 
     if (elementUpdate.change == Style::NoChange)
         return;
 
-    if (!pseudoElement) {
-        auto newPseudoElement = PseudoElement::create(current, pseudoId);
-        pseudoElement = newPseudoElement.ptr();
-        InspectorInstrumentation::pseudoElementCreated(m_document.page(), newPseudoElement);
+    if (newPseudoElement) {
+        InspectorInstrumentation::pseudoElementCreated(m_document.page(), *newPseudoElement);
         if (pseudoId == BEFORE)
-            current.setBeforePseudoElement(WTFMove(newPseudoElement));
+            current.setBeforePseudoElement(newPseudoElement.releaseNonNull());
         else
-            current.setAfterPseudoElement(WTFMove(newPseudoElement));
+            current.setAfterPseudoElement(newPseudoElement.releaseNonNull());
     }
 

trunk/Source/WebCore/style/StyleChange.h

@@ -32 +32 @@
 namespace Style {
 
-enum Change { NoChange, NoInherit, Inherit, Detach, Force };
+enum Change { NoChange, NoInherit, Inherit, Force, Detach };
 
 Change determineChange(const RenderStyle&, const RenderStyle&);

trunk/Source/WebCore/style/StyleTreeResolver.cpp

@@ -201 +201 @@
         return { };
 
-    bool shouldReconstructRenderTree = element.styleValidity() >= Validity::SubtreeAndRenderersInvalid || parent().change == Detach;
-    auto* rendererToUpdate = shouldReconstructRenderTree ? nullptr : element.renderer();
-
-    auto update = createAnimatedElementUpdate(WTFMove(newStyle), rendererToUpdate, m_document);
-
-    if (element.styleResolutionShouldRecompositeLayer())
-        update.recompositeLayer = true;
+    auto update = createAnimatedElementUpdate(WTFMove(newStyle), element, parent().change);
 
     auto* existingStyle = element.renderStyle();
@@ -219 +213 @@
             // In practice this is rare.
             scope().styleResolver.invalidateMatchedPropertiesCache();
-            update.change = Force;
+            update.change = std::max(update.change, Force);
         }
     }
@@ -233 +227 @@
             update.change = Detach;
     }
-
-    if (update.change != Detach && (parent().change == Force || element.styleValidity() >= Validity::SubtreeInvalid))
-        update.change = Force;
 
     return update;
@@ -254 +245 @@
 }
 
-ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderStyle> newStyle, RenderElement* rendererToUpdate, Document& document)
-{
-    ElementUpdate update;
+ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderStyle> newStyle, Element& element, Change parentChange)
+{
+    auto validity = element.styleValidity();
+    bool recompositeLayer = element.styleResolutionShouldRecompositeLayer();
+
+    auto makeUpdate = [&] (std::unique_ptr<RenderStyle> style, Change change) {
+        if (validity >= Validity::SubtreeInvalid)
+            change = std::max(change, validity == Validity::SubtreeAndRenderersInvalid ? Detach : Force);
+        if (parentChange >= Force)
+            change = std::max(change, parentChange);
+        return ElementUpdate { WTFMove(style), change, recompositeLayer };
+    };
+
+    auto* renderer = element.renderer();
+
+    bool shouldReconstruct = validity >= Validity::SubtreeAndRenderersInvalid || parentChange == Detach;
+    if (shouldReconstruct)
+        return makeUpdate(WTFMove(newStyle), Detach);
+
+    if (!renderer) {
+        auto keepsDisplayContents = newStyle->display() == CONTENTS && element.hasDisplayContents();
+        // Some inherited property might have changed.
+        return makeUpdate(WTFMove(newStyle), keepsDisplayContents ? Inherit : Detach);
+    }
 
     std::unique_ptr<RenderStyle> animatedStyle;
-    if (rendererToUpdate && document.frame()->animation().updateAnimations(*rendererToUpdate, *newStyle, animatedStyle))
-        update.recompositeLayer = true;
+    if (element.document().frame()->animation().updateAnimations(*renderer, *newStyle, animatedStyle))
+        recompositeLayer = true;
 
     if (animatedStyle) {
-        update.change = determineChange(rendererToUpdate->style(), *animatedStyle);
+        auto change = determineChange(renderer->style(), *animatedStyle);
         // If animation forces render tree reconstruction pass the original style. The animation will be applied on renderer construction.
         // FIXME: We should always use the animated style here.
-        update.style = update.change == Detach ? WTFMove(newStyle) : WTFMove(animatedStyle);
-    } else {
-        update.change = rendererToUpdate ? determineChange(rendererToUpdate->style(), *newStyle) : Detach;
-        update.style = WTFMove(newStyle);
-    }
-
-    return update;
+        auto style = change == Detach ? WTFMove(newStyle) : WTFMove(animatedStyle);
+        return makeUpdate(WTFMove(style), change);
+    }
+
+    auto change = determineChange(renderer->style(), *newStyle);
+    return makeUpdate(WTFMove(newStyle), change);
 }
 

trunk/Source/WebCore/style/StyleTreeResolver.h

@@ -33 +33 @@
 #include "StyleSharingResolver.h"
 #include "StyleUpdate.h"
+#include "StyleValidity.h"
 #include <wtf/Function.h>
 #include <wtf/RefPtr.h>
@@ -54 +55 @@
     std::unique_ptr<Update> resolve(Change);
 
-    static ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, RenderElement* existingRenderer, Document&);
+    static ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, Element&, Change parentChange);
 
 private: