Title: [195216] releases/WebKitGTK/webkit-2.10
Revision
195216
Author
[email protected]
Date
2016-01-18 05:20:31 -0800 (Mon, 18 Jan 2016)

Log Message

Merge r194706 - Directly-composited animated GIFs never resume once scrolled offscreen
https://bugs.webkit.org/show_bug.cgi?id=152817
<rdar://problem/19982020>

Reviewed by Daniel Bates.

Source/WebCore:

Directly-composited animated GIFs would never resume once scrolled
offscreen. This is because calling repaint() in this case would not
cause BitmapImage::draw() to be called and the animation would thus
not be resumed. To address the problem,
repaintForPausedImageAnimationsIfNeeded() now calls
RenderBoxModelObject::contentChanged(ImageChanged) in addition to
repaint() to make sure the animation actually gets resumed, even in
the directly-composited animated GIF case.

Test: fast/images/composited-animated-gif-outside-viewport.html

* platform/graphics/BitmapImage.h:
Make currentFrame() public so it can be exposed via Internals for the
purpose of testing.

* rendering/RenderElement.cpp:
(WebCore::RenderElement::repaintForPausedImageAnimationsIfNeeded):
Call RenderBoxModelObject::contentChanged(ImageChanged) in addition to
calling repaint() to make sure the animation actually gets resumed in
the directly-composited animated GIFs case.

* testing/Internals.cpp:
(WebCore::Internals::imageFrameIndex):
* testing/Internals.h:
* testing/Internals.idl:
Expose new "unsigned long imageFrameIndex(Element)" operation on
Internals so layout tests can better check if an image is actually
animating. Previously, we would rely on the output of
internals.hasPausedImageAnimations(Element) but this is not sufficient
to cover this bug as our rendering code believed it has resumed the
animations but the GIF was not actually animating due to it being
directly-composited.

LayoutTests:

Add a layout test to check that directly-composited animated GIFs are
properly suspended / resumed based on visibility inside the viewport.

* fast/images/composited-animated-gif-outside-viewport-expected.txt: Added.
* fast/images/composited-animated-gif-outside-viewport.html: Added.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog (195215 => 195216)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog	2016-01-18 13:08:58 UTC (rev 195215)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog	2016-01-18 13:20:31 UTC (rev 195216)
@@ -1,3 +1,17 @@
+2016-01-07  Chris Dumez  <[email protected]>
+
+        Directly-composited animated GIFs never resume once scrolled offscreen
+        https://bugs.webkit.org/show_bug.cgi?id=152817
+        <rdar://problem/19982020>
+
+        Reviewed by Daniel Bates.
+
+        Add a layout test to check that directly-composited animated GIFs are
+        properly suspended / resumed based on visibility inside the viewport.
+
+        * fast/images/composited-animated-gif-outside-viewport-expected.txt: Added.
+        * fast/images/composited-animated-gif-outside-viewport.html: Added.
+
 2016-01-06  Brent Fulgham  <[email protected]>
 
         Port blocking bypass issue using 307 redirect

