Make MediaDecoder::CanPlayThrough run on the main thread

RESOLVED FIXED in Firefox 43

Status

()

defect
P2
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

unspecified
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(2 attachments)

No description provided.
Blocks: 1179498
Bug 1203374. Part 1 - extract the code of computing canplaythrough so it is reusable.
1. extract MediaDecoder::Statistics to its own file.
2. sort out include orders of MediaDecoderStateMachine.cpp
Attachment #8659065 - Flags: review?(jyavenard)
Bug 1203374. Part 2 - duplicate the implementation of MediaDecoder::CanPlayThrough so MDSM can call its own CanPlayThrough() on its own thread.
Attachment #8659066 - Flags: review?(jyavenard)
Comment on attachment 8659066 [details]
MozReview Request: Bug 1203374. Part 2 - duplicate the implementation of MediaDecoder::CanPlayThrough so MDSM can call its own CanPlayThrough() on its own thread.

https://reviewboard.mozilla.org/r/18749/#review16783

r=me with comment addressed.

::: dom/media/MediaDecoderStateMachine.cpp:2802
(Diff revision 1)
> +  ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());

Do you need to hold the monitor here?
after all ; GetStatistics() is thread-safe already

and IsRealTime() is only ever accessed on the MDSM task queue
Attachment #8659066 - Flags: review?(jyavenard) → review+
Comment on attachment 8659065 [details]
MozReview Request: Bug 1203374. Part 1 - extract the code of computing canplaythrough so it is reusable.

https://reviewboard.mozilla.org/r/18747/#review16785
Attachment #8659065 - Flags: review?(jyavenard) → review+
https://reviewboard.mozilla.org/r/18749/#review16783

> Do you need to hold the monitor here?
> after all ; GetStatistics() is thread-safe already
> 
> and IsRealTime() is only ever accessed on the MDSM task queue

It is just a convention that all(most) MDSM functions are run in the task queue with the monitor held. When we finish the refactoring, we will be able to remove the monitor from all functions of MDSM.

Thanks for the review.
Assignee: nobody → jwwang
Priority: -- → P2
https://hg.mozilla.org/mozilla-central/rev/d849c8c7a995
https://hg.mozilla.org/mozilla-central/rev/d27278fc308e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.