Changeset 128082 in webkit
- Timestamp:
- Sep 10, 2012 11:22:51 AM (12 years ago)
- Location:
- trunk/Source
- Files:
-
- 5 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/Platform/ChangeLog
r128064 r128082 1 2012-09-10 Adrienne Walker <enne@google.com> 2 3 [chromium] Fix deadlock between WebMediaPlayerClientImpl dtor and PutCurrentFrame 4 https://bugs.webkit.org/show_bug.cgi?id=96010 5 6 Reviewed by James Robinson. 7 8 Add some additional clarifying comments. 9 10 * chromium/public/WebVideoFrameProvider.h: 11 (Client): 12 (WebVideoFrameProvider): 13 1 14 2012-09-10 Tommy Widenflycht <tommyw@google.com> 2 15 -
trunk/Source/Platform/chromium/public/WebVideoFrameProvider.h
r114335 r128082 47 47 public: 48 48 // Provider will call this method to tell the client to stop using it. 49 // stopUsingProvider() may be called from any thread. 49 // stopUsingProvider() may be called from any thread. The client should 50 // block until it has putCurrentFrame any outstanding frames. 50 51 virtual void stopUsingProvider() = 0; 51 52 … … 58 59 }; 59 60 60 // May be called from any thread. 61 // May be called from any thread, but there must be some external guarantee 62 // that the provider is not destroyed before this call returns. 61 63 virtual void setVideoFrameProviderClient(Client*) = 0; 62 64 … … 64 66 // Calls to this method should always be followed with a call to putCurrentFrame(). 65 67 // The ownership of the object is not transferred to the caller and 66 // the caller should not free the returned object. 68 // the caller should not free the returned object. Only the current provider 69 // client should call this function. 67 70 virtual WebVideoFrame* getCurrentFrame() = 0; 68 71 // This function releases the lock on the video frame in chromium. It should 69 72 // always be called after getCurrentFrame(). Frames passed into this method 70 // should no longer be referenced after the call is made. 73 // should no longer be referenced after the call is made. Only the current 74 // provider client should call this function. 71 75 virtual void putCurrentFrame(WebVideoFrame*) = 0; 72 76 }; -
trunk/Source/WebKit/chromium/ChangeLog
r128058 r128082 1 2012-09-10 Adrienne Walker <enne@google.com> 2 3 [chromium] Fix deadlock between WebMediaPlayerClientImpl dtor and PutCurrentFrame 4 https://bugs.webkit.org/show_bug.cgi?id=96010 5 6 Reviewed by James Robinson. 7 8 The key fix here is that the destructor no longer has a mutex. 9 The m_compositingMutex was supposedly protecting races between 10 ~WebMediaPlayerClientImpl and setVideoFrameProviderClient. The 11 former is only called from the main thread and the latter is called 12 from the compositor thread only when the main thread is blocked (and 13 it already asserts that this is the case). 14 15 In addition, the m_providerMutex in CCVideoLayerImpl prevents the 16 destruction of WebMediaPlayerClientImpl, thus keeping the frame 17 acquired via getCurrentFrame alive until putCurrentFrame is called. 18 These functions are only called by the client, and comments are added 19 to the interface to better document this. 20 21 To prevent a race between load() and getCurrentFrame/putCurrentFrame 22 (which are called from different threads) a new m_webMediaPlayerMutex 23 to replace part of what the old m_compositingMutex was doing. 24 25 * src/WebMediaPlayerClientImpl.cpp: 26 (WebKit::WebMediaPlayerClientImpl::~WebMediaPlayerClientImpl): 27 (WebKit::WebMediaPlayerClientImpl::load): 28 (WebKit::WebMediaPlayerClientImpl::loadInternal): 29 (WebKit::WebMediaPlayerClientImpl::setVideoFrameProviderClient): 30 (WebKit::WebMediaPlayerClientImpl::getCurrentFrame): 31 (WebKit::WebMediaPlayerClientImpl::putCurrentFrame): 32 * src/WebMediaPlayerClientImpl.h: 33 (WebMediaPlayerClientImpl): 34 1 35 2012-09-10 Rick Byers <rbyers@chromium.org> 2 36 -
trunk/Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp
r127952 r128082 93 93 { 94 94 #if USE(ACCELERATED_COMPOSITING) 95 MutexLocker locker(m_compositingMutex);96 95 if (m_videoFrameProviderClient) 97 96 m_videoFrameProviderClient->stopUsingProvider(); 97 // No need for a lock here, as getCurrentFrame/putCurrentFrame can't be 98 // called now that the client is no longer using this provider. Also, load() 99 // and this destructor are called from the same thread. 98 100 if (m_webMediaPlayer) 99 101 m_webMediaPlayer->setStreamTextureClient(0); … … 313 315 m_url = url; 314 316 317 MutexLocker locker(m_webMediaPlayerMutex); 315 318 if (m_preload == MediaPlayer::None) { 316 MutexLocker locker(m_compositingMutex);317 319 #if ENABLE(WEB_AUDIO) 318 320 m_audioSourceProvider.wrap(0); // Clear weak reference to m_webMediaPlayer's WebAudioSourceProvider. … … 326 328 void WebMediaPlayerClientImpl::loadInternal() 327 329 { 328 MutexLocker locker(m_compositingMutex);329 330 #if ENABLE(WEB_AUDIO) 330 331 m_audioSourceProvider.wrap(0); // Clear weak reference to m_webMediaPlayer's WebAudioSourceProvider. … … 767 768 void WebMediaPlayerClientImpl::setVideoFrameProviderClient(WebVideoFrameProvider::Client* client) 768 769 { 769 MutexLocker locker(m_ compositingMutex);770 MutexLocker locker(m_webMediaPlayerMutex); 770 771 if (m_videoFrameProviderClient) 771 772 m_videoFrameProviderClient->stopUsingProvider(); … … 777 778 WebVideoFrame* WebMediaPlayerClientImpl::getCurrentFrame() 778 779 { 779 MutexLocker locker(m_compositingMutex); 780 // This function is called only by the client. 781 MutexLocker locker(m_webMediaPlayerMutex); 780 782 ASSERT(!m_currentVideoFrame); 783 ASSERT(m_videoFrameProviderClient); 781 784 if (m_webMediaPlayer) 782 785 m_currentVideoFrame = m_webMediaPlayer->getCurrentFrame(); … … 786 789 void WebMediaPlayerClientImpl::putCurrentFrame(WebVideoFrame* videoFrame) 787 790 { 788 MutexLocker locker(m_compositingMutex); 791 // This function is called only by the client. 792 MutexLocker locker(m_webMediaPlayerMutex); 789 793 ASSERT(videoFrame == m_currentVideoFrame); 794 ASSERT(m_videoFrameProviderClient); 790 795 if (!videoFrame) 791 796 return; -
trunk/Source/WebKit/chromium/src/WebMediaPlayerClientImpl.h
r127675 r128082 198 198 #endif 199 199 200 Mutex m_ compositingMutex; // Guards m_currentVideoFrame and m_videoFrameProviderClient.200 Mutex m_webMediaPlayerMutex; // Guards the m_webMediaPlayer 201 201 WebCore::MediaPlayer* m_mediaPlayer; 202 202 OwnPtr<WebMediaPlayer> m_webMediaPlayer;
Note: See TracChangeset
for help on using the changeset viewer.