1157121 - [B2G] The speaker status is wrong, when we resume the FM.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

[Alastor Wu [:alwu]]

Only happen on v2.1.

STR.
1. Switch routing to the speaker 
 - speaker-switch button lights is on
2. Stop FM
3. Resume FM, the routing is still out from the speaker
 - speaker-switch button lights is off (ERROR)

Comment 1

[Alastor Wu [:alwu]]

Attachment: [v2.1] Update speaker status

Wait for try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f480a0ae209

Comment 2

[jingmei.zhang]

(In reply to Alastor Wu [:alwu] from comment #1)
> Created attachment 8597078 [details] [diff] [review]
> [v2.1] Update speaker status
> 
> Wait for try-server result.
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f480a0ae209

Hi  Alastor,

We need to modify the codes in speaker manager API at the same time.

With your patch in comment1,open fm, play fm ,open speaker,go back to homescreen,fm will play in headset other than speaker ;(

Comment 3

[Alastor Wu [:alwu]]

Attachment: [v2.1] Update speaker status

Here is the revised patch.
It seems that the v2.1 try-server has some questions. 
I will ask the review after finishing the test.

Comment 4

[Alastor Wu [:alwu]]

Attachment: [v2.1] Update speaker status

Add the revised version.

Comment 5

[Alastor Wu [:alwu]]

Try msg only : https://treeherder.mozilla.org/#/jobs?repo=try&revision=40eea568143e
Try msg + patch : https://treeherder.mozilla.org/#/jobs?repo=try&revision=1562b76b95f4

Comment 6

[Alastor Wu [:alwu]]

Hi, Ryan,
On comment5, there are lots crashed on the try-server even if I only commit the test to the try command.
Do you know why? I don't think this changeset would caused such serious error.
Thanks a lots!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Try pushes without any changes are redundant. Please save resources and just look at the b2g34 tree on Treeherder.
https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g34_v2_1

The results on your Try push look fine considering that Try attempts to run everything that mozilla-central supports, which is substantially more than b2g34 does.  All you need to verify is that the suites that run on b2g34 at the link above are still passing with your patch. Bonus points for using better Try syntax to avoid running extra builds/tests in the first place.
https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices

Comment 8

[Alastor Wu [:alwu]]

In v2.1, the condition of the issue is in the description.
In v2.0/v2.2/m-c, the speaker status would be reset to the headphone.
That is all because of we forgot to check the speaker status. 

In bug1089526, I only add the checking in AudioChannelService, but forgot to add it in the AudioChannelServiceChild.

Comment 9

[Alastor Wu [:alwu]]

Attachment: Update speaker status

Comment 10

[Alastor Wu [:alwu]]

Try-server result. It seems fine, except the last one mochitest. 
However, I also saw the crash on other people's test, I think this is not resulted by my patch.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=008b181f599e

Comment 11

[Alastor Wu [:alwu]]

Comment on attachment 8599216 [details] [diff] [review]
Update speaker status

Hi, Baku,
Could you help me review this patch :)?
The details of this issue is in the comment8. 
Very appreciate!

Comment 12

[Andrea Marchesini [:baku]]

Comment on attachment 8599216 [details] [diff] [review]
Update speaker status

Review of attachment 8599216 [details] [diff] [review]:
-----------------------------------------------------------------

can you help me to bring this code to the new implementation of AudioChannelService?

::: dom/audiochannel/AudioChannelService.cpp
@@ +378,5 @@
> +   *  (1) apps in the foreground.
> +   *  (2) apps in the backgrund and inactive.
> +   *  Notice : check the state when the visible status is stable, because there
> +   *  has lantency in passing the visibility events.
> +  **/

same here.

::: dom/audiochannel/AudioChannelServiceChild.cpp
@@ +99,5 @@
> +   *  (1) apps in the foreground.
> +   *  (2) apps in the backgrund and inactive.
> +   *  Notice : modify only when the visible status is stable, because there
> +   *  has lantency in passing the visibility events.
> +  **/

indentation of this last line.

Comment 13

[Alastor Wu [:alwu]]

Attachment: Update speaker status. r=baku.

Sure, I would help to rebase it after landing the new architecture :)

