Title: [116833] trunk/Source/WebCore
Revision
116833
Author
[email protected]
Date
2012-05-11 19:04:46 -0700 (Fri, 11 May 2012)

Log Message

[chromium] Prevent deadlock on CCVideoLayerImpl destruction
https://bugs.webkit.org/show_bug.cgi?id=86258

Reviewed by James Robinson.

~CCVideoLayerImpl had a common deadlock issue where if it got
destroyed before WebMediaPlayerClientImpl, it would take a lock,
call WebMediaPlayerClientImpl::setVideoFrameProviderClient(0),
which in turn would call CCVideoLayerImpl::stopUsingProvider(),
which would try to take the same lock and would deadlock.

CCVideoLayerImpl is only created and destroyed during tree
synchronization in a commit or during synchronous compositor thread
destruction. In either case, the main thread is blocked, and so no
lock needs to be taken at all.

* platform/graphics/chromium/cc/CCVideoLayerImpl.cpp:
(WebCore::CCVideoLayerImpl::CCVideoLayerImpl):
(WebCore::CCVideoLayerImpl::~CCVideoLayerImpl):
(WebCore::CCVideoLayerImpl::stopUsingProvider):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (116832 => 116833)


--- trunk/Source/WebCore/ChangeLog	2012-05-12 02:01:51 UTC (rev 116832)
+++ trunk/Source/WebCore/ChangeLog	2012-05-12 02:04:46 UTC (rev 116833)
@@ -1,3 +1,26 @@
+2012-05-11  Adrienne Walker  <[email protected]>
+
+        [chromium] Prevent deadlock on CCVideoLayerImpl destruction
+        https://bugs.webkit.org/show_bug.cgi?id=86258
+
+        Reviewed by James Robinson.
+
+        ~CCVideoLayerImpl had a common deadlock issue where if it got
+        destroyed before WebMediaPlayerClientImpl, it would take a lock,
+        call WebMediaPlayerClientImpl::setVideoFrameProviderClient(0),
+        which in turn would call CCVideoLayerImpl::stopUsingProvider(),
+        which would try to take the same lock and would deadlock.
+
+        CCVideoLayerImpl is only created and destroyed during tree
+        synchronization in a commit or during synchronous compositor thread
+        destruction. In either case, the main thread is blocked, and so no
+        lock needs to be taken at all.
+
+        * platform/graphics/chromium/cc/CCVideoLayerImpl.cpp:
+        (WebCore::CCVideoLayerImpl::CCVideoLayerImpl):
+        (WebCore::CCVideoLayerImpl::~CCVideoLayerImpl):
+        (WebCore::CCVideoLayerImpl::stopUsingProvider):
+
 2012-05-11  Jeffrey Pfau  <[email protected]>
 
         REGRESSION (r114170): Scroll areas in nested frames improperly placed when tiled drawing is enabled

Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.cpp (116832 => 116833)


--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.cpp	2012-05-12 02:01:51 UTC (rev 116832)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.cpp	2012-05-12 02:04:46 UTC (rev 116833)
@@ -80,12 +80,17 @@
     , m_frame(0)
 {
     memcpy(m_streamTextureMatrix, flipTransform, sizeof(m_streamTextureMatrix));
-    provider->setVideoFrameProviderClient(this);
+
+    // This only happens during a commit on the compositor thread while the main
+    // thread is blocked. That makes this a thread-safe call to set the video
+    // frame provider client that does not require a lock. The same is true of
+    // the call in the destructor.
+    m_provider->setVideoFrameProviderClient(this);
 }
 
 CCVideoLayerImpl::~CCVideoLayerImpl()
 {
-    MutexLocker locker(m_providerMutex);
+    // See comment in constructor for why this doesn't need a lock.
     if (m_provider) {
         m_provider->setVideoFrameProviderClient(0);
         m_provider = 0;
@@ -96,7 +101,10 @@
 
 void CCVideoLayerImpl::stopUsingProvider()
 {
+    // Block the provider from shutting down until this client is done
+    // using the frame.
     MutexLocker locker(m_providerMutex);
+    ASSERT(!m_frame);
     m_provider = 0;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to