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)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.5+, firefox43 fixed, b2g-master fixed)

RESOLVED FIXED
FxOS-S5 (21Aug)
blocking-b2g 2.5+
Tracking Status
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.
Blocks: 1183870
Attached file bug-1191207.log (obsolete) (deleted) —
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
Hi Alastor, Could you take a look? Thanks.
Flags: needinfo?(alwu)
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)
Blocks: 1130350
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
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 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-
Attached file Debug log and stack (deleted) —
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.
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)
I don't know. Please ask Fabrice/baku, or #b2g.
Flags: needinfo?(ehsan)
Hi, Baku, Do you have any suggestion about the comment6? The root cause analysis is in the comment3. Very appreciate :)
Flags: needinfo?(amarchesini)
Blocking since this blocks bug 1183870.
blocking-b2g: --- → 2.5+
(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.
Attached file Debug log and stack [on device] (deleted) —
Dump the log and stack from device.
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)
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)
(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)
Attachment #8644176 - Flags: feedback?(amarchesini) → feedback+
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)
Attachment #8644176 - Flags: review?(amarchesini) → review+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S5 (21Aug)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: