Closed Bug 1196358 Opened 9 years ago Closed 9 years ago

[B2G] Volume setting is wrong after reboot the phone

Categories

(Firefox OS Graveyard :: Gaia::System::Audio Mgmt, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
FxOS-S8 (02Oct)
Tracking Status
firefox44 --- fixed

People

(Reporter: NicholasN, Assigned: alwu)

References

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

Description:
The test fails after the device opens the power menu. According to the assertion it is failing to find "Ring incoming calls" in the list of options. It's possible that the test should have changed the state of the phone prior to opening the menu, so that it would begin with calls silenced, and "Ring incoming calls" would appear in the list.

Repro Steps:
1) Update phone to 20150819030215
2) Long tap the power button.
3) Observe listed options.

Actual:
Device fails to find "Ring incoming calls" in power down menu.

Expected:
Device finds all options correctly.
Traceback (most recent call last):
File "/var/lib/jenkins/jobs/flame-kk.ui.adhoc.bitbar/workspace/.env/lib/python2.7/site-packages/marionette_client-0.18-py2.7.egg/marionette/marionette_test.py", line 296, in run
testMethod()
File "/var/lib/jenkins/jobs/flame-kk.ui.adhoc.bitbar/workspace/tests/python/gaia-ui-tests/gaiatest/tests/functional/system/test_power_button_long_press.py", line 28, in test_power_button_long_press
self.assertIn(item, sleep_menu_items)
AssertionError: 'Ring incoming calls' not found in [u'Turn on airplane mode', u'Silence incoming calls', u'Restart', u'Power off']


Environmental Variables:
Device firmware (base)     L1TC000118D0
Device firmware (date)     19 Aug 2015 10:36:31
Device firmware (incremental)     eng.cltbld.20150819.063618
Device firmware (release)     4.4.2
Device identifier     flame
Device memory     219772 kB
Device serial     94f7ce17
Device uptime     0 days 0 hours 2 minutes 12 seconds
Gaia date     18 Aug 2015 16:54:35
Gaia revision     1e1197e0e8e6
Gecko build     20150819030215
Gecko revision     f384789a29dcfd514d25d4a16a97ec5309612d78
Gecko version     43.0a1

Reproducible manually: NO

Repro frequency: 5/5
Assignee: nobody → martijn.martijn
The problem is that this doesn't seem to work anymore: http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/gaia_test.py#24
 'audio.volume.notification': 0,  # mute audio notifications
Instead, this settings seems to be 8.

I think there is a real bug here.
Str:
- Press the volume down key, until the sound is completely 0. Make sure that in Settings->Ringtones & Notifications the volume is now shown to be 0, too.
- Restart the phone.

Expected result:
- The volume for notifications is still 0, in Settings->Ringtones & Notifications, the volume is 0.

Actual result:
- The volume for noticications is 8 again, in Settings->Ringtones & Notifications, the volume is also 8.

I can reproduce this with:
Build ID               20150819030215
Gaia Revision          1e1197e0e8e64307aa382ffba4711d1c661de7ca
Gaia Date              2015-08-18 16:54:35
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/f384789a29dcfd514d25d4a16a97ec5309612d78
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150727.063909
Firmware Date          Mon Jul 27 06:39:20 EDT 2015
Bootloader             L1TC000118D0

I couldn't reproduce with:
Build ID               20150818030206
Gaia Revision          d9d99f32762975a370f1abd34a3512bd6fe29111
Gaia Date              2015-08-17 16:47:27
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/90d9b7c391d38ae118865bd87b5d011feee6dded
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150727.063909
Firmware Date          Mon Jul 27 06:39:20 EDT 2015
Bootloader             L1TC000118D0
Assignee: martijn.martijn → nobody
Component: Gaia::UI Tests → Gaia::System::Audio Mgmt
Oh, I think this is a regression from bug 1179181. Alastor, can you take a look at this?
Blocks: 1179181
Flags: needinfo?(alwu)
Hi, Martin,
I think you pressed the volume button too early after reboot the phone, if you wait for a little bit time and then adjust the volume again, you can find that the volume is correct.

