REGRESSION(99539): Infinite repaint loop with SVGImage and deferred repaint timers
authorschenney@chromium.org <schenney@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Apr 2012 15:51:24 +0000 (15:51 +0000)
committerschenney@chromium.org <schenney@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Apr 2012 15:51:24 +0000 (15:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=78315

Reviewed by Dimitri Glazkov.

The existing fix for this issue was failing to check if the frameView object
was currently _in_ layout, in addition to whether it needs layout. Calling the
redraw method while in layout leads to a debug assertion and potential infinite
layout loops. Now we check whether we need layout or are in layout. We also add
a check when the repaint timer fires to ensure we do not call redraw during layout
at that point.

This patch was tested with tens of thousands of runs on layout test cases that
previously crashed at a rate of about 1 in 25. Now we see no crashes and no test
failures.

No new tests, as this exists to fix flaky existing tests.

* svg/graphics/SVGImageCache.cpp:
(WebCore::SVGImageCache::imageContentChanged):
(WebCore::SVGImageCache::redrawTimerFired):

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

Source/WebCore/ChangeLog
Source/WebCore/svg/graphics/SVGImageCache.cpp

index 4bd6628..1d11ee5 100644 (file)
@@ -1,3 +1,27 @@
+2012-04-05  Stephen Chenney  <schenney@chromium.org>
+
+        REGRESSION(99539): Infinite repaint loop with SVGImage and deferred repaint timers
+        https://bugs.webkit.org/show_bug.cgi?id=78315
+
+        Reviewed by Dimitri Glazkov.
+
+        The existing fix for this issue was failing to check if the frameView object
+        was currently _in_ layout, in addition to whether it needs layout. Calling the
+        redraw method while in layout leads to a debug assertion and potential infinite
+        layout loops. Now we check whether we need layout or are in layout. We also add
+        a check when the repaint timer fires to ensure we do not call redraw during layout
+        at that point.
+
+        This patch was tested with tens of thousands of runs on layout test cases that
+        previously crashed at a rate of about 1 in 25. Now we see no crashes and no test
+        failures.
+
+        No new tests, as this exists to fix flaky existing tests.
+
+        * svg/graphics/SVGImageCache.cpp:
+        (WebCore::SVGImageCache::imageContentChanged):
+        (WebCore::SVGImageCache::redrawTimerFired):
+
 2012-04-05  Keishi Hattori  <keishi@webkit.org>
 
         Hide datalist element
index fb38fd5..2869f61 100644 (file)
@@ -84,9 +84,8 @@ void SVGImageCache::imageContentChanged()
 
     // 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 (frameView && (frameView->needsLayout() || frameView->isInLayout())) {
         if (!m_redrawTimer.isActive())
             m_redrawTimer.startOneShot(0);
     } else
@@ -113,7 +112,14 @@ void SVGImageCache::redraw()
 
 void SVGImageCache::redrawTimerFired(Timer<SVGImageCache>*)
 {
-    redraw();
+    // We have no guarantee that the frame does not require layout when the timer fired.
+    // So be sure to check again in case it is still not safe to run redraw.
+    FrameView* frameView = m_svgImage->frameView();
+    if (frameView && (frameView->needsLayout() || frameView->isInLayout())) {
+        if (!m_redrawTimer.isActive())
+            m_redrawTimer.startOneShot(0);
+    } else
+       redraw();
 }
 
 Image* SVGImageCache::lookupOrCreateBitmapImageForRenderer(const RenderObject* renderer)