Closed
Bug 1160064
Opened 9 years ago
Closed 9 years ago
Give WatchManager an owner thread and manage lifetimes better
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: bholley, Assigned: bholley)
References
Details
Attachments
(5 files)
2.17 KB,
patch
|
jwwang
:
review+
|
Details | Diff | Splinter Review |
8.75 KB,
patch
|
jwwang
:
review+
|
Details | Diff | Splinter Review |
7.24 KB,
patch
|
jwwang
:
review+
|
Details | Diff | Splinter Review |
8.21 KB,
patch
|
jwwang
:
review+
|
Details | Diff | Splinter Review |
6.23 KB,
patch
|
jwwang
:
review+
|
Details | Diff | Splinter Review |
Followup from bug 1159558.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8600177 -
Flags: review?(jwwang)
Assignee | ||
Comment 2•9 years ago
|
||
The goal here is to hoist all meaningful watcher/mirror/canonical manipulation onto the owner thread. But since that must necessarily happen asynchronously, we need to make sure that canonicals are in a sane state immediately upon creation, since otherwise a mirror from another thread may attempt to connect to a not-yet-initialized canonical.
Attachment #8600178 -
Flags: review?(jwwang)
Assignee | ||
Comment 3•9 years ago
|
||
We take this as an opportunity to remove connect-during-initialization. Always connecting from the owner thread feels like a stronger invariant.
Attachment #8600179 -
Flags: review?(jwwang)
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8600180 -
Flags: review?(jwwang)
Assignee | ||
Comment 5•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c932d5fdaaed
Updated•9 years ago
|
Attachment #8600177 -
Flags: review?(jwwang) → review+
Updated•9 years ago
|
Attachment #8600178 -
Flags: review?(jwwang) → review+
Assignee | ||
Comment 6•9 years ago
|
||
The patches in this bug can cause us to log things before some of the Ensure*Log macros are called. Let's just do this right.
Attachment #8600210 -
Flags: review?(jwwang)
Assignee | ||
Comment 7•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=904b8ab90fd4
Updated•9 years ago
|
Attachment #8600179 -
Flags: review?(jwwang) → review+
Updated•9 years ago
|
Attachment #8600210 -
Flags: review?(jwwang) → review+
Comment 8•9 years ago
|
||
Comment on attachment 8600180 [details] [diff] [review] Part 4 - Give WatchManager an owner thread and make sure everything happens there. v1 Review of attachment 8600180 [details] [diff] [review]: ----------------------------------------------------------------- Good!
Attachment #8600180 -
Flags: review?(jwwang) → review+
Assignee | ||
Comment 9•9 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #7) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=904b8ab90fd4 https://treeherder.mozilla.org/#/jobs?repo=try&revision=904b8ab90fd4 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/65c1a77ed87c remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/931d1e1603f4 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fe579e65165e remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/6979a9e3e60a remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/924a8ed0b89c
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/65c1a77ed87c https://hg.mozilla.org/mozilla-central/rev/931d1e1603f4 https://hg.mozilla.org/mozilla-central/rev/fe579e65165e https://hg.mozilla.org/mozilla-central/rev/6979a9e3e60a https://hg.mozilla.org/mozilla-central/rev/924a8ed0b89c
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment 11•9 years ago
|
||
This breaks building when PR_LOGGING is not set due to line 130: gMediaDecoderLog = PR_NewLogModule("MediaDecoder"); Suggest wrapping that line with: #ifdef PR_LOGGING gMediaDecoderLog = PR_NewLogModule("MediaDecoder"); #endif since the extern is only defined when PR_LOGGING is defined.
Comment 12•9 years ago
|
||
I have created a new bug to track the build breakage: bug 1161186
You need to log in
before you can comment on or make changes to this bug.
Description
•