Closed Bug 1267600 Opened 8 years ago Closed 8 years ago

Ask the main thread to shut down the SystemClockDriver if needed

Categories

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

45 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox47 --- fixed
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: padenot, Assigned: padenot)

References

Details

Attachments

(3 files)

      No description provided.
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/
Attachment #8745305 - Flags: review?(rjesup)
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
Attachment #8745305 - Flags: review?(rjesup) → review+
Assignee: nobody → padenot
Rank: 15
Priority: -- → P1
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
Flags: needinfo?(rjesup)
https://hg.mozilla.org/mozilla-central/rev/7c761e514be2
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Target Milestone: mozilla48 → mozilla49
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
Flags: needinfo?(rjesup)
Attachment #8746007 - Flags: approval-mozilla-beta?
Attachment #8746007 - Flags: approval-mozilla-aurora?
No longer blocks: 1221910
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.
Attachment #8746007 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Note: patch doesn't apply cleanly to Aurora; MainStreamGraphShutdownThreadRunnable has changed a lot
Flags: needinfo?(padenot)
This takes care of the nsRunnable -> Runnable rename.
Here is a patch for Aurora.
Flags: needinfo?(padenot)
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+
Attachment #8746007 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.