Do not use FloatRect::infiniteRect() to flag full repaints.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Aug 2014 04:03:42 +0000 (04:03 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Aug 2014 04:03:42 +0000 (04:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135900

Reviewed by Simon Fraser.

Converting FloatRect::infiniteRect() to IntRect leads to value overflow
and we end up with invalid repaint rectangle. Use a boolean flag to indicate
full repaint request.

Covered by existing tests.

Source/WebCore:

* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::GraphicsLayerCA):
(WebCore::GraphicsLayerCA::setNeedsDisplay):
(WebCore::GraphicsLayerCA::setNeedsDisplayInRect):
(WebCore::GraphicsLayerCA::repaintLayerDirtyRects):
* platform/graphics/ca/GraphicsLayerCA.h:
* platform/graphics/ca/PlatformCALayer.h:
* platform/graphics/ca/TileGrid.cpp:
(WebCore::TileGrid::setTileNeedsDisplayInRect):
* platform/graphics/ca/mac/PlatformCALayerMac.h:
* platform/graphics/ca/mac/PlatformCALayerMac.mm:
(PlatformCALayerMac::setNeedsDisplay):
(PlatformCALayerMac::setNeedsDisplayInRect):

Source/WebKit2:

* WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:
(WebKit::PlatformCALayerRemote::setNeedsDisplayInRect):
(WebKit::PlatformCALayerRemote::setNeedsDisplay):
* WebProcess/WebPage/mac/PlatformCALayerRemote.h:
* WebProcess/WebPage/mac/PlatformCALayerRemoteCustom.h:
* WebProcess/WebPage/mac/PlatformCALayerRemoteCustom.mm:
(WebKit::PlatformCALayerRemoteCustom::setNeedsDisplayInRect):
(WebKit::PlatformCALayerRemoteCustom::setNeedsDisplay):
* WebProcess/WebPage/mac/PlatformCALayerRemoteTiledBacking.cpp:
(WebKit::PlatformCALayerRemoteTiledBacking::setNeedsDisplayInRect):
(WebKit::PlatformCALayerRemoteTiledBacking::setNeedsDisplay):
* WebProcess/WebPage/mac/PlatformCALayerRemoteTiledBacking.h:

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

19 files changed:
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/GraphicsLayer.cpp
Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h
Source/WebCore/platform/graphics/ca/PlatformCALayer.h
Source/WebCore/platform/graphics/ca/TileGrid.cpp
Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.h
Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm
Source/WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp
Source/WebCore/platform/graphics/ca/win/PlatformCALayerWin.h
Source/WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp
Source/WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp
Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.h
Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemoteCustom.h
Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemoteCustom.mm
Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemoteTiledBacking.cpp
Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemoteTiledBacking.h

index ed41708..100d036 100644 (file)
@@ -1,3 +1,30 @@
+2014-08-15  Zalan Bujtas  <zalan@apple.com>
+
+        Do not use FloatRect::infiniteRect() to flag full repaints.
+        https://bugs.webkit.org/show_bug.cgi?id=135900
+
+        Reviewed by Simon Fraser.
+
+        Converting FloatRect::infiniteRect() to IntRect leads to value overflow
+        and we end up with invalid repaint rectangle. Use a boolean flag to indicate
+        full repaint request.
+
+        Covered by existing tests.
+
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::GraphicsLayerCA):
+        (WebCore::GraphicsLayerCA::setNeedsDisplay):
+        (WebCore::GraphicsLayerCA::setNeedsDisplayInRect):
+        (WebCore::GraphicsLayerCA::repaintLayerDirtyRects):
+        * platform/graphics/ca/GraphicsLayerCA.h:
+        * platform/graphics/ca/PlatformCALayer.h:
+        * platform/graphics/ca/TileGrid.cpp:
+        (WebCore::TileGrid::setTileNeedsDisplayInRect):
+        * platform/graphics/ca/mac/PlatformCALayerMac.h:
+        * platform/graphics/ca/mac/PlatformCALayerMac.mm:
+        (PlatformCALayerMac::setNeedsDisplay):
+        (PlatformCALayerMac::setNeedsDisplayInRect):
+
 2014-08-15  Benjamin Poulain  <benjamin@webkit.org>
 
         Unify the modes style resolution modes SharingRules and StyleInvalidation
