Closed
Bug 1177399
Opened 9 years ago
Closed 9 years ago
Enable AudioChannelService in desktop by default
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla42
People
(Reporter: baku, Assigned: baku)
References
Details
Attachments
(2 files, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
We need this component for a couple of new features. Let's use this bug as a meta bug for any possible dependence.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → amarchesini
Comment 2•9 years ago
|
||
Comment on attachment 8632694 [details] [diff] [review] default.patch Review of attachment 8632694 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/app/mobile.js @@ +795,5 @@ > > +// Make <audio>, <video> and webAudio talk to the AudioChannelService. > +pref("media.useAudioChannelService", true); > +// Add Mozilla AudioChannel APIs. > +pref("media.useAudioChannelAPI", false); You should be able to remove these.
Attachment #8632694 -
Flags: review?(ehsan) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 5•9 years ago
|
||
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=11651363&repo=mozilla-inbound
Flags: needinfo?(amarchesini)
Comment 7•9 years ago
|
||
also seems https://treeherder.mozilla.org/logviewer.html#?job_id=11651196&repo=mozilla-inbound is related
Assignee | ||
Comment 8•9 years ago
|
||
It took a while to debug it properly. This patch does: 1. Unregister the actor when unlinked by CC 2. Removed part of a test because enabling the ACS, we enable the AudioChannel Policy and we check if the page has the right permission to change the audiochannel. The permission check has covered by other tests.
Flags: needinfo?(amarchesini)
Attachment #8635275 -
Flags: review?(ehsan)
Comment 9•9 years ago
|
||
Comment on attachment 8635275 [details] [diff] [review] debug.patch Review of attachment 8635275 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the below fixed. ::: dom/audiochannel/AudioChannelAgent.cpp @@ +15,5 @@ > +NS_IMPL_CYCLE_COLLECTION_CLASS(AudioChannelAgent) > + > +NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(AudioChannelAgent) > + if (tmp->mIsRegToService) { > + tmp->NotifyStoppedPlaying(); Do you mind refactoring this logic into a helper method and call it both from the Unlink function and the destructor, so that in the future we won't forget to change one but not the other?
Attachment #8635275 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 10•9 years ago
|
||
just because we have an intermittent failure for this CC thing, I split the patch in 2 parts. Point 1 of the previous comment is not in this new patch.
Attachment #8635275 -
Attachment is obsolete: true
Attachment #8635285 -
Flags: review?(ehsan)
Assignee | ||
Updated•9 years ago
|
Attachment #8635285 -
Attachment is obsolete: true
Attachment #8635285 -
Flags: review?(ehsan)
Assignee | ||
Updated•9 years ago
|
Attachment #8635275 -
Attachment is obsolete: false
Assignee | ||
Comment 11•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a459b176febd
Attachment #8635275 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Updated•9 years ago
|
Attachment #8632880 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #8632880 -
Attachment is obsolete: false
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0184189bf1c5 https://hg.mozilla.org/integration/mozilla-inbound/rev/b1b8616162b4
Keywords: checkin-needed
Assignee | ||
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/37a7604e5e1e
https://hg.mozilla.org/mozilla-central/rev/0184189bf1c5 https://hg.mozilla.org/mozilla-central/rev/b1b8616162b4 https://hg.mozilla.org/mozilla-central/rev/37a7604e5e1e
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•