Bug 1147857: be careful about WebRTC stats query creation r=jib
authorRandell Jesup <rjesup@jesup.org>
Fri, 27 Mar 2015 15:52:56 -0400
changeset 236191 d56c2eb468f65272e20b3e9562ef05c47be75e57
parent 236190 dcc49f2ed01607ec3e9b9955c9abb55005d52679
child 236192 fadf551562b882a3265ea0e10eee74bb028bfcd9
push id57617
push userrjesup@wgate.com
push dateFri, 27 Mar 2015 19:58:12 +0000
treeherdermozilla-inbound@d56c2eb468f6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjib
bugs1147857
milestone39.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 1147857: be careful about WebRTC stats query creation r=jib
media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
@@ -284,24 +284,30 @@ PeerConnectionCtx::EverySecondTelemetryC
   }
   MOZ_ASSERT(stsThread);
 
   nsAutoPtr<RTCStatsQueries> queries(new RTCStatsQueries);
   for (auto p = ctx->mPeerConnections.begin();
         p != ctx->mPeerConnections.end(); ++p) {
     if (p->second->HasMedia()) {
       queries->append(nsAutoPtr<RTCStatsQuery>(new RTCStatsQuery(true)));
-      p->second->BuildStatsQuery_m(nullptr, // all tracks
-                                   queries->back());
+      if (NS_WARN_IF(NS_FAILED(p->second->BuildStatsQuery_m(nullptr, // all tracks
+                                                            queries->back())))) {
+        queries->popBack();
+      } else {
+        MOZ_ASSERT(queries->back()->report);
+      }
     }
   }
-  rv = RUN_ON_THREAD(stsThread,
-                     WrapRunnableNM(&EverySecondTelemetryCallback_s, queries),
-                     NS_DISPATCH_NORMAL);
-  NS_ENSURE_SUCCESS_VOID(rv);
+  if (!queries->empty()) {
+    rv = RUN_ON_THREAD(stsThread,
+                       WrapRunnableNM(&EverySecondTelemetryCallback_s, queries),
+                       NS_DISPATCH_NORMAL);
+    NS_ENSURE_SUCCESS_VOID(rv);
+  }
 }
 #endif
 
 nsresult PeerConnectionCtx::Initialize() {
   initGMP();
 
 #ifdef MOZILLA_INTERNAL_API
   mConnectionCounter = 0;
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -2802,27 +2802,32 @@ PeerConnectionImpl::EndOfLocalCandidates
 
 #ifdef MOZILLA_INTERNAL_API
 nsresult
 PeerConnectionImpl::BuildStatsQuery_m(
     mozilla::dom::MediaStreamTrack *aSelector,
     RTCStatsQuery *query) {
 
   if (!HasMedia()) {
-    return NS_OK;
+    return NS_ERROR_UNEXPECTED;
   }
 
-  if (!mMedia->ice_ctx() || !mThread) {
-    CSFLogError(logTag, "Could not build stats query, critical components of "
-                        "PeerConnectionImpl not set.");
+  // Note: mMedia->ice_ctx() is deleted on STS thread; so make sure we grab and hold
+  // a ref instead of making multiple calls.  NrIceCtx uses threadsafe refcounting.
+  query->iceCtx = mMedia->ice_ctx();
+  if (!query->iceCtx) {
+    CSFLogError(logTag, "Could not build stats query, no ice_ctx");
+    return NS_ERROR_UNEXPECTED;
+  }
+  if (!mThread) {
+    CSFLogError(logTag, "Could not build stats query, no MainThread");
     return NS_ERROR_UNEXPECTED;
   }
 
   nsresult rv = GetTimeSinceEpoch(&(query->now));
-
   if (NS_FAILED(rv)) {
     CSFLogError(logTag, "Could not build stats query, could not get timestamp");
     return rv;
   }
 
   // We do not use the pcHandle here, since that's risky to expose to content.
   query->report = new RTCStatsReportInternalConstruct(
       NS_ConvertASCIItoUTF16(mName.c_str()),
@@ -2879,18 +2884,16 @@ PeerConnectionImpl::BuildStatsQuery_m(
       }
     } else {
       for (auto it = pipelines.begin(); it != pipelines.end(); ++it) {
         query->pipelines.AppendElement(it->second);
       }
     }
   }
 
-  query->iceCtx = mMedia->ice_ctx();
-
   // From the list of MediaPipelines, determine the set of NrIceMediaStreams
   // we are interested in.
   std::set<size_t> levelsToGrab;
   if (aSelector) {
     for (size_t p = 0; p < query->pipelines.Length(); ++p) {
       size_t level = query->pipelines[p]->level();
       levelsToGrab.insert(level);
     }