Closed
Bug 1157476
Opened 10 years ago
Closed 10 years ago
Remove synchronous dispatch in AndroidMediaResourceServer::Start
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: bholley, Assigned: bholley)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
8.00 KB,
patch
|
cajbir
:
review+
|
Details | Diff | Splinter Review |
This seems to run on the main thread anyway, and I think we can guarantee that we always initialize this thing on the main thread before using it off-main-thread. I'm hacking around it for now in bug 1144486, but we should fix it up properly.
Is there any reason not to do this at startup in MediaDecoder::InitStatics?
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(cajbir.bugzilla)
Assignee | ||
Comment 1•10 years ago
|
||
On IRC, cajbir and I decided we could do this in the MediaDecoder constructor.
Flags: needinfo?(cajbir.bugzilla)
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #2)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=eede6af36b0c
https://treeherder.mozilla.org/#/jobs?repo=try&revision=545486190bc7
Green.
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8597583 -
Flags: review?(cajbir.bugzilla)
Comment 5•10 years ago
|
||
Comment on attachment 8597583 [details] [diff] [review]
Remove synchronous dispatch in AndroidMediaResourceServer::Start. v1
Review of attachment 8597583 [details] [diff] [review]:
-----------------------------------------------------------------
r+ if you've tested that this works on physical hardware.
Attachment #8597583 -
Flags: review?(cajbir.bugzilla) → review+
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to cajbir (:cajbir) from comment #5)
> r+ if you've tested that this works on physical hardware.
I'm not in a position to do that easily. Can you suggest someone who's familiar with the feature who can do it?
This passes the robocop tests that exercise this codepath, FWIW.
Flags: needinfo?(cajbir.bugzilla)
Assignee | ||
Comment 7•10 years ago
|
||
And in particular, the failure mode here is going to be a very clear MOZ_CRASH (when the server is used without having been initialized), which is unlikely to go unnoticed.
Comment 8•10 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #6)
>
> This passes the robocop tests that exercise this codepath, FWIW.
If the codepath is being tested via robocop that's fine.
Flags: needinfo?(cajbir.bugzilla)
Assignee | ||
Comment 9•10 years ago
|
||
Assignee: nobody → bobbyholley
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•