Closed
Bug 1267600
Opened 9 years ago
Closed 9 years ago
Ask the main thread to shut down the SystemClockDriver if needed
Categories
(Core :: Audio/Video: MediaStreamGraph, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: padenot, Assigned: padenot)
References
Details
Attachments
(3 files)
(deleted),
text/x-review-board-request
|
jesup
:
review+
|
Details |
(deleted),
patch
|
ritu
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•9 years ago
|
||
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 2•9 years ago
|
||
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+
Updated•9 years ago
|
Assignee: nobody → padenot
Rank: 15
Priority: -- → P1
Comment 4•9 years ago
|
||
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)
Comment 6•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7c761e514be2
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•9 years ago
|
Target Milestone: mozilla48 → mozilla49
Assignee | ||
Comment 7•9 years ago
|
||
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?
status-firefox47:
--- → affected
status-firefox48:
--- → affected
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+
Comment 9•9 years ago
|
||
Note: patch doesn't apply cleanly to Aurora; MainStreamGraphShutdownThreadRunnable has changed a lot
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+
Comment 14•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/4b58059be1b9
You need to log in
before you can comment on or make changes to this bug.
Description
•