- Revision
- 229491
- Author
- [email protected]
- Date
- 2018-03-09 16:38:02 -0800 (Fri, 09 Mar 2018)
Log Message
RealtimeOutgoingAudioSource and RealtimeOutgoingVideoSource should be destroyed on the main thread
https://bugs.webkit.org/show_bug.cgi?id=183483
<rdar://problem/38214152>
Reviewed by Eric Carlson.
When dereferencing from libwebrtc code path, schedule a call to deref on main thread.
WebCore dereferencing is happening in the main thread so this guarantees destruction on the main thread.
Covered by updated mock libwebrtc peer connection backend.
We make mock senders to keep a reference to their source which are RealtimeOutgoingXXSource.
We then make mock peer connection backend to free the mock senders in a background thread.
* Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
(WebCore::LibWebRTCPeerConnectionBackend::doStop):
* platform/mediastream/RealtimeOutgoingAudioSource.cpp:
(WebCore::RealtimeOutgoingAudioSource::stop):
* platform/mediastream/RealtimeOutgoingAudioSource.h:
* platform/mediastream/RealtimeOutgoingVideoSource.cpp:
(WebCore::RealtimeOutgoingVideoSource::stop):
* platform/mediastream/RealtimeOutgoingVideoSource.h:
* testing/MockLibWebRTCPeerConnection.cpp:
(WebCore::ThreadKeeper::create):
(WebCore::ThreadKeeper::setThread):
(WebCore::MockLibWebRTCPeerConnection::~MockLibWebRTCPeerConnection):
* testing/MockLibWebRTCPeerConnection.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (229490 => 229491)
--- trunk/Source/WebCore/ChangeLog 2018-03-10 00:12:35 UTC (rev 229490)
+++ trunk/Source/WebCore/ChangeLog 2018-03-10 00:38:02 UTC (rev 229491)
@@ -1,3 +1,32 @@
+2018-03-09 Youenn Fablet <[email protected]>
+
+ RealtimeOutgoingAudioSource and RealtimeOutgoingVideoSource should be destroyed on the main thread
+ https://bugs.webkit.org/show_bug.cgi?id=183483
+ <rdar://problem/38214152>
+
+ Reviewed by Eric Carlson.
+
+ When dereferencing from libwebrtc code path, schedule a call to deref on main thread.
+ WebCore dereferencing is happening in the main thread so this guarantees destruction on the main thread.
+
+ Covered by updated mock libwebrtc peer connection backend.
+ We make mock senders to keep a reference to their source which are RealtimeOutgoingXXSource.
+ We then make mock peer connection backend to free the mock senders in a background thread.
+
+ * Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
+ (WebCore::LibWebRTCPeerConnectionBackend::doStop):
+ * platform/mediastream/RealtimeOutgoingAudioSource.cpp:
+ (WebCore::RealtimeOutgoingAudioSource::stop):
+ * platform/mediastream/RealtimeOutgoingAudioSource.h:
+ * platform/mediastream/RealtimeOutgoingVideoSource.cpp:
+ (WebCore::RealtimeOutgoingVideoSource::stop):
+ * platform/mediastream/RealtimeOutgoingVideoSource.h:
+ * testing/MockLibWebRTCPeerConnection.cpp:
+ (WebCore::ThreadKeeper::create):
+ (WebCore::ThreadKeeper::setThread):
+ (WebCore::MockLibWebRTCPeerConnection::~MockLibWebRTCPeerConnection):
+ * testing/MockLibWebRTCPeerConnection.h:
+
2018-03-09 Jer Noble <[email protected]>
Unconditionalize more methods in VideoFullscreenInterface (and related classes)
Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp (229490 => 229491)
--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp 2018-03-10 00:12:35 UTC (rev 229490)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp 2018-03-10 00:38:02 UTC (rev 229491)
@@ -192,6 +192,8 @@
m_endpoint->stop();
+ m_audioSources.clear();
+ m_videoSources.clear();
m_statsPromises.clear();
m_remoteStreams.clear();
m_pendingReceivers.clear();
Modified: trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.cpp (229490 => 229491)
--- trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.cpp 2018-03-10 00:12:35 UTC (rev 229490)
+++ trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.cpp 2018-03-10 00:38:02 UTC (rev 229491)
@@ -63,6 +63,7 @@
void RealtimeOutgoingAudioSource::stop()
{
+ ASSERT(isMainThread());
m_silenceAudioTimer.stop();
m_audioSource->removeObserver(*this);
}
Modified: trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.h (229490 => 229491)
--- trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.h 2018-03-10 00:12:35 UTC (rev 229490)
+++ trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.h 2018-03-10 00:38:02 UTC (rev 229491)
@@ -73,8 +73,10 @@
void AddRef() const final { ref(); }
rtc::RefCountReleaseStatus Release() const final
{
- deref();
- return refCount() ? rtc::RefCountReleaseStatus::kDroppedLastRef : rtc::RefCountReleaseStatus::kOtherRefsRemained;
+ callOnMainThread([this] {
+ deref();
+ });
+ return rtc::RefCountReleaseStatus::kOtherRefsRemained;
}
SourceState state() const final { return kLive; }
Modified: trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp (229490 => 229491)
--- trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp 2018-03-10 00:12:35 UTC (rev 229490)
+++ trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp 2018-03-10 00:38:02 UTC (rev 229491)
@@ -62,6 +62,7 @@
void RealtimeOutgoingVideoSource::stop()
{
+ ASSERT(isMainThread());
m_videoSource->removeObserver(*this);
m_blackFrameTimer.stop();
m_isStopped = true;
Modified: trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h (229490 => 229491)
--- trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h 2018-03-10 00:12:35 UTC (rev 229490)
+++ trunk/Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h 2018-03-10 00:38:02 UTC (rev 229491)
@@ -54,8 +54,10 @@
void AddRef() const final { ref(); }
rtc::RefCountReleaseStatus Release() const final
{
- deref();
- return refCount() ? rtc::RefCountReleaseStatus::kDroppedLastRef : rtc::RefCountReleaseStatus::kOtherRefsRemained;
+ callOnMainThread([this] {
+ deref();
+ });
+ return rtc::RefCountReleaseStatus::kOtherRefsRemained;
}
void setApplyRotation(bool shouldApplyRotation) { m_shouldApplyRotation = shouldApplyRotation; }
Modified: trunk/Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp (229490 => 229491)
--- trunk/Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp 2018-03-10 00:12:35 UTC (rev 229490)
+++ trunk/Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp 2018-03-10 00:38:02 UTC (rev 229491)
@@ -33,6 +33,7 @@
#include <wtf/Function.h>
#include <wtf/MainThread.h>
#include <wtf/NeverDestroyed.h>
+#include <wtf/Threading.h>
namespace WebCore {
@@ -68,6 +69,12 @@
provider->setPeerConnectionFactory(MockLibWebRTCPeerConnectionFactory::create(String(testCase)));
}
+MockLibWebRTCPeerConnection::~MockLibWebRTCPeerConnection()
+{
+ // Free senders in a different thread like an actual peer connection would probably do.
+ Thread::create("MockLibWebRTCPeerConnection thread", [senders = WTFMove(m_senders)] { });
+}
+
class MockLibWebRTCPeerConnectionForIceCandidates : public MockLibWebRTCPeerConnection {
public:
explicit MockLibWebRTCPeerConnectionForIceCandidates(webrtc::PeerConnectionObserver& observer) : MockLibWebRTCPeerConnection(observer) { }
Modified: trunk/Source/WebCore/testing/MockLibWebRTCPeerConnection.h (229490 => 229491)
--- trunk/Source/WebCore/testing/MockLibWebRTCPeerConnection.h 2018-03-10 00:12:35 UTC (rev 229490)
+++ trunk/Source/WebCore/testing/MockLibWebRTCPeerConnection.h 2018-03-10 00:38:02 UTC (rev 229491)
@@ -41,7 +41,7 @@
class MockLibWebRTCPeerConnection : public webrtc::PeerConnectionInterface {
public:
- virtual ~MockLibWebRTCPeerConnection() = default;
+ ~MockLibWebRTCPeerConnection();
protected:
explicit MockLibWebRTCPeerConnection(webrtc::PeerConnectionObserver& observer) : m_observer(observer) { }
@@ -151,7 +151,7 @@
bool m_enabled { true };
std::string m_id;
- webrtc::AudioSourceInterface* m_source { nullptr };
+ rtc::scoped_refptr<webrtc::AudioSourceInterface> m_source;
};
class MockLibWebRTCVideoTrack : public webrtc::VideoTrackInterface {
@@ -173,7 +173,7 @@
bool m_enabled;
std::string m_id;
- webrtc::VideoTrackSourceInterface* m_source { nullptr };
+ rtc::scoped_refptr<webrtc::VideoTrackSourceInterface> m_source;
};
class MockLibWebRTCDataChannel : public webrtc::DataChannelInterface {
@@ -210,8 +210,11 @@
class MockRtpSender : public webrtc::RtpSenderInterface {
public:
MockRtpSender(rtc::scoped_refptr<webrtc::MediaStreamTrackInterface>&& track) : m_track(WTFMove(track)) { }
-
- bool SetTrack(webrtc::MediaStreamTrackInterface*) final { return false; }
+ bool SetTrack(webrtc::MediaStreamTrackInterface* track) final
+ {
+ m_track = track;
+ return true;
+ }
rtc::scoped_refptr<webrtc::MediaStreamTrackInterface> track() const final { return m_track; }
uint32_t ssrc() const { return 0; }
