Closed
Bug 1191207
Opened 9 years ago
Closed 9 years ago
The state of an audio channel will just become inactive after go to homescreen
Categories
(Firefox OS Graveyard :: AudioChannel, defect)
Tracking
(blocking-b2g:2.5+, firefox43 fixed, b2g-master fixed)
People
(Reporter: evanxd, Assigned: alwu)
References
Details
Attachments
(3 files, 1 obsolete file)
The state of an audio channel will just become inactive after go to homescreen.
The bug only occurs in B2G desktop client. I'll add a UI test to reproduce the bug later.
Reporter | ||
Comment 1•9 years ago
|
||
STR:
1. Add the patch[1] in your gaia repository.
2. Run the UI test: `make test-integration-test TEST_FILES=apps/music/test/marionette/bug_1191207_test.js VERBOSE=1`
Excepted log:
{"window":"app://system.gaiamobile.org/index.html","level":"log","message":"bug-1191207: audiochannelstatechangedAppWindow_1_content: true","filename":"app://system.gaiamobile.org/js/audio_channel_service.js","lineNumber":58,"functionName":"._handle_audiochannelstatechanged","timeStamp":1438765315169}
{"window":"app://system.gaiamobile.org/index.html","level":"log","message":"bug-1191207: go to homescreen","filename":"app://system.gaiamobile.org/js/app_window_manager.js","lineNumber":461,"functionName":"._handle_home","timeStamp":1438765315650}
Actual log:
{"window":"app://system.gaiamobile.org/index.html","level":"log","message":"bug-1191207: audiochannelstatechangedAppWindow_1_content: true","filename":"app://system.gaiamobile.org/js/audio_channel_service.js","lineNumber":58,"functionName":"._handle_audiochannelstatechanged","timeStamp":1438765315169}
{"window":"app://system.gaiamobile.org/index.html","level":"log","message":"bug-1191207: go to homescreen","filename":"app://system.gaiamobile.org/js/app_window_manager.js","lineNumber":461,"functionName":"._handle_home","timeStamp":1438765315650}
{"window":"app://system.gaiamobile.org/index.html","level":"log","message":"bug-1191207: audiochannelstatechangedAppWindow_1_content: false","filename":"app://system.gaiamobile.org/js/audio_channel_service.js","lineNumber":58,"functionName":"._handle_audiochannelstatechanged","timeStamp":1438765315685}
The attachment is the full log.
[1]: https://github.com/evanxd/gaia/commit/6c95c5579d11d94f22c3875585442c48307b6977
Assignee | ||
Comment 3•9 years ago
|
||
The problem is that the HTMLMediaElement::WindowVolumeChanged() is called by non chrome caller and this issue ONLY happens on b2g desktop (simulator).
I guess the reason might be that the system app in b2g is not running on the chrome process, so we would get this error.
The issue is like this,
> (1) Muted by default.
In b2g, the audio would be muted by default.
Every audio would be muted by HTMLMediaElement::WindowVolumeChanged().
The |mMuted| would be set to MUTED_BY_AUDIO_CHANNEL, and then we send the active event to system app, waiting its response.
> (2) System app opens the audio
After receiving the audio active event, the system app would check whether the audio can be playback.
If so, the system app opens the audio.
The code flow of this operation is via system app to AudioChannelService, then to AudioChannelAgent and finally to the callback, WindowVolumeChanged().
In normal situation, we would go into HTMLMediaElement::UpdateChannelMuteState(), and then cancel the MUTED_BY_AUDIO_CHANNEL.
However, I think that the system app might be not in the chrome process, so we got fail on here [1].
[1] https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp#4524
Assignee: nobody → alwu
Flags: needinfo?(alwu)
Assignee | ||
Comment 4•9 years ago
|
||
I think we don't need to check whether the caller is the chrome process.
Each process have an independent AudioChannelService, and the AudioChannelService would notify the agents of that process. Finally, the agent would trigger the WindowVolumeChanged of its callback.
That means the caller might be any content process, so we don't need to check the IsCallerChrome(). Compare with IsCallerChrome(), I think whether WindowVolumeChanged is called by main thread is more important.
---
Try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf1ec9d73ef5
Attachment #8643561 -
Attachment is obsolete: true
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8644176 [details] [diff] [review]
Don't need to check whether the call is from chrome process
Hi, Ehsan,
Sorry to bother you,
Could you help me review this patch?
This issue ONLY happens in the b2g desktop, not in b2g device.
The preliminary analysis is in the comment3.
My solution proposal is in the comment4.
Very appreciate :)
Attachment #8644176 -
Flags: review?(ehsan)
Comment 6•9 years ago
|
||
Comment on attachment 8644176 [details] [diff] [review]
Don't need to check whether the call is from chrome process
Review of attachment 8644176 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/html/HTMLMediaElement.cpp
@@ -4519,5 @@
> }
>
> NS_IMETHODIMP HTMLMediaElement::WindowVolumeChanged(float aVolume, bool aMuted)
> {
> - NS_ENSURE_TRUE(nsContentUtils::IsCallerChrome(), NS_ERROR_NOT_AVAILABLE);
I think you may have misunderstood what IsCallerChrome() does. It has nothing to do with chrome vs content process. It returns true when the caller has chrome *privileges* (as in, either it's C++ code or JS code with chrome privileges), and if this patch fixes something, that is an indication that this function is called by unprivileged JS somehow, I think, which is very weird and perhaps a sign of another issue. Minusing this for now while we investigate why this is happening.
Do you mind attaching a C++ and JS stack for the problematic call, please?
Attachment #8644176 -
Flags: review?(ehsan) → review-
Assignee | ||
Comment 7•9 years ago
|
||
The test step is open the music using the music app in b2g desktop.
I set the breakpoint in HTMLMediaElement::WindowVolumeChanged(). There are three times we would call the WindowVolumeChanged(). The first one is called by chrome privileges, and the others aren't.
Assignee | ||
Comment 8•9 years ago
|
||
Hi, Ehsan,
Is possible that the system app is running on unprivileged JS in "b2g-desktop"?
Because all the code called from the system app would be regard as non-chrome.
Thanks :)
Flags: needinfo?(ehsan)
Assignee | ||
Comment 10•9 years ago
|
||
Hi, Baku,
Do you have any suggestion about the comment6?
The root cause analysis is in the comment3.
Very appreciate :)
Flags: needinfo?(amarchesini)
Comment 12•9 years ago
|
||
(In reply to Alastor Wu [:alwu] from comment #8)
> Hi, Ehsan,
> Is possible that the system app is running on unprivileged JS in
> "b2g-desktop"?
> Because all the code called from the system app would be regard as
> non-chrome.
> Thanks :)
I'm quite sure the System app runs in unprivileged JS both on devices and on B2G Desktop.
Assignee | ||
Comment 13•9 years ago
|
||
Dump the log and stack from device.
Assignee | ||
Comment 14•9 years ago
|
||
Hi, Julien,
Could you tell me more details about that?
Even if the app is the certified, would it still run in the unprivileged JS?
I guess that the issue can only exist in b2g desktop might be because the b2g desktop doesn't have out-of-process.
Very appreciate :)
Flags: needinfo?(felash)
Assignee | ||
Comment 15•9 years ago
|
||
Comment on attachment 8644176 [details] [diff] [review]
Don't need to check whether the call is from chrome process
Hi, Baku,
According to the comment12, do you think could we use this method to solve the problem?
Very appreciate!
Flags: needinfo?(amarchesini)
Attachment #8644176 -
Flags: review- → feedback?(amarchesini)
Comment 16•9 years ago
|
||
(In reply to Alastor Wu [:alwu] from comment #14)
> Hi, Julien,
> Could you tell me more details about that?
> Even if the app is the certified, would it still run in the unprivileged JS?
Yes :)
All Gaia apps are unprivileged (using the meaning of "Chrome privilege").
The System app has some special _permissions_ (I think some are even implicit) that allows to access some API that only the System app can use. But it's always unprivileged JS.
> I guess that the issue can only exist in b2g desktop might be because the
> b2g desktop doesn't have out-of-process.
Ah I can definitely assure you that the problem exists on device. See bug 1183870 and bug 1195191.
Flags: needinfo?(felash)
Updated•9 years ago
|
Attachment #8644176 -
Flags: feedback?(amarchesini) → feedback+
Assignee | ||
Comment 17•9 years ago
|
||
Comment on attachment 8644176 [details] [diff] [review]
Don't need to check whether the call is from chrome process
Hi, Baku,
According the comment14, I think this method is reasonable, so could you help me review this patch?
Because all Gaia apps are unprivileged, we don't need to check the IsCallerChrome().
Very appreciate :)
Attachment #8644176 -
Flags: review?(amarchesini)
Updated•9 years ago
|
Attachment #8644176 -
Flags: review?(amarchesini) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 18•9 years ago
|
||
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S5 (21Aug)
Updated•9 years ago
|
status-b2g-master:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•