Bug 1028588 - Fix dangerous public destructors in media/webrtc/ - r=rjesup
authorBenoit Jacob <bjacob@mozilla.com>
Thu, 26 Jun 2014 09:31:20 -0400
changeset 191030 d9d6555b5a1b7e6093303649e0eed2ccf7d122fd
parent 191029 16244b1384abf153f1bf945a59f890b7930442ca
child 191031 99c8203bca19b4ae8a7a6ada0112601af12aa8dd
push id7483
push userryanvm@gmail.com
push dateThu, 26 Jun 2014 21:15:06 +0000
treeherderfx-team@3519e987aa3b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrjesup
bugs1028588
milestone33.0a1
Bug 1028588 - Fix dangerous public destructors in media/webrtc/ - r=rjesup
media/webrtc/signaling/src/peerconnection/MediaStreamList.h
media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
media/webrtc/signaling/test/FakeMediaStreams.h
media/webrtc/signaling/test/signaling_unittests.cpp
--- a/media/webrtc/signaling/src/peerconnection/MediaStreamList.h
+++ b/media/webrtc/signaling/src/peerconnection/MediaStreamList.h
@@ -28,29 +28,30 @@ class MediaStreamList : public nsISuppor
 {
 public:
   enum StreamType {
     Local,
     Remote
   };
 
   MediaStreamList(sipcc::PeerConnectionImpl* peerConnection, StreamType type);
-  virtual ~MediaStreamList();
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(MediaStreamList)
 
   virtual JSObject* WrapObject(JSContext *cx)
     MOZ_OVERRIDE;
   nsISupports* GetParentObject();
 
   DOMMediaStream* IndexedGetter(uint32_t index, bool& found);
   uint32_t Length();
 
 private:
+  virtual ~MediaStreamList();
+
   nsRefPtr<sipcc::PeerConnectionImpl> mPeerConnection;
   StreamType mType;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // MediaStreamList_h__
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
@@ -126,24 +126,16 @@ public:
       rv = observerService->AddObserver(this,
                                         NS_XPCOM_SHUTDOWN_OBSERVER_ID,
                                         false);
       MOZ_ALWAYS_TRUE(NS_SUCCEEDED(rv));
 #endif
       (void) rv;
     }
 
