1089526 - [FFOS2.0][Woodduck][FM]The speaker close when playing Radio make a active call then hang out.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: AudioChannel, defect, P2)

Description

[sync-1]

DEFECT DESCRIPTION:
 Speakers close.
 
  REPRODUCING PROCEDURES:
 1.Access FM Radio and select a channel -> Select speakers
 2.Make a incoming call or out going call then hang out
 
  EXPECTED BEHAVIOUR:
 Should be speakers on.
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:100%
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[shawn ku [:sku]]

Hi Wayne:
 Please help check this issue first.

Shawn

Comment 2

[Wayne Chen [:xwaynec]]

I can't check this issue today, because the code released in this week has problem. FM doesn't have sound when I switch to speaker mode.

Comment 3

[Wayne Chen [:xwaynec]]

To continue comment 2, 'frameworks/av' git log 8c6a63a2e687d1c2c3f68c27d82a60d2175e896e is ok.

Comment 4

[Wayne Chen [:xwaynec]]

Hi Randy,

as offline talk, please help check this bug.
thanks

Comment 5

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

I would like to let speaker can keep on when this application register the audio channel type larger than content even switch to background.

Comment 6

[Josh Cheng [:josh]]

Triage:
Minor issue. Low user impact.
Hi Howie,
Can you nominate this as 2.2? Thanks!

Comment 7

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

I

Comment 8

[howie [:howie]]

ok, the corresponding OS functional team should go through the 2.2 feature nomination.

Comment 9

[sync-1]

