[webkit-changes] [200381] trunk

Tue, 03 May 2016 11:00:50 -0700

Title: [200381] trunk
Revision
200381
Author
[email protected]
Date
2016-05-03 11:00:04 -0700 (Tue, 03 May 2016)

Log Message

REGRESSION (r198943): Transitions don't work if they animate display property
https://bugs.webkit.org/show_bug.cgi?id=157244
<rdar://problem/26042189>

Reviewed by Simon Fraser.

Source/WebCore:

Test: transitions/transition-display-property.html

* style/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::updateBeforeOrAfterPseudoElement):

    Call the common function for ::before/::after updates.

* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolveElement):
(WebCore::Style::TreeResolver::createAnimatedElementUpdate):

    If animation forces render tree reconstruction use the original rather than animated style for update.
    Because animations are tied to renderers we start them during renderer construction in this case.

    Factor to a function.

(WebCore::Style::elementImplicitVisibility):
* style/StyleTreeResolver.h:

LayoutTests:

* transitions/transition-display-property-expected.html: Added.
* transitions/transition-display-property.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (200380 => 200381)


--- trunk/LayoutTests/ChangeLog	2016-05-03 17:58:23 UTC (rev 200380)
+++ trunk/LayoutTests/ChangeLog	2016-05-03 18:00:04 UTC (rev 200381)
@@ -1,3 +1,14 @@
+2016-05-02  Antti Koivisto  <[email protected]>
+
+        REGRESSION (r198943): Transitions don't work if they animate display property
+        https://bugs.webkit.org/show_bug.cgi?id=157244
+        <rdar://problem/26042189>
+
+        Reviewed by Simon Fraser.
+
+        * transitions/transition-display-property-expected.html: Added.
+        * transitions/transition-display-property.html: Added.
+
 2016-05-03  Chris Dumez  <[email protected]>
 
         Unreviewed, drop outdated layout test after r200375.

Added: trunk/LayoutTests/transitions/transition-display-property-expected.html (0 => 200381)


--- trunk/LayoutTests/transitions/transition-display-property-expected.html	                        (rev 0)
+++ trunk/LayoutTests/transitions/transition-display-property-expected.html	2016-05-03 18:00:04 UTC (rev 200381)
@@ -0,0 +1,4 @@
+<style>
+test { display:inline-block; height:10px; width:10px; border: 2px solid green; }
+</style>
+<test></test>

Added: trunk/LayoutTests/transitions/transition-display-property.html (0 => 200381)


--- trunk/LayoutTests/transitions/transition-display-property.html	                        (rev 0)
+++ trunk/LayoutTests/transitions/transition-display-property.html	2016-05-03 18:00:04 UTC (rev 200381)
@@ -0,0 +1,23 @@
+<style>
+test { transition:0.1s; display:block; height:100px; border: 2px solid green; }
+.animate { display:inline-block; height:10px; width:10px; }
+</style>
+<test></test>
+<script>
+var test = document.querySelector("test");
+var count = 0;
+function testUntilComplete()
+{
+    if (test.offsetWidth == 10 || ++count == 10)
+        testRunner.notifyDone();
+    else
+        setTimeout(testUntilComplete, 0.1);
+}
+
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testUntilComplete();
+}
+test.offsetWidth;
+test.className = "animate";
+</script>

Modified: trunk/Source/WebCore/ChangeLog (200380 => 200381)


--- trunk/Source/WebCore/ChangeLog	2016-05-03 17:58:23 UTC (rev 200380)
+++ trunk/Source/WebCore/ChangeLog	2016-05-03 18:00:04 UTC (rev 200381)
@@ -1,3 +1,30 @@
+2016-05-02  Antti Koivisto  <[email protected]>
+
+        REGRESSION (r198943): Transitions don't work if they animate display property
+        https://bugs.webkit.org/show_bug.cgi?id=157244
+        <rdar://problem/26042189>
+
+        Reviewed by Simon Fraser.
+
+        Test: transitions/transition-display-property.html
+
+        * style/RenderTreeUpdater.cpp:
+        (WebCore::RenderTreeUpdater::updateBeforeOrAfterPseudoElement):
+
+            Call the common function for ::before/::after updates.
+
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::TreeResolver::resolveElement):
+        (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
+
+            If animation forces render tree reconstruction use the original rather than animated style for update.
+            Because animations are tied to renderers we start them during renderer construction in this case.
+
+            Factor to a function.
+
+        (WebCore::Style::elementImplicitVisibility):
+        * style/StyleTreeResolver.h:
+
 2016-05-03  Pranjal Jumde  <[email protected]>
 
         WorkerGlobalScope's self, location and navigator attributes should not be replaceable

Modified: trunk/Source/WebCore/style/RenderTreeUpdater.cpp (200380 => 200381)


--- trunk/Source/WebCore/style/RenderTreeUpdater.cpp	2016-05-03 17:58:23 UTC (rev 200380)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.cpp	2016-05-03 18:00:04 UTC (rev 200381)
@@ -480,17 +480,10 @@
         return;
     }
 