-  virtual ~PeerConnectionCtxShutdown()
-    {
-      nsCOMPtr<nsIObserverService> observerService =
-        services::GetObserverService();
-      if (observerService)
-        observerService->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
-    }
-
   NS_IMETHODIMP Observe(nsISupports* aSubject, const char* aTopic,
                         const char16_t* aData) {
     if (strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {
       CSFLogDebug(logTag, "Shutting down PeerConnectionCtx");
       sipcc::PeerConnectionCtx::Destroy();
 
       nsCOMPtr<nsIObserverService> observerService =
         services::GetObserverService();
@@ -155,16 +147,25 @@ public:
       MOZ_ALWAYS_TRUE(NS_SUCCEEDED(rv));
 
       // Make sure we're not deleted while still inside ::Observe()
       nsRefPtr<PeerConnectionCtxShutdown> kungFuDeathGrip(this);
       sipcc::PeerConnectionCtx::gPeerConnectionCtxShutdown = nullptr;
     }
     return NS_OK;
   }
+
+private:
+  virtual ~PeerConnectionCtxShutdown()
+    {
+      nsCOMPtr<nsIObserverService> observerService =
+        services::GetObserverService();
+      if (observerService)
+        observerService->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
+    }
 };
 
 NS_IMPL_ISUPPORTS(PeerConnectionCtxShutdown, nsIObserver);
 }
 
 using namespace mozilla;
 namespace sipcc {
 
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
@@ -206,17 +206,16 @@ class PeerConnectionImpl MOZ_FINAL : pub
                                      public DOMMediaStream::PrincipalChangeObserver,
 #endif
                                      public sigslot::has_slots<>
 {
   struct Internal; // Avoid exposing c includes to bindings
 
 public:
   PeerConnectionImpl(const mozilla::dom::GlobalObject* aGlobal = nullptr);
-  virtual ~PeerConnectionImpl();
 
   enum Error {
     kNoError                          = 0,
     kInvalidConstraintsType           = 1,
     kInvalidCandidateType             = 2,
     kInvalidMediastreamTrack          = 3,
     kInvalidState                     = 4,
     kInvalidSessionDescription        = 5,
@@ -561,16 +560,17 @@ public:
   static nsresult ExecuteStatsQuery_s(RTCStatsQuery *query);
 
   // for monitoring changes in stream ownership
   // PeerConnectionMedia can't do it because it doesn't know about principals
   virtual void PrincipalChanged(DOMMediaStream* aMediaStream) MOZ_OVERRIDE;
 #endif
 
 private:
+  virtual ~PeerConnectionImpl();
   PeerConnectionImpl(const PeerConnectionImpl&rhs);
   PeerConnectionImpl& operator=(PeerConnectionImpl);
   NS_IMETHODIMP Initialize(PeerConnectionObserver& aObserver,
                            nsGlobalWindow* aWindow,
                            const IceConfiguration* aConfiguration,
                            const RTCConfiguration* aRTCConfiguration,
                            nsISupports* aThread);
 
--- a/media/webrtc/signaling/test/FakeMediaStreams.h
+++ b/media/webrtc/signaling/test/FakeMediaStreams.h
@@ -100,27 +100,28 @@ class Fake_MediaStream {
   mozilla::Mutex mMutex;  // Lock to prevent the listener list from being modified while
                           // executing Periodic().
 };
 
 class Fake_MediaPeriodic : public nsITimerCallback {
 public:
 Fake_MediaPeriodic(Fake_MediaStream *aStream) : mStream(aStream),
                                                 mCount(0) {}
-  virtual ~Fake_MediaPeriodic() {}
   void Detach() {
     mStream = nullptr;
   }
 
   int GetTimesCalled() { return mCount; }
 
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSITIMERCALLBACK
 
 protected:
+  virtual ~Fake_MediaPeriodic() {}
+
   Fake_MediaStream *mStream;
   int mCount;
 };
 
 
 class Fake_SourceMediaStream : public Fake_MediaStream {
  public:
   Fake_SourceMediaStream() : mSegmentsAdded(0),
@@ -206,26 +207,27 @@ class Fake_SourceMediaStream : public Fa
   bool mStop;
   nsRefPtr<Fake_MediaPeriodic> mPeriodic;
   nsCOMPtr<nsITimer> mTimer;
 };
 
 
 class Fake_DOMMediaStream : public nsIDOMMediaStream
 {
+protected:
+  virtual ~Fake_DOMMediaStream() {
+    // Note: memory leak
+    mMediaStream->Stop();
+  }
+
 public:
   Fake_DOMMediaStream() : mMediaStream(new Fake_MediaStream()) {}
   Fake_DOMMediaStream(Fake_MediaStream *stream) :
       mMediaStream(stream) {}
 
-  virtual ~Fake_DOMMediaStream() {
-    // Note: memory leak
-    mMediaStream->Stop();
-  }
-
   NS_DECL_THREADSAFE_ISUPPORTS
 
   static already_AddRefed<Fake_DOMMediaStream>
   CreateSourceStream(nsIDOMWindow* aWindow, uint32_t aHintContents) {
     Fake_SourceMediaStream *source = new Fake_SourceMediaStream();
 
     nsRefPtr<Fake_DOMMediaStream> ds = new Fake_DOMMediaStream(source);
     ds->SetHintContents(aHintContents);
--- a/media/webrtc/signaling/test/signaling_unittests.cpp
+++ b/media/webrtc/signaling/test/signaling_unittests.cpp
@@ -235,16 +235,19 @@ enum mediaPipelineFlags
   PIPELINE_SEND = (1<<2),
   PIPELINE_VIDEO = (1<<3),
   PIPELINE_RTCP_NACK = (1<<4)
 };
 
 
 class TestObserver : public AFakePCObserver
 {
+protected:
+  ~TestObserver() {}
+
 public:
   TestObserver(sipcc::PeerConnectionImpl *peerConnection,
                const std::string &aName) :
     AFakePCObserver(peerConnection, aName) {}
 
   size_t MatchingCandidates(const std::string& cand) {
     size_t count = 0;
 
@@ -682,23 +685,24 @@ class ParsedSDP {
   int num_lines;
 };
 
 
 // This class wraps the PeerConnection object and ensures that all calls
 // into it happen on the main thread.
 class PCDispatchWrapper : public nsSupportsWeakReference
 {
+ protected:
+  virtual ~PCDispatchWrapper() {}
+
  public:
   PCDispatchWrapper(sipcc::PeerConnectionImpl *peerConnection) {
     pc_ = peerConnection;
   }
 
-  virtual ~PCDispatchWrapper() {}
-
   NS_DECL_THREADSAFE_ISUPPORTS
 
   sipcc::PeerConnectionImpl *pcImpl() const {
     return pc_;
   }
 
   const nsRefPtr<sipcc::PeerConnectionMedia>& media() const {
     return pc_->media();