The first MediaEngineSource::Start() happens outside of SourceListener [1], so if it fails it still thinks the source has started. So later when stopping the MediaEngineSource it is in a bad state (`MOZ_ASSERT(mState == kStarted`), but `mState == kStopped` [2]). Note that this assert is debug only but for sanity we should get this fixed. [1] https://searchfox.org/mozilla-central/rev/a539f8f7fe288d0b4d8d6a32510be20f52bb7a0f/dom/media/MediaManager.cpp#1407 [2] https://searchfox.org/mozilla-central/rev/a539f8f7fe288d0b4d8d6a32510be20f52bb7a0f/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp#298

Comment on attachment 8951622 [details] Bug 1436694 - Make a PostTask variant that returns a MozPromise. https://reviewboard.mozilla.org/r/220902/#review227112 Lgtm.

Comment on attachment 8951623 [details] Bug 1436694 - MozPromisify device initialization and move it to SourceListener. https://reviewboard.mozilla.org/r/220904/#review227122 lgtm. ::: dom/media/MediaManager.cpp:1420 (Diff revision 1) > + auto streamError = MakeRefPtr<MediaStreamError>(window->AsInner(), *error); > + onFailure->OnError(streamError); inline?

Comment on attachment 8951623 [details] Bug 1436694 - MozPromisify device initialization and move it to SourceListener. https://reviewboard.mozilla.org/r/220904/#review227122 > inline? Not sure I follow. Inline what to where?

Comment on attachment 8951622 [details] Bug 1436694 - Make a PostTask variant that returns a MozPromise. https://reviewboard.mozilla.org/r/220902/#review228034 jya asked me about MozPromise::Private, and I got caught up doing a quick (superficial) fly-by review. :-) Just a couple of suggestions: ::: dom/media/MediaManager.cpp:2154 (Diff revision 1) > +MediaManager::PostTask(const char* aName, > + std::function<void(const RefPtr<typename MozPromiseType::Private>&)>&& aFunction) I'm not a fan of std::function, when you could instead make this a templated function in the header (in which case change `Move` into `Forward` below). ::: dom/media/MediaManager.cpp:2157 (Diff revision 1) > + auto p = MakeRefPtr<typename MozPromiseType::Private>(aName); > + MediaManager::PostTask(NS_NewRunnableFunction( > + aName, [p, func = Move(aFunction)]() { func(p); })); > + return p.get(); You could use a MozPromiseHolder instead of MozPromise::Private, see example in https://searchfox.org/mozilla-central/source/dom/media/MediaRecorder.cpp#1751-1772 .

Comment on attachment 8953060 [details] Bug 1436694 - Clarify that MediaEngineSources can be double-stopped. https://reviewboard.mozilla.org/r/222304/#review228602

Comment on attachment 8951622 [details] Bug 1436694 - Make a PostTask variant that returns a MozPromise. https://reviewboard.mozilla.org/r/220902/#review228824 ::: dom/media/MediaManager.cpp:2152 (Diff revision 2) > NS_ASSERTION(Get(), "MediaManager singleton?"); > NS_ASSERTION(Get()->mMediaThread, "No thread yet"); > Get()->mMediaThread->message_loop()->PostTask(Move(task)); > } > > +template<typename MozPromiseType, typename FuncionType> 'Funcion' -> 'Function'

Comment on attachment 8951622 [details] Bug 1436694 - Make a PostTask variant that returns a MozPromise. https://reviewboard.mozilla.org/r/220902/#review229060

Pushed by pehrsons@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ae428bfb913f Make a PostTask variant that returns a MozPromise. r=jib, r=jya https://hg.mozilla.org/integration/mozilla-inbound/rev/6c38cc382d21 MozPromisify device initialization and move it to SourceListener. r=jib https://hg.mozilla.org/integration/mozilla-inbound/rev/f5cc71d38e4a Clarify that MediaEngineSources can be double-stopped. r=padenot

Backed out for frequent assertion failures at MediaEngineWebRTCAudio.cpp: https://hg.mozilla.org/integration/mozilla-inbound/rev/6e0b46a7a0a05b1e3f1d46147a46bbe37a7f55f5 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=1aff350b83b8d504dbaa3d5d8d4fe6cd99512786&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=164350480&repo=mozilla-inbound&lineNumber=10939 [task 2018-02-26T15:24:16.209Z] 15:24:16 INFO - GECKO(1045) | [1101:WebRTCPD #2]: E/signaling [WebRTCPD #2|WebrtcAudioSessionConduit] AudioConduit.cpp:797: A/V sync: sync delta: 0ms, audio jitter delay 53ms, playout delay 0ms [task 2018-02-26T15:24:16.392Z] 15:24:16 INFO - GECKO(1045) | [1101:Main Thread]: W/signaling [main|PeerConnectionMedia] PeerConnectionMedia.cpp:1184: GetTransmitPipelinesMatching: none found for (nil) [task 2018-02-26T15:24:16.611Z] 15:24:16 INFO - GECKO(1045) | (stun/INFO) STUN-CLIENT(pgCr|IP4:172.17.0.4:36422/UDP|IP4:172.17.0.4:49302/UDP(host(IP4:172.17.0.4:36422/UDP)|candidate:0 1 UDP 2122252543 172.17.0.4 49302 typ host)): Timed out [task 2018-02-26T15:24:16.613Z] 15:24:16 INFO - GECKO(1045) | (ice/INFO) ICE-PEER(PC:1519658652948303 (id=2147483750 url=http://mochi.test:8888/tests/dom/media/tests/mochitest/test_peerConnection_verifyAudioAfter:default)/CAND-PAIR(pgCr): setting pair to state FAILED: pgCr|IP4:172.17.0.4:36422/UDP|IP4:172.17.0.4:49302/UDP(host(IP4:172.17.0.4:36422/UDP)|candidate:0 1 UDP 2122252543 172.17.0.4 49302 typ host) [task 2018-02-26T15:24:16.710Z] 15:24:16 INFO - GECKO(1045) | [1101:WebRTCPD #2]: E/signaling [WebRTCPD #2|WebrtcAudioSessionConduit] AudioConduit.cpp:797: A/V sync: sync delta: 0ms, audio jitter delay 48ms, playout delay 0ms [task 2018-02-26T15:24:17.398Z] 15:24:17 INFO - GECKO(1045) | [1101:Main Thread]: W/signaling [main|PeerConnectionMedia] PeerConnectionMedia.cpp:1184: GetTransmitPipelinesMatching: none found for (nil) [task 2018-02-26T15:24:17.757Z] 15:24:17 INFO - GECKO(1045) | --DOMWINDOW == 11 (0x7f81840c7000) [pid = 1045] [serial = 52] [outer = (nil)] [url = chrome://browser/content/webrtcIndicator.xul] [task 2018-02-26T15:24:17.758Z] 15:24:17 INFO - GECKO(1045) | --DOMWINDOW == 10 (0x7f8182f38000) [pid = 1045] [serial = 50] [outer = (nil)] [url = chrome://browser/content/webrtcIndicator.xul] [task 2018-02-26T15:24:17.969Z] 15:24:17 INFO - GECKO(1045) | Assertion failure: (aGraph->IterationEnd() == 0 && mLastAppendTime == 0) || aGraph->IterationEnd() > mLastAppendTime (Iteration time didn't advance since last append), at /builds/worker/workspace/build/src/dom/media/webrtc/MediaEngineWebRTCAudio.cpp:115 [task 2018-02-26T15:24:52.127Z] 15:24:52 INFO - GECKO(1045) | #01: mozilla::MediaEngineWebRTCMicrophoneSource::Pull [dom/media/webrtc/MediaEngineWebRTCAudio.cpp:767] [task 2018-02-26T15:24:52.128Z] 15:24:52 INFO - [task 2018-02-26T15:24:52.129Z] 15:24:52 INFO - GECKO(1045) | #02: mozilla::MediaDevice::Pull [dom/media/MediaManager.cpp:1031] [task 2018-02-26T15:24:52.130Z] 15:24:52 INFO - [task 2018-02-26T15:24:52.132Z] 15:24:52 INFO - GECKO(1045) | #03: mozilla::SourceListener::NotifyPull [dom/media/MediaManager.cpp:4251]

Pushed by pehrsons@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/59f19fc034cf Make a PostTask variant that returns a MozPromise. r=jib, r=jya https://hg.mozilla.org/integration/mozilla-inbound/rev/0451fe123f5b MozPromisify device initialization and move it to SourceListener. r=jib https://hg.mozilla.org/integration/mozilla-inbound/rev/04b165b560de Clarify that MediaEngineSources can be double-stopped. r=padenot

https://hg.mozilla.org/mozilla-central/rev/59f19fc034cf https://hg.mozilla.org/mozilla-central/rev/0451fe123f5b https://hg.mozilla.org/mozilla-central/rev/04b165b560de