-    Style::ElementUpdate elementUpdate;
-
     auto newStyle = RenderStyle::clonePtr(*current.renderer()->getCachedPseudoStyle(pseudoId, &current.renderer()->style()));
 
-    std::unique_ptr<RenderStyle> animatedStyle;
-    if (renderer && m_document.frame()->animation().updateAnimations(*renderer, *newStyle, animatedStyle))
-        elementUpdate.isSynthetic = true;
+    auto elementUpdate = Style::TreeResolver::createAnimatedElementUpdate(WTFMove(newStyle), renderer, m_document);
 
-    elementUpdate.style = animatedStyle ? WTFMove(animatedStyle) : WTFMove(newStyle);
-    elementUpdate.change = renderer ? Style::determineChange(renderer->style(), *elementUpdate.style) : Style::Detach;
-
     if (elementUpdate.change == Style::NoChange)
         return;

Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (200380 => 200381)


--- trunk/Source/WebCore/style/StyleTreeResolver.cpp	2016-05-03 17:58:23 UTC (rev 200380)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp	2016-05-03 18:00:04 UTC (rev 200381)
@@ -185,22 +185,14 @@
 {
     auto newStyle = styleForElement(element, parent().style);
 
-    auto* renderer = element.renderer();
-
     if (!affectsRenderedSubtree(element, *newStyle))
         return { };
 
-    ElementUpdate update;
+    bool shouldReconstructRenderTree = element.styleChangeType() == ReconstructRenderTree || parent().change == Detach;
+    auto* rendererToUpdate = shouldReconstructRenderTree ? nullptr : element.renderer();
 
-    bool needsNewRenderer = !renderer || element.styleChangeType() == ReconstructRenderTree || parent().change == Detach;
+    auto update = createAnimatedElementUpdate(WTFMove(newStyle), rendererToUpdate, m_document);
 
-    std::unique_ptr<RenderStyle> animatedStyle;
-    if (!needsNewRenderer && m_document.frame()->animation().updateAnimations(*renderer, *newStyle, animatedStyle))
-        update.isSynthetic = true;
-
-    update.style = animatedStyle ? WTFMove(animatedStyle) : WTFMove(newStyle);
-    update.change = needsNewRenderer ? Detach : determineChange(renderer->style(), *update.style);
-
     if (element.styleChangeType() == SyntheticStyleChange)
         update.isSynthetic = true;
 
@@ -210,7 +202,7 @@
 
         // If "rem" units are used anywhere in the document, and if the document element's font size changes, then force font updating
         // all the way down the tree. This is simpler than having to maintain a cache of objects (and such font size changes should be rare anyway).
-        if (m_document.authorStyleSheets().usesRemUnits() && update.change != NoChange && renderer && renderer->style().fontSize() != update.style->fontSize()) {
+        if (m_document.authorStyleSheets().usesRemUnits() && update.change != NoChange && element.renderer() && element.renderer()->style().fontSize() != update.style->fontSize()) {
             // Cached RenderStyles may depend on the rem units.
             scope().styleResolver.invalidateMatchedPropertiesCache();
             update.change = Force;
@@ -228,6 +220,27 @@
     return update;
 }
 
+ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderStyle> newStyle, RenderElement* rendererToUpdate, Document& document)
+{
+    ElementUpdate update;
+
+    std::unique_ptr<RenderStyle> animatedStyle;
+    if (rendererToUpdate && document.frame()->animation().updateAnimations(*rendererToUpdate, *newStyle, animatedStyle))
+        update.isSynthetic = true;
+
+    if (animatedStyle) {
+        update.change = determineChange(rendererToUpdate->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;
+}
+
 #if PLATFORM(IOS)
 static EVisibility elementImplicitVisibility(const Element* element)
 {

Modified: trunk/Source/WebCore/style/StyleTreeResolver.h (200380 => 200381)


--- trunk/Source/WebCore/style/StyleTreeResolver.h	2016-05-03 17:58:23 UTC (rev 200380)
+++ trunk/Source/WebCore/style/StyleTreeResolver.h	2016-05-03 18:00:04 UTC (rev 200381)
@@ -60,6 +60,8 @@
 
     std::unique_ptr<Update> resolve(Change);
 
+    static ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, RenderElement* existingRenderer, Document&);
+
 private:
     std::unique_ptr<RenderStyle> styleForElement(Element&, const RenderStyle& inheritedStyle);
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to