Bug 1048261 - Safe dispatch from DTLS connect to PeerConnectionImpl. r=jesup, r=bwc, a=abillings
authorMartin Thomson <martin.thomson@gmail.com>
Mon, 04 Aug 2014 16:31:45 -0400
changeset 208248 adb28e85421f
parent 208247 c2fc1e357ca0
child 208249 22589028e561
push id3788
push userryanvm@gmail.com
push date2014-08-06 15:01 +0000
treeherdermozilla-beta@adb28e85421f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup, bwc, abillings
bugs1048261
milestone32.0
Bug 1048261 - Safe dispatch from DTLS connect to PeerConnectionImpl. r=jesup, r=bwc, a=abillings
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
@@ -133,16 +133,17 @@ PeerConnectionImpl* PeerConnectionImpl::
   CSFLogDebug(logTag, "Created PeerConnection: %p", pc);
 
   return pc;
 }
 
 
 PeerConnectionMedia::PeerConnectionMedia(PeerConnectionImpl *parent)
     : mParent(parent),
+      mParentHandle(parent->GetHandle()),
       mIceCtx(nullptr),
       mDNSResolver(new mozilla::NrIceResolver()),
       mMainThread(mParent->GetMainThread()),
       mSTSThread(mParent->GetSTSThread()) {}
 
 nsresult PeerConnectionMedia::Init(const std::vector<NrIceStunServer>& stun_servers,
                                    const std::vector<NrIceTurnServer>& turn_servers)
 {
@@ -530,29 +531,42 @@ PeerConnectionMedia::IceStreamReady(NrIc
 {
   MOZ_ASSERT(aStream);
 
   CSFLogDebug(logTag, "%s: %s", __FUNCTION__, aStream->name().c_str());
 }
 
 
 void
-PeerConnectionMedia::DtlsConnected(TransportLayer *dtlsLayer,
-                                   TransportLayer::State state)
+PeerConnectionMedia::DtlsConnected_s(TransportLayer *dtlsLayer,
+                                     TransportLayer::State state)
 {
   dtlsLayer->SignalStateChange.disconnect(this);
 
   bool privacyRequested = false;
   // TODO (Bug 952678) set privacy mode, ask the DTLS layer about that
+  // This has to be a dispatch to a static method, we could be going away
   GetMainThread()->Dispatch(
-    WrapRunnable(mParent, &PeerConnectionImpl::SetDtlsConnected, privacyRequested),
+    WrapRunnableNM(&PeerConnectionMedia::DtlsConnected_m,
+                   mParentHandle, privacyRequested),
     NS_DISPATCH_NORMAL);
 }
 
 void
+PeerConnectionMedia::DtlsConnected_m(const std::string& aParentHandle,
+                                     bool aPrivacyRequested)
+{
+  PeerConnectionWrapper pcWrapper(aParentHandle);
+  PeerConnectionImpl* pc = pcWrapper.impl();
+  if (pc) {
+    pc->SetDtlsConnected(aPrivacyRequested);
+  }
+}
+
+void
 PeerConnectionMedia::AddTransportFlow(int aIndex, bool aRtcp,
                                       const RefPtr<TransportFlow> &aFlow)
 {
   int index_inner = aIndex * 2 + (aRtcp ? 1 : 0);
 
   MOZ_ASSERT(!mTransportFlows[index_inner]);
   mTransportFlows[index_inner] = aFlow;
 
@@ -561,17 +575,17 @@ PeerConnectionMedia::AddTransportFlow(in
     NS_DISPATCH_NORMAL);
 }
 
 void
 PeerConnectionMedia::ConnectDtlsListener_s(const RefPtr<TransportFlow>& aFlow)
 {
   TransportLayer* dtls = aFlow->GetLayer(TransportLayerDtls::ID());
   if (dtls) {
-    dtls->SignalStateChange.connect(this, &PeerConnectionMedia::DtlsConnected);
+    dtls->SignalStateChange.connect(this, &PeerConnectionMedia::DtlsConnected_s);
   }
 }
 
 #ifdef MOZILLA_INTERNAL_API
 /**
  * Tells you if any local streams is isolated.  Obviously, we want all the
  * streams to be isolated equally so that they can all be sent or not.  But we
  * can't make that determination for certain, because stream principals change.
@@ -707,22 +721,23 @@ RefPtr<MediaPipeline> SourceStreamInfo::
   }
 
   return nullptr;
 }
 
 bool RemoteSourceStreamInfo::SetUsingBundle_m(int aLevel, bool decision) {
   ASSERT_ON_THREAD(mParent->GetMainThread());
 
-  RefPtr<MediaPipeline> pipeline(GetPipelineByLevel_m(aLevel));
+  // Avoid adding and dropping an extra ref
+  MediaPipeline *pipeline = GetPipelineByLevel_m(aLevel);
 
   if (pipeline) {
     RUN_ON_THREAD(mParent->GetSTSThread(),
                   WrapRunnable(
-                      pipeline,
+                      RefPtr<MediaPipeline>(pipeline),
                       &MediaPipeline::SetUsingBundle_s,
                       decision
                   ),
                   NS_DISPATCH_NORMAL);
     return true;
   }
   return false;
 }
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
@@ -351,18 +351,20 @@ class PeerConnectionMedia : public sigsl
 
     return mTransportFlows[index_inner];
   }
 
   // Add a transport flow
   void AddTransportFlow(int aIndex, bool aRtcp,
                         const mozilla::RefPtr<mozilla::TransportFlow> &aFlow);
   void ConnectDtlsListener_s(const mozilla::RefPtr<mozilla::TransportFlow>& aFlow);
-  void DtlsConnected(mozilla::TransportLayer* aFlow,
-                     mozilla::TransportLayer::State state);
+  void DtlsConnected_s(mozilla::TransportLayer* aFlow,
+                       mozilla::TransportLayer::State state);
+  static void DtlsConnected_m(const std::string& aParentHandle,
+                              bool aPrivacyRequested);
 
   mozilla::RefPtr<mozilla::MediaSessionConduit> GetConduit(int aStreamIndex, bool aReceive) {
     int index_inner = aStreamIndex * 2 + (aReceive ? 0 : 1);
 
     if (mConduits.find(index_inner) == mConduits.end())
       return nullptr;
 
     return mConduits[index_inner];
@@ -395,16 +397,18 @@ class PeerConnectionMedia : public sigsl
   void IceGatheringStateChange(mozilla::NrIceCtx* ctx,
                                mozilla::NrIceCtx::GatheringState state);
   void IceConnectionStateChange(mozilla::NrIceCtx* ctx,
                                 mozilla::NrIceCtx::ConnectionState state);
   void IceStreamReady(mozilla::NrIceMediaStream *aStream);
 
   // The parent PC
   PeerConnectionImpl *mParent;
+  // and a loose handle on it for event driven stuff
+  std::string mParentHandle;
 
   // A list of streams returned from GetUserMedia
   // This is only accessed on the main thread (with one special exception)
   nsTArray<nsRefPtr<LocalSourceStreamInfo> > mLocalSourceStreams;
 
   // A list of streams provided by the other side
   // This is only accessed on the main thread (with one special exception)
   nsTArray<nsRefPtr<RemoteSourceStreamInfo> > mRemoteSourceStreams;