Modified: trunk/Source/WebCore/ChangeLog (111977 => 111978)
--- trunk/Source/WebCore/ChangeLog 2012-03-24 05:21:26 UTC (rev 111977)
+++ trunk/Source/WebCore/ChangeLog 2012-03-24 05:54:38 UTC (rev 111978)
@@ -1,3 +1,32 @@
+2012-03-23 Dana Jansens <[email protected]>
+
+ [chromium] When prepainting fails, tiles dirty rects may be cleared
+ https://bugs.webkit.org/show_bug.cgi?id=82107
+
+ Reviewed by Adrienne Walker.
+
+ When prepainting, if a tile is unable to be reserved due to memory
+ limits, we bail out of prepareToUpdateTiles. But we would have
+ cleared the dirty rect of any previous tiles. This leaves them
+ in a bad state where their textures are reserved, but their textureIds
+ are set to 0, and they are not marked dirty. This means that they will
+ not be updated and displayed if they become visible, since it is
+ assumed that valid textures with zero textureId must have a dirty
+ region.
+
+ We fix this by not clearing the dirty rects until we know we are
+ going to update the layer.
+
+ Unit test: TiledLayerChromiumTest.pushTilesAfterIdlePaintFailed
+
+ * platform/graphics/chromium/TiledLayerChromium.cpp:
+ (WebCore::TiledLayerChromium::prepareToUpdateTiles):
+ * platform/graphics/chromium/cc/CCTiledLayerImpl.cpp:
+ (WebCore::CCTiledLayerImpl::hasTextureIdForTileAt):
+ (WebCore):
+ * platform/graphics/chromium/cc/CCTiledLayerImpl.h:
+ (CCTiledLayerImpl):
+
2012-03-23 Stephanie Lewis <[email protected]>
https://bugs.webkit.org/show_bug.cgi?id=81963 WebProcess can get stuck in GC during many low memory signals.
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCTiledLayerImpl.cpp (111977 => 111978)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCTiledLayerImpl.cpp 2012-03-24 05:21:26 UTC (rev 111977)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCTiledLayerImpl.cpp 2012-03-24 05:54:38 UTC (rev 111978)
@@ -101,6 +101,11 @@
return m_tiler->tileAt(i, j);
}
+bool CCTiledLayerImpl::hasTextureIdForTileAt(int i, int j) const
+{
+ return hasTileAt(i, j) && tileAt(i, j)->textureId();
+}
+
DrawableTile* CCTiledLayerImpl::tileAt(int i, int j) const
{
return static_cast<DrawableTile*>(m_tiler->tileAt(i, j));
Modified: trunk/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp (111977 => 111978)
--- trunk/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp 2012-03-24 05:21:26 UTC (rev 111977)
+++ trunk/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp 2012-03-24 05:54:38 UTC (rev 111978)
@@ -69,7 +69,7 @@
class FakeTextureAllocator : public TextureAllocator {
public:
- virtual unsigned createTexture(const IntSize&, GC3Denum) { return 0; }
+ virtual unsigned createTexture(const IntSize&, GC3Denum) { return 1; }
virtual void deleteTexture(unsigned, const IntSize&, GC3Denum) { }
};
@@ -86,7 +86,12 @@
}
virtual ~Texture() { }
- virtual void updateRect(GraphicsContext3D*, TextureAllocator*, const IntRect&, const IntRect&) { m_layer->updateRect(); }
+ virtual void updateRect(GraphicsContext3D*, TextureAllocator* allocator, const IntRect&, const IntRect&)
+ {
+ if (allocator)
+ texture()->allocate(allocator);
+ m_layer->updateRect();
+ }
virtual void prepareRect(const IntRect&) { m_layer->prepareRect(); }
private:
@@ -144,10 +149,8 @@
: CCTiledLayerImpl(id) { }
virtual ~FakeCCTiledLayerImpl() { }
- bool hasTileAt(int i, int j)
- {
- return CCTiledLayerImpl::hasTileAt(i, j);
- }
+ using CCTiledLayerImpl::hasTileAt;
+ using CCTiledLayerImpl::hasTextureIdForTileAt;
};
class FakeTiledLayerChromium : public TiledLayerChromium {
@@ -434,6 +437,88 @@
}
}
+TEST(TiledLayerChromiumTest, pushTilesAfterIdlePaintFailed)
+{
+ OwnPtr<TextureManager> textureManager = TextureManager::create(1024*1024, 1024*1024, 1024);
+ DebugScopedSetImplThread implThread;
+ RefPtr<FakeTiledLayerChromium> layer1 = adoptRef(new FakeTiledLayerChromium(textureManager.get()));
+ OwnPtr<FakeCCTiledLayerImpl> layerImpl1(adoptPtr(new FakeCCTiledLayerImpl(0)));
+ RefPtr<FakeTiledLayerChromium> layer2 = adoptRef(new FakeTiledLayerChromium(textureManager.get()));
+ OwnPtr<FakeCCTiledLayerImpl> layerImpl2(adoptPtr(new FakeCCTiledLayerImpl(0)));
+
+ FakeTextureAllocator textureAllocator;
+ CCTextureUpdater updater(&textureAllocator);
+
+ // For this test we have two layers. layer1 exhausts most texture memory, leaving room for 2 more tiles from
+ // layer2, but not all three tiles. First we paint layer1, and one tile from layer2. Then when we idle paint
+ // layer2, we will fail on the third tile of layer2, and this should not leave the second tile in a bad state.
+
+ // This requires 4*30000 bytes of memory.
+ IntRect layer2Rect(0, 0, 100, 300);
+ layer2->setBounds(layer2Rect.size());
+ layer2->setVisibleLayerRect(layer2Rect);
+ layer2->invalidateRect(layer2Rect);
+
+ // This uses 960000 bytes, leaving 88576 bytes of memory left, which is enough for 2 tiles only in the other layer.
+ IntRect layerRect(IntPoint::zero(), IntSize(100, 2400));
+ layer1->setBounds(layerRect.size());
+ layer1->setVisibleLayerRect(layerRect);
+ layer1->invalidateRect(layerRect);
+ layer1->prepareToUpdate(layerRect, 0);
+
+ // Paint a single tile in layer2 so that it will idle paint.
+ layer2->prepareToUpdate(IntRect(0, 0, 100, 100), 0);
+
+ // We should need idle-painting for both remaining tiles in layer2.
+ EXPECT_TRUE(layer2->needsIdlePaint(layer2Rect));
+
+ // Commit the frame over to impl.
+ layer1->updateCompositorResources(0, updater);
+ layer2->updateCompositorResources(0, updater);
+ updater.update(0, 5000);
+ layer1->pushPropertiesTo(layerImpl1.get());
+ layer2->pushPropertiesTo(layerImpl2.get());
+
+ // Now idle paint layer2. We are going to run out of memory though!
+ layer2->prepareToUpdate(IntRect(0, 0, 100, 100), 0);
+ layer2->prepareToUpdateIdle(layer2Rect, 0);
+
+ // Oh well, commit the frame and push.
+ layer1->updateCompositorResources(0, updater);
+ layer2->updateCompositorResources(0, updater);
+ updater.update(0, 5000);
+ layer1->pushPropertiesTo(layerImpl1.get());
+ layer2->pushPropertiesTo(layerImpl2.get());
+
+ // Sanity check, we should have textures for the big layer.
+ EXPECT_TRUE(layerImpl1->hasTextureIdForTileAt(0, 0));
+
+ // We should only have the first tile from layer2 since it failed to idle update.
+ EXPECT_TRUE(layerImpl2->hasTileAt(0, 0));
+ EXPECT_TRUE(layerImpl2->hasTextureIdForTileAt(0, 0));
+ EXPECT_FALSE(layerImpl2->hasTileAt(0, 1));
+ EXPECT_FALSE(layerImpl2->hasTileAt(0, 2));
+
+ // Now if layer2 becomes fully visible, we should be able to paint it and push valid textures.
+ textureManager->unprotectAllTextures();
+
+ layer2->prepareToUpdate(layer2Rect, 0);
+ layer1->prepareToUpdate(IntRect(), 0);
+
+ layer1->updateCompositorResources(0, updater);
+ layer2->updateCompositorResources(0, updater);
+ updater.update(0, 5000);
+ layer1->pushPropertiesTo(layerImpl1.get());
+ layer2->pushPropertiesTo(layerImpl2.get());
+
+ EXPECT_TRUE(layerImpl2->hasTileAt(0, 0));
+ EXPECT_TRUE(layerImpl2->hasTileAt(0, 1));
+ EXPECT_TRUE(layerImpl2->hasTileAt(0, 2));
+ EXPECT_TRUE(layerImpl2->hasTextureIdForTileAt(0, 0));
+ EXPECT_TRUE(layerImpl2->hasTextureIdForTileAt(0, 1));
+ EXPECT_TRUE(layerImpl2->hasTextureIdForTileAt(0, 2));
+}
+
TEST(TiledLayerChromiumTest, pushIdlePaintedOccludedTiles)
{
OwnPtr<TextureManager> textureManager = TextureManager::create(4*1024*1024, 2*1024*1024, 1024);
