Diff
Modified: trunk/Source/WebCore/ChangeLog (112445 => 112446)
--- trunk/Source/WebCore/ChangeLog 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebCore/ChangeLog 2012-03-28 21:45:07 UTC (rev 112446)
@@ -1,3 +1,32 @@
+2012-03-28 Nat Duca <[email protected]>
+
+ [chromium] Scheduler should not tell FrameRateController to begin a frame when we dont swap
+ https://bugs.webkit.org/show_bug.cgi?id=82516
+
+ Reviewed by James Robinson.
+
+ * platform/graphics/chromium/LayerRendererChromium.cpp:
+ (WebCore::LayerRendererChromium::swapBuffers):
+ * platform/graphics/chromium/LayerRendererChromium.h:
+ (LayerRendererChromium):
+ * platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:
+ (WebCore::CCLayerTreeHostImpl::swapBuffers):
+ * platform/graphics/chromium/cc/CCLayerTreeHostImpl.h:
+ (CCLayerTreeHostImpl):
+ * platform/graphics/chromium/cc/CCScheduler.cpp:
+ (WebCore::CCScheduler::processScheduledActions):
+ * platform/graphics/chromium/cc/CCScheduler.h:
+ (WebCore::CCScheduledActionDrawAndSwapResult::CCScheduledActionDrawAndSwapResult):
+ (CCScheduledActionDrawAndSwapResult):
+ (WebCore):
+ (CCSchedulerClient):
+ * platform/graphics/chromium/cc/CCThreadProxy.cpp:
+ (WebCore::CCThreadProxy::scheduledActionDrawAndSwapInternal):
+ (WebCore::CCThreadProxy::scheduledActionDrawAndSwapIfPossible):
+ (WebCore::CCThreadProxy::scheduledActionDrawAndSwapForced):
+ * platform/graphics/chromium/cc/CCThreadProxy.h:
+ (CCThreadProxy):
+
2012-03-26 Eric Uhrhane <[email protected]>
FileWriter has two race conditions
Modified: trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp (112445 => 112446)
--- trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp 2012-03-28 21:45:07 UTC (rev 112446)
@@ -1073,13 +1073,13 @@
m_context->finish();
}
-void LayerRendererChromium::swapBuffers(const IntRect& subBuffer)
+bool LayerRendererChromium::swapBuffers(const IntRect& subBuffer)
{
// FIXME: Remove this once gpu process supports ignoring swap buffers command while framebuffer is discarded.
// Alternatively (preferably?), protect all cc code so as not to attempt a swap after a framebuffer discard.
if (m_isFramebufferDiscarded) {
m_client->setFullRootLayerDamage();
- return;
+ return false;
}
TRACE_EVENT("LayerRendererChromium::swapBuffers", this, 0);
@@ -1098,6 +1098,7 @@
m_context->prepareTexture();
m_headsUpDisplay->onSwapBuffers();
+ return true;
}
void LayerRendererChromium::onSwapBuffersComplete()
Modified: trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h (112445 => 112446)
--- trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h 2012-03-28 21:45:07 UTC (rev 112446)
@@ -108,7 +108,7 @@
void finish();
// puts backbuffer onscreen
- void swapBuffers(const IntRect& subBuffer);
+ bool swapBuffers(const IntRect& subBuffer);
static void debugGLCall(GraphicsContext3D*, const char* command, const char* file, int line);
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp (112445 => 112446)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp 2012-03-28 21:45:07 UTC (rev 112446)
@@ -434,10 +434,10 @@
return m_layerRenderer ? m_layerRenderer->contentsTextureAllocator() : 0;
}
-void CCLayerTreeHostImpl::swapBuffers()
+bool CCLayerTreeHostImpl::swapBuffers()
{
ASSERT(m_layerRenderer);
- m_layerRenderer->swapBuffers(enclosingIntRect(m_rootDamageRect));
+ return m_layerRenderer->swapBuffers(enclosingIntRect(m_rootDamageRect));
}
void CCLayerTreeHostImpl::didLoseContext()
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h (112445 => 112446)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h 2012-03-28 21:45:07 UTC (rev 112446)
@@ -114,7 +114,7 @@
const LayerRendererCapabilities& layerRendererCapabilities() const;
TextureAllocator* contentsTextureAllocator() const;
- void swapBuffers();
+ bool swapBuffers();
void readback(void* pixels, const IntRect&);
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCScheduler.cpp (112445 => 112446)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCScheduler.cpp 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCScheduler.cpp 2012-03-28 21:45:07 UTC (rev 112446)
@@ -163,17 +163,18 @@
m_client->scheduledActionCommit();
break;
case CCSchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE: {
- bool drawSuccess = m_client->scheduledActionDrawAndSwapIfPossible();
- m_stateMachine.didDrawIfPossibleCompleted(drawSuccess);
- if (drawSuccess)
+ CCScheduledActionDrawAndSwapResult result = m_client->scheduledActionDrawAndSwapIfPossible();
+ m_stateMachine.didDrawIfPossibleCompleted(result.didDraw);
+ if (result.didSwap)
m_frameRateController->didBeginFrame();
break;
}
- case CCSchedulerStateMachine::ACTION_DRAW_FORCED:
- m_client->scheduledActionDrawAndSwapForced();
- m_frameRateController->didBeginFrame();
+ case CCSchedulerStateMachine::ACTION_DRAW_FORCED: {
+ CCScheduledActionDrawAndSwapResult result = m_client->scheduledActionDrawAndSwapForced();
+ if (result.didSwap)
+ m_frameRateController->didBeginFrame();
break;
- case CCSchedulerStateMachine::ACTION_BEGIN_CONTEXT_RECREATION:
+ } case CCSchedulerStateMachine::ACTION_BEGIN_CONTEXT_RECREATION:
m_client->scheduledActionBeginContextRecreation();
break;
}
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCScheduler.h (112445 => 112446)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCScheduler.h 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCScheduler.h 2012-03-28 21:45:07 UTC (rev 112446)
@@ -35,14 +35,29 @@
class CCThread;
+struct CCScheduledActionDrawAndSwapResult {
+ CCScheduledActionDrawAndSwapResult()
+ : didDraw(false)
+ , didSwap(false)
+ {
+ }
+ CCScheduledActionDrawAndSwapResult(bool didDraw, bool didSwap)
+ : didDraw(didDraw)
+ , didSwap(didSwap)
+ {
+ }
+ bool didDraw;
+ bool didSwap;
+};
+
class CCSchedulerClient {
public:
virtual bool canDraw() = 0;
virtual bool hasMoreResourceUpdates() const = 0;
virtual void scheduledActionBeginFrame() = 0;
- virtual bool scheduledActionDrawAndSwapIfPossible() = 0;
- virtual void scheduledActionDrawAndSwapForced() = 0;
+ virtual CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapIfPossible() = 0;
+ virtual CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapForced() = 0;
virtual void scheduledActionUpdateMoreResources() = 0;
virtual void scheduledActionCommit() = 0;
virtual void scheduledActionBeginContextRecreation() = 0;
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp (112445 => 112446)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp 2012-03-28 21:45:07 UTC (rev 112446)
@@ -561,13 +561,16 @@
m_mainThreadProxy->postTask(createCCThreadTask(this, &CCThreadProxy::beginContextRecreation));
}
-bool CCThreadProxy::scheduledActionDrawAndSwapInternal(bool forcedDraw)
+CCScheduledActionDrawAndSwapResult CCThreadProxy::scheduledActionDrawAndSwapInternal(bool forcedDraw)
{
TRACE_EVENT("CCThreadProxy::scheduledActionDrawAndSwap", this, 0);
+ CCScheduledActionDrawAndSwapResult result;
+ result.didDraw = false;
+ result.didSwap = false;
ASSERT(isImplThread());
ASSERT(m_layerTreeHostImpl);
if (!m_layerTreeHostImpl)
- return false;
+ return result;
// FIXME: compute the frame display time more intelligently
double monotonicTime = monotonicallyIncreasingTime();
@@ -577,8 +580,10 @@
m_layerTreeHostImpl->animate(monotonicTime, wallClockTime);
CCLayerTreeHostImpl::FrameData frame;
bool drawFrame = m_layerTreeHostImpl->prepareToDraw(frame) || forcedDraw;
- if (drawFrame)
+ if (drawFrame) {
m_layerTreeHostImpl->drawLayers(frame);
+ result.didDraw = true;
+ }
// Check for a pending compositeAndReadback.
if (m_readbackRequestOnImplThread) {
@@ -590,7 +595,7 @@
}
if (drawFrame)
- m_layerTreeHostImpl->swapBuffers();
+ result.didSwap = m_layerTreeHostImpl->swapBuffers();
// Process any finish request
if (m_finishAllRenderingCompletionEventOnImplThread) {
@@ -607,17 +612,17 @@
}
ASSERT(drawFrame || (!drawFrame && !forcedDraw));
- return drawFrame;
+ return result;
}
-bool CCThreadProxy::scheduledActionDrawAndSwapIfPossible()
+CCScheduledActionDrawAndSwapResult CCThreadProxy::scheduledActionDrawAndSwapIfPossible()
{
return scheduledActionDrawAndSwapInternal(false);
}
-void CCThreadProxy::scheduledActionDrawAndSwapForced()
+CCScheduledActionDrawAndSwapResult CCThreadProxy::scheduledActionDrawAndSwapForced()
{
- scheduledActionDrawAndSwapInternal(true);
+ return scheduledActionDrawAndSwapInternal(true);
}
void CCThreadProxy::didCommitAndDrawFrame()
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h (112445 => 112446)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h 2012-03-28 21:45:07 UTC (rev 112446)
@@ -82,8 +82,8 @@
virtual bool canDraw();
virtual bool hasMoreResourceUpdates() const;
virtual void scheduledActionBeginFrame();
- virtual bool scheduledActionDrawAndSwapIfPossible();
- virtual void scheduledActionDrawAndSwapForced();
+ virtual CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapIfPossible();
+ virtual CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapForced();
virtual void scheduledActionUpdateMoreResources();
virtual void scheduledActionCommit();
virtual void scheduledActionBeginContextRecreation();
@@ -127,7 +127,7 @@
void layerTreeHostClosedOnImplThread(CCCompletionEvent*);
void setFullRootLayerDamageOnImplThread();
void recreateContextOnImplThread(CCCompletionEvent*, GraphicsContext3D*, bool* recreateSucceeded, LayerRendererCapabilities*);
- bool scheduledActionDrawAndSwapInternal(bool forcedDraw);
+ CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapInternal(bool forcedDraw);
// Accessed on main thread only.
bool m_animateRequested;
Modified: trunk/Source/WebKit/chromium/ChangeLog (112445 => 112446)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-03-28 21:45:07 UTC (rev 112446)
@@ -1,3 +1,27 @@
+2012-03-28 Nat Duca <[email protected]>
+
+ [chromium] Scheduler should not tell FrameRateController to begin a frame when we dont swap
+ https://bugs.webkit.org/show_bug.cgi?id=82516
+
+ Reviewed by James Robinson.
+
+ * tests/CCSchedulerTest.cpp:
+ (WebKitTests::FakeCCSchedulerClient::reset):
+ (WebKitTests::FakeCCSchedulerClient::hasAction):
+ (FakeCCSchedulerClient):
+ (WebKitTests::FakeCCSchedulerClient::scheduledActionDrawAndSwapIfPossible):
+ (WebKitTests::FakeCCSchedulerClient::scheduledActionDrawAndSwapForced):
+ (WebKitTests::FakeCCSchedulerClient::setDrawWillHappen):
+ (WebKitTests::FakeCCSchedulerClient::setSwapWillHappenIfDrawHappens):
+ (WebKitTests::SchedulerClientThatSetNeedsDrawInsideDraw::scheduledActionDrawAndSwapIfPossible):
+ (WebKitTests::SchedulerClientThatSetNeedsDrawInsideDraw::scheduledActionDrawAndSwapForced):
+ (SchedulerClientThatSetNeedsDrawInsideDraw):
+ (WebKitTests::TEST):
+ (WebKitTests::SchedulerClientThatSetNeedsCommitInsideDraw::scheduledActionDrawAndSwapIfPossible):
+ (WebKitTests::SchedulerClientThatSetNeedsCommitInsideDraw::scheduledActionDrawAndSwapForced):
+ (SchedulerClientThatSetNeedsCommitInsideDraw):
+ (WebKitTests):
+
2012-03-26 Shawn Singh <[email protected]>
[chromium] layer->clipRect() is not initialized for layers that create a renderSurface.
Modified: trunk/Source/WebKit/chromium/tests/CCSchedulerTest.cpp (112445 => 112446)
--- trunk/Source/WebKit/chromium/tests/CCSchedulerTest.cpp 2012-03-28 21:43:10 UTC (rev 112445)
+++ trunk/Source/WebKit/chromium/tests/CCSchedulerTest.cpp 2012-03-28 21:45:07 UTC (rev 112446)
@@ -45,7 +45,8 @@
m_actions.clear();
m_hasMoreResourceUpdates = false;
m_canDraw = true;
- m_drawSuccess = true;
+ m_drawWillHappen = true;
+ m_swapWillHappenIfDrawHappens = true;
m_numDraws = 0;
}
@@ -56,32 +57,42 @@
int numActions() const { return static_cast<int>(m_actions.size()); }
const char* action(int i) const { return m_actions[i]; }
+ bool hasAction(const char* action) const
+ {
+ for (size_t i = 0; i < m_actions.size(); i++)
+ if (!strcmp(m_actions[i], action))
+ return true;
+ return false;
+ }
+
virtual bool canDraw() { return m_canDraw; }
virtual bool hasMoreResourceUpdates() const { return m_hasMoreResourceUpdates; }
virtual void scheduledActionBeginFrame() { m_actions.push_back("scheduledActionBeginFrame"); }
- virtual bool scheduledActionDrawAndSwapIfPossible()
+ virtual CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapIfPossible()
{
m_actions.push_back("scheduledActionDrawAndSwapIfPossible");
m_numDraws++;
- return m_drawSuccess;
+ return CCScheduledActionDrawAndSwapResult(m_drawWillHappen, m_drawWillHappen && m_swapWillHappenIfDrawHappens);
}
- virtual void scheduledActionDrawAndSwapForced()
+ virtual CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapForced()
{
m_actions.push_back("scheduledActionDrawAndSwapForced");
- m_numDraws++;
+ return CCScheduledActionDrawAndSwapResult(true, m_swapWillHappenIfDrawHappens);
}
virtual void scheduledActionUpdateMoreResources() { m_actions.push_back("scheduledActionUpdateMoreResources"); }
virtual void scheduledActionCommit() { m_actions.push_back("scheduledActionCommit"); }
virtual void scheduledActionBeginContextRecreation() { m_actions.push_back("scheduledActionBeginContextRecreation"); }
- void setDrawSuccess(bool drawSuccess) { m_drawSuccess = drawSuccess; }
+ void setDrawWillHappen(bool drawWillHappen) { m_drawWillHappen = drawWillHappen; }
+ void setSwapWillHappenIfDrawHappens(bool swapWillHappenIfDrawHappens) { m_swapWillHappenIfDrawHappens = swapWillHappenIfDrawHappens; }
protected:
bool m_hasMoreResourceUpdates;
bool m_canDraw;
- bool m_drawSuccess;
+ bool m_drawWillHappen;
+ bool m_swapWillHappenIfDrawHappens;
int m_numDraws;
std::vector<const char*> m_actions;
};
@@ -163,7 +174,7 @@
void setScheduler(CCScheduler* scheduler) { m_scheduler = scheduler; }
virtual void scheduledActionBeginFrame() { }
- virtual bool scheduledActionDrawAndSwapIfPossible()
+ virtual CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapIfPossible()
{
// Only setNeedsRedraw the first time this is called
if (!m_numDraws)
@@ -171,7 +182,12 @@
return FakeCCSchedulerClient::scheduledActionDrawAndSwapIfPossible();
}
- virtual void scheduledActionDrawAndSwapForced() { }
+ virtual CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapForced()
+ {
+ ASSERT_NOT_REACHED();
+ return CCScheduledActionDrawAndSwapResult(true, true);
+ }
+
virtual void scheduledActionUpdateMoreResources() { }
virtual void scheduledActionCommit() { }
virtual void scheduledActionBeginContextRecreation() { }
@@ -216,7 +232,7 @@
OwnPtr<CCScheduler> scheduler = CCScheduler::create(&client, adoptPtr(new CCFrameRateController(timeSource)));
client.setScheduler(scheduler.get());
scheduler->setVisible(true);
- client.setDrawSuccess(false);
+ client.setDrawWillHappen(false);
scheduler->setNeedsRedraw();
EXPECT_TRUE(scheduler->redrawPending());
@@ -240,7 +256,7 @@
EXPECT_TRUE(timeSource->active());
// Draw successfully.
- client.setDrawSuccess(true);
+ client.setDrawWillHappen(true);
timeSource->tick();
EXPECT_EQ(3, client.numDraws());
EXPECT_TRUE(scheduler->commitPending());
@@ -256,7 +272,7 @@
void setScheduler(CCScheduler* scheduler) { m_scheduler = scheduler; }
virtual void scheduledActionBeginFrame() { }
- virtual bool scheduledActionDrawAndSwapIfPossible()
+ virtual CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapIfPossible()
{
// Only setNeedsCommit the first time this is called
if (!m_numDraws)
@@ -264,7 +280,12 @@
return FakeCCSchedulerClient::scheduledActionDrawAndSwapIfPossible();
}
- virtual void scheduledActionDrawAndSwapForced() { }
+ virtual CCScheduledActionDrawAndSwapResult scheduledActionDrawAndSwapForced()
+ {
+ ASSERT_NOT_REACHED();
+ return CCScheduledActionDrawAndSwapResult(true, true);
+ }
+
virtual void scheduledActionUpdateMoreResources() { }
virtual void scheduledActionCommit() { }
virtual void scheduledActionBeginContextRecreation() { }
@@ -308,7 +329,7 @@
OwnPtr<CCScheduler> scheduler = CCScheduler::create(&client, adoptPtr(new CCFrameRateController(timeSource)));
client.setScheduler(scheduler.get());
scheduler->setVisible(true);
- client.setDrawSuccess(false);
+ client.setDrawWillHappen(false);
scheduler->setNeedsRedraw();
EXPECT_TRUE(scheduler->redrawPending());
@@ -332,7 +353,7 @@
EXPECT_TRUE(timeSource->active());
// Draw successfully.
- client.setDrawSuccess(true);
+ client.setDrawWillHappen(true);
timeSource->tick();
EXPECT_EQ(3, client.numDraws());
EXPECT_TRUE(scheduler->commitPending());
@@ -380,10 +401,33 @@
EXPECT_TRUE(timeSource->active());
// Fail to draw, this should not start a frame.
- client.setDrawSuccess(false);
+ client.setDrawWillHappen(false);
timeSource->tick();
EXPECT_EQ(3, client.numDraws());
EXPECT_EQ(0, controllerPtr->numFramesPending());
}
+TEST(CCSchedulerTest, NoBeginFrameWhenSwapFailsDuringForcedCommit)
+{
+ RefPtr<FakeCCTimeSource> timeSource = adoptRef(new FakeCCTimeSource());
+ FakeCCSchedulerClient client;
+ OwnPtr<FakeCCFrameRateController> controller = adoptPtr(new FakeCCFrameRateController(timeSource));
+ FakeCCFrameRateController* controllerPtr = controller.get();
+ OwnPtr<CCScheduler> scheduler = CCScheduler::create(&client, controller.release());
+
+ EXPECT_EQ(0, controllerPtr->numFramesPending());
+
+ // Tell the client that it will fail to swap.
+ client.setDrawWillHappen(true);
+ client.setSwapWillHappenIfDrawHappens(false);
+
+ // Get the compositor to do a scheduledActionDrawAndSwapForced.
+ scheduler->setNeedsRedraw();
+ scheduler->setNeedsForcedRedraw();
+ EXPECT_TRUE(client.hasAction("scheduledActionDrawAndSwapForced"));
+
+ // We should not have told the frame rate controller that we began a frame.
+ EXPECT_EQ(0, controllerPtr->numFramesPending());
}
+
+}
