Backed out 2 changesets (bug 1512958) for frequently failing mda at /dom/media/MediaDecoderStateMachine.cpp a=backout
authorCoroiu Cristina <ccoroiu@mozilla.com>
Sat, 22 Dec 2018 06:14:56 +0200
changeset 451803 66865fdea5afa8c7613300502f6f1a33d704754a
parent 451802 42d4ad944a37049139e65a175a9b9c0961a3e3b9
child 451808 324627be0cbc1632386eda812585da9c14fb5e70
child 451812 1909ebb2e51fe6ba267f39eb6a9ce311fb40f821
push id35255
push userccoroiu@mozilla.com
push dateSat, 22 Dec 2018 04:15:16 +0000
treeherdermozilla-central@66865fdea5af [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1512958
milestone66.0a1
backs out74101900e7d484cc9ddcba2cd867ca172b961ea0
1d4816ef6e013b44000d5c633245ec8effef810d
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
Backed out 2 changesets (bug 1512958) for frequently failing mda at /dom/media/MediaDecoderStateMachine.cpp a=backout Backed out changeset 74101900e7d4 (bug 1512958) Backed out changeset 1d4816ef6e01 (bug 1512958)
dom/media/mediasink/DecodedStream.cpp
dom/media/mediasink/DecodedStream.h
dom/media/test/mochitest.ini
dom/media/test/test_bug1512958.html
--- a/dom/media/mediasink/DecodedStream.cpp
+++ b/dom/media/mediasink/DecodedStream.cpp
@@ -56,50 +56,52 @@ class DecodedStreamGraphListener {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodedStreamGraphListener)
  public:
   DecodedStreamGraphListener(
       SourceMediaStream* aStream, TrackID aAudioTrackID,
       MozPromiseHolder<DecodedStream::EndedPromise>&& aAudioEndedHolder,
       TrackID aVideoTrackID,
       MozPromiseHolder<DecodedStream::EndedPromise>&& aVideoEndedHolder,
       AbstractThread* aMainThread)
-      : mAudioTrackListener(IsTrackIDExplicit(aAudioTrackID)
+      : mMutex("DecodedStreamGraphListener::mMutex"),
+        mAudioTrackListener(IsTrackIDExplicit(aAudioTrackID)
                                 ? MakeRefPtr<DecodedStreamTrackListener>(
                                       this, aStream, aAudioTrackID)
                                 : nullptr),
-        mAudioTrackID(aAudioTrackID),
-        mAudioEndedHolder(std::move(aAudioEndedHolder)),
         mVideoTrackListener(IsTrackIDExplicit(aVideoTrackID)
                                 ? MakeRefPtr<DecodedStreamTrackListener>(
                                       this, aStream, aVideoTrackID)
                                 : nullptr),
+        mAudioTrackID(aAudioTrackID),
+        mAudioEndedHolder(std::move(aAudioEndedHolder)),
         mVideoTrackID(aVideoTrackID),
         mVideoEndedHolder(std::move(aVideoEndedHolder)),
-        mStream(aStream),
         mAbstractMainThread(aMainThread) {
-    MOZ_ASSERT(NS_IsMainThread());
     if (mAudioTrackListener) {
-      mStream->AddTrackListener(mAudioTrackListener, mAudioTrackID);
+      aStream->AddTrackListener(mAudioTrackListener, mAudioTrackID);
     } else {
       mAudioEndedHolder.ResolveIfExists(true, __func__);
     }
 
     if (mVideoTrackListener) {
-      mStream->AddTrackListener(mVideoTrackListener, mVideoTrackID);
+      aStream->AddTrackListener(mVideoTrackListener, mVideoTrackID);
     } else {
       mVideoEndedHolder.ResolveIfExists(true, __func__);
     }
   }
 
-  void NotifyOutput(TrackID aTrackID, StreamTime aCurrentTrackTime) {
+  void NotifyOutput(const RefPtr<SourceMediaStream>& aStream, TrackID aTrackID,
+                    StreamTime aCurrentTrackTime) {
     if (aTrackID != mAudioTrackID && mAudioTrackID != TRACK_NONE) {
       // Only audio playout drives the clock forward, if present.
       return;
     }
-    mOnOutput.Notify(mStream->StreamTimeToMicroseconds(aCurrentTrackTime));
+    if (aStream) {
+      mOnOutput.Notify(aStream->StreamTimeToMicroseconds(aCurrentTrackTime));
+    }
   }
 
   TrackID AudioTrackID() const { return mAudioTrackID; }
 
   TrackID VideoTrackID() const { return mVideoTrackID; }
 
   void DoNotifyTrackEnded(TrackID aTrackID) {
     MOZ_ASSERT(NS_IsMainThread());
@@ -108,63 +110,59 @@ class DecodedStreamGraphListener {
     } else if (aTrackID == mVideoTrackID) {
       mVideoEndedHolder.ResolveIfExists(true, __func__);
     } else {
       MOZ_CRASH("Unexpected track id");
     }
   }
 
   void Forget() {
-    MOZ_ASSERT(NS_IsMainThread());
-
-    if (mAudioTrackListener && !mStream->IsDestroyed()) {
-      mStream->EndTrack(mAudioTrackID);
-      mStream->RemoveTrackListener(mAudioTrackListener, mAudioTrackID);
-    }
+    RefPtr<DecodedStreamGraphListener> self = this;
+    mAbstractMainThread->Dispatch(
+        NS_NewRunnableFunction("DecodedStreamGraphListener::Forget", [self]() {
+          MOZ_ASSERT(NS_IsMainThread());
+          self->mAudioEndedHolder.ResolveIfExists(false, __func__);
+          self->mVideoEndedHolder.ResolveIfExists(false, __func__);
+        }));
+    MutexAutoLock lock(mMutex);
     mAudioTrackListener = nullptr;
-    mAudioEndedHolder.ResolveIfExists(false, __func__);
-
-    if (mVideoTrackListener && !mStream->IsDestroyed()) {
-      mStream->EndTrack(mVideoTrackID);
-      mStream->RemoveTrackListener(mVideoTrackListener, mVideoTrackID);
-    }
     mVideoTrackListener = nullptr;
-    mVideoEndedHolder.ResolveIfExists(false, __func__);
   }
 
   MediaEventSource<int64_t>& OnOutput() { return mOnOutput; }
 
  private:
   ~DecodedStreamGraphListener() {
     MOZ_ASSERT(mAudioEndedHolder.IsEmpty());
     MOZ_ASSERT(mVideoEndedHolder.IsEmpty());
   }
 
   MediaEventProducer<int64_t> mOnOutput;
 
+  Mutex mMutex;
+  // Members below are protected by mMutex.
+  RefPtr<DecodedStreamTrackListener> mAudioTrackListener;
+  RefPtr<DecodedStreamTrackListener> mVideoTrackListener;
   // Main thread only.
-  RefPtr<DecodedStreamTrackListener> mAudioTrackListener;
   const TrackID mAudioTrackID;
   MozPromiseHolder<DecodedStream::EndedPromise> mAudioEndedHolder;
-  RefPtr<DecodedStreamTrackListener> mVideoTrackListener;
   const TrackID mVideoTrackID;
   MozPromiseHolder<DecodedStream::EndedPromise> mVideoEndedHolder;
 
-  const RefPtr<SourceMediaStream> mStream;
   const RefPtr<AbstractThread> mAbstractMainThread;
 };
 
 DecodedStreamTrackListener::DecodedStreamTrackListener(
     DecodedStreamGraphListener* aGraphListener, SourceMediaStream* aStream,
     mozilla::TrackID aTrackID)
     : mGraphListener(aGraphListener), mStream(aStream), mTrackID(aTrackID) {}
 
 void DecodedStreamTrackListener::NotifyOutput(MediaStreamGraph* aGraph,
                                               StreamTime aCurrentTrackTime) {
-  mGraphListener->NotifyOutput(mTrackID, aCurrentTrackTime);
+  mGraphListener->NotifyOutput(mStream, mTrackID, aCurrentTrackTime);
 }
 
 void DecodedStreamTrackListener::NotifyEnded() {
   mStream->Graph()->DispatchToMainThreadStableState(
       NewRunnableMethod<mozilla::TrackID>(
           "DecodedStreamGraphListener::DoNotifyTrackEnded", mGraphListener,
           &DecodedStreamGraphListener::DoNotifyTrackEnded, mTrackID));
 }
@@ -447,28 +445,30 @@ bool DecodedStream::IsPlaying() const {
 }
 
 void DecodedStream::Shutdown() {
   AssertOwnerThread();
   mPrincipalHandle.DisconnectIfConnected();
   mWatchManager.Shutdown();
 }
 
-void DecodedStream::DestroyData(UniquePtr<DecodedStreamData>&& aData) {
+void DecodedStream::DestroyData(UniquePtr<DecodedStreamData> aData) {
   AssertOwnerThread();
 
   if (!aData) {
     return;
   }
 
   mOutputListener.Disconnect();
 
-  NS_DispatchToMainThread(
-      NS_NewRunnableFunction("DecodedStream::DestroyData",
-                             [data = std::move(aData)]() { data->Forget(); }));
+  DecodedStreamData* data = aData.release();
+  data->Forget();
+  nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction("DecodedStream::DestroyData",
+                                                   [=]() { delete data; });
+  NS_DispatchToMainThread(r.forget());
 }
 
 void DecodedStream::SetPlaying(bool aPlaying) {
   AssertOwnerThread();
 
   // Resume/pause matters only when playback started.
   if (mStartTime.isNothing()) {
     return;
--- a/dom/media/mediasink/DecodedStream.h
+++ b/dom/media/mediasink/DecodedStream.h
@@ -71,17 +71,17 @@ class DecodedStream : public MediaSink {
 
  protected:
   virtual ~DecodedStream();
 
  private:
   media::TimeUnit FromMicroseconds(int64_t aTime) {
     return media::TimeUnit::FromMicroseconds(aTime);
   }
-  void DestroyData(UniquePtr<DecodedStreamData>&& aData);
+  void DestroyData(UniquePtr<DecodedStreamData> aData);
   void SendAudio(double aVolume, bool aIsSameOrigin,
                  const PrincipalHandle& aPrincipalHandle);
   void SendVideo(bool aIsSameOrigin, const PrincipalHandle& aPrincipalHandle);
   StreamTime SentDuration();
   void SendData();
   void NotifyOutput(int64_t aTime);
 
   void AssertOwnerThread() const {
--- a/dom/media/test/mochitest.ini
+++ b/dom/media/test/mochitest.ini
@@ -762,19 +762,16 @@ skip-if = (android_version == '23' && de
 skip-if = android_version >= '19' # bug 1198168, android(bug 1232305)
 tags=capturestream
 [test_bug1242338.html]
 skip-if = toolkit == 'android' # bug 1306916, bug 1329566, android(bug 1232305)
 [test_bug1242594.html]
 [test_bug1248229.html]
 skip-if = android_version == '17' # bug 1306917, 1323778, android(bug 1232305)
 tags=capturestream
-[test_bug1512958.html]
-skip-if = toolkit == 'android' # android(bug 1232305)
-tags=msg capturestream
 [test_can_play_type.html]
 skip-if = (android_version == '23' && debug) || (android_version == '25' && debug) # android(bug 1232305)
 [test_can_play_type_mpeg.html]
 skip-if = (android_version == '23' && debug) || (android_version == '25' && debug) # android(bug 1232305)
 [test_can_play_type_no_ogg.html]
 skip-if = (android_version == '23' && debug) || (android_version == '25' && debug) # android(bug 1232305)
 [test_can_play_type_ogg.html]
 skip-if = (android_version == '23' && debug) || (android_version == '25' && debug) # android(bug 1232305)
deleted file mode 100644
--- a/dom/media/test/test_bug1512958.html
+++ /dev/null
@@ -1,74 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <title>Test that pausing and resuming a captured media element with audio doesn't stall</title>
-  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
-  <script type="text/javascript" src="manifest.js"></script>
-</head>
-<body>
-<audio id="a"></audio>
-<pre id="test">
-<script class="testbody" type="text/javascript">
-function dumpEvent({target, type}) {
-  info(`${target.name} GOT EVENT ${type} currentTime=${target.currentTime} ` +
-       `paused=${target.paused} ended=${target.ended} ` +
-       `readyState=${target.readyState}`);
-}
-
-function wait(ms) {
-  return new Promise(resolve => setTimeout(resolve, ms));
-}
-
-const a = document.getElementById('a');
-
-const events = ["timeupdate", "seeking", "seeked", "ended", "playing", "pause"];
-for (let ev of events) {
-  a.addEventListener(ev, dumpEvent);
-}
-
-(async _ => {
-  try {
-    SimpleTest.waitForExplicitFinish();
-    SimpleTest.requestFlakyTimeout("Timeouts for shortcutting test-timeout");
-
-    const test = getPlayableAudio(gTrackTests.filter(t => t.duration > 2));
-    if (!test) {
-      todo(false, "No playable audio");
-      return;
-    }
-
-    // Start playing and capture
-    a.src = test.name;
-    a.name = test.name;
-    const ac = new AudioContext();
-    const src = ac.createMediaElementSource(a);
-    a.play();
-    do {
-      await new Promise(r => a.ontimeupdate = r);
-    } while(a.currentTime == 0)
-
-    // Pause to trigger recreating tracks in DecodedStream
-    a.pause();
-    await new Promise(r => a.onpause = r);
-
-    // Resuming should now work. Bug 1512958 would cause a stall because the
-    // original track wasn't ended and we'd block on it.
-    // See https://bugzilla.mozilla.org/show_bug.cgi?id=1512958#c5
-    a.play();
-    await new Promise(r => a.onplaying = r);
-    a.currentTime = test.duration - 1;
-    await Promise.race([
-      new Promise(res => a.onended = res),
-      wait(30000).then(_ => Promise.reject(new Error("Timeout"))),
-    ]);
-  } catch(e) {
-    ok(false, e);
-  } finally {
-    SimpleTest.finish();
-  }
-})();
-</script>
-</pre>
-</body>
-</html>