Ask the main thread to shut down the SystemClockDriver if needed

RESOLVED FIXED in Firefox 47

Status

()

defect
P1
normal
Rank:
15
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: padenot, Assigned: padenot)

Tracking

45 Branch
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed, firefox48 fixed, firefox49 fixed)

Details

Attachments

(3 attachments)

Assignee

Description

3 years ago
No description provided.
Assignee

Comment 1

3 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 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)

Comment 6

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7c761e514be2
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Target Milestone: mozilla48 → mozilla49
Assignee

Comment 7

3 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?
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)
Assignee

Comment 10

3 years ago
This takes care of the nsRunnable -> Runnable rename.
Assignee

Comment 11

3 years ago
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.