From f8b323b72db7cd6ca4eb3040199a8afb91fff2f4 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Sat, 19 Oct 2013 10:48:41 -0400 Subject: [PATCH] Backed out changeset dc2b71e57211 (bug 928221) because it calls a non-existing GetWeakReferent function --- dom/media/PeerConnection.js | 15 --------- dom/webidl/PeerConnectionObserver.webidl | 5 --- .../src/peerconnection/PeerConnectionImpl.cpp | 33 ++++--------------- .../src/peerconnection/PeerConnectionImpl.h | 27 +++++++++------ media/webrtc/signaling/test/FakePCObserver.h | 3 +- .../signaling/test/signaling_unittests.cpp | 2 +- 6 files changed, 26 insertions(+), 59 deletions(-) diff --git a/dom/media/PeerConnection.js b/dom/media/PeerConnection.js index bf269c34f056..f8595d68e1a3 100644 --- a/dom/media/PeerConnection.js +++ b/dom/media/PeerConnection.js @@ -891,7 +891,6 @@ RTCError.prototype = { // This is a separate object because we don't want to expose it to DOM. function PeerConnectionObserver() { this._dompc = null; - this._guard = new WeakReferent(this); } PeerConnectionObserver.prototype = { classDescription: "PeerConnectionObserver", @@ -1193,23 +1192,9 @@ PeerConnectionObserver.prototype = { getSupportedConstraints: function(dict) { return dict; - }, - - get weakReferent() { - return this._guard; } }; -// A PeerConnectionObserver member that c++ can do weak references on - -function WeakReferent(parent) { - this._parent = parent; // prevents parent from going away without us -} -WeakReferent.prototype = { - QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports, - Ci.nsISupportsWeakReference]), -}; - this.NSGetFactory = XPCOMUtils.generateNSGetFactory( [GlobalPCList, RTCIceCandidate, RTCSessionDescription, RTCPeerConnection, RTCStatsReport, PeerConnectionObserver] diff --git a/dom/webidl/PeerConnectionObserver.webidl b/dom/webidl/PeerConnectionObserver.webidl index a8bdbd73bfe2..162ff8b4c083 100644 --- a/dom/webidl/PeerConnectionObserver.webidl +++ b/dom/webidl/PeerConnectionObserver.webidl @@ -4,8 +4,6 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ -interface nsISupports; - [ChromeOnly, JSImplementation="@mozilla.org/dom/peerconnectionobserver;1", Constructor (object domPC)] @@ -42,9 +40,6 @@ interface PeerConnectionObserver void onAddTrack(); void onRemoveTrack(); - /* Used by c++ to know when Observer goes away */ - readonly attribute nsISupports weakReferent; - /* Helper function to access supported constraints defined in webidl. Needs to * be in a separate webidl object we hold, so putting it here was convenient. */ diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp index 4b4a185c84ce..5f17923913c7 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp @@ -655,7 +655,7 @@ PeerConnectionImpl::Initialize(PeerConnectionObserver& aObserver, MOZ_ASSERT(aThread); mThread = do_QueryInterface(aThread); - mPCObserver.Set(&aObserver); + mPCObserver.Init(&aObserver); // Find the STS thread @@ -1383,6 +1383,12 @@ PeerConnectionImpl::CloseInt() { PC_AUTO_ENTER_API_CALL_NO_CHECK(); + // Clear raw pointer to observer since PeerConnection.js does not guarantee + // the observer's existence past Close(). + // + // Any outstanding runnables hold RefPtr<> references to observer. + mPCObserver.Close(); + if (mInternal->mCall) { CSFLogInfo(logTag, "%s: Closing PeerConnectionImpl %s; " "ending call", __FUNCTION__, mHandle.c_str()); @@ -1756,30 +1762,5 @@ PeerConnectionImpl::GetRemoteStreams(nsTArray #endif } -// WeakConcretePtr gets around WeakPtr not working on concrete types by using -// the attribute getWeakReferent, a member that supports weak refs, as a guard. - -void -PeerConnectionImpl::WeakConcretePtr::Set(PeerConnectionObserver *aObserver) { - mObserver = aObserver; -#ifdef MOZILLA_INTERNAL_API - MOZ_ASSERT(NS_IsMainThread()); - JSErrorResult rv; - nsCOMPtr tmp = aObserver->GetWeakReferent(rv); - MOZ_ASSERT(!rv.Failed()); - mWeakPtr = do_GetWeakReference(tmp); -#else - mWeakPtr = do_GetWeakReference(aObserver); -#endif -} - -PeerConnectionObserver * -PeerConnectionImpl::WeakConcretePtr::MayGet() { -#ifdef MOZILLA_INTERNAL_API - MOZ_ASSERT(NS_IsMainThread()); -#endif - nsCOMPtr guard = do_QueryReferent(mWeakPtr); - return (!guard) ? nullptr : mObserver; -} } // end sipcc namespace diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h index de8fef6eb121..237ba93d51d0 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h @@ -12,9 +12,6 @@ #include "prlock.h" #include "mozilla/RefPtr.h" -#include "nsWeakPtr.h" -#include "nsIWeakReferenceUtils.h" // for the definition of nsWeakPtr -#include "IPeerConnection.h" #include "sigslot.h" #include "nricectx.h" #include "nricemediastream.h" @@ -505,19 +502,29 @@ private: mozilla::dom::PCImplIceState mIceState; nsCOMPtr mThread; - // WeakConcretePtr to PeerConnectionObserver. TODO: Remove after bug 928535 + // We hold a raw pointer to PeerConnectionObserver (no WeakRefs to concretes!) + // which is an invariant guaranteed to exist between Initialize() and Close(). + // We explicitly clear it in Close(). We wrap it in a helper, to encourage + // testing against nullptr before use. Use in Runnables requires wrapping + // access in RefPtr<> since they may execute after close. This is only safe + // to use on the main thread // - // This is only safe to use on the main thread // TODO: Remove if we ever properly wire PeerConnection for cycle-collection. - class WeakConcretePtr + class WeakReminder { public: - WeakConcretePtr() : mObserver(nullptr) {} - void Set(PeerConnectionObserver *aObserver); - PeerConnectionObserver *MayGet(); + WeakReminder() : mObserver(nullptr) {} + void Init(PeerConnectionObserver *aObserver) { + mObserver = aObserver; + } + void Close() { + mObserver = nullptr; + } + PeerConnectionObserver *MayGet() { + return mObserver; + } private: PeerConnectionObserver *mObserver; - nsWeakPtr mWeakPtr; } mPCObserver; nsCOMPtr mWindow; diff --git a/media/webrtc/signaling/test/FakePCObserver.h b/media/webrtc/signaling/test/FakePCObserver.h index ddb427341683..54761a1c8cdc 100644 --- a/media/webrtc/signaling/test/FakePCObserver.h +++ b/media/webrtc/signaling/test/FakePCObserver.h @@ -21,7 +21,6 @@ #include "nsIDOMMediaStream.h" #include "mozilla/dom/PeerConnectionObserverEnumsBinding.h" #include "PeerConnectionImpl.h" -#include "nsWeakReference.h" namespace sipcc { class PeerConnectionImpl; @@ -33,7 +32,7 @@ class nsIDOMDataChannel; namespace test { using namespace mozilla::dom; -class AFakePCObserver : public nsSupportsWeakReference +class AFakePCObserver : public nsISupports { protected: typedef mozilla::ErrorResult ER; diff --git a/media/webrtc/signaling/test/signaling_unittests.cpp b/media/webrtc/signaling/test/signaling_unittests.cpp index be6e9a6ea666..8122decd5dc7 100644 --- a/media/webrtc/signaling/test/signaling_unittests.cpp +++ b/media/webrtc/signaling/test/signaling_unittests.cpp @@ -273,7 +273,7 @@ public: NS_IMETHODIMP OnIceCandidate(uint16_t level, const char *mid, const char *cand, ER&); }; -NS_IMPL_ISUPPORTS1(TestObserver, nsISupportsWeakReference) +NS_IMPL_ISUPPORTS0(TestObserver) NS_IMETHODIMP TestObserver::OnCreateOfferSuccess(const char* offer, ER&)