Title: [177135] trunk
Revision
177135
Author
cdu...@apple.com
Date
2014-12-10 23:41:13 -0800 (Wed, 10 Dec 2014)

Log Message

http://omfgdogs.info/ only animates when you resize the window
https://bugs.webkit.org/show_bug.cgi?id=139435
<rdar://problem/19190493>

Reviewed by Simon Fraser.

Source/WebCore:

After r163928, we would fail to animate a gif if:
- it is used as a background image of a 0-height html element
- it is used as a background image of a 0-height body element whose
  background is delegated to the root (because the root has no
  background).

This is because in such cases, shouldRepaintForImageAnimation()
should use the background rect instead of the renderer's overflow
rect to determine if the image is inside the viewport. Both cases
are addressed in this patch.

Tests: fast/images/animated-gif-body-delegated-background-image.html
       fast/images/animated-gif-body-outside-viewport.html
       fast/images/animated-gif-html-background-image.html

* rendering/RenderElement.cpp:
(WebCore::shouldRepaintForImageAnimation):
* testing/Internals.cpp:
(WebCore::Internals::hasPausedImageAnimations):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

Add layout tests to make sure a gif image is still animated if:
- It is used as a background image of a 0-height html element
- It is used as a background image of a 0-height body element that
  is delegated to the root (because the root has no background)

Also add a layout test to make sure we still stop the gif animation
if it is used as background image of a body that is outside the
viewport.

* fast/images/animated-gif-body-delegated-background-image-expected.txt: Added.
* fast/images/animated-gif-body-delegated-background-image.html: Added.
* fast/images/animated-gif-body-outside-viewport-expected.txt: Added.
* fast/images/animated-gif-body-outside-viewport.html: Added.
* fast/images/animated-gif-html-background-image-expected.txt: Added.
* fast/images/animated-gif-html-background-image.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (177134 => 177135)


--- trunk/LayoutTests/ChangeLog	2014-12-11 07:37:20 UTC (rev 177134)
+++ trunk/LayoutTests/ChangeLog	2014-12-11 07:41:13 UTC (rev 177135)
@@ -1,3 +1,27 @@
+2014-12-10  Chris Dumez  <cdu...@apple.com>
+
+        http://omfgdogs.info/ only animates when you resize the window
+        https://bugs.webkit.org/show_bug.cgi?id=139435
+        <rdar://problem/19190493>
+
+        Reviewed by Simon Fraser.
+
+        Add layout tests to make sure a gif image is still animated if:
+        - It is used as a background image of a 0-height html element
+        - It is used as a background image of a 0-height body element that
+          is delegated to the root (because the root has no background)
+
+        Also add a layout test to make sure we still stop the gif animation
+        if it is used as background image of a body that is outside the
+        viewport.
+
+        * fast/images/animated-gif-body-delegated-background-image-expected.txt: Added.
+        * fast/images/animated-gif-body-delegated-background-image.html: Added.
+        * fast/images/animated-gif-body-outside-viewport-expected.txt: Added.
+        * fast/images/animated-gif-body-outside-viewport.html: Added.
+        * fast/images/animated-gif-html-background-image-expected.txt: Added.
+        * fast/images/animated-gif-html-background-image.html: Added.
+
 2014-12-10  Zalan Bujtas  <za...@apple.com>
 
         Continuously repainting large parts of Huffington Post.

Added: trunk/LayoutTests/fast/images/animated-gif-body-delegated-background-image-expected.txt (0 => 177135)


--- trunk/LayoutTests/fast/images/animated-gif-body-delegated-background-image-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/images/animated-gif-body-delegated-background-image-expected.txt	2014-12-11 07:41:13 UTC (rev 177135)
@@ -0,0 +1,11 @@
+Make sure the background image of the body element is animated if the body has 0 height and the background is delegated to the root
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS bodyHeight is "0px"
+PASS isBackgroundAnimated is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/images/animated-gif-body-delegated-background-image.html (0 => 177135)


--- trunk/LayoutTests/fast/images/animated-gif-body-delegated-background-image.html	                        (rev 0)
+++ trunk/LayoutTests/fast/images/animated-gif-body-delegated-background-image.html	2014-12-11 07:41:13 UTC (rev 177135)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body style="background-image: url('resources/animated.gif'); background-repeat: repeat; height: 100%; width: 100%;" _onload_="runTest()">
+<script>
+// The test passes if the background image is animated.
+jsTestIsAsync = true;
+
+function checkBackgroundAnimated() {
+  isBackgroundAnimated = !internals.hasPausedImageAnimations(document.body);
+  bodyHeight = window.getComputedStyle(document.body)["height"];
+  description("Make sure the background image of the body element is animated if the body has 0 height and the background is delegated to the root");
+  shouldBeEqualToString("bodyHeight", "0px");
+  shouldBeTrue("isBackgroundAnimated");
+  finishJSTest();
+}
+
+function imageLoaded() {
+  setTimeout(checkBackgroundAnimated, 200);
+}
+
+function runTest() {
+  if (!window.internals)
+    return;
+
+  var img = new Image();
+  // Make sure the image is loaded before we check if it is animated.
+  img._onload_ = imageLoaded;
+  img.src = ""
+}
+</script>
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/fast/images/animated-gif-body-outside-viewport-expected.txt (0 => 177135)


