From 885093324617fb57df3a7fe427584283e1223fdd Mon Sep 17 00:00:00 2001 From: "jamesr@google.com" Date: Thu, 22 Sep 2011 07:53:28 +0000 Subject: [PATCH] Unreviewed, rolling out r95699. http://trac.webkit.org/changeset/95699 https://bugs.webkit.org/show_bug.cgi?id=67417 Makes many chromium compositor tests crash Source/WebCore: * platform/CrossThreadCopier.h: * platform/graphics/chromium/LayerRendererChromium.cpp: (WebCore::LayerRendererChromium::~LayerRendererChromium): * platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp: (WebCore::CCHeadsUpDisplay::enabled): * platform/graphics/chromium/cc/CCLayerImpl.cpp: (WebCore::CCLayerImpl::CCLayerImpl): (WebCore::CCLayerImpl::~CCLayerImpl): * platform/graphics/chromium/cc/CCLayerTreeHost.cpp: (WebCore::CCLayerTreeHost::CCLayerTreeHost): (WebCore::CCLayerTreeHost::commitTo): (WebCore::CCLayerTreeHost::setNeedsRedraw): * platform/graphics/chromium/cc/CCLayerTreeHost.h: (WebCore::CCSettings::CCSettings): * platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp: (WebCore::CCLayerTreeHostImpl::CCLayerTreeHostImpl): (WebCore::CCLayerTreeHostImpl::~CCLayerTreeHostImpl): * platform/graphics/chromium/cc/CCProxy.h: * platform/graphics/chromium/cc/CCSingleThreadProxy.cpp: (WebCore::CCSingleThreadProxy::finishAllRendering): (WebCore::CCSingleThreadProxy::setNeedsCommit): (WebCore::CCSingleThreadProxy::commitIfNeeded): * platform/graphics/chromium/cc/CCThreadProxy.cpp: (WebCore::CCThreadProxy::CCThreadProxy): (WebCore::CCThreadProxy::~CCThreadProxy): (WebCore::CCThreadProxy::compositeAndReadback): (WebCore::CCThreadProxy::finishAllRendering): (WebCore::CCThreadProxy::isStarted): (WebCore::CCThreadProxy::initializeLayerRenderer): (WebCore::CCThreadProxy::setNeedsCommit): (WebCore::CCThreadProxy::setNeedsCommitAndRedraw): (WebCore::CCThreadProxy::setNeedsRedraw): (WebCore::CCThreadProxy::start): (WebCore::CCThreadProxy::stop): (WebCore::CCThreadProxy::beginFrameAndCommitOnCCThread): (WebCore::CCThreadProxy::beginFrameAndCommit): (WebCore::CCThreadProxy::commitOnCCThread): (WebCore::CCThreadProxy::drawLayersOnCCThread): (WebCore::CCThreadProxy::setNeedsCommitOnCCThread): (WebCore::CCThreadProxy::setNeedsCommitAndRedrawOnCCThread): (WebCore::CCThreadProxy::setNeedsRedrawOnCCThread): * platform/graphics/chromium/cc/CCThreadProxy.h: Source/WebKit/chromium: * tests/CCLayerTreeHostTest.cpp: (WTF::CCLayerTreeHostTest::doBeginTest): (WTF::TEST_F): * tests/TreeSynchronizerTest.cpp: (WebCore::TEST): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@95702 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebCore/ChangeLog | 51 +++++ Source/WebCore/platform/CrossThreadCopier.h | 6 - .../graphics/chromium/LayerRendererChromium.cpp | 1 - .../graphics/chromium/cc/CCHeadsUpDisplay.cpp | 3 - .../platform/graphics/chromium/cc/CCLayerImpl.cpp | 2 - .../graphics/chromium/cc/CCLayerTreeHost.cpp | 15 +- .../graphics/chromium/cc/CCLayerTreeHost.h | 7 - .../graphics/chromium/cc/CCLayerTreeHostImpl.cpp | 2 - .../platform/graphics/chromium/cc/CCProxy.h | 13 +- .../graphics/chromium/cc/CCSingleThreadProxy.cpp | 13 +- .../graphics/chromium/cc/CCThreadProxy.cpp | 247 +++++---------------- .../platform/graphics/chromium/cc/CCThreadProxy.h | 25 +-- Source/WebKit/chromium/ChangeLog | 14 ++ .../WebKit/chromium/tests/CCLayerTreeHostTest.cpp | 21 +- .../WebKit/chromium/tests/TreeSynchronizerTest.cpp | 23 -- 15 files changed, 139 insertions(+), 304 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 9210646..c6307ec 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,54 @@ +2011-09-22 James Robinson + + Unreviewed, rolling out r95699. + http://trac.webkit.org/changeset/95699 + https://bugs.webkit.org/show_bug.cgi?id=67417 + + Makes many chromium compositor tests crash + + * platform/CrossThreadCopier.h: + * platform/graphics/chromium/LayerRendererChromium.cpp: + (WebCore::LayerRendererChromium::~LayerRendererChromium): + * platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp: + (WebCore::CCHeadsUpDisplay::enabled): + * platform/graphics/chromium/cc/CCLayerImpl.cpp: + (WebCore::CCLayerImpl::CCLayerImpl): + (WebCore::CCLayerImpl::~CCLayerImpl): + * platform/graphics/chromium/cc/CCLayerTreeHost.cpp: + (WebCore::CCLayerTreeHost::CCLayerTreeHost): + (WebCore::CCLayerTreeHost::commitTo): + (WebCore::CCLayerTreeHost::setNeedsRedraw): + * platform/graphics/chromium/cc/CCLayerTreeHost.h: + (WebCore::CCSettings::CCSettings): + * platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp: + (WebCore::CCLayerTreeHostImpl::CCLayerTreeHostImpl): + (WebCore::CCLayerTreeHostImpl::~CCLayerTreeHostImpl): + * platform/graphics/chromium/cc/CCProxy.h: + * platform/graphics/chromium/cc/CCSingleThreadProxy.cpp: + (WebCore::CCSingleThreadProxy::finishAllRendering): + (WebCore::CCSingleThreadProxy::setNeedsCommit): + (WebCore::CCSingleThreadProxy::commitIfNeeded): + * platform/graphics/chromium/cc/CCThreadProxy.cpp: + (WebCore::CCThreadProxy::CCThreadProxy): + (WebCore::CCThreadProxy::~CCThreadProxy): + (WebCore::CCThreadProxy::compositeAndReadback): + (WebCore::CCThreadProxy::finishAllRendering): + (WebCore::CCThreadProxy::isStarted): + (WebCore::CCThreadProxy::initializeLayerRenderer): + (WebCore::CCThreadProxy::setNeedsCommit): + (WebCore::CCThreadProxy::setNeedsCommitAndRedraw): + (WebCore::CCThreadProxy::setNeedsRedraw): + (WebCore::CCThreadProxy::start): + (WebCore::CCThreadProxy::stop): + (WebCore::CCThreadProxy::beginFrameAndCommitOnCCThread): + (WebCore::CCThreadProxy::beginFrameAndCommit): + (WebCore::CCThreadProxy::commitOnCCThread): + (WebCore::CCThreadProxy::drawLayersOnCCThread): + (WebCore::CCThreadProxy::setNeedsCommitOnCCThread): + (WebCore::CCThreadProxy::setNeedsCommitAndRedrawOnCCThread): + (WebCore::CCThreadProxy::setNeedsRedrawOnCCThread): + * platform/graphics/chromium/cc/CCThreadProxy.h: + 2011-09-22 Nat Duca [chromium] Make CCThreadProxy draw diff --git a/Source/WebCore/platform/CrossThreadCopier.h b/Source/WebCore/platform/CrossThreadCopier.h index 39d11b5..7ce7f98 100644 --- a/Source/WebCore/platform/CrossThreadCopier.h +++ b/Source/WebCore/platform/CrossThreadCopier.h @@ -41,7 +41,6 @@ namespace WebCore { - class IntRect; class KURL; class ResourceError; class ResourceRequest; @@ -64,14 +63,9 @@ namespace WebCore { template struct CrossThreadCopierBase : public CrossThreadCopierPassThrough { }; - // To allow a type to be passed across threads using its copy constructor, add a forward declaration of the type and - // a CopyThreadCopierBase : public CrossThreadCopierPassThrough { }; to this file. template<> struct CrossThreadCopierBase : public CrossThreadCopierPassThrough { }; - template<> struct CrossThreadCopierBase : public CrossThreadCopierPassThrough { - }; - // Custom copy methods. template struct CrossThreadCopierBase { typedef typename WTF::RemoveTemplate::Type TypeWithoutRefPtr; diff --git a/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp b/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp index 1946a96..4dae613 100644 --- a/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp +++ b/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp @@ -182,7 +182,6 @@ bool LayerRendererChromium::initialize() LayerRendererChromium::~LayerRendererChromium() { - ASSERT(CCProxy::isMainThread()); m_headsUpDisplay.clear(); // Explicitly destroy the HUD before the TextureManager dies. cleanupSharedObjects(); } diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp b/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp index dc27148..30aa82c 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp +++ b/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp @@ -89,9 +89,6 @@ void CCHeadsUpDisplay::onPresent() bool CCHeadsUpDisplay::enabled() const { - // FIXME: HUD does not work in compositor thread mode. - if (settings().enableCompositorThread) - return false; return settings().showPlatformLayerTree || settings().showFPSCounter; } diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp b/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp index 64168c2..c986591 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp +++ b/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp @@ -55,12 +55,10 @@ CCLayerImpl::CCLayerImpl(int id) , m_debugBorderColor(0, 0, 0, 0) , m_debugBorderWidth(0) { - ASSERT(CCProxy::isImplThread()); } CCLayerImpl::~CCLayerImpl() { - ASSERT(CCProxy::isImplThread()); } void CCLayerImpl::addChild(PassRefPtr child) diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp b/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp index bda6004..a257b41 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp +++ b/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp @@ -56,7 +56,6 @@ CCLayerTreeHost::CCLayerTreeHost(CCLayerTreeHostClient* client, PassRefPtrdeleteEvictedTextures(hostImpl->context()); updateCompositorResources(m_updateList, hostImpl->context()); + clearPendingUpdate(); hostImpl->setVisible(m_visible); hostImpl->setZoomAnimatorScale(m_zoomAnimatorScale); hostImpl->setViewport(viewportSize()); hostImpl->layerRenderer()->setContentsTextureMemoryUseBytes(m_contentsTextureManager->currentMemoryUseBytes()); + m_contentsTextureManager->unprotectAllTextures(); // Synchronize trees, if one exists at all... if (rootLayer()) @@ -142,12 +138,6 @@ void CCLayerTreeHost::commitTo(CCLayerTreeHostImpl* hostImpl) m_frameNumber++; } -void CCLayerTreeHost::commitComplete() -{ - clearPendingUpdate(); - m_contentsTextureManager->unprotectAllTextures(); -} - PassOwnPtr CCLayerTreeHost::createCompositorThread() { return m_client->createCompositorThread(); @@ -224,6 +214,7 @@ void CCLayerTreeHost::setNeedsCommitAndRedraw() void CCLayerTreeHost::setNeedsRedraw() { #if USE(THREADED_COMPOSITING) + TRACE_EVENT("CCLayerTreeHost::setNeedsRedraw", this, 0); m_proxy->setNeedsRedraw(); #else m_client->scheduleComposite(); diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.h b/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.h index b7b9048..fee2956 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.h +++ b/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.h @@ -67,12 +67,6 @@ struct CCSettings { , enableCompositorThread(false) , showFPSCounter(false) , showPlatformLayerTree(false) { } - CCSettings(bool acceleratePainting, bool compositeOffscreen, bool enableCompositorThread, bool showFPSCounter, bool showPlatformLayerTree) - : acceleratePainting(acceleratePainting) - , compositeOffscreen(compositeOffscreen) - , enableCompositorThread(enableCompositorThread) - , showFPSCounter(showFPSCounter) - , showPlatformLayerTree(showPlatformLayerTree) { } bool acceleratePainting; bool compositeOffscreen; @@ -102,7 +96,6 @@ public: // CCLayerTreeHost interface to CCProxy. void animateAndLayout(double frameBeginTime); - void commitComplete(); void commitTo(CCLayerTreeHostImpl*); PassOwnPtr createCompositorThread(); PassRefPtr createLayerTreeHostContext3D(); diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp b/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp index 4a1c39d..b304836 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp +++ b/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp @@ -48,12 +48,10 @@ CCLayerTreeHostImpl::CCLayerTreeHostImpl(const CCSettings& settings) , m_frameNumber(0) , m_settings(settings) { - ASSERT(CCProxy::isImplThread()); } CCLayerTreeHostImpl::~CCLayerTreeHostImpl() { - ASSERT(CCProxy::isImplThread()); TRACE_EVENT("CCLayerTreeHostImpl::~CCLayerTreeHostImpl()", this, 0); if (m_layerRenderer) m_layerRenderer->close(); diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCProxy.h b/Source/WebCore/platform/graphics/chromium/cc/CCProxy.h index b222b99..f5e418b 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCProxy.h +++ b/Source/WebCore/platform/graphics/chromium/cc/CCProxy.h @@ -75,20 +75,19 @@ public: static bool isImplThread(); #endif - // Temporary hack while render_widget still does scheduling for CCLayerTreeHostMainThreadI - virtual GraphicsContext3D* context() = 0; - // Testing hooks virtual void loseCompositorContext(int numTimes) = 0; -#ifndef NDEBUG - static void setImplThread(bool); - static void setImplThread(WTF::ThreadIdentifier); -#endif + // Temporary hack while render_widget still does scheduling for CCLayerTreeHostMainThreadI + virtual GraphicsContext3D* context() = 0; protected: CCProxy() { } friend class ScopedSetImplThread; +#ifndef NDEBUG + static void setImplThread(bool); + static void setImplThread(WTF::ThreadIdentifier); +#endif }; } diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp b/Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp index 002a4ab..2d27d72 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp +++ b/Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp @@ -111,15 +111,8 @@ GraphicsContext3D* CCSingleThreadProxy::context() void CCSingleThreadProxy::finishAllRendering() { ASSERT(isMainThread()); - if (!recreateContextIfNeeded()) - return; - - commitIfNeeded(); - - { - ScopedSetImplThread impl; - m_layerTreeHostImpl->finishAllRendering(); - } + ScopedSetImplThread impl; + m_layerTreeHostImpl->finishAllRendering(); } bool CCSingleThreadProxy::isStarted() const @@ -164,7 +157,6 @@ void CCSingleThreadProxy::setNeedsCommit() m_layerTreeHost->commitTo(m_layerTreeHostImpl.get()); m_layerTreeHostImpl->commitComplete(); } - m_layerTreeHost->commitComplete(); } void CCSingleThreadProxy::setNeedsCommitAndRedraw() @@ -258,7 +250,6 @@ void CCSingleThreadProxy::commitIfNeeded() m_layerTreeHost->commitTo(m_layerTreeHostImpl.get()); m_layerTreeHostImpl->commitComplete(); } - m_layerTreeHost->commitComplete(); } bool CCSingleThreadProxy::doComposite() diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp b/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp index 3d4ed5e..9a84da6 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp +++ b/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp @@ -31,7 +31,6 @@ #include "cc/CCLayerTreeHost.h" #include "cc/CCMainThreadTask.h" #include "cc/CCThreadTask.h" -#include #include using namespace WTF; @@ -49,14 +48,8 @@ PassOwnPtr CCThreadProxy::create(CCLayerTreeHost* layerTreeHost) } CCThreadProxy::CCThreadProxy(CCLayerTreeHost* layerTreeHost) - : m_commitRequested(false) + : m_commitPending(false) , m_layerTreeHost(layerTreeHost) - , m_started(false) - , m_lastExecutedBeginFrameAndCommitSequenceNumber(-1) - , m_numBeginFrameAndCommitsIssuedOnCCThread(0) - , m_beginFrameAndCommitPendingOnCCThread(false) - , m_drawTaskPostedOnCCThread(false) - , m_redrawRequestedOnCCThread(false) { TRACE_EVENT("CCThreadProxy::CCThreadProxy", this, 0); ASSERT(isMainThread()); @@ -73,7 +66,8 @@ CCThreadProxy::~CCThreadProxy() { TRACE_EVENT("CCThreadProxy::~CCThreadProxy", this, 0); ASSERT(isMainThread()); - ASSERT(!m_started); + ASSERT(!m_layerTreeHostImpl); // Make sure stop() got called. + ASSERT(!m_layerTreeHost); // Make sure stop() got called. numProxies--; if (!numProxies) { @@ -84,29 +78,8 @@ CCThreadProxy::~CCThreadProxy() bool CCThreadProxy::compositeAndReadback(void *pixels, const IntRect& rect) { - ASSERT(isMainThread()); - ASSERT(m_layerTreeHost); - - finishAllRendering(); - bool success = false; - CCCompletionEvent completion; - ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::drawLayersAndReadbackOnCCThread, AllowCrossThreadAccess(&completion), AllowCrossThreadAccess(&success), AllowCrossThreadAccess(pixels), rect)); - completion.wait(); - return success; -} - -void CCThreadProxy::drawLayersAndReadbackOnCCThread(CCCompletionEvent* completion, bool* success, void* pixels, const IntRect& rect) -{ - ASSERT(CCProxy::isImplThread()); - if (!m_layerTreeHostImpl) { - *success = false; - completion->signal(); - return; - } - drawLayersOnCCThread(); - m_layerTreeHostImpl->readback(pixels, rect); - *success = m_layerTreeHostImpl->isContextLost(); - completion->signal(); + ASSERT_NOT_REACHED(); + return false; } GraphicsContext3D* CCThreadProxy::context() @@ -116,34 +89,12 @@ GraphicsContext3D* CCThreadProxy::context() void CCThreadProxy::finishAllRendering() { - ASSERT(CCProxy::isMainThread()); - // If a commit is pending, perform the commit first. - if (m_commitRequested) { - // This bit of code is uglier than it should be because returning - // pointers via the CCThread task model is really messy. Effectively, we - // are making a blocking call to createBeginFrameAndCommitTaskOnCCThread, - // and trying to get the CCMainThread::Task it returns so we can run it. - OwnPtr beginFrameAndCommitTask; - { - CCMainThread::Task* taskPtr = 0; - CCCompletionEvent completion; - ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::createBeginFrameAndCommitTaskOnCCThread, AllowCrossThreadAccess(&completion), AllowCrossThreadAccess(&taskPtr))); - completion.wait(); - beginFrameAndCommitTask = adoptPtr(taskPtr); - } - - beginFrameAndCommitTask->performTask(); - } - // Make sure all GL drawing is finished on the impl thread. - CCCompletionEvent completion; - ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::finishAllRenderingOnCCThread, AllowCrossThreadAccess(&completion))); - completion.wait(); + ASSERT_NOT_REACHED(); } bool CCThreadProxy::isStarted() const { - ASSERT(CCProxy::isMainThread()); - return m_started; + return m_layerTreeHostImpl; } bool CCThreadProxy::initializeLayerRenderer() @@ -160,7 +111,7 @@ bool CCThreadProxy::initializeLayerRenderer() // Make a blocking call to initializeLayerRendererOnCCThread. The results of that call // are pushed into the initializeSucceeded and capabilities local variables. CCCompletionEvent completion; - bool initializeSucceeded = false; + bool initializeSucceeded; LayerRendererCapabilities capabilities; ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::initializeLayerRendererOnCCThread, AllowCrossThreadAccess(contextPtr), AllowCrossThreadAccess(&completion), AllowCrossThreadAccess(&initializeSucceeded), AllowCrossThreadAccess(&capabilities))); @@ -184,57 +135,43 @@ void CCThreadProxy::loseCompositorContext(int numTimes) void CCThreadProxy::setNeedsCommit() { ASSERT(isMainThread()); - if (m_commitRequested) + if (m_commitPending) return; TRACE_EVENT("CCThreadProxy::setNeedsCommit", this, 0); - m_commitRequested = true; - ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::updateSchedulerStateOnCCThread, m_commitRequested, true)); + m_commitPending = true; + ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::setNeedsCommitOnCCThread)); } void CCThreadProxy::setNeedsCommitAndRedraw() { ASSERT(isMainThread()); - if (m_commitRequested) + if (m_commitPending) return; - m_commitRequested = true; TRACE_EVENT("CCThreadProxy::setNeedsCommitAndRedraw", this, 0); - ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::updateSchedulerStateOnCCThread, m_commitRequested, true)); + m_commitPending = true; + ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::setNeedsCommitAndRedrawOnCCThread)); } void CCThreadProxy::setNeedsRedraw() { ASSERT(isMainThread()); - if (m_commitRequested) // Implies that a commit is in flight. - return; - // Unlike setNeedsCommit that tracks whether a commit message has been sent, - // setNeedsRedraw always sends a message to the compositor thread. This is - // because the compositor thread can draw without telling the main - // thread. This should not be much of a problem because calls to - // setNeedsRedraw messages are uncommon (only triggered by WM_PAINT/etc), - // compared to setNeedsCommitAndRedraw messages. - TRACE_EVENT("CCThreadProxy::setNeedsRedraw", this, 0); - ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::updateSchedulerStateOnCCThread, false, true)); + ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::setNeedsRedrawOnCCThread)); } void CCThreadProxy::start() { - ASSERT(isMainThread()); // Create LayerTreeHostImpl. CCCompletionEvent completion; ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::initializeImplOnCCThread, AllowCrossThreadAccess(&completion))); completion.wait(); - - m_started = true; } void CCThreadProxy::stop() { TRACE_EVENT("CCThreadProxy::stop", this, 0); ASSERT(isMainThread()); - ASSERT(m_started); - // Synchronously deletes the impl. CCCompletionEvent completion; ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::layerTreeHostClosedOnCCThread, AllowCrossThreadAccess(&completion))); @@ -242,166 +179,82 @@ void CCThreadProxy::stop() ASSERT(!m_layerTreeHostImpl); // verify that the impl deleted. m_layerTreeHost = 0; - m_started = false; } -void CCThreadProxy::finishAllRenderingOnCCThread(CCCompletionEvent* completion) +void CCThreadProxy::beginFrameAndCommitOnCCThread() { - TRACE_EVENT("CCThreadProxy::finishAllRenderingOnCCThread", this, 0); - ASSERT(isImplThread()); - ASSERT(!m_beginFrameAndCommitPendingOnCCThread); - if (m_redrawRequestedOnCCThread) { - drawLayersOnCCThread(); - m_layerTreeHostImpl->present(); - m_redrawRequestedOnCCThread = false; - } - m_layerTreeHostImpl->finishAllRendering(); - completion->signal(); -} - -void CCThreadProxy::createBeginFrameAndCommitTaskOnCCThread(CCCompletionEvent* completion, CCMainThread::Task** taskPtr) -{ - OwnPtr task = createBeginFrameAndCommitTaskOnCCThread(); - *taskPtr = task.leakPtr(); - completion->signal(); -} - -PassOwnPtr CCThreadProxy::createBeginFrameAndCommitTaskOnCCThread() -{ - TRACE_EVENT("CCThreadProxy::createBeginFrameAndCommitTaskOnCCThread", this, 0); + TRACE_EVENT("CCThreadProxy::beginFrameAndCommitOnCCThread", this, 0); ASSERT(isImplThread()); - double frameBeginTime = currentTime(); - m_beginFrameAndCommitPendingOnCCThread = true; - - // NOTE, it is possible to receieve a request for a - // beginFrameAndCommitOnCCThread from finishAllRendering while a - // beginFrameAndCommitOnCCThread is enqueued. Since it CCMainThread doesn't - // provide a threadsafe way to cancel tasks, it is important that - // beginFrameAndCommit be structured to understand that it may get called at - // a point that it shouldn't. We do this by assigning a sequence number to - // every new beginFrameAndCommit task. Then, beginFrameAndCommit tracks the - // last executed sequence number, dropping beginFrameAndCommit with sequence - // numbers below the last executed one. - int thisTaskSequenceNumber = m_numBeginFrameAndCommitsIssuedOnCCThread; - m_numBeginFrameAndCommitsIssuedOnCCThread++; - return createMainThreadTask(this, &CCThreadProxy::beginFrameAndCommit, thisTaskSequenceNumber, frameBeginTime); + // TEMP HACK so we can exercise this code in unit tests. + CCMainThread::postTask(createMainThreadTask(this, &CCThreadProxy::beginFrameAndCommit, 0.0)); } -void CCThreadProxy::beginFrameAndCommit(int sequenceNumber, double frameBeginTime) +void CCThreadProxy::beginFrameAndCommit(double frameBeginTime) { - TRACE_EVENT("CCThreadProxy::beginFrameAndCommit", this, 0); ASSERT(isMainThread()); if (!m_layerTreeHost) return; - // Drop beginFrameAndCommit calls that occur out of sequence. See createBeginFrameAndCommitTaskOnCCThread for - // an explanation of how out-of-sequence beginFrameAndCommit tasks can occur. - if (sequenceNumber < m_lastExecutedBeginFrameAndCommitSequenceNumber) { - TRACE_EVENT("EarlyOut_StaleBeginFrameAndCommit", this, 0); - return; - } - m_lastExecutedBeginFrameAndCommitSequenceNumber = sequenceNumber; - - ASSERT(m_commitRequested); - - // FIXME: recreate the context if it was requested by the impl thread + TRACE_EVENT("CCThreadProxy::requestFrameAndCommit", this, 0); { TRACE_EVENT("CCLayerTreeHost::animateAndLayout", this, 0); m_layerTreeHost->animateAndLayout(frameBeginTime); } - ASSERT(m_lastExecutedBeginFrameAndCommitSequenceNumber == sequenceNumber); + m_commitPending = false; - // Clear the commit flag after animateAndLayout here --- objects that only - // layout when painted will trigger another setNeedsCommit inside - // updateLayers. - m_commitRequested = false; - - m_layerTreeHost->updateLayers(); - - { - // Blocking call to CCThreadProxy::commitOnCCThread - TRACE_EVENT("commit", this, 0); - CCCompletionEvent completion; - ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::commitOnCCThread, AllowCrossThreadAccess(&completion))); - completion.wait(); - } - - m_layerTreeHost->commitComplete(); - - ASSERT(m_lastExecutedBeginFrameAndCommitSequenceNumber == sequenceNumber); + // Blocking call to CCThreadProxy::performCommit + CCCompletionEvent completion; + ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::commitOnCCThread, AllowCrossThreadAccess(&completion))); + completion.wait(); } void CCThreadProxy::commitOnCCThread(CCCompletionEvent* completion) { - TRACE_EVENT("CCThreadProxy::beginFrameAndCommitOnCCThread", this, 0); ASSERT(isImplThread()); - ASSERT(m_beginFrameAndCommitPendingOnCCThread); - m_beginFrameAndCommitPendingOnCCThread = false; - if (!m_layerTreeHostImpl) { - completion->signal(); - return; - } + TRACE_EVENT("CCThreadProxy::commitOnCCThread", this, 0); m_layerTreeHostImpl->beginCommit(); - m_layerTreeHost->commitTo(m_layerTreeHostImpl.get()); - m_layerTreeHostImpl->commitComplete(); - + { + TRACE_EVENT("CCLayerTreeHost::commit", this, 0); + m_layerTreeHost->commitTo(m_layerTreeHostImpl.get()); + } completion->signal(); - if (m_redrawRequestedOnCCThread) - scheduleDrawTaskOnCCThread(); -} - -void CCThreadProxy::scheduleDrawTaskOnCCThread() -{ - ASSERT(isImplThread()); - if (m_drawTaskPostedOnCCThread) - return; - TRACE_EVENT("CCThreadProxy::scheduleDrawTaskOnCCThread", this, 0); - ASSERT(m_layerTreeHostImpl); - m_drawTaskPostedOnCCThread = true; - ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::drawLayersAndPresentOnCCThread)); + m_layerTreeHostImpl->commitComplete(); + setNeedsRedrawOnCCThread(); } -void CCThreadProxy::drawLayersAndPresentOnCCThread() +void CCThreadProxy::drawLayersOnCCThread() { TRACE_EVENT("CCThreadProxy::drawLayersOnCCThread", this, 0); ASSERT(isImplThread()); - if (!m_layerTreeHostImpl) - return; - - drawLayersOnCCThread(); - m_layerTreeHostImpl->present(); - m_redrawRequestedOnCCThread = false; - m_drawTaskPostedOnCCThread = false; + if (m_layerTreeHostImpl) + m_layerTreeHostImpl->drawLayers(); } -void CCThreadProxy::drawLayersOnCCThread() +void CCThreadProxy::setNeedsCommitOnCCThread() { - TRACE_EVENT("CCThreadProxy::drawLayersOnCCThread", this, 0); + TRACE_EVENT("CCThreadProxy::setNeedsCommitOnCCThread", this, 0); ASSERT(isImplThread()); ASSERT(m_layerTreeHostImpl); - - m_layerTreeHostImpl->drawLayers(); - ASSERT(!m_layerTreeHostImpl->isContextLost()); + // FIXME: Not yet implemented, see https://bugs.webkit.org/show_bug.cgi?id=67417 + ASSERT_NOT_REACHED(); } -void CCThreadProxy::updateSchedulerStateOnCCThread(bool commitRequested, bool redrawRequested) +void CCThreadProxy::setNeedsCommitAndRedrawOnCCThread() { - TRACE_EVENT("CCThreadProxy::updateSchedulerStateOnCCThread", this, 0); + TRACE_EVENT("CCThreadProxy::setNeedsCommitAndRedrawOnCCThread", this, 0); ASSERT(isImplThread()); ASSERT(m_layerTreeHostImpl); + // TEMP HACK so we can exercise this code in unit tests. + CCMainThread::postTask(createMainThreadTask(this, &CCThreadProxy::beginFrameAndCommit, 0.0)); +} - // FIXME: use CCScheduler to decide when to manage the conversion of this - // commit request into an actual createBeginFrameAndCommitTaskOnCCThread call. - m_redrawRequestedOnCCThread |= redrawRequested; - if (!m_beginFrameAndCommitPendingOnCCThread) - CCMainThread::postTask(createBeginFrameAndCommitTaskOnCCThread()); - - // If no commit is pending, but a redraw is requested, then post a redraw right away - if (!m_beginFrameAndCommitPendingOnCCThread && m_redrawRequestedOnCCThread) - scheduleDrawTaskOnCCThread(); - +void CCThreadProxy::setNeedsRedrawOnCCThread() +{ + TRACE_EVENT("CCThreadProxy::setNeedsRedrawOnCCThread", this, 0); + // TEMP HACK so we can exercise this code in unit tests. + drawLayersOnCCThread(); } void CCThreadProxy::initializeImplOnCCThread(CCCompletionEvent* completion) diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h b/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h index 5a0ea67..2467724 100644 --- a/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h +++ b/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h @@ -27,7 +27,6 @@ #include "cc/CCCompletionEvent.h" #include "cc/CCLayerTreeHostImpl.h" -#include "cc/CCMainThread.h" #include "cc/CCProxy.h" #include @@ -59,36 +58,28 @@ private: explicit CCThreadProxy(CCLayerTreeHost*); // Called on CCMainThread - void beginFrameAndCommit(int sequenceNumber, double frameBeginTime); + void beginFrameAndCommit(double frameBeginTime); // Called on CCThread - PassOwnPtr createBeginFrameAndCommitTaskOnCCThread(); - void createBeginFrameAndCommitTaskOnCCThread(CCCompletionEvent*, CCMainThread::Task**); + void beginFrameAndCommitOnCCThread(); void commitOnCCThread(CCCompletionEvent*); - void drawLayersAndPresentOnCCThread(); void drawLayersOnCCThread(); - void drawLayersAndReadbackOnCCThread(CCCompletionEvent*, bool* success, void* pixels, const IntRect&); - void finishAllRenderingOnCCThread(CCCompletionEvent*); void initializeImplOnCCThread(CCCompletionEvent*); void initializeLayerRendererOnCCThread(GraphicsContext3D*, CCCompletionEvent*, bool* initializeSucceeded, LayerRendererCapabilities*); void setNeedsCommitOnCCThread(); - void updateSchedulerStateOnCCThread(bool commitRequested, bool redrawRequested); + void setNeedsCommitAndRedrawOnCCThread(); + void setNeedsRedrawOnCCThread(); void layerTreeHostClosedOnCCThread(CCCompletionEvent*); - void scheduleDrawTaskOnCCThread(); + + // Used on main-thread only. + bool m_commitPending; // Accessed on main thread only. - bool m_commitRequested; CCLayerTreeHost* m_layerTreeHost; LayerRendererCapabilities m_layerRendererCapabilitiesMainThreadCopy; - bool m_started; - int m_lastExecutedBeginFrameAndCommitSequenceNumber; - // Used on the CCThread only + // Used on the CCThread, but checked on main thread during initialization/shutdown. OwnPtr m_layerTreeHostImpl; - int m_numBeginFrameAndCommitsIssuedOnCCThread; - bool m_beginFrameAndCommitPendingOnCCThread; - bool m_drawTaskPostedOnCCThread; - bool m_redrawRequestedOnCCThread; }; } diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog index d34db66..6b4e285 100644 --- a/Source/WebKit/chromium/ChangeLog +++ b/Source/WebKit/chromium/ChangeLog @@ -1,3 +1,17 @@ +2011-09-22 James Robinson + + Unreviewed, rolling out r95699. + http://trac.webkit.org/changeset/95699 + https://bugs.webkit.org/show_bug.cgi?id=67417 + + Makes many chromium compositor tests crash + + * tests/CCLayerTreeHostTest.cpp: + (WTF::CCLayerTreeHostTest::doBeginTest): + (WTF::TEST_F): + * tests/TreeSynchronizerTest.cpp: + (WebCore::TEST): + 2011-09-22 Nat Duca [chromium] Make CCThreadProxy draw diff --git a/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp b/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp index 64c2161..ff4ffbe 100644 --- a/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp +++ b/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp @@ -170,10 +170,6 @@ public: { } -#if !USE(THREADED_COMPOSITING) - virtual void scheduleComposite() { } -#endif - private: explicit MockLayerTreeHostClient(TestHooks* testHooks) : m_testHooks(testHooks) { } @@ -190,11 +186,8 @@ private: // // The test continues until someone calls endTest. endTest can be called on any thread, but be aware that // ending the test is an asynchronous process. -class CCLayerTreeHostTest : public testing::TestWithParam, TestHooks { +class CCLayerTreeHostTest : public testing::Test, TestHooks { public: - virtual void SetUp() - { - } virtual void afterTest() = 0; virtual void beginTest() = 0; @@ -294,8 +287,10 @@ void CCLayerTreeHostTest::doBeginTest() m_running = true; m_client = MockLayerTreeHostClient::create(this); + CCSettings settings; + settings.enableCompositorThread = true; RefPtr rootLayer; - m_layerTreeHost = MockLayerTreeHost::create(this, m_client.get(), rootLayer, GetParam()); + m_layerTreeHost = MockLayerTreeHost::create(this, m_client.get(), rootLayer, settings); ASSERT(m_layerTreeHost); m_beginning = true; @@ -304,11 +299,6 @@ void CCLayerTreeHostTest::doBeginTest() if (m_endWhenBeginReturns) onEndTest(static_cast(this)); } -INSTANTIATE_TEST_CASE_P( - ProxyTests, CCLayerTreeHostTest, - testing::Values( - CCSettings(false, false, false, false, false), - CCSettings(false, false, true, false, false))); void CCLayerTreeHostTest::endTest() { @@ -555,10 +545,9 @@ private: }; TEST_F(CCLayerTreeHostTestSetNeedsRedraw, run) { - CCSettings setings; runTest(); } } // namespace -#endif +#endif // USE(THREADED_COMPOSITING) diff --git a/Source/WebKit/chromium/tests/TreeSynchronizerTest.cpp b/Source/WebKit/chromium/tests/TreeSynchronizerTest.cpp index c37de73..8adf476 100644 --- a/Source/WebKit/chromium/tests/TreeSynchronizerTest.cpp +++ b/Source/WebKit/chromium/tests/TreeSynchronizerTest.cpp @@ -28,29 +28,12 @@ #include "LayerChromium.h" #include "cc/CCLayerImpl.h" -#include "cc/CCProxy.h" #include using namespace WebCore; namespace { -class ScopedSetImplThread { -public: - ScopedSetImplThread() - { -#ifndef NDEBUG - CCProxy::setImplThread(true); -#endif - } - ~ScopedSetImplThread() - { -#ifndef NDEBUG - CCProxy::setImplThread(false); -#endif - } -}; - class MockCCLayerImpl : public CCLayerImpl { public: static PassRefPtr create(int layerId) @@ -133,7 +116,6 @@ void expectTreesAreIdentical(LayerChromium* layer, CCLayerImpl* ccLayer) // Constructs a very simple tree and synchronizes it without trying to reuse any preexisting layers. TEST(TreeSynchronizerTest, syncSimpleTreeFromEmpty) { - ScopedSetImplThread impl; RefPtr layerTreeRoot = LayerChromium::create(0); layerTreeRoot->addChild(LayerChromium::create(0)); layerTreeRoot->addChild(LayerChromium::create(0)); @@ -146,7 +128,6 @@ TEST(TreeSynchronizerTest, syncSimpleTreeFromEmpty) // Constructs a very simple tree and synchronizes it attempting to reuse some layers TEST(TreeSynchronizerTest, syncSimpleTreeReusingLayers) { - ScopedSetImplThread impl; Vector ccLayerDestructionList; RefPtr layerTreeRoot = MockLayerChromium::create(&ccLayerDestructionList); @@ -172,7 +153,6 @@ TEST(TreeSynchronizerTest, syncSimpleTreeReusingLayers) TEST(TreeSynchronizerTest, syncSimpleTreeAndProperties) { - ScopedSetImplThread impl; RefPtr layerTreeRoot = LayerChromium::create(0); layerTreeRoot->addChild(LayerChromium::create(0)); layerTreeRoot->addChild(LayerChromium::create(0)); @@ -204,7 +184,6 @@ TEST(TreeSynchronizerTest, syncSimpleTreeAndProperties) TEST(TreeSynchronizerTest, reuseCCLayersAfterStructuralChange) { - ScopedSetImplThread impl; Vector ccLayerDestructionList; // Set up a tree with this sort of structure: @@ -251,7 +230,6 @@ TEST(TreeSynchronizerTest, reuseCCLayersAfterStructuralChange) // Constructs a very simple tree, synchronizes it, then synchronizes to a totally new tree. All layers from the old tree should be deleted. TEST(TreeSynchronizerTest, syncSimpleTreeThenDestroy) { - ScopedSetImplThread impl; Vector ccLayerDestructionList; RefPtr oldLayerTreeRoot = MockLayerChromium::create(&ccLayerDestructionList); @@ -282,7 +260,6 @@ TEST(TreeSynchronizerTest, syncSimpleTreeThenDestroy) // Constructs+syncs a tree with mask, replica, and replica mask layers. TEST(TreeSynchronizerTest, syncMaskReplicaAndReplicaMaskLayers) { - ScopedSetImplThread impl; RefPtr layerTreeRoot = LayerChromium::create(0); layerTreeRoot->addChild(LayerChromium::create(0)); layerTreeRoot->addChild(LayerChromium::create(0)); -- 2.7.4