(In reply to comment #8)
 > Comment from Mozilla:ok, the corresponding OS functional team should go through
 > the 2.2 feature nomination.
 
 Dear Mozilla，
 The feature  will be OK  on 2.2 branch?If ok ,can we merge the pitch to our Woodduck?

Comment 10

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Yes, I think it should be ok to uplife on 2.0m.

Comment 11

[sync-1]

(In reply to comment #10)
 > Comment from Mozilla:Yes, I think it should be ok to uplife on 2.0m.
 
 How can I get the update file or patch?

Comment 12

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

The patch is cooking...

Comment 13

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: patch v1

Let audiochannel can resume speaker if previous application has turned it on.

Comment 14

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

The patch v1 would let audioChannelService to recover the speaker status even the application in the background.

Hi Germán, 
I just check and found you want to control the speaker even fm application in the background. 
Could you have a change on app side to avoid this? 

In wiki document, we has this rule that avoid application to control speaker. 
https://wiki.mozilla.org/WebAPI/SpeakerManager
==>If a background app calls forcespeaker=true that doesn't change anything. 'speakerforced' remains false everywhere. 

You try to get speakerforced first (return false) and set forcespeaker = false, so that would turn off the speaker in device.

Comment 15

[Germán Toro del Valle (:gtorodelvalle)]

Hi Randy,
I am sorry but I am kind of lost here :( Would you be so kind to point me to the piece of code you are referring to, please?
Thank you very much! ;)

Comment 16

[Germán Toro del Valle (:gtorodelvalle)]

Need-infoing Randy regarding comment 15 ;)

Comment 17

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

I think it should be
fm.js:L861 
  window._previousSpeakerForcedState = speakerManager.speakerforced;
L:877
          // Re-enable the speaker if it was previously forced.
  speakerManager.forcespeaker = !!window._previousSpeakerForcedState;

speakerManager would recover the original speaker status on this case.
You could remove those control logic After this patch check-in the codebase.

Comment 18

[sync-1]

(In reply to comment #13)
 > Created an attachment (id=1008999) [details]
 > patch v1
 
 Dear Mozilla，
 The patch V1 refer to the 2.2 branch or 2.0 ?I cann't find the pices of code On our 2.0M.Like as follow:
 @@ -871,23 +869,16 @@ AudioChannelService::Observe(nsISupports
  
      uint64_t innerID;
      nsresult rv = wrapper->GetData(&innerID);
      if (NS_WARN_IF(NS_FAILED(rv))) {
        return rv;
      }
  
      mAgents.Enumerate(WindowDestroyedEnumerator, &innerID);
 -
 -#ifdef MOZ_WIDGET_GONK
 -    bool active = AnyAudioChannelIsActive();
 -    for (uint32_t i = 0; i < mSpeakerManager.Length(); i++) {
 -      mSpeakerManager[i]->SetAudioChannelActive(active);
 -    }
 -#endif
    }
  
    return NS_OK;
  }
 Could you give me a patch on our woodduck 2.0M?

Comment 19

[sync-1]

(In reply to comment #16)
 > Comment from Mozilla:Hi Randy,
 > I am sorry but I am kind of lost here :( Would you be so kind to point me to
 > the piece of code you are referring to, please?
 > Thank you very much! ;)
 
 Hi，Mozilla
 I don't find the dom/media/MediaRecorder.cpp file in our Woodduck,can you give me a patch based on 2.0M?

Comment 20

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

ah, sorry, you don't need the MediaRecorder.cpp's patch.

Comment 21

[Josh Cheng [:josh]]

HI Reporter,
Have you successfully verified the problem after the patch? Thanks!

Comment 22

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: fm_gaia.patch

changes on fm app.

Comment 23

[sync-1]

(In reply to comment #22)
 > Comment from Mozilla:HI Reporter,
 > Have you successfully verified the problem after the patch? Thanks!
 
 Dear Mozilla，
 I merged the patch to our project ,but it compile failed.I am debuging it,please help me to verified the patch on our Woodduck too,thanks!

Comment 24

[sync-1]

(In reply to comment #24)
 > (In reply to comment #22)
 > > Comment from Mozilla:HI Reporter,
 > > Have you successfully verified the problem after the patch? Thanks!
 > Dear Mozilla，
 > I merged the patch to our project ,but it compile failed.I am debuging
 > it,please help me to verified the patch on our Woodduck too,thanks!
 
 Dear Mozilla,
 I have merged the patch and successfully compiled the project ,but it doesn't work!When hang out the active call（MO/MT）,the FM sound still come from headset.

Comment 25

[Josh Cheng [:josh]]

HI Reporter,
Can you provide console log? Thanks!

Comment 26

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Did you change the fm radio side?

Comment 27

[sync-1]

(In reply to comment #27)
 > Comment from Mozilla:Did you change the fm radio side?
 
 ah,how to change the fm radio side?

Comment 28

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

I mean the  fm_gaia.patch. 
You can apply it on fm radio application (gaia/apps/fm).

Comment 29

[sync-1]

Created an attachment (id=1027870)
 Woodduck FM file
 
 Dear Mozilla，
 The attachment is the gaia/apps/fm.js based on our woodduck.In this fm.js I remove the follow code:
 fm.js:L876 
   window._previousSpeakerForcedState = speakerManager.speakerforced;
 L:893          // Re-enable the speaker if it was previously forced.
   speakerManager.forcespeaker = !!window._previousSpeakerForcedState;
 
 But it doesn't work ,and when make a incoming call ,the FM will be crashed.

Comment 30

[sync-1]

Created an attachment (id=1027870)
 Woodduck FM file
 
 Dear Mozilla，
 The attachment is the gaia/apps/fm.js based on our woodduck.In this fm.js I remove the follow code:
 fm.js:L876 
   window._previousSpeakerForcedState = speakerManager.speakerforced;
 L:893          // Re-enable the speaker if it was previously forced.
   speakerManager.forcespeaker = !!window._previousSpeakerForcedState;
 
 But it doesn't work ,and when make a incoming call ,the FM will be crashed.

Comment 31

[sync-1]

Attachment: Woodduck FM file

Created an attachment (id=1027870)
 Woodduck FM file
 
 Dear Mozilla，
 The attachment is the gaia/apps/fm.js based on our woodduck.In this fm.js I remove the follow code:
 fm.js:L876 
   window._previousSpeakerForcedState = speakerManager.speakerforced;
 L:893          // Re-enable the speaker if it was previously forced.
   speakerManager.forcespeaker = !!window._previousSpeakerForcedState;
 
 But it doesn't work ,and when make a incoming call ,the FM will be crashed.

Comment 32

[sync-1]

Created an attachment (id=1028155)
 When incoming a call,the fm will be crashed
 
 Dear Mozilla，
 When the FM play in the background,hand out the MO/MT,the sound will be recover to the speaker.But at the FM play screen,hand out the incoming call,the FM will be crashed.The attachment is the console log,please check it,thanks!

Comment 33

[sync-1]

Created an attachment (id=1028155)
 When incoming a call,the fm will be crashed
 
 Dear Mozilla，
 When the FM play in the background,hand out the MO/MT,the sound will be recover to the speaker.But at the FM play screen,hand out the incoming call,the FM will be crashed.The attachment is the console log,please check it,thanks!

Comment 34

[sync-1]

Attachment: When incoming a call,the fm will be crashed

Created an attachment (id=1028155)
 When incoming a call,the fm will be crashed
 
 Dear Mozilla，
 When the FM play in the background,hand out the MO/MT,the sound will be recover to the speaker.But at the FM play screen,hand out the incoming call,the FM will be crashed.The attachment is the console log,please check it,thanks!

Comment 35

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Hi sync-1, could you provide the call stack for this problem?

Comment 36

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: patch v2

Fix fm radio crash as described in comment 34.

Comment 37

[sync-1]

(In reply to comment #34)
 > (In reply to comment #33)
 > > Comment from Mozilla:Fix fm radio crash as described in comment 34.
 > Dear Mozilla:
 > I want to know what time can solve the fm crashed problem?How can I provide
 > assistance?
 
 Dear Mozilla，
 According to our team's evaluation,in order to reduce the development risk,we hope Mozilla can solve this problem based on our Woodduck ,Thks!

Comment 38

[Josh Cheng [:josh]]

Hi Reporter,
Could you try again with new patch : https://bugzilla.mozilla.org/attachment.cgi?id=8525820?

Comment 39

[sync-1]

Hi,Mozilla,
 This feature is ok,thanks for your help!

Comment 40

[Josh Cheng [:josh]]

Hi Randy,
Can you raise review request? Partner has verified the patch works!

Comment 41

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: patch v2.1

Hi Baku, 
Could you review this?

Comment 42

[Josh Cheng [:josh]]

Hi Randy,
Is the patch also good for 2.0M? Thanks!

Comment 43

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

It need to rebaes to 2.0m branch.
I think it should be ok.

Comment 44

[Josh Cheng [:josh]]

Hi Baku,
Could you help to review this patch? Thanks!

Comment 45

[Andrea Marchesini [:baku]]

If we don't want this for 2.0, probably it's better to have this patch built on top of bug 1113086.
Randy Lin, what do you think about it?

Comment 46

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

The target of refactor audiochannel is on v3. So I think if this bug isn't 2.2+, we can keep this patch for partner and got it fix after bug 1113086.

Comment 47

[Josh Cheng [:josh]]

Hi Andrea, Hi Randy,
This issue is reported by partner in 2.0M. I think we can apply this temp patch on 2.0M and leave bug 1113086 for V3.0. Thanks!

Comment 48

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Comment on attachment 8531396 [details] [diff] [review]
patch v2.1

Hi Andreas, 
Partner want to fix this one, so still needs your help to review that. Thanks..

Comment 49

[Andrea Marchesini [:baku]]

Comment on attachment 8531396 [details] [diff] [review]
patch v2.1

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

I wrote a couple of questions to understand this code better. Thanks.

::: dom/audiochannel/AudioChannelService.cpp
@@ +353,5 @@
>    data->mState = GetStateInternal(data->mChannel, CONTENT_PROCESS_ID_MAIN,
>                                  aElementHidden, oldElementHidden);
> +#ifdef MOZ_WIDGET_GONK
> +  for (uint32_t i = 0; i < mSpeakerManager.Length(); i++) {
> +    mSpeakerManager[i]->SetAudioChannelActive(AUDIO_CHANNEL_STATE_MUTED != data->mState);

Well... the fact that this audiochannel is muted, doesn't mean that we cannot have something else playing.
Why do you want to mute all?

::: dom/audiochannel/AudioChannelServiceChild.cpp
@@ +94,5 @@
>    data->mState = state;
>    cc->SendAudioChannelChangedNotification();
> +#ifdef MOZ_WIDGET_GONK
> +  for (uint32_t i = 0; i < mSpeakerManager.Length(); i++) {
> +    mSpeakerManager[i]->SetAudioChannelActive(AUDIO_CHANNEL_STATE_MUTED != state);

Same issue here.

::: dom/speakermanager/SpeakerManager.cpp
@@ +191,5 @@
>    SpeakerManagerService *service =
>      SpeakerManagerService::GetOrCreateSpeakerManagerService();
>    MOZ_ASSERT(service);
> +  if (mForcespeaker && mVisible) {
> +    service->ForceSpeaker(mForcespeaker && mVisible);

Why is it true that we want t ofroce the speak when the visibility is set to true?

::: dom/speakermanager/SpeakerManagerService.cpp
@@ +141,5 @@
>  {
> +  // Content process and switch to background with no audio and speaker forced.
> +  // Then disable speaker
> +  for (uint32_t i = 0; i < mRegisteredSpeakerManagers.Length(); i++) {
> +    mRegisteredSpeakerManagers[i]->SetAudioChannelActive(aIsActive);

Which AudioChannel?

Comment 50

[Josh Cheng [:josh]]

Dear Alastor,
Could you help to check this bug and resume Randy's work?
Thank you very much!

Comment 51

[Alastor Wu [:alwu]]

Ok, I would check it later.
Keep ni to trace.

Comment 52

[Alastor Wu [:alwu]]

Hi, Baku,
I would resume Randy's work on this problem.
So let me describe about my understanding.

> ::: dom/audiochannel/AudioChannelService.cpp
> @@ +353,5 @@
> >    data->mState = GetStateInternal(data->mChannel, CONTENT_PROCESS_ID_MAIN,
> >                                  aElementHidden, oldElementHidden);
> > +#ifdef MOZ_WIDGET_GONK
> > +  for (uint32_t i = 0; i < mSpeakerManager.Length(); i++) {
> > +    mSpeakerManager[i]->SetAudioChannelActive(AUDIO_CHANNEL_STATE_MUTED != data->mState);
> 
> Well... the fact that this audiochannel is muted, doesn't mean that we
> cannot have something else playing.
> Why do you want to mute all?
> 

Every time the AudioChannelAgent come to check its playing state, we also need to ensure whether this agent have used the speaker. If the speaker has been enabled before, we need to resume it when the AudioChannelAgent is permitted to continue playback, is it right?

I can't also understand this code. But I think there exists a better way to active/inactive the speakerManager. We could add the variable which stored AudioChannelType in SpeakerManager, then switch the speaker to the correct audio output by checking the type of AudioChannelAgents.

How do you think :)?

> ::: dom/speakermanager/SpeakerManager.cpp
> @@ +191,5 @@
> >    SpeakerManagerService *service =
> >      SpeakerManagerService::GetOrCreateSpeakerManagerService();
> >    MOZ_ASSERT(service);
> > +  if (mForcespeaker && mVisible) {
> > +    service->ForceSpeaker(mForcespeaker && mVisible);
> 
> Why is it true that we want to force the speak when the visibility is set to
> true?
> 

Only the foreground app can force the speaker on.
See comment14.

> ::: dom/speakermanager/SpeakerManagerService.cpp
> @@ +141,5 @@
> >  {
> > +  // Content process and switch to background with no audio and speaker forced.
> > +  // Then disable speaker
> > +  for (uint32_t i = 0; i < mRegisteredSpeakerManagers.Length(); i++) {
> > +    mRegisteredSpeakerManagers[i]->SetAudioChannelActive(aIsActive);
> 
> Which AudioChannel?

I think we don't need to know the AudioChannel type.
Only the SpeakerManager whose mForceSpeaker is true can set the speaker on. 
Therefore, they can make the decision of active/inactive speaker themselves by checking its mForceSpeaker.

Comment 53

[Josh Cheng [:josh]]

Hi Baku,
Could you provide some feedback for comment 52?
Thanks!

Comment 54

[Alastor Wu [:alwu]]

I will update the new patch later.

Comment 55

[Alastor Wu [:alwu]]

Attachment: Modify the checking timing of the speaker states

Hi, Baku :)
Here is my new patch to solve this bug!
Following is the description of the root cause and the solution.
Could you help me to review this?
Very appreciate!

---

[Root cause]
This bug is resulted from the timing issue of delivering events. In this case, when we hang up the phone call, the SpeakerManager receives the "visibility-change" first, then the AudioChannelService receives the "inner-window-destroyed".

First event would turn the speaker on, and the second one turns the speaker off. (this event was triggered before registering the new agent)

[Solution]
Everytime the agents asked its state, then we also check whether the speaker setting is correct. 
Here are two places we will check the speaker state, the first one is in the |UnregisterAudioChannelAgent|, and the second one is in the |GetState()|.

Comment 56

[Alastor Wu [:alwu]]

Attachment: Correct the error function name

Correct the error function name.
|TuruOnSpeaker()| -> |TurnOnSpeaker()|.

Comment 57

[Josh Cheng [:josh]]

Hi Baku,
Could you help to review the patch per comment 56?
Thanks!

Comment 58

[Andrea Marchesini [:baku]]

Comment on attachment 8561297 [details] [diff] [review]
Modify the checking timing of the speaker states

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

::: dom/audiochannel/AudioChannelServiceChild.cpp
@@ -135,5 @@
>    }
> -#ifdef MOZ_WIDGET_GONK
> -  bool active = AnyAudioChannelIsActive();
> -  for (uint32_t i = 0; i < mSpeakerManager.Length(); i++) {
> -    mSpeakerManager[i]->SetAudioChannelActive(active);

Why don't you need it in the child process?

::: dom/speakermanager/SpeakerManagerService.cpp
@@ +150,1 @@
>                                 aTopic, const char16_t* aData)

can you indent it like this:

SpeakerManagerService::Observe(nsISupports* aSubject,
                               const char* aTopic,
                               const char16_t* aData)

Comment 59

[Alastor Wu [:alwu]]

Attachment: Modify the checking timing of the speaker states. r=baku.

Hi, Baku,

> 
> Why don't you need it in the child process?
> 

It's my fault. These codes should be keep it. I have already added it back.
Thanks for your review!!!

Comment 60

[Alastor Wu [:alwu]]

Attachment: Correct the error function name. r=baku.

Comment 61

[Keven Kuo [:kkuo]]

Hi! Kai-Zhen,

check in needed?

--
Keven

Comment 62

[Kai-Zhen Li [:seinlin][:kli]]

Alastor, is the patch ready to checkin to v2.0m branch? Does it need to checkin to master branch?

Comment 63

[Alastor Wu [:alwu]]

Yes, I think it can be landed to v2.0m, I would update the available patches.
It doesn't need to be landed to master, because we will refactor the audio channel soon.

Comment 64

[Kai-Zhen Li [:seinlin][:kli]]

When I merge these two patch into 2.0m branch, there are conflicts in the following files.
? dom/audiochannel/AudioChannelService.cpp.rej
? dom/speakermanager/SpeakerManager.cpp.rej
? dom/speakermanager/SpeakerManagerService.cpp.rej

Could you rebase patches for 2.0m branch?
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m

Comment 65

[Alastor Wu [:alwu]]

Attachment: [v2.0m] Modify the checking timing of the speaker states. r=baku.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
The timing of checking speaker states errors

User impact if declined: 
This would fix the problem of the speaker states error. And the patch doesn't need to be landed in the master, because we will refactor the audio channel soon (Refactoring audio channel would solve lots of audio channel issue). 

Testing completed: 
You can compare these patches, one is try message, another is the changeset.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d310ac186623
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9f043c7548e

Risk to taking this patch (and alternatives if risky): None

String or UUID changes made by this patch: None

Comment 66

[Alastor Wu [:alwu]]

Attachment: [v2.0m] Correct the error function name. r=baku.

The reason is as same as comment65.
These patches need to be landed together.

Comment 67

[Kai-Zhen Li [:seinlin][:kli]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/6f602a6f4859
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/514b7663aac6

Comment 68

[Ryan VanderMeulen [:RyanVM]]

(In reply to Alastor Wu [:alwu] from comment #63)
> Yes, I think it can be landed to v2.0m, I would update the available patches.
> It doesn't need to be landed to master, because we will refactor the audio
> channel soon.

What about b2g34/b2g37 (v2.1/v2.2)?

Comment 69

[Alastor Wu [:alwu]]

If all other versions is affected, I will rebase the patches for them.
Thanks Ryan, I would update the patches later (including master)

Comment 70

[Alastor Wu [:alwu]]

Attachment: [v2.1] Correct the error function name. r=baku.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Spelling error of the function name
User impact if declined: None
Testing completed: Yes
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: Correct the spelling error

Comment 71

[Alastor Wu [:alwu]]

Attachment: [v2.2/v3.0] Correct the error function name. r=baku.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Spelling error of the function name
User impact if declined: None
Testing completed: Yes
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: Correct the spelling error

Comment 72

[Alastor Wu [:alwu]]

Attachment: [v2.1/v2.2/v3.0] Modify the checking timing of the speaker states. r=baku.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): The timing of checking speaker states errors
User impact if declined: Users will feel wrong because the speaker state is not as their expectation.
Testing completed: Yes
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None

Comment 73

[Alastor Wu [:alwu]]

Comment on attachment 8579093 [details] [diff] [review]
[v2.2/v3.0] Correct the error function name. r=baku.

># HG changeset patch
># User Alastor Wu <[e-mail]>
># Date 1424082550 -28800
>#      Mon Feb 16 18:29:10 2015 +0800
># Node ID 4c08fff5dd69147dda67a550edec50673077ea3c
># Parent  83251e534b337bc0c428df7a2c830e9cb1651240
>Bug 1089526 - Correct function name. r=baku.
>
>diff --git a/dom/speakermanager/SpeakerManagerService.cpp b/dom/speakermanager/SpeakerManagerService.cpp
>--- a/dom/speakermanager/SpeakerManagerService.cpp
>+++ b/dom/speakermanager/SpeakerManagerService.cpp
>@@ -71,36 +71,36 @@ SpeakerManagerService::Shutdown()
>   }
> }
> 
> NS_IMPL_ISUPPORTS(SpeakerManagerService, nsIObserver)
> 
> void
> SpeakerManagerService::ForceSpeaker(bool aEnable, uint64_t aChildId)
> {
>-  TuruOnSpeaker(aEnable);
>+  TurnOnSpeaker(aEnable);
>   if (aEnable) {
>     mSpeakerStatusSet.Put(aChildId);
>   }
>   Notify();
>   return;
> }
> 
> void
> SpeakerManagerService::ForceSpeaker(bool aEnable, bool aVisible)
> {
>   // b2g main process without oop
>-  TuruOnSpeaker(aEnable && aVisible);
>+  TurnOnSpeaker(aEnable && aVisible);
>   mVisible = aVisible;
>   mOrgSpeakerStatus = aEnable;
>   Notify();
> }
> 
> void
>-SpeakerManagerService::TuruOnSpeaker(bool aOn)
>+SpeakerManagerService::TurnOnSpeaker(bool aOn)
> {
>   nsCOMPtr<nsIAudioManager> audioManager = do_GetService(NS_AUDIOMANAGER_CONTRACTID);
>   NS_ENSURE_TRUE_VOID(audioManager);
>   if (aOn) {
>     audioManager->SetForceForUse(nsIAudioManager::USE_MEDIA, nsIAudioManager::FORCE_SPEAKER);
>   } else {
>     audioManager->SetForceForUse(nsIAudioManager::USE_MEDIA, nsIAudioManager::FORCE_NONE);
>   }
>@@ -141,38 +141,38 @@ void
> SpeakerManagerService::SetAudioChannelActive(bool aIsActive)
> {
>   if (!aIsActive && !mVisible) {
>     ForceSpeaker(!mOrgSpeakerStatus, mVisible);
>   }
> }
> 
> NS_IMETHODIMP
>-SpeakerManagerService::Observe(nsISupports* aSubject, const char* 
>+SpeakerManagerService::Observe(nsISupports* aSubject, const char*
>                                aTopic, const char16_t* aData)
> {
>   if (!strcmp(aTopic, "ipc:content-shutdown")) {
>     nsCOMPtr<nsIPropertyBag2> props = do_QueryInterface(aSubject);
>     if (!props) {
>       NS_WARNING("ipc:content-shutdown message without property bag as subject");
>       return NS_OK;
>     }
> 
>     uint64_t childID = 0;
>     nsresult rv = props->GetPropertyAsUint64(NS_LITERAL_STRING("childID"),
>                                              &childID);
>     if (NS_SUCCEEDED(rv)) {
>         // If the audio has paused by audiochannel,
>         // the enable flag should be false and don't need to handle.
>         if (mSpeakerStatusSet.Contains(childID)) {
>-          TuruOnSpeaker(false);
>+          TurnOnSpeaker(false);
>           mSpeakerStatusSet.Remove(childID);
>         }
>         if (mOrgSpeakerStatus) {
>-          TuruOnSpeaker(!mOrgSpeakerStatus);
>+          TurnOnSpeaker(!mOrgSpeakerStatus);
>           mOrgSpeakerStatus = false;
>         }
>     } else {
>       NS_WARNING("ipc:content-shutdown message without childID property");
>     }
>   }
>   return NS_OK;
> }
>diff --git a/dom/speakermanager/SpeakerManagerService.h b/dom/speakermanager/SpeakerManagerService.h
>--- a/dom/speakermanager/SpeakerManagerService.h
>+++ b/dom/speakermanager/SpeakerManagerService.h
>@@ -53,17 +53,17 @@ public:
> 
> protected:
>   SpeakerManagerService();
> 
>   virtual ~SpeakerManagerService();
>   // Notify to UA if device speaker status changed
>   virtual void Notify();
> 
>-  void TuruOnSpeaker(bool aEnable);
>+  void TurnOnSpeaker(bool aEnable);
> 
>   nsTArray<nsRefPtr<SpeakerManager> > mRegisteredSpeakerManagers;
>   // Set for remember all the child speaker status
>   nsCheapSet<nsUint64HashKey> mSpeakerStatusSet;
>   // The Speaker status assign by UA
>   bool mOrgSpeakerStatus;
> 
>   bool mVisible;

Comment 74

[Alastor Wu [:alwu]]

Try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=df0cef8faf68

Comment 75

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/48e7d4065ae2
https://hg.mozilla.org/integration/b2g-inbound/rev/f8e635d611be

Comment 76

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

https://hg.mozilla.org/mozilla-central/rev/48e7d4065ae2
https://hg.mozilla.org/mozilla-central/rev/f8e635d611be

Comment 77

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/db41cb442414
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e75ae086a742

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/dd4d7f88b8bb
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f52f3c2236da

Comment 78

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/dd4d7f88b8bb
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/f52f3c2236da

Comment 79

[jingmei.zhang]

Hi Alastor，

I encounter with one question with your patch:

1>open fm,set speaker on

2>pause fm,then in AudioChannelServiceChild::UnregisterAudioChannelAgent,we use 'mSpeakerManager[i]->SetAudioChannelActive(active)' to set speaker off,and 'speakerManager.onspeakerforcedchange' in fm.js set the 'speaker-switch' button to gray

3>replay fm.
Here comes the question:it seems that we have not done anything in gecko to reset the speaker on in SpeakerManager,then in fm UI,we can see the 'speaker-switch' button keeps to gray.

In user perspective,speaker should keep on in such a situation.

Can you help to check the issue?

Comment 80

[Alastor Wu [:alwu]]

OK, I will check it later.
Keep ni for tracking.

Comment 81

[Alastor Wu [:alwu]]

Hi, JingMei,
When we replay the FM, what situation is your expectation?
Is "the FM still coming out from the speaker and display the correct button image" OR  "to recover the FM coming out from the headphone"?
Thanks :)

Comment 82

[jingmei.zhang]

(In reply to Alastor Wu [:alwu] from comment #81)
> Hi, JingMei,
> When we replay the FM, what situation is your expectation?
> Is "the FM still coming out from the speaker and display the correct button
> image" OR  "to recover the FM coming out from the headphone"?
> Thanks :)

Hi Alastor，,

I think speaker should keep its original status:

1>If it comes out from headset before paused,then when replay,it should keep on playing in headset
2>If it comes out from speaker before paused,then when replay,it should keep on playing in speaker

So what do you think?

Comment 83

[Alastor Wu [:alwu]]

Thanks, JingMei.
I found that the speaker status will be reset to default in the Flame when we stop the FM.
Let me ask the UX for the result.

--

Hi, Jenny,
Could you help me confirm some problems?
When users resume the FM, what the speaker status should be?
Is back to default or keeping the original status?
Thanks :)

Comment 84

[Jenny Lee]

Hi Alastor,

In this case, FM should keep playing with speaker. Thanks!

Comment 85

[Alastor Wu [:alwu]]

I file another bug to solve it (See Bug 1157121)

Comment 86

[Shine(Leave from Mozilla)]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/11241