Added: releases/WebKitGTK/webkit-2.10/LayoutTests/fast/images/composited-animated-gif-outside-viewport-expected.txt (0 => 195216)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/fast/images/composited-animated-gif-outside-viewport-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/fast/images/composited-animated-gif-outside-viewport-expected.txt	2016-01-18 13:20:31 UTC (rev 195216)
@@ -0,0 +1,15 @@
+Make sure directly-composited animated GIFs properly get suspended / resumed based on visibility inside viewport.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS isImageAnimated() became true
+Scroll down so that the image is no longer visible
+PASS isImageAnimated() became false
+Animation was paused, scroll up so that the image is visible again
+PASS isImageAnimated() became true
+PASS internals.imageFrameIndex(testImage) became different from frameIndexImageWasPausedAt
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: releases/WebKitGTK/webkit-2.10/LayoutTests/fast/images/composited-animated-gif-outside-viewport.html (0 => 195216)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/fast/images/composited-animated-gif-outside-viewport.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/fast/images/composited-animated-gif-outside-viewport.html	2016-01-18 13:20:31 UTC (rev 195216)
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html style="width: 1600px; height: 1200px">
+<head>
+<script src=""
+</head>
+<body _onload_="runTest()">
+<img id="testImage" src="" style="-webkit-transform: translatez(0);">
+<script>
+description("Make sure directly-composited animated GIFs properly get suspended / resumed based on visibility inside viewport.");
+jsTestIsAsync = true;
+
+var testImage = document.getElementById("testImage");
+var frameIndexImageWasPausedAt = 0;
+
+function isImageAnimated()
+{
+  return !internals.hasPausedImageAnimations(testImage);
+}
+
+function checkFrameIndexAndFinish()
+{
+  // Actually make sure that the frame index changes.
+  shouldBecomeDifferent("internals.imageFrameIndex(testImage)", "frameIndexImageWasPausedAt", finishJSTest);
+}
+
+function scrollUp()
+{
+  frameIndexImageWasPausedAt = internals.imageFrameIndex(testImage);
+  
+  debug("Animation was paused, scroll up so that the image is visible again");
+  window.scrollBy(0, -600);
+
+  shouldBecomeEqual("isImageAnimated()", "true", checkFrameIndexAndFinish);
+}
+
+function scrollDown()
+{
+  debug("Scroll down so that the image is no longer visible");
+  window.scrollBy(0, 600);
+  shouldBecomeEqual("isImageAnimated()", "false", scrollUp);
+}
+
+function runTest()
+{
+  if (!window.internals)
+    return;
+
+  shouldBecomeEqual("isImageAnimated()", "true", scrollDown);
+}
+</script>
+<script src=""
+</body>
+</html>

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog (195215 => 195216)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog	2016-01-18 13:08:58 UTC (rev 195215)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog	2016-01-18 13:20:31 UTC (rev 195216)
@@ -1,3 +1,44 @@
+2016-01-07  Chris Dumez  <[email protected]>
+
+        Directly-composited animated GIFs never resume once scrolled offscreen
+        https://bugs.webkit.org/show_bug.cgi?id=152817
+        <rdar://problem/19982020>
+
+        Reviewed by Daniel Bates.
+
+        Directly-composited animated GIFs would never resume once scrolled
+        offscreen. This is because calling repaint() in this case would not
+        cause BitmapImage::draw() to be called and the animation would thus
+        not be resumed. To address the problem,
+        repaintForPausedImageAnimationsIfNeeded() now calls
+        RenderBoxModelObject::contentChanged(ImageChanged) in addition to
+        repaint() to make sure the animation actually gets resumed, even in
+        the directly-composited animated GIF case.
+
+        Test: fast/images/composited-animated-gif-outside-viewport.html
+
+        * platform/graphics/BitmapImage.h:
+        Make currentFrame() public so it can be exposed via Internals for the
+        purpose of testing.
+
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::repaintForPausedImageAnimationsIfNeeded):
+        Call RenderBoxModelObject::contentChanged(ImageChanged) in addition to
+        calling repaint() to make sure the animation actually gets resumed in
+        the directly-composited animated GIFs case.
+
+        * testing/Internals.cpp:
+        (WebCore::Internals::imageFrameIndex):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+        Expose new "unsigned long imageFrameIndex(Element)" operation on
+        Internals so layout tests can better check if an image is actually
+        animating. Previously, we would rely on the output of
+        internals.hasPausedImageAnimations(Element) but this is not sufficient
+        to cover this bug as our rendering code believed it has resumed the
+        animations but the GIF was not actually animating due to it being
+        directly-composited.
+
 2016-01-07  Michael Catanzaro  <[email protected]>
 
         [SOUP] Consider reducing max simultaneous connections

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/platform/graphics/BitmapImage.h (195215 => 195216)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/platform/graphics/BitmapImage.h	2016-01-18 13:08:58 UTC (rev 195215)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/platform/graphics/BitmapImage.h	2016-01-18 13:20:31 UTC (rev 195216)
@@ -185,6 +185,8 @@
 
     bool allowSubsampling() const { return m_allowSubsampling; }
     void setAllowSubsampling(bool allowSubsampling) { m_allowSubsampling = allowSubsampling; }
