Diff
Modified: trunk/LayoutTests/ChangeLog (163927 => 163928)
--- trunk/LayoutTests/ChangeLog 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/LayoutTests/ChangeLog 2014-02-12 02:55:22 UTC (rev 163928)
@@ -1,3 +1,13 @@
+2014-02-11 Antti Koivisto <[email protected]>
+
+ GIF animations should be suspended when outside of viewport
+ https://bugs.webkit.org/show_bug.cgi?id=128632
+
+ Reviewed by Andreas Kling.
+
+ * fast/repaint/no-animation-outside-viewport-expected.txt: Added.
+ * fast/repaint/no-animation-outside-viewport.html: Added.
+
2014-02-11 Ryosuke Niwa <[email protected]>
Add failing test expectations to asynchronous spellchecking tests.
Added: trunk/LayoutTests/fast/repaint/no-animation-outside-viewport-expected.txt (0 => 163928)
--- trunk/LayoutTests/fast/repaint/no-animation-outside-viewport-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/repaint/no-animation-outside-viewport-expected.txt 2014-02-12 02:55:22 UTC (rev 163928)
@@ -0,0 +1,10 @@
+Test that animated gif outside viewport does not trigger repaint.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS repaintRects is ""
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/repaint/no-animation-outside-viewport.html (0 => 163928)
--- trunk/LayoutTests/fast/repaint/no-animation-outside-viewport.html (rev 0)
+++ trunk/LayoutTests/fast/repaint/no-animation-outside-viewport.html 2014-02-12 02:55:22 UTC (rev 163928)
@@ -0,0 +1,39 @@
+<html>
+<head>
+<script>jsTestIsAsync = true;</script>
+<script src=""
+<script>
+ description("Test that animated gif outside viewport does not trigger repaint.");
+ function startTrackingRepaints()
+ {
+ document.body.offsetTop;
+ window.internals.startTrackingRepaints();
+ testRunner.display();
+ setTimeout(logRepaints, 200);
+ }
+
+ function logRepaints()
+ {
+ repaintRects = window.internals.repaintRectsAsText();
+ window.internals.stopTrackingRepaints();
+
+ shouldBeEqualToString("repaintRects", "");
+
+ finishJSTest();
+ }
+
+ function start() {
+ if (!window.testRunner || !window.internals)
+ return;
+
+ var img = new Image();
+ img._onload_ = startTrackingRepaints;
+ img.src = ""
+ }
+</script>
+</head>
+<body _onload_="start()">
+<div style="height:600px"></div>
+<img src=""
+<script src=""
+</html>
Modified: trunk/Source/WebCore/ChangeLog (163927 => 163928)
--- trunk/Source/WebCore/ChangeLog 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/ChangeLog 2014-02-12 02:55:22 UTC (rev 163928)
@@ -1,3 +1,72 @@
+2014-02-11 Antti Koivisto <[email protected]>
+
+ GIF animations should be suspended when outside of viewport
+ https://bugs.webkit.org/show_bug.cgi?id=128632
+
+ Reviewed by Andreas Kling.
+
+ Animations are driven by the paint cycle. Speculative tiles keep animations outside the actual viewport going.
+
+ Pause animations when they are outside the viewport by not painting them.
+
+ Test: fast/repaint/no-animation-outside-viewport.html
+
+ * loader/cache/CachedImage.cpp:
+ (WebCore::CachedImage::animationAdvanced):
+
+ Call animation specific newImageAnimationFrameAvailable instead of the generic notifyObservers.
+
+ * loader/cache/CachedImage.h:
+
+ Removed now unnecessary resumeAnimatingImagesForLoader mechanism.
+ Remove unnecessary shouldPauseAnimation. Pausing is now always done when by avoiding repaint.
+
+ * loader/cache/CachedImageClient.h:
+ (WebCore::CachedImageClient::newImageAnimationFrameAvailable):
+ * page/FrameView.cpp:
+ (WebCore::FrameView::scrollPositionChanged):
+
+ Check if we have image animations to resume when scroll position changes.
+
+ * page/Page.cpp:
+ (WebCore::Page::resumeAnimatingImages):
+
+ Use the same mechanism when resuming background tabs etc.
+
+ * platform/graphics/BitmapImage.cpp:
+ (WebCore::BitmapImage::internalAdvanceAnimation):
+
+ Remove the shouldPauseAnimation test, always rely on pausing on invalidation.
+
+ * platform/graphics/ImageObserver.h:
+ * rendering/RenderBoxModelObject.cpp:
+ * rendering/RenderElement.cpp:
+ (WebCore::RenderElement::RenderElement):
+ (WebCore::RenderElement::~RenderElement):
+ (WebCore::shouldRepaintForImageAnimation):
+
+ Factor the pausing conditions to a function. Test that the animation is withing the
+ visible rect.
+
+ (WebCore::RenderElement::newImageAnimationFrameAvailable):
+
+ Add renderer to the paused animation set if we don't continue the animation.
+
+ (WebCore::RenderElement::repaintForPausedImageAnimationsIfNeeded):
+
+ Resume the paused animations by triggering repaint.
+
+ * rendering/RenderElement.h:
+ (WebCore::RenderElement::setHasPausedImageAnimations):
+ (WebCore::RenderElement::hasPausedImageAnimations):
+ * rendering/RenderObject.cpp:
+ * rendering/RenderObject.h:
+ * rendering/RenderView.cpp:
+ (WebCore::RenderView::addRendererWithPausedImageAnimations):
+ (WebCore::RenderView::removeRendererWithPausedImageAnimations):
+ (WebCore::RenderView::resumePausedImageAnimationsIfNeeded):
+ * rendering/RenderView.h:
+
2014-02-11 Andreas Kling <[email protected]>
Remove unused RenderNamedFlowThread::previousRendererForNode().
Modified: trunk/Source/WebCore/loader/cache/CachedImage.cpp (163927 => 163928)
--- trunk/Source/WebCore/loader/cache/CachedImage.cpp 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/loader/cache/CachedImage.cpp 2014-02-12 02:55:22 UTC (rev 163928)
@@ -499,25 +499,13 @@
CachedResource::didAccessDecodedData(timeStamp);
}
-bool CachedImage::shouldPauseAnimation(const Image* image)
-{
- if (!image || image != m_image)
- return false;
-
- CachedResourceClientWalker<CachedImageClient> w(m_clients);
- while (CachedImageClient* c = w.next()) {
- if (c->willRenderImage(this))
- return false;
- }
-
- return true;
-}
-
void CachedImage::animationAdvanced(const Image* image)
{
if (!image || image != m_image)
return;
- notifyObservers();
+ CachedResourceClientWalker<CachedImageClient> clientWalker(m_clients);
+ while (CachedImageClient* client = clientWalker.next())
+ client->newImageAnimationFrameAvailable(*this);
}
void CachedImage::changedInRect(const Image* image, const IntRect& rect)
@@ -527,27 +515,6 @@
notifyObservers(&rect);
}
-void CachedImage::resumeAnimatingImagesForLoader(CachedResourceLoader* loader)
-{
- const CachedResourceLoader::DocumentResourceMap& resources = loader->allCachedResources();
-
- for (CachedResourceLoader::DocumentResourceMap::const_iterator it = resources.begin(), end = resources.end(); it != end; ++it) {
- const CachedResourceHandle<CachedResource>& resource = it->value;
- if (!resource || !resource->isImage())
- continue;
- CachedImage* cachedImage = toCachedImage(resource.get());
- if (!cachedImage->hasImage())
- continue;
- Image* image = cachedImage->image();
- if (!image->isBitmapImage())
- continue;
- BitmapImage* bitmapImage = toBitmapImage(image);
- if (!bitmapImage->canAnimate())
- continue;
- cachedImage->animationAdvanced(bitmapImage);
- }
-}
-
bool CachedImage::currentFrameKnownToBeOpaque(const RenderElement* renderer)
{
Image* image = imageForRenderer(renderer);
Modified: trunk/Source/WebCore/loader/cache/CachedImage.h (163927 => 163928)
--- trunk/Source/WebCore/loader/cache/CachedImage.h 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/loader/cache/CachedImage.h 2014-02-12 02:55:22 UTC (rev 163928)
@@ -84,8 +84,6 @@
bool isManuallyCached() const { return m_isManuallyCached; }
virtual bool mustRevalidateDueToCacheHeaders(CachePolicy) const;
- static void resumeAnimatingImagesForLoader(CachedResourceLoader*);
-
#if ENABLE(DISK_IMAGE_CACHE)
virtual bool canUseDiskImageCache() const override;
virtual void useDiskImageCache() override;
@@ -128,7 +126,6 @@
virtual void decodedSizeChanged(const Image*, int delta) override;
virtual void didDraw(const Image*) override;
- virtual bool shouldPauseAnimation(const Image*) override;
virtual void animationAdvanced(const Image*) override;
virtual void changedInRect(const Image*, const IntRect&) override;
Modified: trunk/Source/WebCore/loader/cache/CachedImageClient.h (163927 => 163928)
--- trunk/Source/WebCore/loader/cache/CachedImageClient.h 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/loader/cache/CachedImageClient.h 2014-02-12 02:55:22 UTC (rev 163928)
@@ -36,15 +36,12 @@
static CachedResourceClientType expectedType() { return ImageType; }
virtual CachedResourceClientType resourceClientType() const override { return expectedType(); }
- // Called whenever a frame of an image changes, either because we got more data from the network or
- // because we are animating. If not null, the IntRect is the changed rect of the image.
+ // Called whenever a frame of an image changes because we got more data from the network.
+ // If not null, the IntRect is the changed rect of the image.
virtual void imageChanged(CachedImage*, const IntRect* = 0) { }
- // Called to find out if this client wants to actually display the image. Used to tell when we
- // can halt animation. Content nodes that hold image refs for example would not render the image,
- // but RenderImages would (assuming they have visibility: visible and their render tree isn't hidden
- // e.g., in the b/f cache or in a background tab).
- virtual bool willRenderImage(CachedImage*) { return false; }
+ // Called when GIF animation progresses.
+ virtual void newImageAnimationFrameAvailable(CachedImage& image) { imageChanged(&image); }
};
}
Modified: trunk/Source/WebCore/page/FrameView.cpp (163927 => 163928)
--- trunk/Source/WebCore/page/FrameView.cpp 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/page/FrameView.cpp 2014-02-12 02:55:22 UTC (rev 163928)
@@ -1947,6 +1947,7 @@
sendWillRevealEdgeEventsIfNeeded(oldPosition, newPosition);
if (RenderView* renderView = this->renderView()) {
+ renderView->resumePausedImageAnimationsIfNeeded();
if (renderView->usesCompositing())
renderView->compositor().frameViewDidScroll();
}
Modified: trunk/Source/WebCore/page/Page.cpp (163927 => 163928)
--- trunk/Source/WebCore/page/Page.cpp 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/page/Page.cpp 2014-02-12 02:55:22 UTC (rev 163928)
@@ -1144,8 +1144,10 @@
// Drawing models which cache painted content while out-of-window (WebKit2's composited drawing areas, etc.)
// require that we repaint animated images to kickstart the animation loop.
- for (Frame* frame = m_mainFrame.get(); frame; frame = frame->tree().traverseNext())
- CachedImage::resumeAnimatingImagesForLoader(frame->document()->cachedResourceLoader());
+ for (Frame* frame = m_mainFrame.get(); frame; frame = frame->tree().traverseNext()) {
+ if (auto* renderView = frame->contentRenderer())
+ renderView->resumePausedImageAnimationsIfNeeded();
+ }
}
void Page::setViewState(ViewState::Flags viewState)
Modified: trunk/Source/WebCore/platform/graphics/BitmapImage.cpp (163927 => 163928)
--- trunk/Source/WebCore/platform/graphics/BitmapImage.cpp 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/platform/graphics/BitmapImage.cpp 2014-02-12 02:55:22 UTC (rev 163928)
@@ -709,13 +709,6 @@
// Stop the animation.
stopAnimation();
-#if !PLATFORM(IOS)
- // See if anyone is still paying attention to this animation. If not, we don't
- // advance and will remain suspended at the current frame until the animation is resumed.
- if (!skippingFrames && imageObserver()->shouldPauseAnimation(this))
- return false;
-#endif
-
++m_currentFrame;
bool advancedAnimation = true;
bool destroyAll = false;
Modified: trunk/Source/WebCore/platform/graphics/ImageObserver.h (163927 => 163928)
--- trunk/Source/WebCore/platform/graphics/ImageObserver.h 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/platform/graphics/ImageObserver.h 2014-02-12 02:55:22 UTC (rev 163928)
@@ -40,7 +40,6 @@
virtual void decodedSizeChanged(const Image*, int delta) = 0;
virtual void didDraw(const Image*) = 0;
- virtual bool shouldPauseAnimation(const Image*) = 0;
virtual void animationAdvanced(const Image*) = 0;
virtual void changedInRect(const Image*, const IntRect&) = 0;
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (163927 => 163928)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2014-02-12 02:55:22 UTC (rev 163928)
@@ -27,6 +27,7 @@
#include "RenderBoxModelObject.h"
#include "Frame.h"
+#include "FrameView.h"
#include "GraphicsContext.h"
#include "HTMLFrameOwnerElement.h"
#include "HTMLNames.h"
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (163927 => 163928)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2014-02-12 02:55:22 UTC (rev 163928)
@@ -69,6 +69,7 @@
, m_ancestorLineBoxDirty(false)
, m_hasInitializedStyle(false)
, m_renderInlineAlwaysCreatesLineBoxes(false)
+ , m_hasPausedImageAnimations(false)
, m_firstChild(nullptr)
, m_lastChild(nullptr)
, m_style(std::move(style))
@@ -81,6 +82,7 @@
, m_ancestorLineBoxDirty(false)
, m_hasInitializedStyle(false)
, m_renderInlineAlwaysCreatesLineBoxes(false)
+ , m_hasPausedImageAnimations(false)
, m_firstChild(nullptr)
, m_lastChild(nullptr)
, m_style(std::move(style))
@@ -119,6 +121,8 @@
}
#endif
}
+ if (m_hasPausedImageAnimations)
+ view().removeRendererWithPausedImageAnimations(*this);
}
RenderPtr<RenderElement> RenderElement::createFor(Element& element, PassRef<RenderStyle> style)
@@ -1275,6 +1279,47 @@
return borderImage && borderImage->canRender(this, style().effectiveZoom()) && borderImage->isLoaded();
}
+static bool shouldRepaintForImageAnimation(const RenderElement& renderer, const IntRect& visibleRect)
+{
+ const Document& document = renderer.document();
+ if (document.inPageCache())
+ return false;
+ auto& frameView = renderer.view().frameView();
+ if (frameView.isOffscreen())
+ return false;
+#if PLATFORM(IOS)
+ if (frameView.timersPaused())
+ return false;
+#endif
+ if (document.activeDOMObjectsAreSuspended())
+ return false;
+ if (renderer.style().visibility() != VISIBLE)
+ return false;
+ if (!visibleRect.intersects(renderer.absoluteBoundingBoxRect()))
+ return false;
+
+ return true;
+}
+
+void RenderElement::newImageAnimationFrameAvailable(CachedImage& image)
+{
+ auto visibleRect = view().frameView().visibleContentRect();
+ if (!shouldRepaintForImageAnimation(*this, visibleRect)) {
+ view().addRendererWithPausedImageAnimations(*this);
+ return;
+ }
+ imageChanged(&image);
+}
+
+bool RenderElement::repaintForPausedImageAnimationsIfNeeded(const IntRect& visibleRect)
+{
+ ASSERT(m_hasPausedImageAnimations);
+ if (!shouldRepaintForImageAnimation(*this, visibleRect))
+ return false;
+ repaint();
+ return true;
+}
+
RenderNamedFlowThread* RenderElement::renderNamedFlowThreadWrapper()
{
auto renderer = this;
Modified: trunk/Source/WebCore/rendering/RenderElement.h (163927 => 163928)
--- trunk/Source/WebCore/rendering/RenderElement.h 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/rendering/RenderElement.h 2014-02-12 02:55:22 UTC (rev 163928)
@@ -141,6 +141,10 @@
bool hasBlendMode() const { return false; }
#endif
+ bool repaintForPausedImageAnimationsIfNeeded(const IntRect& visibleRect);
+ bool hasPausedImageAnimations() const { return m_hasPausedImageAnimations; }
+ void setHasPausedImageAnimations(bool b) { m_hasPausedImageAnimations = b; }
+
RenderNamedFlowThread* renderNamedFlowThreadWrapper();
protected:
@@ -200,12 +204,14 @@
StyleDifference adjustStyleDifference(StyleDifference, unsigned contextSensitiveProperties) const;
RenderStyle* cachedFirstLineStyle() const;
+ virtual void newImageAnimationFrameAvailable(CachedImage&) final override;
+
unsigned m_baseTypeFlags : 6;
bool m_ancestorLineBoxDirty : 1;
bool m_hasInitializedStyle : 1;
- // Specific to RenderInline.
bool m_renderInlineAlwaysCreatesLineBoxes : 1;
+ bool m_hasPausedImageAnimations : 1;
RenderObject* m_firstChild;
RenderObject* m_lastChild;
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (163927 => 163928)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2014-02-12 02:55:22 UTC (rev 163928)
@@ -2203,26 +2203,6 @@
}
#endif
-bool RenderObject::willRenderImage(CachedImage*)
-{
- // Without visibility we won't render (and therefore don't care about animation).
- if (style().visibility() != VISIBLE)
- return false;
-
-#if PLATFORM(IOS)
- if (document().frame()->timersPaused())
- return false;
-#else
- // We will not render a new image when Active DOM is suspended
- if (document().activeDOMObjectsAreSuspended())
- return false;
-#endif
-
- // If we're not in a window (i.e., we're dormant from being put in the b/f cache or in a background tab)
- // then we don't want to render either.
- return !document().inPageCache() && !document().view()->isOffscreen();
-}
-
int RenderObject::maximalOutlineSize(PaintPhase p) const
{
if (p != PaintPhaseOutline && p != PaintPhaseSelfOutline && p != PaintPhaseChildOutlines)
Modified: trunk/Source/WebCore/rendering/RenderObject.h (163927 => 163928)
--- trunk/Source/WebCore/rendering/RenderObject.h 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2014-02-12 02:55:22 UTC (rev 163928)
@@ -847,7 +847,6 @@
virtual void imageChanged(CachedImage*, const IntRect* = 0) override;
virtual void imageChanged(WrappedImagePtr, const IntRect* = 0) { }
- virtual bool willRenderImage(CachedImage*) override;
void selectionStartEnd(int& spos, int& epos) const;
Modified: trunk/Source/WebCore/rendering/RenderView.cpp (163927 => 163928)
--- trunk/Source/WebCore/rendering/RenderView.cpp 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/rendering/RenderView.cpp 2014-02-12 02:55:22 UTC (rev 163928)
@@ -1178,6 +1178,37 @@
return *m_imageQualityController;
}
+void RenderView::addRendererWithPausedImageAnimations(RenderElement& renderer)
+{
+ if (renderer.hasPausedImageAnimations()) {
+ ASSERT(m_renderersWithPausedImageAnimation.contains(&renderer));
+ return;
+ }
+ renderer.setHasPausedImageAnimations(true);
+ m_renderersWithPausedImageAnimation.add(&renderer);
+}
+
+void RenderView::removeRendererWithPausedImageAnimations(RenderElement& renderer)
+{
+ ASSERT(renderer.hasPausedImageAnimations());
+ ASSERT(m_renderersWithPausedImageAnimation.contains(&renderer));
+
+ renderer.setHasPausedImageAnimations(false);
+ m_renderersWithPausedImageAnimation.remove(&renderer);
+}
+
+void RenderView::resumePausedImageAnimationsIfNeeded()
+{
+ auto visibleRect = frameView().visibleContentRect();
+ Vector<RenderElement*, 10> toRemove;
+ for (auto* renderer : m_renderersWithPausedImageAnimation) {
+ if (renderer->repaintForPausedImageAnimationsIfNeeded(visibleRect))
+ toRemove.append(renderer);
+ }
+ for (auto& renderer : toRemove)
+ removeRendererWithPausedImageAnimations(*renderer);
+}
+
RenderView::RepaintRegionAccumulator::RepaintRegionAccumulator(RenderView* view)
: m_rootView(view ? view->document().topDocument().renderView() : nullptr)
{
Modified: trunk/Source/WebCore/rendering/RenderView.h (163927 => 163928)
--- trunk/Source/WebCore/rendering/RenderView.h 2014-02-12 02:52:19 UTC (rev 163927)
+++ trunk/Source/WebCore/rendering/RenderView.h 2014-02-12 02:55:22 UTC (rev 163928)
@@ -27,6 +27,7 @@
#include "PODFreeListArena.h"
#include "Region.h"
#include "RenderBlockFlow.h"
+#include <wtf/HashSet.h>
#include <wtf/OwnPtr.h>
namespace WebCore {
@@ -219,6 +220,10 @@
void didCreateRenderer() { ++m_rendererCount; }
void didDestroyRenderer() { --m_rendererCount; }
+ void resumePausedImageAnimationsIfNeeded();
+ void addRendererWithPausedImageAnimations(RenderElement&);
+ void removeRendererWithPausedImageAnimations(RenderElement&);
+
class RepaintRegionAccumulator {
WTF_MAKE_NONCOPYABLE(RepaintRegionAccumulator);
public:
@@ -340,6 +345,8 @@
#if ENABLE(CSS_FILTERS)
bool m_hasSoftwareFilters;
#endif
+
+ HashSet<RenderElement*> m_renderersWithPausedImageAnimation;
};
RENDER_OBJECT_TYPE_CASTS(RenderView, isRenderView())
