Backed out changeset dc2b71e57211 (bug 928221) because it calls a non-existing GetWeakReferent function

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]

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.
    */

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<nsRefPtr<mozilla::DOMMediaStream >
 #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<nsISupports> 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<nsISupports> guard = do_QueryReferent(mWeakPtr);
-  return (!guard) ? nullptr : mObserver;
-}
 
 }  // end sipcc namespace

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<nsIThread> 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<nsPIDOMWindow> mWindow;
 

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;

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&)