The reason why we need to wait for a little time is that Gaia fetch the volume from the database too slowly [1]. If Gaia can do it early, it would be perfect.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/sound_manager.js#L719
Flags: needinfo?(alwu)
In addition, in Gecko, if it doesn't have the correct volume data in database, we would reset it using the default value [1]. I think that is why you get the "8" for the notification.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/AudioManager.cpp#88
Hi, Martin,
If the purpose of this test case is to check the default volume value, maybe we should change the test value to the fit the Gecko's default value?
Flags: needinfo?(martijn.martijn)
(In reply to Alastor Wu [:alwu] from comment #6)
> Hi, Martin,
> If the purpose of this test case is to check the default volume value, maybe
> we should change the test value to the fit the Gecko's default value?

Currently it's not. It's testing the case where the volume for notification is 0.

Regarding the str from comment 1, when I wait 2 minutes before restarting the phone (after I set the notification sound to 0), I still get the default value back.
That's clearly a bug, no?
Flags: needinfo?(martijn.martijn) → needinfo?(alwu)
Attached file Log : volume setting (obsolete) —
Hi, Martin,
Could you describe more detailed about what is the testing do?

In addition, the reproducing steps in comment1 doesn't work on my device. The log displays the time that Gaia gets the volume data from the database, and the volume setting still keep the same after the phone reboots.

---

Here is my testing device,
Gaia-Rev        250c2f7af4ab4b55d7934f87562b0d0187bcb85f
Gecko-Rev       258464:29b2df16e961
Build-ID        20150820104421
Version         43.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.alastor.20150318.121016
FW-Date         三  3月 18 12:10:32 CST 2015
Bootloader      L1TC00011880
Flags: needinfo?(alwu) → needinfo?(martijn.martijn)
Attached file Log : volume setting
Add the time when we reboot the phone.
Attachment #8650376 - Attachment is obsolete: true
Oh, I found some errors in Gecko, checking for more details.
Serial: 807fd0eb (State: device)
Build ID               20150819030215
This is my testing device:
Gaia Revision          1e1197e0e8e64307aa382ffba4711d1c661de7ca
Gaia Date              2015-08-18 16:54:35
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/f384789a29dcfd514d25d4a16a97ec5309612d78
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150727.063909
Firmware Date          Mon Jul 27 06:39:20 EDT 2015
Bootloader             L1TC000118D0

Setting the audio to 0:
W/AudioFlinger(  201): Thread AudioOut_2 cannot connect to the power manager service
W/AudioFlinger(  201): Thread AudioOut_2 cannot connect to the power manager service
E/AudioFlinger(  201): no wake lock to update!
V/WLAN_PSA(  216): NL MSG, len[048], NL type[0x11] WNI type[0x5050] len[028]
D/audio_hw_primary(  201): out_set_parameters: enter: usecase(0: deep-buffer-playback) kvpairs: fm_volume=0.0000000000
D/audio_hw_extn(  201): audio_extn_set_anc_parameters: anc_enabled:0
D/audio_hw_fm(  201): audio_extn_fm_set_parameters: set_fm_volume usecase
D/audio_hw_fm(  201): fm_set_volume: (0.000000)

On restart, I see this, what might be useful info:
I/AudioFlinger(  202): AudioFlinger's thread 0xb5cff008 ready to run
W/AudioFlinger(  202): Thread AudioOut_2 cannot connect to the power manager service
D/audio_hw_primary(  202): out_set_parameters: enter: usecase(0: deep-buffer-playback) kvpairs: routing=2
D/audio_hw_extn(  202): audio_extn_set_anc_parameters: anc_enabled:0
W/AudioFlinger(  202): Thread AudioOut_2 cannot connect to the power manager service
W/Gonk    (  206): Could not open boot animation
W/Gonk    (  206): Show solid color frame for bootAnim
W/AudioFlinger(  202): Thread AudioOut_2 cannot connect to the power manager service
E/AudioFlinger(  202): no wake lock to update!
I/audio_a2dp_hw(  202): adev_open:  adev_open in A2dp_hw module
I/AudioFlinger(  202): loadHwModule() Loaded a2dp audio interface from A2DP Audio HW HAL (audio) handle 3
I/AudioFlinger(  202): loadHwModule() Loaded usb audio interface from USB audio HW HAL (audio) handle 4
I/r_submix(  202): adev_open(name=audio_hw_if)
I/r_submix(  202): adev_init_check()
I/AudioFlinger(  202): loadHwModule() Loaded r_submix audio interface from Wifi Display audio HAL (audio) handle 5
I/AudioPolicyService(  202): Loaded audio policy from LEGACY Audio Policy HAL (audio_policy)
Flags: needinfo?(martijn.martijn)
Assignee: nobody → alwu
Bug 1196358 - update volume setting to database when the volume changing.
Comment on attachment 8650876 [details]
MozReview Request: Bug 1196358 - update volume setting to database when the volume changing. r=sotaro.

Hi, Baku,
This bug is resulted from bug1179181, could you help me review this issue again?

In previous design, I update the volume setting to database in two situation,
(1) Switch the different devices
(2) AudioManager die

However, it seems that the update in (2) can't be done successfully. I guess the SettingService maybe be closed before the AudioManger. 

Therefore, I need to update the volume setting when the volume is changed.

Very appreciate!!!!

---

Try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec3888496767
Attachment #8650876 - Flags: review?(amarchesini)
No longer blocks: 1179181
Depends on: 1179181
Summary: test_power_button_long_press.py: "AssertionError: 'Ring incoming calls' not found in [u'Turn on airplane mode', u'Silence incoming calls', u'Restart', u'Power off']" → [B2G] Volume setting is wrong after reboot the phone
Comment on attachment 8650876 [details]
MozReview Request: Bug 1196358 - update volume setting to database when the volume changing. r=sotaro.

It seems that we got some test case fails, need modification.
Attachment #8650876 - Flags: review?(amarchesini)
Alastor, are you still working on this? Otherwise, I guess we need to disable our tests that are affected by this, for now.
Flags: needinfo?(alwu)
Hi, Martin,
Could we disable the test first?
Because now I got the test case fails, I still need to figure out what is the problem.
Thanks!
Flags: needinfo?(alwu) → needinfo?(martijn.martijn)
Comment on attachment 8653560 [details] [review]
[gaia] mwargers:1196358_disable > mozilla-b2g:master

Merged this: https://github.com/mozilla-b2g/gaia/commit/628a4c4a0ca74fad7107883e1832527293e0575e
This disabled test_cleanup_gaia.py and test_power_button_long_press.py
Flags: needinfo?(martijn.martijn)
New try-server result, fix the fails.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2502a3e10b6e
This issue is that we don't change the value in setting service when we reboot the phone.

The present design is to send the volume changing request in AudioManager's dtor. However, it seems that the setting service can't receive it at that time. Therefore, I want to send the request everytime when we change the volume.
Comment on attachment 8650876 [details]
MozReview Request: Bug 1196358 - update volume setting to database when the volume changing. r=sotaro.

Bug 1196358 - update volume setting to database when the volume changing.
Attachment #8650876 - Flags: review?(sotaro.ikeda.g)
https://reviewboard.mozilla.org/r/16795/#review16905

I have one comment. Other changes looks good.

::: dom/system/gonk/AudioManager.h:24
(Diff revision 3)
> +#include "nsISettingsService.h"

You might want to avoid the change by adding "class nsISettingsServiceLock;" to header file.
Comment on attachment 8650876 [details]
MozReview Request: Bug 1196358 - update volume setting to database when the volume changing. r=sotaro.

Bug 1196358 - update volume setting to database when the volume changing.
Comment on attachment 8650876 [details]
MozReview Request: Bug 1196358 - update volume setting to database when the volume changing. r=sotaro.

https://reviewboard.mozilla.org/r/16797/#review17949
Attachment #8650876 - Flags: review?(sotaro.ikeda.g) → review+
Try-server result is on the comment23.
Keywords: checkin-needed
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=2848628&repo=b2g-inbound
Flags: needinfo?(alwu)
Attachment #8650876 - Attachment description: MozReview Request: Bug 1196358 - update volume setting to database when the volume changing. → MozReview Request: Bug 1196358 - update volume setting to database when the volume changing. r=sotaro.
Attachment #8650876 - Flags: review+
Comment on attachment 8650876 [details]
MozReview Request: Bug 1196358 - update volume setting to database when the volume changing. r=sotaro.

Bug 1196358 - update volume setting to database when the volume changing. r=sotaro.
Flags: needinfo?(alwu)
Keywords: checkin-needed
Hi, this don't apply to b2g-inbound cleanly:

applying c7b9f6443a6e
patching file dom/system/gonk/AudioManager.cpp
Hunk #4 FAILED at 1116
1 out of 5 hunks FAILED -- saving rejects to file dom/system/gonk/AudioManager.cpp.rej

can you take a look, thanks!
Flags: needinfo?(alwu)
Hi, Carsten,
Could you try it again? 
Because I don't have any conflicts when I apply this patch into latest Gecko (264360:5abe3c4deab9).
Thanks!
Flags: needinfo?(alwu) → needinfo?(cbook)
Comment on attachment 8650876 [details]
MozReview Request: Bug 1196358 - update volume setting to database when the volume changing. r=sotaro.

Bug 1196358 - update volume setting to database when the volume changing. r=sotaro.
I can see the conflicts now, rebase it and request check-in again.
Thanks!
Flags: needinfo?(cbook)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/62392cb2aeee
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S8 (02Oct)
Hi, Martin,
I think we can re-enable the test now :)
Thanks!
Flags: needinfo?(martijn.martijn)
Alastor, I still get the same failure when trying to run test_cleanup_gaia.py. I did flash the base image as well as gaia+gecko:
Build ID               20151001150204
Gaia Revision          bd8ff00faac97ad6a2df5a6217910b8d295d56a3
Gaia Date              2015-10-01 06:59:53
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/2c1fb007137dcb68b1862a79553b53f1a34c99c3
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151001.182807
Firmware Date          Thu Oct  1 18:28:19 EDT 2015
Bootloader             L1TC000118D0

I think your patch is already in there, right?
Flags: needinfo?(martijn.martijn) → needinfo?(alwu)
Keep NI, I'll check it later.
(In reply to Alastor Wu [:alwu] from comment #44)
> Keep NI, I'll check it later.

I filed bug 1218775 now, I NI you there.
Flags: needinfo?(alwu)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: