Closed Bug 1223297 Opened 9 years ago Closed 8 years ago

[Testcase] App should have the ability to playback different kinds of audio channel

Categories

(Firefox OS Graveyard :: AudioChannel, defect)

Product:
Firefox OS Graveyard
Component:
AudioChannel
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox46 fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox46 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

Details

Attachments

(1 file)

MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.
40 bytes, text/x-review-board-request
baku
: review+
Details 
When the app have the multiple audio channels, we should make sure each channel can be playback successfully. See more details in [1].

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1186135#c10
Blocks: 1223298
Now I can't get the multiple audio channel channels in the mochitest.
Even I added the permission in the manifest, the other channels (except normal) still can't be generated.
Still finding the reason...
The test can pass on my local environment, but fail on the try-server.
Still trying to find the solution...
I guess the problem might be caused by [1], we can't get the app from the AppService.
Still debug for more details.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/html/nsBrowserElement.cpp#596
This test can't be ran on the emulator, because the test of emulator is running on the content process.

Because of that, we can't get the app manifest in [1], then we can't get the app in [2].
Therefore, we would only have the "normal" type audio channel.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/html/nsGenericHTMLFrameElement.cpp#615
[2] https://dxr.mozilla.org/mozilla-central/source/dom/html/nsBrowserElement.cpp#599
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24923/diff/1-2/
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24923/diff/2-3/
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24923/diff/3-4/
Attachment #8685923 - Flags: review?(amarchesini)
Hi, Baku,
Could you help me review this patch?

It's about to test whether we can receive the "activestatechanged" event from different type audio channels within the same app, the previous bug is [1].

As emulator runs the mochitest tests on the content process, it caused that we can't get the manifest from the mozBrowerFrame [2]. Therefore, I need to use the "chrome" test. 

Very appreciate!!

---

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1186135#c10
[2] https://dxr.mozilla.org/mozilla-central/source/dom/html/nsBrowserElement.cpp?from=nsBrowserElement.cpp#561
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

https://reviewboard.mozilla.org/r/24923/#review22941

::: dom/browser-element/mochitest/browserElement_MultipleAudioChannels.js:40
(Diff revision 4)
> +function showTestInfo(aEnable) {

aOOPCEnabled

::: dom/browser-element/mochitest/browserElement_MultipleAudioChannels.js:62
(Diff revision 4)
> +function runTest(aEnable) {

aOOPCEnabled ?

::: dom/browser-element/mochitest/chrome.ini:5
(Diff revision 4)
> +  ../../../browser/base/content/test/general/audio.ogg

copy this file in this directory.

Good to have more tests!
Attachment #8685923 - Flags: review?(amarchesini)
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

https://reviewboard.mozilla.org/r/24923/#review22943
Attachment #8685923 - Flags: review+
It seems that my patch would cause the "test_browserElement_NotifyChannel.html" time-out.
Need to find the reason.
Attachment #8685923 - Attachment description: MozReview Request: Bug 1223297 - Multiple audio channel test. → MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.
Attachment #8685923 - Flags: review+
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24923/diff/4-5/
Hi, Baku,
Do you have any idea about the test case timeout?
It seems that my patch cause another chrome case timeout, but I have no idea why it happened.
These two tests can work successfully in my local emulator.
Very appreciate!
Flags: needinfo?(amarchesini)
You timeout in the following test, not in yours. This means the the iframe where you do tests is not fully closed?

Here some questions:
1. Do you remove it from the document?
2. If not, do you stop any media element before terminating your test?
Flags: needinfo?(amarchesini)
Summary: [B2G] Add the test to make sure that multiple channels can be playback under the same window → [Testcase] App should have the ability to playback different kinds of audio channel
Keywords: checkin-needed
Keywords: checkin-needed
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24923/diff/5-6/
Attachment #8685923 - Flags: review?(amarchesini)
Attachment #8685923 - Flags: review?(amarchesini) → review+
Keywords: checkin-needed
backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=3496268&repo=b2g-inbound
Flags: needinfo?(alwu)
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24923/diff/6-7/
Attachment #8685923 - Flags: review+ → review?(amarchesini)
Attachment #8685923 - Flags: review?(amarchesini) → review+
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

https://reviewboard.mozilla.org/r/24923/#review24791
Attachment #8685923 - Flags: review+
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24923/diff/7-8/
Attachment #8685923 - Flags: review+
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24923/diff/8-9/
Keywords: checkin-needed
Please do a try push with some desktop platforms too. The patch looks very fishy, because it updates chrome.ini/mochitest.ini files to move around audio.ogg, but the patch itself doesn't move that file. I strongly suspect the test_browserElement_MultipleAudioChannels.html test will fail on desktop.
Flags: needinfo?(alwu)
Keywords: checkin-needed
Hi, Kart,
The chrome.ini/mochitest.ini need "audio.ogg", so this patch also included the changeset of creating a new file.
So I don't very understand what you means.
Flags: needinfo?(bugmail.mozilla)
Keywords: checkin-needed
Ah, my bad. When I imported the patch from reviewboard it didn't have the audio.ogg file but it looks like it's a problem with importing from reviewboard. I've filed bug 1235254 for that. I'll land the patch you pushed to try.
Flags: needinfo?(bugmail.mozilla)
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/210aaacaf2b1 for Android M(4) test failure, the one in bug 1228564 didn't help:

Backout job: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=210aaacaf2b1

E.g. https://treeherder.mozilla.org/logviewer.html#?job_id=19026021&repo=mozilla-inbound

08:03:06     INFO -  51 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | iframe.mute exists
08:03:06     INFO -  52 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | iframe.unmute exists
08:03:06     INFO -  53 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | iframe.getMuted exists
08:03:06     INFO -  54 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | iframe.getVolume exists
08:03:06     INFO -  55 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | iframe.setVolume exists
08:03:06     INFO -  56 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | allowedAudioChannels exist
08:03:06     INFO -  57 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | 1 audio channel by default
08:03:06     INFO -  58 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | Correct class
08:03:06     INFO -  59 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | ac.getVolume exists
08:03:06     INFO -  60 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | ac.setVolume exists
08:03:06     INFO -  61 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | ac.getMuted exists
08:03:06     INFO -  62 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | ac.setMuted exists
08:03:06     INFO -  63 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | ac.isActive exists
08:03:06     INFO -  64 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | undefined assertion name
08:03:06     INFO -  65 INFO Test : audio-muted-by-default
08:03:06     INFO -  66 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | allowedAudioChannels exist
08:03:06     INFO -  67 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | 1 audio channel by default
08:03:06     INFO -  68 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | Correct class
08:03:06     INFO -  69 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | ac.getMuted exists
08:03:06     INFO -  70 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | ac.setMuted exists
08:03:06     INFO -  71 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | activestatechanged event received.
08:03:06     INFO -  72 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | Muted channel by default
08:03:06     INFO -  73 INFO TEST-PASS | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | Unmuted the channel.
08:03:06     INFO -  74 INFO TEST-UNEXPECTED-FAIL | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | Test timed out.
08:03:06     INFO -      reportError@SimpleTest/TestRunner.js:114:7
08:03:06     INFO -      TestRunner._checkForHangs@SimpleTest/TestRunner.js:134:7
08:03:18     INFO -  75 INFO TEST-OK | dom/browser-element/mochitest/test_browserElement_inproc_AudioChannel.html | took 312972ms
Flags: needinfo?(alwu)
Hi, Sebastian,

From the comment38, this patch should pass the tests.
Does this fail come along with landing the bug1228564 at the same time? Or get fail when only landing this bug?

Because I am guessing that the failed might be resulted by landing both this issue and bug1228564, only landing this bug should be no problem.

Thanks!
Flags: needinfo?(alwu) → needinfo?(aryx.bugmail)
After repush my patch to the try-server, now I can see that fail.
Need to investigate it.
Flags: needinfo?(aryx.bugmail)
I guess the time-out might be caused by test_browserElement_inproc_AudioChannel.html doesn't be implemented well in bug1223261.
I am trying to re-write that test to see whether we can avoid the time-out.
Depends on: 1235535
Comment on attachment 8685923 [details]
MozReview Request: Bug 1223297 - Multiple audio channel test. r=baku.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24923/diff/9-10/
Rebase.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b1f2b22e3108
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: