Changeset 100055 in webkit

Timestamp:

Nov 11, 2011 6:05:39 PM (13 years ago)

Author:

commit-queue@webkit.org

Message:

[chromium] Fix CCThreadProxy::setVisible
​https://bugs.webkit.org/show_bug.cgi?id=71903

The behavior is different depending on whether we're showing
or hiding the compositor. This patch fixes both code paths.

Patch by Iain Merrick <​husky@google.com> on 2011-11-11
Reviewed by James Robinson.

• platform/graphics/chromium/cc/CCCompletionEvent.h:

(WebCore::CCCompletionEvent::CCCompletionEvent):
(WebCore::CCCompletionEvent::~CCCompletionEvent):
(WebCore::CCCompletionEvent::wait):
(WebCore::CCCompletionEvent::signal):

• platform/graphics/chromium/cc/CCThreadProxy.cpp:

(WebCore::CCThreadProxy::setVisible):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/platform/graphics/chromium/cc/CCCompletionEvent.h (modified) (5 diffs)
• WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp (modified) (2 diffs)
• WebCore/platform/graphics/chromium/cc/CCThreadProxy.h (modified) (1 diff)
• WebKit/chromium/tests/CCLayerTreeHostTest.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-11-11  Iain Merrick  <husky@google.com>
+
+        [chromium] Fix CCThreadProxy::setVisible
+        https://bugs.webkit.org/show_bug.cgi?id=71903
+
+        The behavior is different depending on whether we're showing
+        or hiding the compositor. This patch fixes both code paths.
+
+        Reviewed by James Robinson.
+
+        * platform/graphics/chromium/cc/CCCompletionEvent.h:
+        (WebCore::CCCompletionEvent::CCCompletionEvent):
+        (WebCore::CCCompletionEvent::~CCCompletionEvent):
+        (WebCore::CCCompletionEvent::wait):
+        (WebCore::CCCompletionEvent::signal):
+        * platform/graphics/chromium/cc/CCThreadProxy.cpp:
+        (WebCore::CCThreadProxy::setVisible):
+
 2011-11-11  Iain Merrick  <husky@google.com>
 

trunk/Source/WebCore/platform/graphics/chromium/cc/CCCompletionEvent.h

@@ -31 +31 @@
 
 // Used for making blocking calls from one thread to another. Use only when
-// absolutely certain that doing-so will not lead to a livelock.
+// absolutely certain that doing-so will not lead to a deadlock.
 //
 // It is safe to destroy this object as soon as wait() returns.
@@ -38 +38 @@
     CCCompletionEvent()
     {
+#ifndef NDEBUG
+        m_waited = false;
+        m_signaled = false;
+#endif
         m_mutex.lock();
     }
@@ -44 +48 @@
     {
         m_mutex.unlock();
+        ASSERT(m_waited);
+        ASSERT(m_signaled);
     }
 
     void wait()
     {
+        ASSERT(!m_waited);
+#ifndef NDEBUG
+        m_waited = true;
+#endif
         m_condition.wait(m_mutex);
     }
@@ -54 +64 @@
     {
         MutexLocker lock(m_mutex);
+        ASSERT(!m_signaled);
+#ifndef NDEBUG
+        m_signaled = true;
+#endif
         m_condition.signal();
     }
@@ -60 +74 @@
     Mutex m_mutex;
     ThreadCondition m_condition;
+#ifndef NDEBUG
+    // Used to assert that wait() and signal() are each called exactly once.
+    bool m_waited;
+    bool m_signaled;
+#endif
 };
 

trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp

