Infinite repaint loop with SVGImageCache and deferred repaint timers
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Mar 2012 20:26:58 +0000 (20:26 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Mar 2012 20:26:58 +0000 (20:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=78315
<rdar://problem/10651634>

Reviewed by Nikolas Zimmermann.

Only defer image redraw on a timer if we're in layout. This breaks
the repaint loop while still preventing us from drawing inside layout.

Completely disable repaint during relayout inside SVGImage::drawSVGToImageBuffer,
preventing deferred repaint timers from being started during that process.

No new tests, as the problem only occurs in a nonstandard configuration.

* page/FrameView.cpp:
(WebCore::FrameView::FrameView):
(WebCore::FrameView::reset):
(WebCore::FrameView::repaintContentRectangle):
(WebCore::FrameView::endDeferredRepaints):
(WebCore::FrameView::startDeferredRepaintTimer):
(WebCore):
(WebCore::FrameView::doDeferredRepaints):
(WebCore::FrameView::deferredRepaintTimerFired):
(WebCore::FrameView::beginDisableRepaints):
(WebCore::FrameView::endDisableRepaints):
* page/FrameView.h:
(FrameView):
(WebCore::FrameView::repaintsDisabled):
* rendering/RenderView.cpp:
(WebCore::RenderView::shouldRepaint):
* svg/graphics/SVGImage.cpp:
(WebCore::SVGImage::drawSVGToImageBuffer):
(WebCore::SVGImage::draw):
(WebCore::SVGImage::frameView):
(WebCore):
* svg/graphics/SVGImage.h:
(WebCore):
* svg/graphics/SVGImageCache.cpp:
(WebCore::SVGImageCache::imageContentChanged):
(WebCore::SVGImageCache::redraw):
(WebCore::SVGImageCache::redrawTimerFired):
(WebCore):
* svg/graphics/SVGImageCache.h:
(SVGImageCache):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@110469 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/rendering/RenderView.cpp
Source/WebCore/svg/graphics/SVGImage.cpp
Source/WebCore/svg/graphics/SVGImage.h
Source/WebCore/svg/graphics/SVGImageCache.cpp
Source/WebCore/svg/graphics/SVGImageCache.h

index 5fafc3b..60eb052 100644 (file)
@@ -1,3 +1,50 @@
+2012-03-12  Tim Horton  <timothy_horton@apple.com>
+
+        Infinite repaint loop with SVGImageCache and deferred repaint timers
+        https://bugs.webkit.org/show_bug.cgi?id=78315
+        <rdar://problem/10651634>
+
+        Reviewed by Nikolas Zimmermann.
+
+        Only defer image redraw on a timer if we're in layout. This breaks
+        the repaint loop while still preventing us from drawing inside layout.
+
+        Completely disable repaint during relayout inside SVGImage::drawSVGToImageBuffer,
+        preventing deferred repaint timers from being started during that process.
+
+        No new tests, as the problem only occurs in a nonstandard configuration.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::FrameView):
+        (WebCore::FrameView::reset):
+        (WebCore::FrameView::repaintContentRectangle):
+        (WebCore::FrameView::endDeferredRepaints):
+        (WebCore::FrameView::startDeferredRepaintTimer):
+        (WebCore):
+        (WebCore::FrameView::doDeferredRepaints):
+        (WebCore::FrameView::deferredRepaintTimerFired):
+        (WebCore::FrameView::beginDisableRepaints):
+        (WebCore::FrameView::endDisableRepaints):
+        * page/FrameView.h:
+        (FrameView):
+        (WebCore::FrameView::repaintsDisabled):
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::shouldRepaint):
+        * svg/graphics/SVGImage.cpp:
+        (WebCore::SVGImage::drawSVGToImageBuffer):
+        (WebCore::SVGImage::draw):
+        (WebCore::SVGImage::frameView):
+        (WebCore):
+        * svg/graphics/SVGImage.h:
+        (WebCore):
+        * svg/graphics/SVGImageCache.cpp:
+        (WebCore::SVGImageCache::imageContentChanged):
+        (WebCore::SVGImageCache::redraw):
+        (WebCore::SVGImageCache::redrawTimerFired):
+        (WebCore):
+        * svg/graphics/SVGImageCache.h:
+        (SVGImageCache):
+
 2012-03-12  Adam Klein  <adamk@chromium.org>
 
         [MutationObservers] Enforce a consistent order of MutationRecord delivery
index 5ecbca3..d99841a 100644 (file)
@@ -138,6 +138,7 @@ FrameView::FrameView(Frame* frame)
     , m_wasScrolledByUser(false)
     , m_inProgrammaticScroll(false)
     , m_deferredRepaintTimer(this, &FrameView::deferredRepaintTimerFired)
