Bug 825514: Add safety check to PeerConnectionCtx shutdown r=ekr
authorRandell Jesup <rjesup@jesup.org>
Mon, 31 Dec 2012 12:34:44 -0500
changeset 126370 f6e45f550d309efb453b929e919465446ab4097a
parent 126369 1a2e77002690fc920753e1e716bb493cc6d97ced
child 126371 6a15f30fdfb348ae0177b7860d7c00771653c830
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersekr
bugs825514
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 825514: Add safety check to PeerConnectionCtx shutdown r=ekr
media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
media/webrtc/signaling/test/signaling_unittests.cpp
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
@@ -132,19 +132,21 @@ nsresult PeerConnectionCtx::InitializeGl
 PeerConnectionCtx* PeerConnectionCtx::GetInstance() {
   MOZ_ASSERT(gInstance);
   return gInstance;
 }
 
 void PeerConnectionCtx::Destroy() {
   CSFLogDebug(logTag, "%s", __FUNCTION__);
 
-  gInstance->Cleanup();
-  delete gInstance;
-  gInstance = NULL;
+  if (gInstance) {
+    gInstance->Cleanup();
+    delete gInstance;
+    gInstance = NULL;
+  }
 }
 
 nsresult PeerConnectionCtx::Initialize() {
   mCCM = CSF::CallControlManager::create();
 
   NS_ENSURE_TRUE(mCCM.get(), NS_ERROR_FAILURE);
 
   // Add the local audio codecs
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -226,27 +226,23 @@ PeerConnectionImpl::PeerConnectionImpl()
 }
 
 PeerConnectionImpl::~PeerConnectionImpl()
 {
   PC_AUTO_ENTER_API_CALL_NO_CHECK();
   PeerConnectionCtx::GetInstance()->mPeerConnections.erase(mHandle);
   CloseInt(false);
 
-#if 0
-  // TODO(ekr@rtfm.com): figure out how to shut down PCCtx.
-  // bug 820011.
-
   // Since this and Initialize() occur on MainThread, they can't both be
   // running at once
-  // Might be more optimal to release off a timer (and XPCOM Shutdown)
-  // to avoid churn
-  if (PeerConnectionCtx::GetInstance()->mPeerConnections.empty())
-    Shutdown();
-#endif
+
+  // Right now, we delete PeerConnectionCtx at XPCOM shutdown only, but we
+  // probably want to shut it down more aggressively to save memory.  We
+  // could shut down here when there are no uses.  It might be more optimal
+  // to release off a timer (and XPCOM Shutdown) to avoid churn
 
   /* We should release mPCObserver on the main thread, but also prevent a double free.
   nsCOMPtr<nsIThread> mainThread;
   NS_GetMainThread(getter_AddRefs(mainThread));
   NS_ProxyRelease(mainThread, mPCObserver);
   */
 }
 
@@ -960,22 +956,16 @@ PeerConnectionImpl::ShutdownMedia(bool a
   // Forget the reference so that we can transfer it to
   // SelfDestruct().
   RUN_ON_THREAD(mThread, WrapRunnable(mMedia.forget().get(),
                                       &PeerConnectionMedia::SelfDestruct),
                 aIsSynchronous ? NS_DISPATCH_SYNC : NS_DISPATCH_NORMAL);
 }
 
 void
-PeerConnectionImpl::Shutdown()
-{
-  PeerConnectionCtx::Destroy();
-}
-
-void
 PeerConnectionImpl::onCallEvent(ccapi_call_event_e aCallEvent,
                                 CSF::CC_CallPtr aCall, CSF::CC_CallInfoPtr aInfo)
 {
   PC_AUTO_ENTER_API_CALL_NO_CHECK();
   MOZ_ASSERT(aCall.get());
   MOZ_ASSERT(aInfo.get());
 
   cc_call_state_t event = aInfo->getCallState();
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
@@ -110,17 +110,16 @@ public:
     kRoleOfferer,
     kRoleAnswerer
   };
 
   NS_DECL_ISUPPORTS
   NS_DECL_IPEERCONNECTION
 
   static PeerConnectionImpl* CreatePeerConnection();
-  static void Shutdown();
   static nsresult ConvertConstraints(
     const JS::Value& aConstraints, MediaConstraints* aObj, JSContext* aCx);
   static nsresult MakeMediaStream(uint32_t aHint, nsIDOMMediaStream** aStream);
 
   Role GetRole() const {
     PC_AUTO_ENTER_API_CALL_NO_CHECK();
     return mRole;
   }
--- a/media/webrtc/signaling/test/signaling_unittests.cpp
+++ b/media/webrtc/signaling/test/signaling_unittests.cpp
@@ -841,17 +841,17 @@ private:
       ASSERT_NE(sdp.find("m=application"), std::string::npos);
     }
   }
 };
 
 class SignalingEnvironment : public ::testing::Environment {
  public:
   void TearDown() {
-    sipcc::PeerConnectionImpl::Shutdown();
+    // Signaling is shut down in XPCOM shutdown
   }
 };
 
 class SignalingTest : public ::testing::Test {
 public:
   static void SetUpTestCase() {
     nsIThread *thread;