2011-02-04 Simon Fraser <simon.fraser@apple.com>
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 5 Feb 2011 04:56:59 +0000 (04:56 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 5 Feb 2011 04:56:59 +0000 (04:56 +0000)
        Reviewed by Dan Bernstein.

        Crashes in ShadowBlur via WebKit2 FindController
        https://bugs.webkit.org/show_bug.cgi?id=53830

        Fix a crash cause by re-entering ShadowBlur, and add assertions to
        detect when it happens.

        The re-entrancy occurred when drawRectShadowWithTiling() filled
        the interior of the shadow with fillRect() on the context
        which still had the shadow state set. This would make another ShadowBlur
        on the stack and call into the code again, potentially blowing away
        the image buffer.

        Fix by turning off shadows in the destination context while we're
        drawing the tiled shadow. The non-tiled code path already did this.

        Not testable because CSS shadows clip out the inside of the rect
        being shadowed, and SVG uses fillPath, even for rects.

        * platform/graphics/ShadowBlur.cpp:
        (WebCore::ScratchBuffer::ScratchBuffer):
        (WebCore::ScratchBuffer::getScratchBuffer):
        (WebCore::ScratchBuffer::scheduleScratchBufferPurge):
        (WebCore::ShadowBlur::ShadowBlur):
        (WebCore::ShadowBlur::drawRectShadowWithTiling):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/ShadowBlur.cpp

index bcf8a67a595f55d57b4c05a03cc88fc0172c266e..a56007c3bfc0f0a0b976cd2332d753b77104c45c 100644 (file)
@@ -1,3 +1,32 @@
+2011-02-04  Simon Fraser  <simon.fraser@apple.com>
+
+        Reviewed by Dan Bernstein.
+
+        Crashes in ShadowBlur via WebKit2 FindController
+        https://bugs.webkit.org/show_bug.cgi?id=53830
+        
+        Fix a crash cause by re-entering ShadowBlur, and add assertions to
+        detect when it happens.
+        
+        The re-entrancy occurred when drawRectShadowWithTiling() filled
+        the interior of the shadow with fillRect() on the context
+        which still had the shadow state set. This would make another ShadowBlur
+        on the stack and call into the code again, potentially blowing away
+        the image buffer.
+        
+        Fix by turning off shadows in the destination context while we're
+        drawing the tiled shadow. The non-tiled code path already did this.
+
+        Not testable because CSS shadows clip out the inside of the rect
+        being shadowed, and SVG uses fillPath, even for rects.
+
+        * platform/graphics/ShadowBlur.cpp:
+        (WebCore::ScratchBuffer::ScratchBuffer):
+        (WebCore::ScratchBuffer::getScratchBuffer):
+        (WebCore::ScratchBuffer::scheduleScratchBufferPurge):
+        (WebCore::ShadowBlur::ShadowBlur):
+        (WebCore::ShadowBlur::drawRectShadowWithTiling):
+
 2011-02-04  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Reviewed by Martin Robinson.
index d38609a2629e9f30945eb506840d30ce42751744..60a699bcfd2b10d28ab69b3d69e8fdfd53782868 100644 (file)
@@ -53,11 +53,18 @@ class ScratchBuffer {
 public:
     ScratchBuffer()
         : m_purgeTimer(this, &ScratchBuffer::timerFired)
+#if !ASSERT_DISABLED
+        , m_bufferInUse(false)
+#endif
     {
     }
     
     ImageBuffer* getScratchBuffer(const IntSize& size)
     {
+        ASSERT(!m_bufferInUse);
+#if !ASSERT_DISABLED
+        m_bufferInUse = true;
+#endif
         // We do not need to recreate the buffer if the current buffer is large enough.
         if (m_imageBuffer && m_imageBuffer->width() >= size.width() && m_imageBuffer->height() >= size.height())
             return m_imageBuffer.get();
@@ -71,6 +78,9 @@ public:
 
     void scheduleScratchBufferPurge()
     {
+#if !ASSERT_DISABLED
+        m_bufferInUse = false;
+#endif
         if (m_purgeTimer.isActive())
             m_purgeTimer.stop();
 
@@ -93,6 +103,9 @@ private:
 
     OwnPtr<ImageBuffer> m_imageBuffer;
     Timer<ScratchBuffer> m_purgeTimer;
+#if !ASSERT_DISABLED
+    bool m_bufferInUse;
+#endif
 };
 
 ScratchBuffer& ScratchBuffer::shared()
@@ -106,6 +119,7 @@ ShadowBlur::ShadowBlur(float radius, const FloatSize& offset, const Color& color
     , m_colorSpace(colorSpace)
     , m_blurRadius(radius)
     , m_offset(offset)
+    , m_layerImage(0)
     , m_shadowsIgnoreTransforms(false)
 {
     // Limit blur radius to 128 to avoid lots of very expensive blurring.
@@ -493,6 +507,9 @@ void ShadowBlur::drawRectShadowWithoutTiling(GraphicsContext* graphicsContext, c
 
 void ShadowBlur::drawRectShadowWithTiling(GraphicsContext* graphicsContext, const FloatRect& shadowedRect, const RoundedIntRect::Radii& radii, const IntSize& shadowTemplateSize)
 {
+    graphicsContext->save();
+    graphicsContext->clearShadow();
+
     const float roundedRadius = ceilf(m_blurRadius);
     const float twiceRadius = roundedRadius * 2;
 
@@ -601,6 +618,8 @@ void ShadowBlur::drawRectShadowWithTiling(GraphicsContext* graphicsContext, cons
     destRect = FloatRect(shadowBounds.x(), shadowBounds.maxY() - bottomOffset, leftOffset, bottomOffset);
     graphicsContext->drawImageBuffer(m_layerImage, ColorSpaceDeviceRGB, destRect, tileRect);
 
+    graphicsContext->restore();
+
     m_layerImage = 0;
     // Schedule a purge of the scratch buffer.
     ScratchBuffer::shared().scheduleScratchBufferPurge();