Bug 1546691: Avoid the PCObserver going away spontaneously when network is lost. r=mjf
authorByron Campen [:bwc] <docfaraday@gmail.com>
Sat, 27 Apr 2019 16:29:21 +0000
changeset 471749 e46a4bcb07b769fba96fa572590cc8711586a263
parent 471748 96fc75538a9a52a46e1595f7695587fc677f1d0d
child 471750 d8aabb74946258bdaa6964e7e41fdd409604ae73
push id35934
push usershindli@mozilla.com
push dateMon, 29 Apr 2019 21:53:38 +0000
treeherdermozilla-central@f6766ba4ac77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmjf
bugs1546691
milestone68.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 1546691: Avoid the PCObserver going away spontaneously when network is lost. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D28839
dom/media/PeerConnection.jsm
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
--- a/dom/media/PeerConnection.jsm
+++ b/dom/media/PeerConnection.jsm
@@ -468,26 +468,26 @@ class RTCPeerConnection {
     this.makeGetterSetterEH("onpeeridentity");
     this.makeGetterSetterEH("onidpassertionerror");
     this.makeGetterSetterEH("onidpvalidationerror");
 
     this._pc = new this._win.PeerConnectionImpl();
     this._operationsChain = this._win.Promise.resolve();
 
     this.__DOM_IMPL__._innerObject = this;
-    this._observer = new this._win.PeerConnectionObserver(this.__DOM_IMPL__);
+    const observer = new this._win.PeerConnectionObserver(this.__DOM_IMPL__);
 
     this._warnDeprecatedStatsRemoteAccessNullable = { warn: (key) =>
       this.logWarning(`Detected soon-to-break getStats() use with key="${key}"! stat.isRemote goes away in Firefox 66, but won't warn there!\
  - See https://blog.mozilla.org/webrtc/getstats-isremote-66/`) };
 
     // Add a reference to the PeerConnection to global list (before init).
     _globalPCList.addPC(this);
 
-    this._impl.initialize(this._observer, this._win, rtcConfig,
+    this._impl.initialize(observer, this._win, rtcConfig,
                           Services.tm.currentThread);
 
     this._certificateReady = this._initCertificate(rtcConfig.certificates);
     this._initIdp();
     _globalPCList.notifyLifecycleObservers(this, "initialized");
   }
 
   get _impl() {
@@ -1406,17 +1406,16 @@ class RTCPeerConnection {
         this._remoteIdp.close();
     }
     if (!this._suppressEvents) {
       this._transceivers.forEach(t => t.setStopped());
     }
     this._impl.close();
     this._suppressEvents = true;
     delete this._pc;
-    delete this._observer;
   }
 
   getLocalStreams() {
     this._checkClosed();
     let localStreams = new Set();
     this._transceivers.forEach(transceiver => {
       transceiver.sender.getStreams().forEach(stream => {
         localStreams.add(stream);
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -26,17 +26,16 @@
 #include "nsISimpleEnumerator.h"
 #include "nsServiceManagerUtils.h"
 #include "nsISocketTransportService.h"
 #include "nsIConsoleService.h"
 #include "nsThreadUtils.h"
 #include "nsIPrefService.h"
 #include "nsIPrefBranch.h"
 #include "nsProxyRelease.h"
-#include "nsQueryObject.h"
 #include "prtime.h"
 
 #include "AudioConduit.h"
 #include "VideoConduit.h"
 #include "runnable_utils.h"
 #include "PeerConnectionCtx.h"
 #include "PeerConnectionImpl.h"
 #include "PeerConnectionMedia.h"
@@ -213,25 +212,16 @@ static nsresult InitNSSInContent() {
 
   return NS_OK;
 }
 
 namespace mozilla {
 class DataChannel;
 }
 
-// XXX Workaround for bug 998092 to maintain the existing broken semantics
-template <>
-struct nsISupportsWeakReference::COMTypeInfo<nsSupportsWeakReference, void> {
-  static const nsIID kIID;
-};
-const nsIID
-    nsISupportsWeakReference::COMTypeInfo<nsSupportsWeakReference, void>::kIID =
-        NS_ISUPPORTSWEAKREFERENCE_IID;
-
 namespace mozilla {
 
 RTCStatsQuery::RTCStatsQuery(bool aInternal, bool aRecordTelemetry)
     : internalStats(aInternal),
       recordTelemetry(aRecordTelemetry),
       grabAllLevels(false),
       now(0.0) {}
 
@@ -394,17 +384,17 @@ nsresult PeerConnectionImpl::Initialize(
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aThread);
   if (!mThread) {
     mThread = do_QueryInterface(aThread);
     MOZ_ASSERT(mThread);
   }
   CheckThread();
 
-  mPCObserver = do_GetWeakReference(&aObserver);
+  mPCObserver = &aObserver;
 
   // Find the STS thread
 
   mSTSThread = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &res);
   MOZ_ASSERT(mSTSThread);
 
   // We do callback handling on STS instead of main to avoid media jank.
   // Someday, we may have a dedicated thread for this.
@@ -1133,43 +1123,16 @@ PeerConnectionImpl::CreateDataChannel(
                             getter_AddRefs(retval));
   if (NS_FAILED(rv)) {
     return rv;
   }
   retval.forget(aRetval);
   return NS_OK;
 }
 
-// do_QueryObjectReferent() - Helps get PeerConnectionObserver from nsWeakPtr.
-//
-// nsWeakPtr deals in XPCOM interfaces, while webidl bindings are concrete objs.
-// TODO: Turn this into a central (template) function somewhere (Bug 939178)
-//
-// Without it, each weak-ref call in this file would look like this:
-//
-//  nsCOMPtr<nsISupportsWeakReference> tmp = do_QueryReferent(mPCObserver);
-//  if (!tmp) {
-//    return;
-//  }
-//  RefPtr<nsSupportsWeakReference> tmp2 = do_QueryObject(tmp);
-//  RefPtr<PeerConnectionObserver> pco =
-//    static_cast<PeerConnectionObserver*>(&*tmp2);
-
-static already_AddRefed<PeerConnectionObserver> do_QueryObjectReferent(
-    nsIWeakReference* aRawPtr) {
-  nsCOMPtr<nsISupportsWeakReference> tmp = do_QueryReferent(aRawPtr);
-  if (!tmp) {
-    return nullptr;
-  }
-  RefPtr<nsSupportsWeakReference> tmp2 = do_QueryObject(tmp);
-  RefPtr<PeerConnectionObserver> tmp3 =
-      static_cast<PeerConnectionObserver*>(&*tmp2);
-  return tmp3.forget();
-}
-
 // Not a member function so that we don't need to keep the PC live.
 static void NotifyDataChannel_m(
     const RefPtr<nsDOMDataChannel>& aChannel,
     const RefPtr<PeerConnectionObserver>& aObserver) {
   MOZ_ASSERT(NS_IsMainThread());
   JSErrorResult rv;
   aObserver->NotifyDataChannel(*aChannel, rv);
   aChannel->AppReady();
@@ -1183,24 +1146,20 @@ void PeerConnectionImpl::NotifyDataChann
   MOZ_ASSERT(channel);
   CSFLogDebug(LOGTAG, "%s: channel: %p", __FUNCTION__, channel.get());
 
   RefPtr<nsDOMDataChannel> domchannel;
   nsresult rv = NS_NewDOMDataChannel(channel.forget(), mWindow,
                                      getter_AddRefs(domchannel));
   NS_ENSURE_SUCCESS_VOID(rv);
 
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    return;
-  }
-
-  RUN_ON_THREAD(mThread,
-                WrapRunnableNM(NotifyDataChannel_m, domchannel.forget(), pco),
-                NS_DISPATCH_NORMAL);
+  RUN_ON_THREAD(
+      mThread,
+      WrapRunnableNM(NotifyDataChannel_m, domchannel.forget(), mPCObserver),
+      NS_DISPATCH_NORMAL);
 }
 
 NS_IMETHODIMP
 PeerConnectionImpl::CreateOffer(const RTCOfferOptions& aOptions) {
   JsepOfferOptions options;
   // convert the RTCOfferOptions to JsepOfferOptions
   if (aOptions.mOfferToReceiveAudio.WasPassed()) {
     options.mOfferToReceiveAudio =
@@ -1240,20 +1199,16 @@ static std::unique_ptr<dom::PCErrorData>
   result->mMessage = NS_ConvertASCIItoUTF16(aMessage.c_str());
   return result;
 }
 
 // Used by unit tests and the IDL CreateOffer.
 NS_IMETHODIMP
 PeerConnectionImpl::CreateOffer(const JsepOfferOptions& aOptions) {
   PC_AUTO_ENTER_API_CALL(true);
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    return NS_OK;
-  }
 
   if (!PeerConnectionCtx::GetInstance()->isReady()) {
     // Uh oh. We're not ready yet. Enqueue this operation.
     PeerConnectionCtx::GetInstance()->queueJSEPOperation(
         WrapRunnableNM(DeferredCreateOffer, mHandle, aOptions));
     STAMP_TIMECARD(mTimeCard, "Deferring CreateOffer (not ready)");
     return NS_OK;
   }
@@ -1273,74 +1228,65 @@ PeerConnectionImpl::CreateOffer(const Js
   JsepSession::Result result = mJsepSession->CreateOffer(aOptions, &offer);
   JSErrorResult rv;
   if (result.mError.isSome()) {
     std::string errorString = mJsepSession->GetLastError();
 
     CSFLogError(LOGTAG, "%s: pc = %s, error = %s", __FUNCTION__,
                 mHandle.c_str(), errorString.c_str());
 
-    pco->OnCreateOfferError(*buildJSErrorData(result, errorString), rv);
+    mPCObserver->OnCreateOfferError(*buildJSErrorData(result, errorString), rv);
   } else {
     UpdateSignalingState();
-    pco->OnCreateOfferSuccess(ObString(offer.c_str()), rv);
+    mPCObserver->OnCreateOfferSuccess(ObString(offer.c_str()), rv);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 PeerConnectionImpl::CreateAnswer() {
   PC_AUTO_ENTER_API_CALL(true);
 
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    return NS_OK;
-  }
-
   CSFLogDebug(LOGTAG, "CreateAnswer()");
 
   STAMP_TIMECARD(mTimeCard, "Create Answer");
   // TODO(bug 1098015): Once RTCAnswerOptions is standardized, we'll need to
   // add it as a param to CreateAnswer, and convert it here.
   JsepAnswerOptions options;
   std::string answer;
 
   JsepSession::Result result = mJsepSession->CreateAnswer(options, &answer);
   JSErrorResult rv;
   if (result.mError.isSome()) {
     std::string errorString = mJsepSession->GetLastError();
 
     CSFLogError(LOGTAG, "%s: pc = %s, error = %s", __FUNCTION__,
                 mHandle.c_str(), errorString.c_str());
 
-    pco->OnCreateAnswerError(*buildJSErrorData(result, errorString), rv);
+    mPCObserver->OnCreateAnswerError(*buildJSErrorData(result, errorString),
+                                     rv);
   } else {
     UpdateSignalingState();
-    pco->OnCreateAnswerSuccess(ObString(answer.c_str()), rv);
+    mPCObserver->OnCreateAnswerSuccess(ObString(answer.c_str()), rv);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 PeerConnectionImpl::SetLocalDescription(int32_t aAction, const char* aSDP) {
   PC_AUTO_ENTER_API_CALL(true);
 
   if (!aSDP) {
     CSFLogError(LOGTAG, "%s - aSDP is NULL", __FUNCTION__);
     return NS_ERROR_FAILURE;
   }
 
   JSErrorResult rv;
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    return NS_OK;
-  }
-
   STAMP_TIMECARD(mTimeCard, "Set Local Description");
 
   if (mMedia->AnyLocalTrackHasPeerIdentity()) {
     mPrivacyRequested = Some(true);
   }
 
   mLocalRequestedSDP = aSDP;
 
@@ -1364,24 +1310,25 @@ PeerConnectionImpl::SetLocalDescription(
       return NS_ERROR_FAILURE;
   }
   JsepSession::Result result =
       mJsepSession->SetLocalDescription(sdpType, mLocalRequestedSDP);
   if (result.mError.isSome()) {
     std::string errorString = mJsepSession->GetLastError();
     CSFLogError(LOGTAG, "%s: pc = %s, error = %s", __FUNCTION__,
                 mHandle.c_str(), errorString.c_str());
-    pco->OnSetLocalDescriptionError(*buildJSErrorData(result, errorString), rv);
+    mPCObserver->OnSetLocalDescriptionError(
+        *buildJSErrorData(result, errorString), rv);
   } else {
     if (wasRestartingIce) {
       RecordIceRestartStatistics(sdpType);
     }
-    pco->SyncTransceivers(rv);
+    mPCObserver->SyncTransceivers(rv);
     UpdateSignalingState(sdpType == mozilla::kJsepSdpRollback);
-    pco->OnSetLocalDescriptionSuccess(rv);
+    mPCObserver->OnSetLocalDescriptionSuccess(rv);
   }
 
   return NS_OK;
 }
 
 static void DeferredSetRemote(const std::string& aPcHandle, int32_t aAction,
                               const std::string& aSdp) {
   PeerConnectionWrapper wrapper(aPcHandle);
@@ -1401,21 +1348,16 @@ PeerConnectionImpl::SetRemoteDescription
   PC_AUTO_ENTER_API_CALL(true);
 
   if (!aSDP) {
     CSFLogError(LOGTAG, "%s - aSDP is NULL", __FUNCTION__);
     return NS_ERROR_FAILURE;
   }
 
   JSErrorResult jrv;
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    return NS_OK;
-  }
-
   if (action == IPeerConnection::kActionOffer) {
     if (!PeerConnectionCtx::GetInstance()->isReady()) {
       // Uh oh. We're not ready yet. Enqueue this operation. (This must be a
       // remote offer, or else we would not have gotten this far)
       PeerConnectionCtx::GetInstance()->queueJSEPOperation(WrapRunnableNM(
           DeferredSetRemote, mHandle, action, std::string(aSDP)));
       STAMP_TIMECARD(mTimeCard, "Deferring SetRemote (not ready)");
       return NS_OK;
@@ -1453,18 +1395,18 @@ PeerConnectionImpl::SetRemoteDescription
 
   size_t originalTransceiverCount = mJsepSession->GetTransceivers().size();
   JsepSession::Result result =
       mJsepSession->SetRemoteDescription(sdpType, mRemoteRequestedSDP);
   if (result.mError.isSome()) {
     std::string errorString = mJsepSession->GetLastError();
     CSFLogError(LOGTAG, "%s: pc = %s, error = %s", __FUNCTION__,
                 mHandle.c_str(), errorString.c_str());
-    pco->OnSetRemoteDescriptionError(*buildJSErrorData(result, errorString),
-                                     jrv);
+    mPCObserver->OnSetRemoteDescriptionError(
+        *buildJSErrorData(result, errorString), jrv);
   } else {
     // Iterate over the JSEP transceivers that were just created
     for (size_t i = originalTransceiverCount;
          i < mJsepSession->GetTransceivers().size(); ++i) {
       RefPtr<JsepTransceiver> jsepTransceiver =
           mJsepSession->GetTransceivers()[i];
 
       if (jsepTransceiver->GetMediaType() ==
@@ -1479,22 +1421,22 @@ PeerConnectionImpl::SetRemoteDescription
         return NS_ERROR_FAILURE;
       }
 
       const JsepTrack& receiving(jsepTransceiver->mRecvTrack);
       CSFLogInfo(LOGTAG, "%s: pc = %s, asking JS to create transceiver for %s",
                  __FUNCTION__, mHandle.c_str(), receiving.GetTrackId().c_str());
       switch (receiving.GetMediaType()) {
         case SdpMediaSection::MediaType::kAudio:
-          pco->OnTransceiverNeeded(NS_ConvertASCIItoUTF16("audio"),
-                                   *transceiverImpl, jrv);
+          mPCObserver->OnTransceiverNeeded(NS_ConvertASCIItoUTF16("audio"),
+                                           *transceiverImpl, jrv);
           break;
         case SdpMediaSection::MediaType::kVideo:
-          pco->OnTransceiverNeeded(NS_ConvertASCIItoUTF16("video"),
-                                   *transceiverImpl, jrv);
+          mPCObserver->OnTransceiverNeeded(NS_ConvertASCIItoUTF16("video"),
+                                           *transceiverImpl, jrv);
           break;
         default:
           MOZ_RELEASE_ASSERT(false);
       }
 
       if (jrv.Failed()) {
         nsresult rv = jrv.StealNSResult();
         CSFLogError(LOGTAG,
@@ -1505,21 +1447,21 @@ PeerConnectionImpl::SetRemoteDescription
         return NS_ERROR_FAILURE;
       }
     }
 
     if (wasRestartingIce) {
       RecordIceRestartStatistics(sdpType);
     }
 
-    pco->SyncTransceivers(jrv);
+    mPCObserver->SyncTransceivers(jrv);
 
     UpdateSignalingState(sdpType == mozilla::kJsepSdpRollback);
 
-    pco->OnSetRemoteDescriptionSuccess(jrv);
+    mPCObserver->OnSetRemoteDescriptionSuccess(jrv);
 
     startCallTelem();
   }
 
   return NS_OK;
 }
 
 // WebRTC uses highres time relative to the UNIX epoch (Jan 1, 1970, UTC).
@@ -1578,21 +1520,16 @@ PeerConnectionImpl::AddIceCandidate(
 
   if (mForceIceTcp &&
       std::string::npos != std::string(aCandidate).find(" UDP ")) {
     CSFLogError(LOGTAG, "Blocking remote UDP candidate: %s", aCandidate);
     return NS_OK;
   }
 
   JSErrorResult rv;
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    return NS_OK;
-  }
-
   STAMP_TIMECARD(mTimeCard, "Add Ice Candidate");
 
   CSFLogDebug(LOGTAG, "AddIceCandidate: %s %s", aCandidate, aUfrag);
 
   // When remote candidates are added before our ICE ctx is up and running
   // (the transition to New is async through STS, so this is not impossible),
   // we won't record them as trickle candidates. Is this what we want?
   if (!mIceStartTime.IsNull()) {
@@ -1617,28 +1554,29 @@ PeerConnectionImpl::AddIceCandidate(
   if (!result.mError.isSome()) {
     // We do not bother PCMedia about this before offer/answer concludes.
     // Once offer/answer concludes, PCMedia will extract these candidates from
     // the remote SDP.
     if (mSignalingState == PCImplSignalingState::SignalingStable) {
       mMedia->AddIceCandidate(aCandidate, transportId, aUfrag);
       mRawTrickledCandidates.push_back(aCandidate);
     }
-    pco->OnAddIceCandidateSuccess(rv);
+    mPCObserver->OnAddIceCandidateSuccess(rv);
   } else {
     ++mAddCandidateErrorCount;
     std::string errorString = mJsepSession->GetLastError();
 
     CSFLogError(LOGTAG,
                 "Failed to incorporate remote candidate into SDP:"
                 " res = %u, candidate = %s, level = %i, error = %s",
                 static_cast<unsigned>(*result.mError), aCandidate,
                 level.valueOr(-1), errorString.c_str());
 
-    pco->OnAddIceCandidateError(*buildJSErrorData(result, errorString), rv);
+    mPCObserver->OnAddIceCandidateError(*buildJSErrorData(result, errorString),
+                                        rv);
   }
 
   return NS_OK;
 }
 
 void PeerConnectionImpl::UpdateNetworkState(bool online) {
   if (!mMedia) {
     return;
@@ -1748,21 +1686,16 @@ void PeerConnectionImpl::DumpPacket_m(si
   if (IsClosed()) {
     return;
   }
 
   if (!ShouldDumpPacket(level, type, sending)) {
     return;
   }
 
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    return;
-  }
-
   // TODO: Is this efficient? Should we try grabbing our JS ctx from somewhere
   // else?
   AutoJSAPI jsapi;
   if (!jsapi.Init(GetWindow())) {
     return;
   }
 
   JS::Rooted<JSObject*> jsobj(
@@ -1770,17 +1703,17 @@ void PeerConnectionImpl::DumpPacket_m(si
       JS::NewArrayBufferWithContents(jsapi.cx(), size, packet.release()));
 
   RootedSpiderMonkeyInterface<ArrayBuffer> arrayBuffer(jsapi.cx());
   if (!arrayBuffer.Init(jsobj)) {
     return;
   }
 
   JSErrorResult jrv;
-  pco->OnPacket(level, type, sending, arrayBuffer, jrv);
+  mPCObserver->OnPacket(level, type, sending, arrayBuffer, jrv);
 }
 
 NS_IMETHODIMP
 PeerConnectionImpl::GetRtpSources(
     MediaStreamTrack& aRecvTrack, DOMHighResTimeStamp aRtpSourceTimeNow,
     nsTArray<dom::RTCRtpSourceEntry>& outRtpSources) {
   PC_AUTO_ENTER_API_CALL(true);
   outRtpSources.Clear();
@@ -2371,22 +2304,18 @@ void PeerConnectionImpl::SetSignalingSta
     CloseInt();
     // Uncount this connection as active on the inner window upon close.
     if (mWindow && mActiveOnWindow) {
       mWindow->RemovePeerConnection();
       mActiveOnWindow = false;
     }
   }
 
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    return;
-  }
   JSErrorResult rv;
-  pco->OnStateChange(PCObserverStateType::SignalingState, rv);
+  mPCObserver->OnStateChange(PCObserverStateType::SignalingState, rv);
 }
 
 void PeerConnectionImpl::UpdateSignalingState(bool rollback) {
   mozilla::JsepSignalingState state = mJsepSession->GetState();
 
   PCImplSignalingState newState;
 
   switch (state) {
@@ -2491,29 +2420,24 @@ void PeerConnectionImpl::CandidateReady(
     return;
   }
 
   CSFLogDebug(LOGTAG, "Passing local candidate to content: %s",
               candidate.c_str());
   SendLocalIceCandidateToContent(level, mid, candidate, ufrag);
 }
 
-static void SendLocalIceCandidateToContentImpl(const nsWeakPtr& weakPCObserver,
-                                               uint16_t level,
-                                               const std::string& mid,
-                                               const std::string& candidate,
-                                               const std::string& ufrag) {
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(weakPCObserver);
-  if (!pco) {
-    return;
-  }
-
+static void SendLocalIceCandidateToContentImpl(
+    const RefPtr<PeerConnectionObserver>& aPCObserver, uint16_t level,
+    const std::string& mid, const std::string& candidate,
+    const std::string& ufrag) {
   JSErrorResult rv;
-  pco->OnIceCandidate(level, ObString(mid.c_str()), ObString(candidate.c_str()),
-                      ObString(ufrag.c_str()), rv);
+  aPCObserver->OnIceCandidate(level, ObString(mid.c_str()),
+                              ObString(candidate.c_str()),
+                              ObString(ufrag.c_str()), rv);
 }
 
 void PeerConnectionImpl::SendLocalIceCandidateToContent(
     uint16_t level, const std::string& mid, const std::string& candidate,
     const std::string& ufrag) {
   // We dispatch this because OnSetLocalDescriptionSuccess does a setTimeout(0)
   // to unwind the stack, but the event handlers don't. We need to ensure that
   // the candidates do not skip ahead of the callback.
@@ -2536,17 +2460,17 @@ static bool isSucceeded(PCImplIceConnect
 static bool isFailed(PCImplIceConnectionState state) {
   return state == PCImplIceConnectionState::Failed;
 }
 
 void PeerConnectionImpl::IceConnectionStateChange(
     dom::PCImplIceConnectionState domState) {
   PC_AUTO_ENTER_API_CALL_VOID_RETURN(false);
 
-  CSFLogDebug(LOGTAG, "%s", __FUNCTION__);
+  CSFLogDebug(LOGTAG, "%s: %d", __FUNCTION__, static_cast<int>(domState));
 
   if (domState == mIceConnectionState) {
     // no work to be done since the states are the same.
     // this can happen during ICE rollback situations.
     return;
   }
 
   if (!isDone(mIceConnectionState) && isDone(domState)) {
@@ -2595,23 +2519,18 @@ void PeerConnectionImpl::IceConnectionSt
       break;
     case PCImplIceConnectionState::Closed:
       STAMP_TIMECARD(mTimeCard, "Ice state: closed");
       break;
     default:
       MOZ_ASSERT_UNREACHABLE("Unexpected mIceConnectionState!");
   }
 
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    return;
-  }
-
   WrappableJSErrorResult rv;
-  pco->OnStateChange(PCObserverStateType::IceConnectionState, rv);
+  mPCObserver->OnStateChange(PCObserverStateType::IceConnectionState, rv);
 }
 
 void PeerConnectionImpl::IceGatheringStateChange(
     dom::PCImplIceGatheringState state) {
   PC_AUTO_ENTER_API_CALL_VOID_RETURN(false);
 
   CSFLogDebug(LOGTAG, "%s", __FUNCTION__);
 
@@ -2628,25 +2547,22 @@ void PeerConnectionImpl::IceGatheringSta
       break;
     case PCImplIceGatheringState::Complete:
       STAMP_TIMECARD(mTimeCard, "Ice gathering state: complete");
       break;
     default:
       MOZ_ASSERT_UNREACHABLE("Unexpected mIceGatheringState!");
   }
 
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    return;
-  }
   WrappableJSErrorResult rv;
-  mThread->Dispatch(WrapRunnable(pco, &PeerConnectionObserver::OnStateChange,
-                                 PCObserverStateType::IceGatheringState, rv,
-                                 static_cast<JS::Realm*>(nullptr)),
-                    NS_DISPATCH_NORMAL);
+  mThread->Dispatch(
+      WrapRunnable(mPCObserver, &PeerConnectionObserver::OnStateChange,
+                   PCObserverStateType::IceGatheringState, rv,
+                   static_cast<JS::Realm*>(nullptr)),
+      NS_DISPATCH_NORMAL);
 
   if (mIceGatheringState == PCImplIceGatheringState::Complete) {
     SendLocalIceCandidateToContent(0, "", "", "");
   }
 }
 
 void PeerConnectionImpl::UpdateDefaultCandidate(
     const std::string& defaultAddr, uint16_t defaultPort,
@@ -2967,29 +2883,26 @@ RefPtr<RTCStatsQueryPromise> PeerConnect
 }
 
 void PeerConnectionImpl::DeliverStatsReportToPCObserver_m(
     const std::string& pcHandle, nsresult result,
     const nsAutoPtr<RTCStatsQuery>& query) {
   // Is the PeerConnectionImpl still around?
   PeerConnectionWrapper pcw(pcHandle);
   if (pcw.impl()) {
-    RefPtr<PeerConnectionObserver> pco =
-        do_QueryObjectReferent(pcw.impl()->mPCObserver);
-    if (pco) {
-      JSErrorResult rv;
-      if (NS_SUCCEEDED(result)) {
-        pco->OnGetStatsSuccess(*query->report, rv);
-      } else {
-        pco->OnGetStatsError(ObString("Failed to fetch statistics"), rv);
-      }
-
-      if (rv.Failed()) {
-        CSFLogError(LOGTAG, "Error firing stats observer callback");
-      }
+    JSErrorResult rv;
+    if (NS_SUCCEEDED(result)) {
+      pcw.impl()->mPCObserver->OnGetStatsSuccess(*query->report, rv);
+    } else {
+      pcw.impl()->mPCObserver->OnGetStatsError(
+          ObString("Failed to fetch statistics"), rv);
+    }
+
+    if (rv.Failed()) {
+      CSFLogError(LOGTAG, "Error firing stats observer callback");
     }
   }
 }
 
 void PeerConnectionImpl::RecordLongtermICEStatistics() {
   WebrtcGlobalInformation::StoreLongTermICEStatistics(*this);
 }
 
@@ -3041,30 +2954,24 @@ nsresult PeerConnectionImpl::DTMFState::
                                    nsITimer::TYPE_ONE_SHOT);
 
       mTransceiver->InsertDTMFTone(tone, mDuration);
     }
   } else {
     mSendTimer->Cancel();
   }
 
-  RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
-  if (!pco) {
-    NS_WARNING("Failed to dispatch the RTCDTMFToneChange event!");
-    return NS_OK;  // Return is ignored anyhow
-  }
-
   RefPtr<dom::MediaStreamTrack> sendTrack = mTransceiver->GetSendTrack();
   if (!sendTrack) {
     NS_WARNING("Failed to dispatch the RTCDTMFToneChange event!");
     return NS_OK;  // Return is ignored anyhow
   }
 
   JSErrorResult jrv;
-  pco->OnDTMFToneChange(*sendTrack, eventTone, jrv);
+  mPCObserver->OnDTMFToneChange(*sendTrack, eventTone, jrv);
 
   if (jrv.Failed()) {
     NS_WARNING("Failed to dispatch the RTCDTMFToneChange event!");
   }
 
   return NS_OK;
 }
 
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
@@ -9,17 +9,16 @@
 #include <string>
 #include <vector>
 #include <map>
 #include <cmath>
 
 #include "prlock.h"
 #include "mozilla/RefPtr.h"
 #include "nsAutoPtr.h"
-#include "nsIWeakReferenceUtils.h"  // for the definition of nsWeakPtr
 #include "IPeerConnection.h"
 #include "sigslot.h"
 #include "nsComponentManagerUtils.h"
 #include "nsPIDOMWindow.h"
 #include "nsIUUIDGenerator.h"
 #include "nsIThread.h"
 #include "mozilla/Mutex.h"
 
@@ -579,18 +578,17 @@ class PeerConnectionImpl final
 
   mozilla::dom::PCImplSignalingState mSignalingState;
 
   // ICE State
   mozilla::dom::PCImplIceConnectionState mIceConnectionState;
   mozilla::dom::PCImplIceGatheringState mIceGatheringState;
 
   nsCOMPtr<nsIThread> mThread;
-  // TODO: Remove if we ever properly wire PeerConnection for cycle-collection.
-  nsWeakPtr mPCObserver;
+  RefPtr<PeerConnectionObserver> mPCObserver;
 
   nsCOMPtr<nsPIDOMWindowInner> mWindow;
 
   // The SDP sent in from JS
   std::string mLocalRequestedSDP;
   std::string mRemoteRequestedSDP;
 
   // DTLS fingerprint
@@ -658,17 +656,17 @@ class PeerConnectionImpl final
     virtual ~DTMFState();
 
    public:
     DTMFState();
 
     NS_DECL_NSITIMERCALLBACK
     NS_DECL_THREADSAFE_ISUPPORTS
 
-    nsWeakPtr mPCObserver;
+    RefPtr<PeerConnectionObserver> mPCObserver;
     RefPtr<TransceiverImpl> mTransceiver;
     nsCOMPtr<nsITimer> mSendTimer;
     nsString mTones;
     uint32_t mDuration;
     uint32_t mInterToneGap;
   };
 
   // TODO(bug 1401983): Move DTMF stuff to TransceiverImpl