Closed Bug 1186299 Opened 4 years ago Closed 4 years ago

Consolidate AudioSink::PrepareToShutdown and AudioSink::Shutdown into one function

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

Details

Attachments

(1 file)

Since AudioSink::Shutdown is a sync function, it looks like we can move the code of AudioSink::PrepareToShutdown into AudioSink::Shutdown. Is there any reason for the 2-step shutdown for AudioSink?
Flags: needinfo?(kinetik)
The only reason is that it used to be that the caller needed to hold the decoder monitor for AudioSink::PrepareToShutdown and drop it for Shutdown because of potential reentrancy issues caused by nsIThread::Shutdown spinning the event loop inside AudioSink::Shutdown.
Flags: needinfo?(kinetik)
This should do the work. AudioSink::Shutdown would exit the monitor internally when necessary. AudioSink's API is also made more consistent for these functions are either thread-safe or requiring decoder monitor.
Assignee: nobody → jwwang
Status: NEW → ASSIGNED
Attachment #8637108 - Flags: review?(kinetik)
Attachment #8637108 - Flags: review?(kinetik) → review+
Thanks for the review!
https://hg.mozilla.org/mozilla-central/rev/3091363baf70
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.