Bug 1295352 - Check state in MediaEngines' NotifyPull(). r=jesup draft
authorAndreas Pehrson <pehrsons@gmail.com>
Tue, 23 Aug 2016 10:45:09 +0200
changeset 406785 1ce4c822edb3a282e505423cae0235a98329f9cb
parent 406784 10323afd9ee6e0bb6e3a40b66db6ae3467460a67
child 406786 073c1c1d4f5c4b74b0170beaa0bcd9f1335a009f
push id27827
push userpehrsons@gmail.com
push dateMon, 29 Aug 2016 14:48:02 +0000
reviewersjesup
bugs1295352
milestone51.0a1
Bug 1295352 - Check state in MediaEngines' NotifyPull(). r=jesup GetEndOfAppendedData() returns null and calls a NS_ERROR() if the track we're looking for doesn't exist - to indicate an error in the caller's code. When we end a MediaEngine track we set the state to stopped, which we can use to guard the calls to GetEndOfAppendedData() (and appending data in general). The locking is already in place. MozReview-Commit-ID: DuknmBF883H
dom/media/webrtc/MediaEngineGonkVideoSource.cpp
dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
dom/media/webrtc/MediaEngineTabVideoSource.cpp
--- a/dom/media/webrtc/MediaEngineGonkVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineGonkVideoSource.cpp
@@ -69,18 +69,20 @@ MediaEngineGonkVideoSource::NotifyPull(M
                                        TrackID aID,
                                        StreamTime aDesiredTime,
                                        const PrincipalHandle& aPrincipalHandle)
 {
   VideoSegment segment;
 
   MonitorAutoLock lock(mMonitor);
   // B2G does AddTrack, but holds kStarted until the hardware changes state.
-  // So mState could be kReleased here. We really don't care about the state,
-  // though.
+  // So mState could be kReleased here.
+  if (mState != kStarted) {
+    return;
+  }
 
   // Note: we're not giving up mImage here
   RefPtr<layers::Image> image = mImage;
   StreamTime delta = aDesiredTime - aSource->GetEndOfAppendedData(aID);
   LOGFRAME(("NotifyPull, desired = %" PRIi64 ", delta = %" PRIi64 " %s",
             (int64_t) aDesiredTime, (int64_t) delta, image ? "" : "<null>"));
 
   // Bug 846188 We may want to limit incoming frames to the requested frame rate
--- a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
@@ -328,16 +328,20 @@ void
 MediaEngineRemoteVideoSource::NotifyPull(MediaStreamGraph* aGraph,
                                          SourceMediaStream* aSource,
                                          TrackID aID, StreamTime aDesiredTime,
                                          const PrincipalHandle& aPrincipalHandle)
 {
   VideoSegment segment;
 
   MonitorAutoLock lock(mMonitor);
+  if (mState != kStarted) {
+    return;
+  }
+
   StreamTime delta = aDesiredTime - aSource->GetEndOfAppendedData(aID);
 
   if (delta > 0) {
     // nullptr images are allowed
     AppendToTrack(aSource, mImage, aID, delta, aPrincipalHandle);
   }
 }
 
--- a/dom/media/webrtc/MediaEngineTabVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineTabVideoSource.cpp
@@ -208,16 +208,19 @@ MediaEngineTabVideoSource::Start(SourceM
 void
 MediaEngineTabVideoSource::NotifyPull(MediaStreamGraph*,
                                       SourceMediaStream* aSource,
                                       TrackID aID, StreamTime aDesiredTime,
                                       const PrincipalHandle& aPrincipalHandle)
 {
   VideoSegment segment;
   MonitorAutoLock mon(mMonitor);
+  if (mState != kStarted) {
+    return;
+  }
 
   // Note: we're not giving up mImage here
   RefPtr<layers::SourceSurfaceImage> image = mImage;
   StreamTime delta = aDesiredTime - aSource->GetEndOfAppendedData(aID);
   if (delta > 0) {
     // nullptr images are allowed
     gfx::IntSize size = image ? image->GetSize() : IntSize(0, 0);
     segment.AppendFrame(image.forget().downcast<layers::Image>(), delta, size,