@@ -250 +250 @@
 {
     ASSERT(isMainThread());
-    if (!visible) {
-        CCCompletionEvent completion;
-        s_ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::didBecomeInvisibleOnImplThread, AllowCrossThreadAccess(&completion)));
-        return;
-    }
-    setNeedsRedraw();
-}
-
-void CCThreadProxy::didBecomeInvisibleOnImplThread(CCCompletionEvent* completion)
-{
-    ASSERT(isImplThread());
-    m_layerTreeHost->didBecomeInvisibleOnImplThread(m_layerTreeHostImpl.get());
-    m_schedulerOnImplThread->setVisible(false);
-    m_layerTreeHostImpl->setVisible(false);
+    CCCompletionEvent completion;
+    s_ccThread->postTask(createCCThreadTask(this, &CCThreadProxy::setVisibleOnImplThread, AllowCrossThreadAccess(&completion), visible));
+    completion.wait();
+}
+
+void CCThreadProxy::setVisibleOnImplThread(CCCompletionEvent* completion, bool visible)
+{
+    ASSERT(isImplThread());
+    if (!visible)
+        m_layerTreeHost->didBecomeInvisibleOnImplThread(m_layerTreeHostImpl.get());
+    else
+        m_schedulerOnImplThread->setNeedsRedraw();
+    m_schedulerOnImplThread->setVisible(visible);
+    m_layerTreeHostImpl->setVisible(visible);
     completion->signal();
 }
@@ -448 +448 @@
     m_layerTreeHost->beginCommitOnImplThread(m_layerTreeHostImpl.get());
     CCTextureUpdater updater(m_layerTreeHostImpl->contentsTextureAllocator());
-    m_layerTreeHostImpl->setVisible(m_layerTreeHost->visible());
-    m_schedulerOnImplThread->setVisible(m_layerTreeHostImpl->visible());
     m_layerTreeHost->finishCommitOnImplThread(m_layerTreeHostImpl.get());
 

trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h

@@ -100 +100 @@
     void initializeLayerRendererOnImplThread(GraphicsContext3D*, CCCompletionEvent*, bool* initializeSucceeded, LayerRendererCapabilities*, int* compositorIdentifier);
     void setNeedsAnimateOnImplThread();
-    void didBecomeInvisibleOnImplThread(CCCompletionEvent*);
+    void setVisibleOnImplThread(CCCompletionEvent*, bool visible);
     void layerTreeHostClosedOnImplThread(CCCompletionEvent*);
 

trunk/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp

@@ -210 +210 @@
     }
 
+    void postSetVisibleToMainThread(bool visible)
+    {
+        callOnMainThread(visible ? CCLayerTreeHostTest::dispatchSetVisible : CCLayerTreeHostTest::dispatchSetInvisible, this);
+    }
+
     void timeout()
     {
@@ -270 +275 @@
       if (test->m_layerTreeHost)
           test->m_layerTreeHost->setNeedsRedraw();
+    }
+
+    static void dispatchSetVisible(void* self)
+    {
+      ASSERT(isMainThread());
+      CCLayerTreeHostTest* test = static_cast<CCLayerTreeHostTest*>(self);
+      ASSERT(test);
+      if (test->m_layerTreeHost)
+          test->m_layerTreeHost->setVisible(true);
+    }
+
+    static void dispatchSetInvisible(void* self)
+    {
+      ASSERT(isMainThread());
+      CCLayerTreeHostTest* test = static_cast<CCLayerTreeHostTest*>(self);
+      ASSERT(test);
+      if (test->m_layerTreeHost)
+          test->m_layerTreeHost->setVisible(false);
     }
 
@@ -765 +788 @@
 }
 
+class CCLayerTreeHostTestSetVisible : public CCLayerTreeHostTest {
+public:
+
+    CCLayerTreeHostTestSetVisible()
+        : m_numCommits(0)
+        , m_numDraws(0)
+    {
+    }
+
+    virtual void beginTest()
+    {
+        postSetVisibleToMainThread(false);
+        postSetNeedsRedrawToMainThread(); // This is suppressed while we're invisible.
+        postSetVisibleToMainThread(true); // Triggers the redraw.
+    }
+
+    virtual void drawLayersOnCCThread(CCLayerTreeHostImpl* impl)
+    {
+        EXPECT_TRUE(impl->visible());
+        ++m_numDraws;
+        endTest();
+    }
+
+    virtual void afterTest()
+    {
+        EXPECT_EQ(1, m_numDraws);
+    }
+
+private:
+    int m_numCommits;
+    int m_numDraws;
+};
+
+TEST_F(CCLayerTreeHostTestSetVisible, runMultiThread)
+{
+    runTest(true);
+}
+
 } // namespace