+    , m_disableRepaints(0)
     , m_isTrackingRepaints(false)
     , m_shouldUpdateWhileOffscreen(true)
     , m_deferSetNeedsLayouts(0)
@@ -244,6 +245,7 @@ void FrameView::reset()
     m_isVisuallyNonEmpty = false;
     m_firstVisuallyNonEmptyLayoutCallbackPending = true;
     m_maintainScrollPositionAnchor = 0;
+    m_disableRepaints = 0;
 }
 
 bool FrameView::isFrameView() const 
@@ -1754,7 +1756,7 @@ const unsigned cRepaintRectUnionThreshold = 25;
 void FrameView::repaintContentRectangle(const IntRect& r, bool immediate)
 {
     ASSERT(!m_frame->ownerElement());
-    
+
     if (m_isTrackingRepaints) {
         IntRect repaintRect = r;
         repaintRect.move(-scrollOffset());
@@ -1780,9 +1782,10 @@ void FrameView::repaintContentRectangle(const IntRect& r, bool immediate)
         else
             m_repaintRects[0].unite(paintRect);
         m_repaintCount++;
-    
-        if (!m_deferringRepaints && !m_deferredRepaintTimer.isActive())
-             m_deferredRepaintTimer.startOneShot(delay);
+
+        if (!m_deferringRepaints)
+            startDeferredRepaintTimer(delay);
+
         return;
     }
     
@@ -1835,7 +1838,6 @@ void FrameView::beginDeferredRepaints()
     m_deferringRepaints++;
 }
 
-
 void FrameView::endDeferredRepaints()
 {
     Page* page = m_frame->page();
@@ -1848,18 +1850,26 @@ void FrameView::endDeferredRepaints()
 
     if (--m_deferringRepaints)
         return;
-    
-    if (m_deferredRepaintTimer.isActive())
-        return;
 
     if (double delay = adjustedDeferredRepaintDelay()) {
-        m_deferredRepaintTimer.startOneShot(delay);
+        startDeferredRepaintTimer(delay);
         return;
     }
     
     doDeferredRepaints();
 }
 