--- trunk/LayoutTests/fast/images/animated-gif-body-outside-viewport-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/images/animated-gif-body-outside-viewport-expected.txt	2014-12-11 07:41:13 UTC (rev 177135)
@@ -0,0 +1,12 @@
+Make sure the background image of a body element that is outside the viewport is not animated
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS isBackgroundAnimated is false
+Scroll down so that the body becomes visible.
+PASS isBackgroundAnimated is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/images/animated-gif-body-outside-viewport.html (0 => 177135)


--- trunk/LayoutTests/fast/images/animated-gif-body-outside-viewport.html	                        (rev 0)
+++ trunk/LayoutTests/fast/images/animated-gif-body-outside-viewport.html	2014-12-11 07:41:13 UTC (rev 177135)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html style="background-color: green; width: 1600px; height: 1200px">
+<head>
+<script src=""
+</head>
+<body style="background-image: url('resources/animated.gif'); background-repeat: repeat; position: relative; left: 0px; top: 700px; width: 200px; height: 200px" _onload_="runTest()">
+<script>
+jsTestIsAsync = true;
+
+function checkBackgroundAnimated() {
+  isBackgroundAnimated = !internals.hasPausedImageAnimations(document.body);
+  shouldBeTrue("isBackgroundAnimated");
+  finishJSTest();
+}
+
+function checkBackgroundNotAnimated() {
+  isBackgroundAnimated = !internals.hasPausedImageAnimations(document.body);
+  description("Make sure the background image of a body element that is outside the viewport is not animated");
+  shouldBeFalse("isBackgroundAnimated");
+
+  debug("Scroll down so that the body becomes visible.");
+  window.scrollBy(0, 600);
+  setTimeout(checkBackgroundAnimated, 0);
+}
+
+function imageLoaded() {
+  setTimeout(checkBackgroundNotAnimated, 200);
+}
+
+function runTest() {
+  if (!window.internals)
+    return;
+
+  var img = new Image();
+  // Make sure the image is loaded before we check if it is animated.
+  img._onload_ = imageLoaded;
+  img.src = ""
+}
+</script>
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/fast/images/animated-gif-html-background-image-expected.txt (0 => 177135)


--- trunk/LayoutTests/fast/images/animated-gif-html-background-image-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/images/animated-gif-html-background-image-expected.txt	2014-12-11 07:41:13 UTC (rev 177135)
@@ -0,0 +1,11 @@
+Make sure the background image of the html element is animated if the html element has 0 height
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS htmlHeight is "0px"
+PASS isBackgroundAnimated is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/images/animated-gif-html-background-image.html (0 => 177135)


--- trunk/LayoutTests/fast/images/animated-gif-html-background-image.html	                        (rev 0)
+++ trunk/LayoutTests/fast/images/animated-gif-html-background-image.html	2014-12-11 07:41:13 UTC (rev 177135)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html style="background-image: url('resources/animated.gif'); background-repeat: repeat; height: 0">
+<head>
+<script src=""
+</head>
+<body _onload_="runTest()">
+<script>
+jsTestIsAsync = true;
+
+function checkBackgroundAnimated() {
+  isBackgroundAnimated = !internals.hasPausedImageAnimations(document.documentElement);
+  htmlHeight = window.getComputedStyle(document.documentElement)["height"];
+  description("Make sure the background image of the html element is animated if the html element has 0 height");
+  shouldBeEqualToString("htmlHeight", "0px");
+  shouldBeTrue("isBackgroundAnimated");
+  finishJSTest();
+}
+
+function imageLoaded() {
+  setTimeout(checkBackgroundAnimated, 200);
+}
+
+function runTest() {
+  if (!window.internals)
+    return;
+
+  var img = new Image();
+  // Make sure the image is loaded before we check if it is animated.
+  img._onload_ = imageLoaded;
+  img.src = ""
+}
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (177134 => 177135)


--- trunk/Source/WebCore/ChangeLog	2014-12-11 07:37:20 UTC (rev 177134)
+++ trunk/Source/WebCore/ChangeLog	2014-12-11 07:41:13 UTC (rev 177135)
@@ -1,3 +1,33 @@
+2014-12-10  Chris Dumez  <cdu...@apple.com>
+
+        http://omfgdogs.info/ only animates when you resize the window
+        https://bugs.webkit.org/show_bug.cgi?id=139435
+        <rdar://problem/19190493>
+
+        Reviewed by Simon Fraser.
+
+        After r163928, we would fail to animate a gif if:
+        - it is used as a background image of a 0-height html element
+        - it is used as a background image of a 0-height body element whose
+          background is delegated to the root (because the root has no
+          background).
+
+        This is because in such cases, shouldRepaintForImageAnimation()
+        should use the background rect instead of the renderer's overflow
+        rect to determine if the image is inside the viewport. Both cases
+        are addressed in this patch.
+
+        Tests: fast/images/animated-gif-body-delegated-background-image.html
+               fast/images/animated-gif-body-outside-viewport.html
+               fast/images/animated-gif-html-background-image.html
+
+        * rendering/RenderElement.cpp:
+        (WebCore::shouldRepaintForImageAnimation):
+        * testing/Internals.cpp:
+        (WebCore::Internals::hasPausedImageAnimations):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2014-12-10  Timothy Horton  <timothy_hor...@apple.com>
 
         Fix the build.

Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (177134 => 177135)


