- Revision
- 110469
- Author
- [email protected]
- Date
- 2012-03-12 13:26:58 -0700 (Mon, 12 Mar 2012)
Log Message
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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (110468 => 110469)
--- trunk/Source/WebCore/ChangeLog 2012-03-12 20:26:20 UTC (rev 110468)
+++ trunk/Source/WebCore/ChangeLog 2012-03-12 20:26:58 UTC (rev 110469)
@@ -1,3 +1,50 @@
+2012-03-12 Tim Horton <[email protected]>
+
+ 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 <[email protected]>
[MutationObservers] Enforce a consistent order of MutationRecord delivery
Modified: trunk/Source/WebCore/page/FrameView.cpp (110468 => 110469)
--- trunk/Source/WebCore/page/FrameView.cpp 2012-03-12 20:26:20 UTC (rev 110468)
+++ trunk/Source/WebCore/page/FrameView.cpp 2012-03-12 20:26:58 UTC (rev 110469)
@@ -138,6 +138,7 @@
, 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 @@
m_isVisuallyNonEmpty = false;
m_firstVisuallyNonEmptyLayoutCallbackPending = true;
m_maintainScrollPositionAnchor = 0;
+ m_disableRepaints = 0;
}
bool FrameView::isFrameView() const
@@ -1754,7 +1756,7 @@
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 @@
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 @@
m_deferringRepaints++;
}
-
void FrameView::endDeferredRepaints()
{
Page* page = m_frame->page();
@@ -1848,18 +1850,26 @@
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::doDeferredRepaints()
{
+ if (m_disableRepaints)
+ return;
+
ASSERT(!m_deferringRepaints);
if (!shouldUpdate()) {
m_repaintRects.clear();
@@ -1934,8 +1947,19 @@
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>*)
{
#ifdef INSTRUMENT_LAYOUT_SCHEDULING
Modified: trunk/Source/WebCore/page/FrameView.h (110468 => 110469)
--- trunk/Source/WebCore/page/FrameView.h 2012-03-12 20:26:20 UTC (rev 110468)
+++ trunk/Source/WebCore/page/FrameView.h 2012-03-12 20:26:58 UTC (rev 110469)
@@ -201,8 +201,13 @@
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 @@
Timer<FrameView> m_deferredRepaintTimer;
double m_deferredRepaintDelay;
double m_lastPaintTime;
-
+
+ unsigned m_disableRepaints;
+
bool m_isTrackingRepaints; // Used for testing.
Vector<IntRect> m_trackedRepaintRects;
Modified: trunk/Source/WebCore/rendering/RenderView.cpp (110468 => 110469)
--- trunk/Source/WebCore/rendering/RenderView.cpp 2012-03-12 20:26:20 UTC (rev 110468)
+++ trunk/Source/WebCore/rendering/RenderView.cpp 2012-03-12 20:26:58 UTC (rev 110469)
@@ -298,7 +298,10 @@
if (!m_frameView)
return false;
-
+
+ if (m_frameView->repaintsDisabled())
+ return false;
+
return true;
}
Modified: trunk/Source/WebCore/svg/graphics/SVGImage.cpp (110468 => 110469)
--- trunk/Source/WebCore/svg/graphics/SVGImage.cpp 2012-03-12 20:26:20 UTC (rev 110468)
+++ trunk/Source/WebCore/svg/graphics/SVGImage.cpp 2012-03-12 20:26:58 UTC (rev 110469)
@@ -177,10 +177,15 @@
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 @@
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 @@
if (!m_page)
return;
- FrameView* view = m_page->mainFrame()->view();
+ FrameView* view = frameView();
GraphicsContextStateSaver stateSaver(*context);
context->setCompositeOperation(compositeOp);
@@ -255,6 +262,14 @@
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)
Modified: trunk/Source/WebCore/svg/graphics/SVGImage.h (110468 => 110469)
--- trunk/Source/WebCore/svg/graphics/SVGImage.h 2012-03-12 20:26:20 UTC (rev 110468)
+++ trunk/Source/WebCore/svg/graphics/SVGImage.h 2012-03-12 20:26:58 UTC (rev 110469)
@@ -34,6 +34,7 @@
namespace WebCore {
+class FrameView;
class ImageBuffer;
class Page;
class RenderBox;
@@ -53,6 +54,7 @@
void drawSVGToImageBuffer(ImageBuffer*, const IntSize&, float zoom, ShouldClearBuffer);
RenderBox* embeddedContentBox() const;
+ FrameView* frameView() const;
virtual bool isSVGImage() const { return true; }
virtual IntSize size() const;
Modified: trunk/Source/WebCore/svg/graphics/SVGImageCache.cpp (110468 => 110469)
--- trunk/Source/WebCore/svg/graphics/SVGImageCache.cpp 2012-03-12 20:26:20 UTC (rev 110468)
+++ trunk/Source/WebCore/svg/graphics/SVGImageCache.cpp 2012-03-12 20:26:58 UTC (rev 110469)
@@ -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 @@
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 @@
m_svgImage->imageObserver()->animationAdvanced(m_svgImage);
}
+void SVGImageCache::redrawTimerFired(Timer<SVGImageCache>*)
+{
+ redraw();
+}
+
Image* SVGImageCache::lookupOrCreateBitmapImageForRenderer(const RenderObject* renderer)
{
ASSERT(renderer);
Modified: trunk/Source/WebCore/svg/graphics/SVGImageCache.h (110468 => 110469)
--- trunk/Source/WebCore/svg/graphics/SVGImageCache.h 2012-03-12 20:26:20 UTC (rev 110468)
+++ trunk/Source/WebCore/svg/graphics/SVGImageCache.h 2012-03-12 20:26:58 UTC (rev 110469)
@@ -70,6 +70,7 @@
private:
SVGImageCache(SVGImage*);
+ void redraw();
void redrawTimerFired(Timer<SVGImageCache>*);
struct ImageData {