+void FrameView::startDeferredRepaintTimer(double delay)
+{
+    if (m_deferredRepaintTimer.isActive())
+        return;
+
+    if (m_disableRepaints)
+        return;
+
+    m_deferredRepaintTimer.startOneShot(delay);
+}
+
 void FrameView::checkStopDelayingDeferredRepaints()
 {
     if (!m_deferredRepaintTimer.isActive())
@@ -1876,6 +1886,9 @@ void FrameView::checkStopDelayingDeferredRepaints()
     
 void FrameView::doDeferredRepaints()
 {
+    if (m_disableRepaints)
+        return;
+
     ASSERT(!m_deferringRepaints);
     if (!shouldUpdate()) {
         m_repaintRects.clear();
@@ -1934,7 +1947,18 @@ double FrameView::adjustedDeferredRepaintDelay() const
 void FrameView::deferredRepaintTimerFired(Timer<FrameView>*)
 {
     doDeferredRepaints();
-}    
+}
+
+void FrameView::beginDisableRepaints()
+{
+    m_disableRepaints++;
+}
+
+void FrameView::endDisableRepaints()
+{
+    ASSERT(m_disableRepaints > 0);
+    m_disableRepaints--;
+}
 
 void FrameView::layoutTimerFired(Timer<FrameView>*)
 {
index 1f90bc3..d3af599 100644 (file)
@@ -201,8 +201,13 @@ public:
     void beginDeferredRepaints();
     void endDeferredRepaints();
     void checkStopDelayingDeferredRepaints();
+    void startDeferredRepaintTimer(double delay);
     void resetDeferredRepaintDelay();
 
+    void beginDisableRepaints();
+    void endDisableRepaints();
+    bool repaintsDisabled() { return m_disableRepaints > 0; }
+
 #if ENABLE(DASHBOARD_SUPPORT)
     void updateDashboardRegions();
 #endif
@@ -466,7 +471,9 @@ private:
     Timer<FrameView> m_deferredRepaintTimer;
     double m_deferredRepaintDelay;
     double m_lastPaintTime;
-    
+
+    unsigned m_disableRepaints;
+
     bool m_isTrackingRepaints; // Used for testing.
     Vector<IntRect> m_trackedRepaintRects;
 
index cd3893a..0df9d02 100644 (file)
@@ -298,7 +298,10 @@ bool RenderView::shouldRepaint(const IntRect& r) const
 
     if (!m_frameView)
         return false;
-    
+
+    if (m_frameView->repaintsDisabled())
+        return false;
+
     return true;
 }
 
index 585780f..5cb810a 100644 (file)
@@ -177,10 +177,15 @@ void SVGImage::drawSVGToImageBuffer(ImageBuffer* buffer, const IntSize& size, fl
     ImageObserver* observer = imageObserver();
     ASSERT(observer);
 
-    // Temporarily reset image observer, we don't want to receive any changeInRect() calls due this relayout.
+    // Temporarily reset image observer, we don't want to receive any changeInRect() calls due to this relayout.
     setImageObserver(0);
+
+    // Disable repainting; we don't want deferred repaints to schedule any timers due to this relayout.
+    frame->view()->beginDisableRepaints();
+
     renderer->setContainerSize(size);
     frame->view()->resize(this->size());
+
     if (zoom != 1)
         frame->setPageZoomFactor(zoom);
 
@@ -202,7 +207,9 @@ void SVGImage::drawSVGToImageBuffer(ImageBuffer* buffer, const IntSize& size, fl
     if (frame->view()->needsLayout())
         frame->view()->layout();
 
-    setImageObserver(observer); 
+    setImageObserver(observer);
+
+    frame->view()->endDisableRepaints();
 }
 
 void SVGImage::draw(GraphicsContext* context, const FloatRect& dstRect, const FloatRect& srcRect, ColorSpace, CompositeOperator compositeOp)
@@ -210,7 +217,7 @@ void SVGImage::draw(GraphicsContext* context, const FloatRect& dstRect, const Fl
     if (!m_page)
         return;
 
-    FrameView* view = m_page->mainFrame()->view();
+    FrameView* view = frameView();
 
     GraphicsContextStateSaver stateSaver(*context);
     context->setCompositeOperation(compositeOp);
@@ -255,6 +262,14 @@ RenderBox* SVGImage::embeddedContentBox() const
     return toRenderBox(rootElement->renderer());
 }
 
+FrameView* SVGImage::frameView() const
+{
+    if (!m_page)
+        return 0;
+
+    return m_page->mainFrame()->view();
+}
+
 bool SVGImage::hasRelativeWidth() const
 {
     if (!m_page)
index 7e16049..0fe7b69 100644 (file)
@@ -34,6 +34,7 @@
 
 namespace WebCore {
 
+class FrameView;
 class ImageBuffer;
 class Page;
 class RenderBox;
@@ -53,6 +54,7 @@ public:
 
     void drawSVGToImageBuffer(ImageBuffer*, const IntSize&, float zoom, ShouldClearBuffer);
     RenderBox* embeddedContentBox() const;
+    FrameView* frameView() const;
 
     virtual bool isSVGImage() const { return true; }
     virtual IntSize size() const;
index 0d97a0f..fb38fd5 100644 (file)
@@ -21,6 +21,7 @@
 #include "SVGImageCache.h"
 
 #if ENABLE(SVG)
+#include "FrameView.h"
 #include "GraphicsContext.h"
 #include "ImageBuffer.h"
 #include "RenderSVGRoot.h"
@@ -81,13 +82,18 @@ void SVGImageCache::imageContentChanged()
     for (ImageDataMap::iterator it = m_imageDataMap.begin(); it != end; ++it)
         it->second.imageNeedsUpdate = true;
 
-    // Start redrawing dirty images with a timer, as imageContentChanged() may be called
-    // by the FrameView of the SVGImage which is currently in FrameView::layout().
-    if (!m_redrawTimer.isActive())
-        m_redrawTimer.startOneShot(0);
+    // If we're in the middle of layout, start redrawing dirty
+    // images on a timer; otherwise it's safe to draw immediately.
+
+    FrameView* frameView = m_svgImage->frameView();
+    if (frameView && frameView->needsLayout()) {
+        if (!m_redrawTimer.isActive())
+            m_redrawTimer.startOneShot(0);
+    } else
+       redraw();
 }
 
-void SVGImageCache::redrawTimerFired(Timer<SVGImageCache>*)
+void SVGImageCache::redraw()
 {
     ImageDataMap::iterator end = m_imageDataMap.end();
     for (ImageDataMap::iterator it = m_imageDataMap.begin(); it != end; ++it) {
@@ -105,6 +111,11 @@ void SVGImageCache::redrawTimerFired(Timer<SVGImageCache>*)
     m_svgImage->imageObserver()->animationAdvanced(m_svgImage);
 }
 
+void SVGImageCache::redrawTimerFired(Timer<SVGImageCache>*)
+{
+    redraw();
+}
+
 Image* SVGImageCache::lookupOrCreateBitmapImageForRenderer(const RenderObject* renderer)
 {
     ASSERT(renderer);
index 53d2161..46b7093 100644 (file)
@@ -70,6 +70,7 @@ public:
 
 private:
     SVGImageCache(SVGImage*);
+    void redraw();
     void redrawTimerFired(Timer<SVGImageCache>*);
 
     struct ImageData {