--- trunk/Source/WebCore/rendering/RenderElement.cpp	2014-12-11 07:37:20 UTC (rev 177134)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp	2014-12-11 07:41:13 UTC (rev 177135)
@@ -1354,9 +1354,24 @@
         return false;
     if (renderer.style().visibility() != VISIBLE)
         return false;
-    if (!renderer.isInsideViewport(&visibleRect))
+    if (renderer.view().frameView().isOffscreen())
         return false;
 
+    // Use background rect if we are the root or if we are the body and the background is propagated to the root.
+    // FIXME: This is overly conservative as the image may not be a background-image, in which case it will not
+    // be propagated to the root. At this point, we unfortunately don't have access to the image anymore so we
+    // can no longer check if it is a background image.
+    bool backgroundIsPaintedByRoot = renderer.isRoot();
+    if (renderer.isBody()) {
+        // FIXME: Should share body background propagation code.
+        RenderElement* rootObject = renderer.document().documentElement() ? renderer.document().documentElement()->renderer() : nullptr;
+        backgroundIsPaintedByRoot = &rootObject->rendererForRootBackground() == &renderer;
+
+    }
+    LayoutRect backgroundPaintingRect = backgroundIsPaintedByRoot ? renderer.view().backgroundRect(&renderer.view()) : renderer.absoluteClippedOverflowRect();
+    if (!visibleRect.intersects(enclosingIntRect(backgroundPaintingRect)))
+        return false;
+
     return true;
 }
 
@@ -1367,6 +1382,9 @@
     auto& frameView = view().frameView();
     auto visibleRect = frameView.windowToContents(frameView.windowClipRect());
     if (!shouldRepaintForImageAnimation(*this, visibleRect)) {
+        // FIXME: It would be better to pass the image along with the renderer
+        // so that we can be smarter about detecting if the image is inside the
+        // viewport in repaintForPausedImageAnimationsIfNeeded().
         view().addRendererWithPausedImageAnimations(*this);
         return;
     }

Modified: trunk/Source/WebCore/testing/Internals.cpp (177134 => 177135)


--- trunk/Source/WebCore/testing/Internals.cpp	2014-12-11 07:37:20 UTC (rev 177134)
+++ trunk/Source/WebCore/testing/Internals.cpp	2014-12-11 07:41:13 UTC (rev 177135)
@@ -542,6 +542,15 @@
     return representation;
 }
 
+bool Internals::hasPausedImageAnimations(Element* element, ExceptionCode& ec)
+{
+    if (!element) {
+        ec = INVALID_ACCESS_ERR;
+        return false;
+    }
+    return element->renderer() && element->renderer()->hasPausedImageAnimations();
+}
+
 PassRefPtr<CSSComputedStyleDeclaration> Internals::computedStyleIncludingVisitedInfo(Node* node, ExceptionCode& ec) const
 {
     if (!node) {

Modified: trunk/Source/WebCore/testing/Internals.h (177134 => 177135)


--- trunk/Source/WebCore/testing/Internals.h	2014-12-11 07:37:20 UTC (rev 177134)
+++ trunk/Source/WebCore/testing/Internals.h	2014-12-11 07:41:13 UTC (rev 177135)
@@ -74,6 +74,7 @@
     static void resetToConsistentState(Page*);
 
     String elementRenderTreeAsText(Element*, ExceptionCode&);
+    bool hasPausedImageAnimations(Element*, ExceptionCode&);
 
     String address(Node*);
     bool nodeNeedsStyleRecalc(Node*, ExceptionCode&);

Modified: trunk/Source/WebCore/testing/Internals.idl (177134 => 177135)


--- trunk/Source/WebCore/testing/Internals.idl	2014-12-11 07:37:20 UTC (rev 177134)
+++ trunk/Source/WebCore/testing/Internals.idl	2014-12-11 07:41:13 UTC (rev 177135)
@@ -36,6 +36,9 @@
     [RaisesException] boolean nodeNeedsStyleRecalc(Node node);
     DOMString description(any value);
 
+    // Animated image pausing testing.
+    [RaisesException] boolean hasPausedImageAnimations(Element element);
+
     [RaisesException] DOMString elementRenderTreeAsText(Element element);
     boolean isPreloaded(DOMString url);
     boolean isLoadingFromMemoryCache(DOMString url);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to