index b5d15ec..85d0f3a 100644 (file)
@@ -571,18 +571,19 @@ void GraphicsLayer::resetTrackedRepaints()
 
 void GraphicsLayer::addRepaintRect(const FloatRect& repaintRect)
 {
-    if (m_client.isTrackingRepaints()) {
-        FloatRect largestRepaintRect(FloatPoint(), m_size);
-        largestRepaintRect.intersect(repaintRect);
-        RepaintMap::iterator repaintIt = repaintRectMap().find(this);
-        if (repaintIt == repaintRectMap().end()) {
-            Vector<FloatRect> repaintRects;
-            repaintRects.append(largestRepaintRect);
-            repaintRectMap().set(this, repaintRects);
-        } else {
-            Vector<FloatRect>& repaintRects = repaintIt->value;
-            repaintRects.append(largestRepaintRect);
-        }
+    if (!m_client.isTrackingRepaints())
+        return;
+
+    FloatRect largestRepaintRect(FloatPoint(), m_size);
+    largestRepaintRect.intersect(repaintRect);
+    RepaintMap::iterator repaintIt = repaintRectMap().find(this);
+    if (repaintIt == repaintRectMap().end()) {
+        Vector<FloatRect> repaintRects;
+        repaintRects.append(largestRepaintRect);
+        repaintRectMap().set(this, repaintRects);
+    } else {
+        Vector<FloatRect>& repaintRects = repaintIt->value;
+        repaintRects.append(largestRepaintRect);
     }
 }
 
index 9d8c9ca..896b6ed 100644 (file)
@@ -342,6 +342,7 @@ GraphicsLayerCA::GraphicsLayerCA(GraphicsLayerClient& client)
     : GraphicsLayer(client)
     , m_contentsLayerPurpose(NoContentsLayer)
     , m_isPageTiledBackingLayer(false)
+    , m_needsFullRepaint(false)
     , m_uncommittedChanges(0)
     , m_isCommittingChanges(false)
 {
@@ -709,7 +710,13 @@ void GraphicsLayerCA::setBlendMode(BlendMode blendMode)
 
 void GraphicsLayerCA::setNeedsDisplay()
 {
-    setNeedsDisplayInRect(FloatRect::infiniteRect());
+    if (!drawsContent())
+        return;
+
+    m_needsFullRepaint = true;
+    m_dirtyRects.clear();
+    noteLayerPropertyChanged(DirtyRectsChanged);
+    addRepaintRect(FloatRect(FloatPoint(), m_size));
 }
 
 void GraphicsLayerCA::setNeedsDisplayInRect(const FloatRect& r, ShouldClipToLayer shouldClip)
@@ -717,6 +724,9 @@ void GraphicsLayerCA::setNeedsDisplayInRect(const FloatRect& r, ShouldClipToLaye
     if (!drawsContent())
         return;
 
+    if (m_needsFullRepaint)
+        return;
+
     FloatRect rect(r);
     if (shouldClip == ClipToLayer) {
         FloatRect layerBounds(FloatPoint(), m_size);
@@ -2270,11 +2280,18 @@ void GraphicsLayerCA::pauseCAAnimationOnLayer(AnimatedPropertyID property, const
 
 void GraphicsLayerCA::repaintLayerDirtyRects()
 {
+    if (m_needsFullRepaint) {
+        ASSERT(!m_dirtyRects.size());
+        m_layer->setNeedsDisplay();
+        m_needsFullRepaint = false;
+        return;
+    }
+
     if (!m_dirtyRects.size())
         return;
 
     for (size_t i = 0; i < m_dirtyRects.size(); ++i)
-        m_layer->setNeedsDisplay(&(m_dirtyRects[i]));
+        m_layer->setNeedsDisplayInRect(m_dirtyRects[i]);
     
     m_dirtyRects.clear();
 }
index 8f9037e..db3d2d7 100644 (file)
@@ -482,7 +482,8 @@ private:
     
     ContentsLayerPurpose m_contentsLayerPurpose;
     bool m_isPageTiledBackingLayer : 1;
-    
+    bool m_needsFullRepaint : 1;
+
     Color m_contentsSolidColor;
 
     RetainPtr<CGImageRef> m_uncorrectedContentsImage;
index dbfbf83..fc7d0f2 100644 (file)
@@ -103,7 +103,8 @@ public:
 
     virtual void animationStarted(const String& key, CFTimeInterval beginTime) = 0;
 
-    virtual void setNeedsDisplay(const FloatRect* dirtyRect = 0) = 0;
+    virtual void setNeedsDisplay() = 0;
+    virtual void setNeedsDisplayInRect(const FloatRect& dirtyRect) = 0;
 
     virtual void copyContentsFromLayer(PlatformCALayer*) = 0;
 
index 337e1c9..5fcd55a 100644 (file)
@@ -152,11 +152,11 @@ void TileGrid::setTileNeedsDisplayInRect(const TileIndex& tileIndex, TileInfo& t
     // We could test for intersection with the visible rect. This would reduce painting yet more,
     // but may make scrolling stale tiles into view more frequent.
     if (tileRect.intersects(coverageRectInTileCoords) && tileLayer->superlayer()) {
-        tileLayer->setNeedsDisplay(&tileRepaintRect);
+        tileLayer->setNeedsDisplayInRect(tileRepaintRect);
 
         if (m_controller.rootLayer().owner()->platformCALayerShowRepaintCounter(0)) {
             FloatRect indicatorRect(0, 0, 52, 27);
-            tileLayer->setNeedsDisplay(&indicatorRect);
+            tileLayer->setNeedsDisplayInRect(indicatorRect);
         }
     } else
         tileInfo.hasStaleContent = true;
index bab1f23..9255dfb 100644 (file)
@@ -46,7 +46,8 @@ public:
 
     virtual void setOwner(PlatformCALayerClient*) override;
 
-    virtual void setNeedsDisplay(const FloatRect* dirtyRect = 0) override;
+    virtual void setNeedsDisplay() override;
+    virtual void setNeedsDisplayInRect(const FloatRect& dirtyRect) override;
 
     virtual void copyContentsFromLayer(PlatformCALayer*) override;
 
index 532f9f0..4b3dcb3 100644 (file)
@@ -331,13 +331,17 @@ void PlatformCALayerMac::animationStarted(const String&, CFTimeInterval beginTim
         m_owner->platformCALayerAnimationStarted(beginTime);
 }
 
-void PlatformCALayerMac::setNeedsDisplay(const FloatRect* dirtyRect)
+void PlatformCALayerMac::setNeedsDisplay()
 {
     BEGIN_BLOCK_OBJC_EXCEPTIONS
-    if (dirtyRect)
-        [m_layer.get() setNeedsDisplayInRect:*dirtyRect];
-    else
-        [m_layer.get() setNeedsDisplay];
+    [m_layer.get() setNeedsDisplay];
+    END_BLOCK_OBJC_EXCEPTIONS
+}
+
+void PlatformCALayerMac::setNeedsDisplayInRect(const FloatRect& dirtyRect)
+{
+    BEGIN_BLOCK_OBJC_EXCEPTIONS
+    [m_layer.get() setNeedsDisplayInRect:dirtyRect];
     END_BLOCK_OBJC_EXCEPTIONS
 }
 
index 581aef6..9f0000d 100644 (file)
@@ -208,11 +208,16 @@ void PlatformCALayerWin::animationStarted(const String&, CFTimeInterval beginTim
         m_owner->platformCALayerAnimationStarted(beginTime);
 }
 
-void PlatformCALayerWin::setNeedsDisplay(const FloatRect* dirtyRect)
+void PlatformCALayerWin::setNeedsDisplayInRect(const FloatRect& dirtyRect)
 {
-    intern(this)->setNeedsDisplay(dirtyRect);
+    intern(this)->setNeedsDisplayInRect(dirtyRect);
 }
-    
+
+void PlatformCALayerWin::setNeedsDisplay()
+{
+    intern(this)->setNeedsDisplay();
+}
+
 void PlatformCALayerWin::setNeedsCommit()
 {
     AbstractCACFLayerTreeHost* host = layerTreeHostForLayer(this);
index 2dfc426..fdf5e59 100644 (file)
@@ -37,7 +37,8 @@ public:
     
     ~PlatformCALayerWin();
 
-    virtual void setNeedsDisplay(const FloatRect* dirtyRect = 0) override;
+    virtual void setNeedsDisplayInRect(const FloatRect& dirtyRect) override;
+    virtual void setNeedsDisplay() override;
 
     virtual void copyContentsFromLayer(PlatformCALayer*) override;
 
index 6fee840..e795c78 100644 (file)
@@ -160,16 +160,19 @@ void PlatformCALayerWinInternal::internalSetNeedsDisplay(const FloatRect* dirtyR
         CACFLayerSetNeedsDisplay(owner()->platformLayer(), 0);
 }
 
-void PlatformCALayerWinInternal::setNeedsDisplay(const FloatRect* dirtyRect)
+void PlatformCALayerWinInternal::setNeedsDisplay()
+{
+    internalSetNeedsDisplay(0);
+}
+
+void PlatformCALayerWinInternal::setNeedsDisplayInRect(const FloatRect& dirtyRect)
 {
     if (layerTypeIsTiled(m_owner->layerType())) {
         // FIXME: Only setNeedsDisplay for tiles that are currently visible
         int numTileLayers = tileCount();
-        CGRect rect;
-        if (dirtyRect)
-            rect = *dirtyRect;
+        CGRect rect = dirtyRect;
         for (int i = 0; i < numTileLayers; ++i)
-            CACFLayerSetNeedsDisplay(tileAtIndex(i), dirtyRect ? &rect : 0);
+            CACFLayerSetNeedsDisplay(tileAtIndex(i), &rect);
 
         if (m_owner->owner() && m_owner->owner()->platformCALayerShowRepaintCounter(m_owner)) {
             CGRect layerBounds = m_owner->bounds();
@@ -189,15 +192,15 @@ void PlatformCALayerWinInternal::setNeedsDisplay(const FloatRect* dirtyRect)
                     repaintCounterRect.setY(layerBounds.height() - (layerBounds.y() + repaintCounterRect.height()));
                 internalSetNeedsDisplay(&repaintCounterRect);
             }
-            if (dirtyRect && owner()->owner()->platformCALayerContentsOrientation() == WebCore::GraphicsLayer::CompositingCoordinatesTopDown) {
-                FloatRect flippedDirtyRect = *dirtyRect;
+            if (owner()->owner()->platformCALayerContentsOrientation() == WebCore::GraphicsLayer::CompositingCoordinatesTopDown) {
+                FloatRect flippedDirtyRect = dirtyRect;
                 flippedDirtyRect.setY(owner()->bounds().height() - (flippedDirtyRect.y() + flippedDirtyRect.height()));
                 internalSetNeedsDisplay(&flippedDirtyRect);
                 return;
             }
         }
 
-        internalSetNeedsDisplay(dirtyRect);
+        internalSetNeedsDisplay(&dirtyRect);
     }
     owner()->setNeedsCommit();
 }
index 41e15fa..a3f5ab0 100644 (file)
@@ -48,7 +48,8 @@ public:
     ~PlatformCALayerWinInternal();
 
     void displayCallback(CACFLayerRef, CGContextRef);
-    void setNeedsDisplay(const FloatRect*);
+    void setNeedsDisplayInRect(const FloatRect&);
+    void setNeedsDisplay();
     PlatformCALayer* owner() const { return m_owner; }
 
     void setSublayers(const PlatformCALayerList&);
index e170cc7..5be6aba 100644 (file)
@@ -1,3 +1,29 @@
+2014-08-15  Zalan Bujtas  <zalan@apple.com>
+
+        Do not use FloatRect::infiniteRect() to flag full repaints.
+        https://bugs.webkit.org/show_bug.cgi?id=135900
+
+        Reviewed by Simon Fraser.
+
+        Converting FloatRect::infiniteRect() to IntRect leads to value overflow
+        and we end up with invalid repaint rectangle. Use a boolean flag to indicate
+        full repaint request.
+
+        Covered by existing tests.
+
+        * WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:
+        (WebKit::PlatformCALayerRemote::setNeedsDisplayInRect):
+        (WebKit::PlatformCALayerRemote::setNeedsDisplay):
+        * WebProcess/WebPage/mac/PlatformCALayerRemote.h:
+        * WebProcess/WebPage/mac/PlatformCALayerRemoteCustom.h:
+        * WebProcess/WebPage/mac/PlatformCALayerRemoteCustom.mm:
+        (WebKit::PlatformCALayerRemoteCustom::setNeedsDisplayInRect):
+        (WebKit::PlatformCALayerRemoteCustom::setNeedsDisplay):
+        * WebProcess/WebPage/mac/PlatformCALayerRemoteTiledBacking.cpp:
+        (WebKit::PlatformCALayerRemoteTiledBacking::setNeedsDisplayInRect):
+        (WebKit::PlatformCALayerRemoteTiledBacking::setNeedsDisplay):
+        * WebProcess/WebPage/mac/PlatformCALayerRemoteTiledBacking.h:
+
 2014-08-15  Alexey Proskuryakov  <ap@apple.com>
 
         REGRESSION (r172660): WebKit2.TerminateTwice asserts
index 304c191..e29cd8b 100644 (file)
@@ -203,17 +203,19 @@ void PlatformCALayerRemote::updateBackingStore()
     m_properties.backingStore->ensureBackingStore(m_properties.bounds.size(), m_properties.contentsScale, m_acceleratesDrawing, m_properties.opaque);
 }
 
-void PlatformCALayerRemote::setNeedsDisplay(const FloatRect* rect)
+void PlatformCALayerRemote::setNeedsDisplayInRect(const FloatRect& rect)
 {
     ensureBackingStore();
 
-    if (!rect) {
-        m_properties.backingStore->setNeedsDisplay();
-        return;
-    }
-
     // FIXME: Need to map this through contentsRect/etc.
-    m_properties.backingStore->setNeedsDisplay(enclosingIntRect(*rect));
+    m_properties.backingStore->setNeedsDisplay(enclosingIntRect(rect));
+}
+
+void PlatformCALayerRemote::setNeedsDisplay()
+{
+    ensureBackingStore();
+
+    m_properties.backingStore->setNeedsDisplay();
 }
 
 void PlatformCALayerRemote::copyContentsFromLayer(PlatformCALayer* layer)
index 86f6fcb..f1018f7 100644 (file)
@@ -51,7 +51,8 @@ public:
 
     void recursiveBuildTransaction(RemoteLayerTreeContext&, RemoteLayerTreeTransaction&);
 
-    virtual void setNeedsDisplay(const WebCore::FloatRect* dirtyRect = 0) override;
+    virtual void setNeedsDisplayInRect(const WebCore::FloatRect& dirtyRect) override;
+    virtual void setNeedsDisplay() override;
 
     virtual void copyContentsFromLayer(PlatformCALayer*) override;
 
index e6dd22d..1daa6e2 100644 (file)
@@ -44,7 +44,8 @@ public:
 
     virtual uint32_t hostingContextID() override;
 
-    virtual void setNeedsDisplay(const WebCore::FloatRect* dirtyRect = 0) override;
+    virtual void setNeedsDisplayInRect(const WebCore::FloatRect& dirtyRect) override;
+    virtual void setNeedsDisplay() override;
 
 private:
     PlatformCALayerRemoteCustom(WebCore::PlatformCALayer::LayerType, PlatformLayer *, WebCore::PlatformCALayerClient* owner, RemoteLayerTreeContext&);
index c0cb922..662ce58 100644 (file)
@@ -131,15 +131,20 @@ void PlatformCALayerRemoteCustom::setContents(CFTypeRef contents)
     [m_platformLayer setContents:(id)contents];
 }
 
-void PlatformCALayerRemoteCustom::setNeedsDisplay(const FloatRect* rect)
+void PlatformCALayerRemoteCustom::setNeedsDisplayInRect(const FloatRect& rect)
 {
-    if (m_providesContents) {
-        if (rect)
-            [m_platformLayer setNeedsDisplayInRect:*rect];
-        else
-            [m_platformLayer setNeedsDisplay];
-    } else
-        PlatformCALayerRemote::setNeedsDisplay(rect);
+    if (m_providesContents)
+        [m_platformLayer setNeedsDisplayInRect:rect];
+    else
+        PlatformCALayerRemote::setNeedsDisplayInRect(rect);
+}
+
+void PlatformCALayerRemoteCustom::setNeedsDisplay()
+{
+    if (m_providesContents)
+        [m_platformLayer setNeedsDisplay];
+    else
+        PlatformCALayerRemote::setNeedsDisplay();
 }
 
 } // namespace WebKit
index 88eb1d1..5cdfe5d 100644 (file)
@@ -46,12 +46,14 @@ PlatformCALayerRemoteTiledBacking::~PlatformCALayerRemoteTiledBacking()
 {
 }
 
-void PlatformCALayerRemoteTiledBacking::setNeedsDisplay(const FloatRect* dirtyRect)
+void PlatformCALayerRemoteTiledBacking::setNeedsDisplayInRect(const FloatRect& dirtyRect)
 {
-    if (dirtyRect)
-        m_tileController->setNeedsDisplayInRect(enclosingIntRect(*dirtyRect));
-    else
-        m_tileController->setNeedsDisplay();
+    m_tileController->setNeedsDisplayInRect(enclosingIntRect(dirtyRect));
+}
+
+void PlatformCALayerRemoteTiledBacking::setNeedsDisplay()
+{
+    m_tileController->setNeedsDisplay();
 }
 
 const WebCore::PlatformCALayerList* PlatformCALayerRemoteTiledBacking::customSublayers() const
index fa3e45c..cc1d232 100644 (file)
@@ -40,7 +40,8 @@ private:
 
     virtual WebCore::TiledBacking* tiledBacking() override { return m_tileController.get(); }
 
-    virtual void setNeedsDisplay(const WebCore::FloatRect* dirtyRect = 0) override;
+    virtual void setNeedsDisplayInRect(const WebCore::FloatRect& dirtyRect) override;
+    virtual void setNeedsDisplay() override;
 
     virtual const WebCore::PlatformCALayerList* customSublayers() const override;