- Revision
- 113241
- Author
- commit-qu...@webkit.org
- Date
- 2012-04-04 14:27:36 -0700 (Wed, 04 Apr 2012)
Log Message
[Chromium] Always skip draw and readback if there is nothing
to draw.
https://bugs.webkit.org/show_bug.cgi?id=82680
This avoids corruption from pushing frames that have no valid
content drawn into them.
Also in addition to checking for non-existing root layers, check
for root layers with no content bounds. It's possible to see those
with kForceCompositing mode for empty documents.
Patch by Daniel Sievers <siev...@chromium.org> on 2012-04-04
Reviewed by James Robinson.
Added CCLayerTreeHostTestEmptyContentsShouldNotDraw.
Source/WebCore:
* platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:
(WebCore::CCLayerTreeHostImpl::canDraw):
(WebCore::CCLayerTreeHostImpl::prepareToDraw):
* platform/graphics/chromium/cc/CCThreadProxy.cpp:
(WebCore::CCThreadProxy::scheduledActionDrawAndSwapInternal):
Source/WebKit/chromium:
* tests/CCLayerTreeHostImplTest.cpp:
(WebKitTests::TEST_F):
* tests/CCLayerTreeHostTest.cpp:
(WTF::CCLayerTreeHostTest::doBeginTest):
(WTF):
(CCLayerTreeHostTestEmptyContentsShouldNotDraw):
(WTF::CCLayerTreeHostTestEmptyContentsShouldNotDraw::CCLayerTreeHostTestEmptyContentsShouldNotDraw):
(WTF::CCLayerTreeHostTestEmptyContentsShouldNotDraw::beginTest):
(WTF::CCLayerTreeHostTestEmptyContentsShouldNotDraw::drawLayersOnCCThread):
(WTF::CCLayerTreeHostTestEmptyContentsShouldNotDraw::didCommitAndDrawFrame):
(WTF::CCLayerTreeHostTestEmptyContentsShouldNotDraw::afterTest):
(WTF::TEST_F):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (113240 => 113241)
--- trunk/Source/WebCore/ChangeLog 2012-04-04 21:14:48 UTC (rev 113240)
+++ trunk/Source/WebCore/ChangeLog 2012-04-04 21:27:36 UTC (rev 113241)
@@ -1,3 +1,25 @@
+2012-04-04 Daniel Sievers <siev...@chromium.org>
+
+ [Chromium] Always skip draw and readback if there is nothing
+ to draw.
+ https://bugs.webkit.org/show_bug.cgi?id=82680
+
+ This avoids corruption from pushing frames that have no valid
+ content drawn into them.
+ Also in addition to checking for non-existing root layers, check
+ for root layers with no content bounds. It's possible to see those
+ with kForceCompositing mode for empty documents.
+
+ Reviewed by James Robinson.
+
+ Added CCLayerTreeHostTestEmptyContentsShouldNotDraw.
+
+ * platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:
+ (WebCore::CCLayerTreeHostImpl::canDraw):
+ (WebCore::CCLayerTreeHostImpl::prepareToDraw):
+ * platform/graphics/chromium/cc/CCThreadProxy.cpp:
+ (WebCore::CCThreadProxy::scheduledActionDrawAndSwapInternal):
+
2012-03-15 Jer Noble <jer.no...@apple.com>
Full Screen mode should cancel before navigation.
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp (113240 => 113241)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp 2012-04-04 21:14:48 UTC (rev 113240)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp 2012-04-04 21:27:36 UTC (rev 113241)
@@ -141,7 +141,7 @@
bool CCLayerTreeHostImpl::canDraw()
{
- if (!rootLayer())
+ if (!rootLayer() || rootLayer()->bounds().isEmpty())
return false;
if (viewportSize().isEmpty())
return false;
@@ -376,9 +376,6 @@
frame.renderPasses.clear();
frame.renderSurfaceLayerList.clear();
- if (!rootLayer())
- return false;
-
if (!calculateRenderPasses(frame.renderPasses, frame.renderSurfaceLayerList))
return false;
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h (113240 => 113241)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h 2012-04-04 21:14:48 UTC (rev 113240)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h 2012-04-04 21:27:36 UTC (rev 113241)
@@ -88,7 +88,7 @@
virtual void beginCommit();
virtual void commitComplete();
virtual void animate(double monotonicTime, double wallClockTime);
- // Returns false if problems occured preparing the frame, and we should try to avoid displaying the frame.
+ // Returns false if we should try to avoid displaying the frame, because it has visible checkerboard during an animation.
virtual bool prepareToDraw(FrameData&);
virtual void drawLayers(const FrameData&);
@@ -102,6 +102,8 @@
virtual void setFullRootLayerDamage();
// Implementation
+
+ // Returns false if there is no valid root layer and thus no content that can be drawn.
bool canDraw();
GraphicsContext3D* context();
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp (113240 => 113241)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp 2012-04-04 21:14:48 UTC (rev 113240)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp 2012-04-04 21:27:36 UTC (rev 113241)
@@ -578,7 +578,7 @@
m_inputHandlerOnImplThread->animate(monotonicTime);
m_layerTreeHostImpl->animate(monotonicTime, wallClockTime);
CCLayerTreeHostImpl::FrameData frame;
- bool drawFrame = m_layerTreeHostImpl->prepareToDraw(frame) || forcedDraw;
+ bool drawFrame = m_layerTreeHostImpl->canDraw() && (m_layerTreeHostImpl->prepareToDraw(frame) || forcedDraw);
if (drawFrame) {
m_layerTreeHostImpl->drawLayers(frame);
result.didDraw = true;
@@ -586,9 +586,9 @@
// Check for a pending compositeAndReadback.
if (m_readbackRequestOnImplThread) {
- ASSERT(drawFrame); // This should be a forcedDraw
- m_layerTreeHostImpl->readback(m_readbackRequestOnImplThread->pixels, m_readbackRequestOnImplThread->rect);
- m_readbackRequestOnImplThread->success = !m_layerTreeHostImpl->isContextLost();
+ if (drawFrame)
+ m_layerTreeHostImpl->readback(m_readbackRequestOnImplThread->pixels, m_readbackRequestOnImplThread->rect);
+ m_readbackRequestOnImplThread->success = !m_layerTreeHostImpl->isContextLost() && drawFrame;
m_readbackRequestOnImplThread->completion.signal();
m_readbackRequestOnImplThread = 0;
}
@@ -598,7 +598,6 @@
// Process any finish request
if (m_finishAllRenderingCompletionEventOnImplThread) {
- ASSERT(drawFrame); // This should be a forcedDraw
m_layerTreeHostImpl->finishAllRendering();
m_finishAllRenderingCompletionEventOnImplThread->signal();
m_finishAllRenderingCompletionEventOnImplThread = 0;
@@ -610,7 +609,6 @@
m_mainThreadProxy->postTask(createCCThreadTask(this, &CCThreadProxy::didCommitAndDrawFrame));
}
- ASSERT(drawFrame || (!drawFrame && !forcedDraw));
return result;
}
Modified: trunk/Source/WebKit/chromium/ChangeLog (113240 => 113241)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-04-04 21:14:48 UTC (rev 113240)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-04-04 21:27:36 UTC (rev 113241)
@@ -1,3 +1,32 @@
+2012-04-04 Daniel Sievers <siev...@chromium.org>
+
+ [Chromium] Always skip draw and readback if there is nothing
+ to draw.
+ https://bugs.webkit.org/show_bug.cgi?id=82680
+
+ This avoids corruption from pushing frames that have no valid
+ content drawn into them.
+ Also in addition to checking for non-existing root layers, check
+ for root layers with no content bounds. It's possible to see those
+ with kForceCompositing mode for empty documents.
+
+ Reviewed by James Robinson.
+
+ Added CCLayerTreeHostTestEmptyContentsShouldNotDraw.
+
+ * tests/CCLayerTreeHostImplTest.cpp:
+ (WebKitTests::TEST_F):
+ * tests/CCLayerTreeHostTest.cpp:
+ (WTF::CCLayerTreeHostTest::doBeginTest):
+ (WTF):
+ (CCLayerTreeHostTestEmptyContentsShouldNotDraw):
+ (WTF::CCLayerTreeHostTestEmptyContentsShouldNotDraw::CCLayerTreeHostTestEmptyContentsShouldNotDraw):
+ (WTF::CCLayerTreeHostTestEmptyContentsShouldNotDraw::beginTest):
+ (WTF::CCLayerTreeHostTestEmptyContentsShouldNotDraw::drawLayersOnCCThread):
+ (WTF::CCLayerTreeHostTestEmptyContentsShouldNotDraw::didCommitAndDrawFrame):
+ (WTF::CCLayerTreeHostTestEmptyContentsShouldNotDraw::afterTest):
+ (WTF::TEST_F):
+
2012-04-04 Mark Pilgrim <pilg...@chromium.org>
Call histogramCustomCounts directly
Modified: trunk/Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp (113240 => 113241)
--- trunk/Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp 2012-04-04 21:14:48 UTC (rev 113240)
+++ trunk/Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp 2012-04-04 21:27:36 UTC (rev 113241)
@@ -248,6 +248,7 @@
m_hostImpl->initializeLayerRenderer(createContext());
OwnPtr<CCLayerImpl> root = CCLayerImpl::create(0);
+ root->setBounds(IntSize(1, 1));
root->setScrollable(true);
root->setScrollPosition(IntPoint(0, 0));
root->setMaxScrollPosition(IntSize(100, 100));
Modified: trunk/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp (113240 => 113241)
--- trunk/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp 2012-04-04 21:14:48 UTC (rev 113240)
+++ trunk/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp 2012-04-04 21:27:36 UTC (rev 113241)
@@ -532,6 +532,10 @@
m_client = MockLayerTreeHostClient::create(this);
RefPtr<LayerChromium> rootLayer = LayerChromium::create();
+
+ // Only non-empty root layers will cause drawing to happen.
+ rootLayer->setBounds(IntSize(1, 1));
+
m_layerTreeHost = MockLayerTreeHost::create(this, m_client.get(), rootLayer, m_settings);
ASSERT_TRUE(m_layerTreeHost);
rootLayer->setLayerTreeHost(m_layerTreeHost.get());
@@ -821,6 +825,56 @@
runTestThreaded();
}
+// If the root layer has no content bounds, we should see a commit, but should not be
+// pushing any frames as the contents will be undefined. Regardless, forced draws need
+// to always signal completion.
+class CCLayerTreeHostTestEmptyContentsShouldNotDraw : public CCLayerTreeHostTestThreadOnly {
+public:
+ CCLayerTreeHostTestEmptyContentsShouldNotDraw()
+ : m_numCommits(0)
+ {
+ }
+
+ virtual void beginTest()
+ {
+ }
+
+ virtual void drawLayersOnCCThread(CCLayerTreeHostImpl* impl)
+ {
+ // Only the initial draw should bring us here.
+ EXPECT_FALSE(impl->rootLayer()->bounds().isEmpty());
+ }
+
+ virtual void didCommitAndDrawFrame()
+ {
+ m_numCommits++;
+ if (m_numCommits == 1) {
+ // Put an empty root layer.
+ RefPtr<LayerChromium> rootLayer = LayerChromium::create();
+ m_layerTreeHost->setRootLayer(rootLayer);
+
+ OwnArrayPtr<char> pixels(adoptArrayPtr(new char[4]));
+ m_layerTreeHost->compositeAndReadback(static_cast<void*>(pixels.get()), IntRect(0, 0, 1, 1));
+ } else if (m_numCommits == 2) {
+ m_layerTreeHost->setNeedsCommit();
+ m_layerTreeHost->finishAllRendering();
+ endTest();
+ }
+ }
+
+ virtual void afterTest()
+ {
+ }
+
+private:
+ int m_numCommits;
+};
+
+TEST_F(CCLayerTreeHostTestEmptyContentsShouldNotDraw, runMultiThread)
+{
+ runTestThreaded();
+}
+
// A compositeAndReadback while invisible should force a normal commit without assertion.
class CCLayerTreeHostTestCompositeAndReadbackWhileInvisible : public CCLayerTreeHostTestThreadOnly {
public:
