1267600 - Ask the main thread to shut down the SystemClockDriver if needed

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, defect, P1)

Comment 1

[Paul Adenot (:padenot)]

Attachment: MozReview Request: Bug 1267600 - Ask the main thread to shut down the SystemClockDriver if needed. r?jesup

This also factors in the OfflineAudioDriver case that was correct, but similar,
in the ThreadedDriver dtor, and implements the suggestion that was in the
comment (use .forget()).

Review commit: https://reviewboard.mozilla.org/r/48975/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48975/

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8745305 [details]
MozReview Request: Bug 1267600 - Ask the main thread to shut down the SystemClockDriver if needed.  r?jesup

https://reviewboard.mozilla.org/r/48975/#review45801

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1fa138c4519b

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for bustage.

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/caea9aa0c0cb
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=1fa138c4519b838cc540f3d4a610f128056f47c3


/builds/slave/m-in-l64-000000000000000000000/build/src/dom/media/GraphDriver.cpp:160:66: error: expected class-name before '{' token
/builds/slave/m-in-l64-000000000000000000000/build/src/dom/media/GraphDriver.cpp:186:68: error: conversion from 'mozilla::MediaStreamGraphShutdownThreadRunnable*' to non-scalar type 'nsCOMPtr<nsIRunnable>' requested
gmake[5]: *** [Unified_cpp_dom_media1.o] Error 1
gmake[4]: *** [dom/media/target] Error 2
gmake[4]: *** Waiting for unfinished jobs....
gmake[3]: *** [compile] Error 2
gmake[2]: *** [default] Error 2
gmake[1]: *** [realbuild] Error 2
gmake: *** [build] Error 2

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7c761e514be2

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7c761e514be2

Comment 7

[Paul Adenot (:padenot)]

Attachment: patch that has landed, ready for uplift

Approval Request Comment
[Feature/regressing bug #]: Apparently the initial MSG refactoring (bug 848954)

[User impact if declined]: Shutdown hang, then shutdown crash after the timeout.

[Describe test coverage new/current, TreeHerder]: This is now using the same path for normal MediaStreamGraph than what we used to do with Offline MediaStreamGraph, so this code has been exercised quite a lot. This is also removing NS_WARNINGs and NS_ENSURE_SUCCESS that were blowing up. No test added, but this is probably going to fix intermittents as well. 

[Risks and why]: Low risk, this is using what we used to do for non-real-time graphs to use for real-time graphs. The patch is simple and back be backed out cleanly, worst case.

[String/UUID change made/needed]: none

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8746007 [details] [diff] [review]
patch that has landed, ready for uplift

Crash fix, let's uplift to Aurora48 today and let it bake a few days before uplifting to Beta.

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Note: patch doesn't apply cleanly to Aurora; MainStreamGraphShutdownThreadRunnable has changed a lot

Comment 10

[Paul Adenot (:padenot)]

Attachment: Patch rebased for aurora

This takes care of the nsRunnable -> Runnable rename.

Comment 11

[Paul Adenot (:padenot)]

Here is a patch for Aurora.

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c81c8dd00dcc

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8746007 [details] [diff] [review]
patch that has landed, ready for uplift

Crash fix, this has baked a bit in Nightly and then in Aurora, Beta47+

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/4b58059be1b9