+
+    size_t currentFrame() const { return m_currentFrame; }
     
 private:
     void updateSize(ImageOrientationDescription = ImageOrientationDescription()) const;
@@ -210,7 +212,6 @@
         const FloatPoint& phase, ColorSpace styleColorSpace, CompositeOperator, const FloatRect& destRect);
 #endif
 
-    size_t currentFrame() const { return m_currentFrame; }
     size_t frameCount();
 
     PassNativeImagePtr frameAtIndex(size_t, float presentationScaleHint = 1);

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/rendering/RenderElement.cpp (195215 => 195216)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/rendering/RenderElement.cpp	2016-01-18 13:08:58 UTC (rev 195215)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/rendering/RenderElement.cpp	2016-01-18 13:20:31 UTC (rev 195216)
@@ -1469,7 +1469,13 @@
     ASSERT(m_hasPausedImageAnimations);
     if (!shouldRepaintForImageAnimation(*this, visibleRect))
         return false;
+
     repaint();
+
+    // For directly-composited animated GIFs it does not suffice to call repaint() to resume animation. We need to mark the image as changed.
+    if (is<RenderBoxModelObject>(*this))
+        downcast<RenderBoxModelObject>(*this).contentChanged(ImageChanged);
+
     return true;
 }
 

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.cpp (195215 => 195216)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.cpp	2016-01-18 13:08:58 UTC (rev 195215)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.cpp	2016-01-18 13:20:31 UTC (rev 195216)
@@ -31,6 +31,7 @@
 #include "AnimationController.h"
 #include "ApplicationCacheStorage.h"
 #include "BackForwardController.h"
+#include "BitmapImage.h"
 #include "CachedImage.h"
 #include "CachedResourceLoader.h"
 #include "Chrome.h"
@@ -530,6 +531,21 @@
     return MemoryCache::singleton().size();
 }
 
+size_t Internals::imageFrameIndex(Element* element, ExceptionCode& ec)
+{
+    if (!is<HTMLImageElement>(element)) {
+        ec = TypeError;
+        return 0;
+    }
+
+    auto* cachedImage = downcast<HTMLImageElement>(*element).cachedImage();
+    if (!cachedImage)
+        return 0;
+
+    auto* image = cachedImage->image();
+    return is<BitmapImage>(image) ? downcast<BitmapImage>(*image).currentFrame() : 0;
+}
+
 void Internals::clearPageCache()
 {
     PageCache::singleton().pruneToSizeNow(0, PruningReason::None);

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.h (195215 => 195216)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.h	2016-01-18 13:08:58 UTC (rev 195215)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.h	2016-01-18 13:20:31 UTC (rev 195216)
@@ -102,6 +102,8 @@
     void pruneMemoryCacheToSize(unsigned size);
     unsigned memoryCacheSize() const;
 
+    size_t imageFrameIndex(Element*, ExceptionCode&);
+
     void clearPageCache();
     unsigned pageCacheSize() const;
 

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.idl (195215 => 195216)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.idl	2016-01-18 13:08:58 UTC (rev 195215)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.idl	2016-01-18 13:20:31 UTC (rev 195216)
@@ -205,6 +205,8 @@
 
     [RaisesException] boolean isPageBoxVisible(long pageNumber);
 
+    [RaisesException] unsigned long imageFrameIndex(Element element);
+
     readonly attribute InternalSettings settings;
     readonly attribute unsigned long workerThreadCount;
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to