Aura: Resize locks with --ui-enable-threaded-compositing

cc/thread_proxy.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 
 #include "cc/thread_proxy.h"
 
 #include "CCDelayBasedTimeSource.h"
 #include "CCDrawQuad.h"
@@ skipping 35 matching lines @@
     , m_texturesAcquired(true)
     , m_inCompositeAndReadback(false)
     , m_mainThreadProxy(ScopedThreadProxy::create(Proxy::mainThread()))
     , m_beginFrameCompletionEventOnImplThread(0)
     , m_readbackRequestOnImplThread(0)
     , m_commitCompletionEventOnImplThread(0)
     , m_textureAcquisitionCompletionEventOnImplThread(0)
     , m_nextFrameIsNewlyCommittedFrameOnImplThread(false)
     , m_renderVSyncEnabled(layerTreeHost->settings().renderVSyncEnabled)
     , m_totalCommitCount(0)
+    , m_deferCommits(false)
 {
     TRACE_EVENT0("cc", "ThreadProxy::ThreadProxy");
     DCHECK(isMainThread());
 }
 
 ThreadProxy::~ThreadProxy()
 {
     TRACE_EVENT0("cc", "ThreadProxy::~ThreadProxy");
     DCHECK(isMainThread());
     DCHECK(!m_started);
 }
 
 bool ThreadProxy::compositeAndReadback(void *pixels, const IntRect& rect)
 {
     TRACE_EVENT0("cc", "ThreadPRoxy::compositeAndReadback");
     DCHECK(isMainThread());
     DCHECK(m_layerTreeHost);
+    DCHECK(!m_deferCommits);
 
     if (!m_layerTreeHost->initializeRendererIfNeeded()) {
         TRACE_EVENT0("cc", "compositeAndReadback_EarlyOut_LR_Uninitialized");
         return false;
     }
 
 
     // Perform a synchronous commit.
     {
         DebugScopedSetMainThreadBlocked mainThreadBlocked;
@@ skipping 41 matching lines @@
 void ThreadProxy::requestStartPageScaleAnimationOnImplThread(IntSize targetPosition, bool useAnchor, float scale, double duration)
 {
     DCHECK(Proxy::isImplThread());
     if (m_layerTreeHostImpl.get())
         m_layerTreeHostImpl->startPageScaleAnimation(targetPosition, useAnchor, scale, monotonicallyIncreasingTime(), duration);
 }
 
 void ThreadProxy::finishAllRendering()
 {
     DCHECK(Proxy::isMainThread());
+    DCHECK(!m_deferCommits);
 
     // Make sure all GL drawing is finished on the impl thread.
     DebugScopedSetMainThreadBlocked mainThreadBlocked;
     CompletionEvent completion;
     Proxy::implThread()->postTask(createThreadTask(this, &ThreadProxy::finishAllRenderingOnImplThread, &completion));
     completion.wait();
 }
 
 bool ThreadProxy::isStarted() const
 {
@@ skipping 222 matching lines @@
 }
 
 void ThreadProxy::setNeedsRedraw()
 {
     DCHECK(isMainThread());
     TRACE_EVENT0("cc", "ThreadProxy::setNeedsRedraw");
     Proxy::implThread()->postTask(createThreadTask(this, &ThreadProxy::setFullRootLayerDamageOnImplThread));
     Proxy::implThread()->postTask(createThreadTask(this, &ThreadProxy::setNeedsRedrawOnImplThread));
 }
 
+void ThreadProxy::setDeferCommits(bool deferCommits)
+{
+    DCHECK(isMainThread());
+    DCHECK_NE(m_deferCommits, deferCommits);
+    m_deferCommits = deferCommits;
+
+    if (m_deferCommits)
+        TRACE_EVENT_ASYNC_BEGIN0("cc", "ThreadProxy::setDeferCommits", this);
+    else
+        TRACE_EVENT_ASYNC_END0("cc", "ThreadProxy::setDeferCommits", this);
+
+    if (!m_deferCommits && m_pendingBeginFrameRequest)

[piman, 2012/10/25 00:14:08]

There is a race condition here.
m_pendingBeginFrameRequest is set by scheduledActionBeginFrame on the impl
thread and then the struct is filled up - without a lock. However it's not meant
to be seen by the main thread only after the beginFrame task is posted - adding
an explicit coherency-ensuring ordering relationship
scheduledActionBeginFrame->postTask->runTask->beginFrame.

You can construct a case where the impl thread has set
m_pendingBeginFrameRequest, then the main thread calls setDeferCommits(false)
before the task is posted, and then executes beginFrame before the struct is
filled up (and visible by the main thread) causing inconsistencies.

You can probably solve it by adding a bool, that you would set in beginFrame
when commits are deferred, and here post a task (and reset it) when it is set.
That would guarantee that there's only a single beginFrame "in flight", and so
it would never see inconsistent state.

[jonathan.backer, 2012/10/25 15:19:04]

Nice catch! That would have been impossible to debug.

+        m_mainThreadProxy->postTask(createThreadTask(this, &ThreadProxy::beginFrame));
+}
+
 bool ThreadProxy::commitRequested() const
 {
     DCHECK(isMainThread());
     return m_commitRequested;
 }
 
 void ThreadProxy::setNeedsRedrawOnImplThread()
 {
     DCHECK(isImplThread());
     TRACE_EVENT0("cc", "ThreadProxy::setNeedsRedrawOnImplThread");
@@ skipping 94 matching lines @@
     }
 }
 
 void ThreadProxy::beginFrame()
 {
     TRACE_EVENT0("cc", "ThreadProxy::beginFrame");
     DCHECK(isMainThread());
     if (!m_layerTreeHost)
         return;
 
+    if (m_deferCommits) {
+        TRACE_EVENT0("cc", "EarlyOut_DeferCommits");
+        m_layerTreeHost->didDeferCommit();
+        return;
+    }
+
     if (!m_pendingBeginFrameRequest) {
         TRACE_EVENT0("cc", "EarlyOut_StaleBeginFrameMessage");
         return;
     }
 
     if (m_layerTreeHost->needsSharedContext() && !WebSharedGraphicsContext3D::haveCompositorThreadContext())
         WebSharedGraphicsContext3D::createCompositorThreadContext();
 
     scoped_ptr<BeginFrameAndCommitState> request(m_pendingBeginFrameRequest.Pass());
 
@@ skipping 448 matching lines @@
 ThreadProxy::BeginFrameAndCommitState::BeginFrameAndCommitState()
     : monotonicFrameBeginTime(0)
 {
 }
 
 ThreadProxy::BeginFrameAndCommitState::~BeginFrameAndCommitState()
 {
 }
 
 }  // namespace cc