Bug 1755318: Stop using a strong ref in StunAddrsHandler. r=mjf

StunAddrsHandler is not cycle collected, so it would either need to be made cycle-collected, or it would need to use a weak ref instead. The latter is much easier, since PeerConnectionImpl can clear that weak ref when appropriate. Differential Revision: https://phabricator.services.mozilla.com/D139212
dothq · Mar 1, 2022 · ca51b4e · ca51b4e
1 parent 8d30263
commit ca51b4e
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 17 deletions.
diff --git a/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp b/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp
@@ -3217,8 +3217,12 @@ void PeerConnectionImpl::StartCallTelem() {
 void PeerConnectionImpl::StunAddrsHandler::OnMDNSQueryComplete(
     const nsCString& hostname, const Maybe<nsCString>& address) {
   MOZ_ASSERT(NS_IsMainThread());
-  auto itor = pc_->mQueriedMDNSHostnames.find(hostname.BeginReading());
-  if (itor != pc_->mQueriedMDNSHostnames.end()) {
+  PeerConnectionWrapper pcw(mPcHandle);
+  if (!pcw.impl()) {
+    return;
+  }
+  auto itor = pcw.impl()->mQueriedMDNSHostnames.find(hostname.BeginReading());
+  if (itor != pcw.impl()->mQueriedMDNSHostnames.end()) {
     if (address) {
       for (auto& cand : itor->second) {
         // Replace obfuscated address with actual address
@@ -3232,30 +3236,32 @@ void PeerConnectionImpl::StunAddrsHandler::OnMDNSQueryComplete(
           }
         }
         std::string mungedCandidate = o.str();
-        pc_->StampTimecard("Done looking up mDNS name");
-        pc_->mTransportHandler->AddIceCandidate(
+        pcw.impl()->StampTimecard("Done looking up mDNS name");
+        pcw.impl()->mTransportHandler->AddIceCandidate(
             cand.mTransportId, mungedCandidate, cand.mUfrag, obfuscatedAddr);
       }
     } else {
-      pc_->StampTimecard("Failed looking up mDNS name");
+      pcw.impl()->StampTimecard("Failed looking up mDNS name");
     }
-    pc_->mQueriedMDNSHostnames.erase(itor);
+    pcw.impl()->mQueriedMDNSHostnames.erase(itor);
   }
 }
 
 void PeerConnectionImpl::StunAddrsHandler::OnStunAddrsAvailable(
     const mozilla::net::NrIceStunAddrArray& addrs) {
   CSFLogInfo(LOGTAG, "%s: receiving (%d) stun addrs", __FUNCTION__,
              (int)addrs.Length());
-  if (pc_) {
-    pc_->mStunAddrs = addrs.Clone();
-    pc_->mLocalAddrsRequestState = STUN_ADDR_REQUEST_COMPLETE;
-    pc_->FlushIceCtxOperationQueueIfReady();
-    // If parent process returns 0 STUN addresses, change ICE connection
-    // state to failed.
-    if (!pc_->mStunAddrs.Length()) {
-      pc_->IceConnectionStateChange(dom::RTCIceConnectionState::Failed);
-    }
+  PeerConnectionWrapper pcw(mPcHandle);
+  if (!pcw.impl()) {
+    return;
+  }
+  pcw.impl()->mStunAddrs = addrs.Clone();
+  pcw.impl()->mLocalAddrsRequestState = STUN_ADDR_REQUEST_COMPLETE;
+  pcw.impl()->FlushIceCtxOperationQueueIfReady();
+  // If parent process returns 0 STUN addresses, change ICE connection
+  // state to failed.
+  if (!pcw.impl()->mStunAddrs.Length()) {
+    pcw.impl()->IceConnectionStateChange(dom::RTCIceConnectionState::Failed);
   }
 }
 

diff --git a/dom/media/webrtc/jsapi/PeerConnectionImpl.h b/dom/media/webrtc/jsapi/PeerConnectionImpl.h
@@ -617,7 +617,8 @@ class PeerConnectionImpl final
 
   class StunAddrsHandler : public net::StunAddrsListener {
    public:
-    explicit StunAddrsHandler(PeerConnectionImpl* aPc) : pc_(aPc) {}
+    explicit StunAddrsHandler(PeerConnectionImpl* aPc)
+        : mPcHandle(aPc->GetHandle()) {}
 
     void OnMDNSQueryComplete(const nsCString& hostname,
                              const Maybe<nsCString>& address) override;
@@ -626,7 +627,9 @@ class PeerConnectionImpl final
         const mozilla::net::NrIceStunAddrArray& addrs) override;
 
    private:
-    RefPtr<PeerConnectionImpl> pc_;
+    // This class is not cycle-collected, so we must avoid grabbing a strong
+    // reference.
+    const std::string mPcHandle;
     virtual ~StunAddrsHandler() {}
   };