Comment 14

[Alastor Wu [:alwu]]

Comment on attachment 8611127 [details] [diff] [review]
Update speaker status. r=baku.

This bug is the regression error of bug1089526, and that bug was landed on v2.1/v2.2. 
Therefore, I need to uplift the changeset to these versions.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Forget to check the speaker status
User impact if declined: The FM status would be not correct
Testing completed: Yes
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None

Comment 15

[Alastor Wu [:alwu]]

The try-server result is on the comment10.

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/c93b53c32895

Comment 17

[Josh Cheng [:josh]]

Comment on attachment 8611127 [details] [diff] [review]
Update speaker status. r=baku.

Requesting QA verify on 2.1/2.2/master after patch landed

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/19c7e9b4faa2

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c93b53c32895

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a644263a8225

Comment 21

[Vincent Liu[:vliu]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/19c7e9b4faa2

Comment 22

[Verson Xiong (Leave from Mozilla)]

Attachment: Verify.mp4

This bug has been verified as pass on latest build of Flame v2.1/v2.2/master, Nexus5 v2.2/master and 2.1s_256mb/2.1s_512mb by the STR in Comment 0.
Actually Result: speaker-switch button lights is on.
Reproduce rate: 0/10
See attachment: Verify.mp4

Device:Flame v2.1 build(pass)
Build ID               20150531001203
Gaia Revision          2304a1f6327c2ccf35d6995ee16f2231ed1f22a3
Gaia Date              2015-05-26 13:30:41
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/e52807dee101
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150531.035411
Firmware Date          Sun May 31 03:54:22 EDT 2015
Bootloader             L1TC000118D0

Device:Flame v2.2 build(pass)
Build ID               20150531162502
Gaia Revision          b4582cc394e0919623263997c0cdb0b4751a1403
Gaia Date              2015-05-31 11:06:34
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/78d8b0a4303d
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150531.195816
Firmware Date          Sun May 31 19:58:28 EDT 2015
Bootloader             L1TC000118D0

Device:Flame master build(pass)
Build ID               20150531160203
Gaia Revision          e6dc0f4c583407a4a52a66ce7cb11f058302a762
Gaia Date              2015-05-29 17:20:26
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/f8d21278244b
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150531.192324
Firmware Date          Sun May 31 19:23:35 EDT 2015
Bootloader             L1TC000118D0

Device:Nexus 5 v2.2 build(pass) 
Build ID               20150531002502
Gaia Revision          0a46394dbee0ed2eb71a136cee38ddd8549dd597
Gaia Date              2015-05-30 14:50:16
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ed2f6aeb1d81
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150531.043812
Firmware Date          Sun May 31 04:38:27 EDT 2015
Bootloader             HHZ12f

Device:Nexus 5 master build(pass)
Build ID               20150531160203
Gaia Revision          e6dc0f4c583407a4a52a66ce7cb11f058302a762
Gaia Date              2015-05-29 17:20:26
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/f8d21278244b
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150531.192050
Firmware Date          Sun May 31 19:21:07 EDT 2015
Bootloader             HHZ12f

Device:2.1s_256mb build(pass)
Build ID               20150531001204
Gaia Revision          0493b4dbb59f9617c1c1a45f422303c2aff32a9a
Gaia Date              2015-05-27 19:33:12
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/1aae9fc735b5
Gecko Version          34.0
Device Name            scx15_sp7715ga
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150531.035115
Firmware Date          Sun May 31 03:51:25 EDT 2015

Device:2.1s_512mb build(pass)
Gaia-Rev        2def4255c59065e3ac9265bb165adc1550398477
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/1aae9fc735b5
Build-ID        20150531001204
Version         34.0
Device-Name     scx15_sp7715ea
FW-Release      4.4.2
FW-Incremental  122
FW-Date         Thu Feb  5 12:42:58 CST 2015