Closed Bug 944262 Opened 7 years ago Closed 7 years ago

[B2G]On conference call,we can't switch the speaker between off and on

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Other
defect
Not set
major

Tracking

(blocking-b2g:1.3+, firefox27 wontfix, firefox28 fixed, firefox29 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

RESOLVED FIXED
1.3 C1/1.4 S1(20dec)
blocking-b2g 1.3+
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: ming.li, Assigned: hsinyi)

References

()

Details

(Whiteboard: [Fugu] [v1.2f-uplift-needed])

Attachments

(2 files, 1 obsolete file)

preconditions: on a conference call

then follows:
1.turn the speaker on : it'll be ok

2.turn the speaker off: it'll fail to turn off the speaker.

with some logs,i found that in Telephonyprovider.js ,on a conference call ,when set speakerEnabled ,it'll set the gAudioManager.phoneState to normal .
As a result , the next time we call gAudioManager.setForceForUse ,it fails. 

  set speakerEnabled(aEnabled) {
    if (aEnabled == this.speakerEnabled) {
      return;
    }
    let force = aEnabled ? nsIAudioManager.FORCE_SPEAKER :
                           nsIAudioManager.FORCE_NONE;
    gAudioManager.setForceForUse(nsIAudioManager.USE_COMMUNICATION, force);

    if (!this._activeCall) {  
      gAudioManager.phoneState = nsIAudioManager.PHONE_STATE_NORMAL;
    }
  },
Severity: normal → major
OS: Windows XP → Other
Hardware: x86 → ARM
(In reply to Ming from comment #0)
> preconditions: on a conference call
> 
> then follows:
> 1.turn the speaker on : it'll be ok
> 
> 2.turn the speaker off: it'll fail to turn off the speaker.
> 
> with some logs,i found that in Telephonyprovider.js ,on a conference call
> ,when set speakerEnabled ,it'll set the gAudioManager.phoneState to normal .
> As a result , the next time we call gAudioManager.setForceForUse ,it fails. 
> 
>   set speakerEnabled(aEnabled) {
>     if (aEnabled == this.speakerEnabled) {
>       return;
>     }
>     let force = aEnabled ? nsIAudioManager.FORCE_SPEAKER :
>                            nsIAudioManager.FORCE_NONE;
>     gAudioManager.setForceForUse(nsIAudioManager.USE_COMMUNICATION, force);
> 
>     if (!this._activeCall) {  
>       gAudioManager.phoneState = nsIAudioManager.PHONE_STATE_NORMAL;
>     }
>   },

Thanks for reporting the issue.
It looks we miss conference state check before resetting phone state.

We probably should do 

if (!this._activeCall &&
    conferenceState !== nsITelephonyProvider.CALL_STATE_CONNECTED) {
  gAudioManager.phoneState = nsIAudioManager.PHONE_STATE_NORMAL;
}
Hsinyi, wanna take it?
Flags: needinfo?(htsai)
Taken :)
Assignee: nobody → htsai
Flags: needinfo?(htsai)
fugu and hamachi also have this issue, can I set koi flag?
blocking-b2g: --- → fugu?
blocking-b2g: fugu? → fugu+
Hsinyi, Any update...:)
(In reply to Ken Chang[:ken] from comment #5)
> Hsinyi, Any update...:)

I will try to provide a WIP tomorrow.
Change the bug description because this is a general issue, not specific to any device.
Summary: [fugu][B2G]On conference call,we can't switch the speaker between off and on → [B2G]On conference call,we can't switch the speaker between off and on
Comment on attachment 8343613 [details] [diff] [review]
944262.patch - track the active conference call

This issue happens because we don't track 'active' conference call. Instead, we keep only an 'active' 'single' call. Thus, we always hit |if(!this._activeCall)| in [1] that leads to the problem with setting speaker.

This patch resolves the issue by tracking not only active 'single' call but also active 'conference' call.


[1] http://dxr.mozilla.org/mozilla-central/source/dom/telephony/gonk/TelephonyProvider.js?from=TelephonyProvider.js#472
Attachment #8343613 - Flags: review?(vyang)
Whiteboard: [Fugu needs]
Whiteboard: [Fugu needs]
QA,

Please test on Buri device and see if this is reproducible.
Keywords: qawanted
QA Contact: sparsons
I was not able to repro this issue on the Buri 1.2 Build ID 20131210004008

Gaia   3bede56043379283cb0f6673730f91be88018d13
SourceStamp e535d93d88ad
BuildID 20131210004008
Version 26.0

I also could not repro on Buri 1.3 Build ID: 20131210004003

Gaia   3452fbdb5e1bed0cd27cc6173136537a03e8072f
SourceStamp e0c328d99742
BuildID 20131210004003
Version 28.0a2
Keywords: qawanted
Comment on attachment 8343613 [details] [diff] [review]
944262.patch - track the active conference call

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

Thank you! I'm going to attach a test script for this.  Hold on a minute.
Attachment #8343613 - Flags: review?(vyang) → review+
Attached patch 944262.patch-2 - test cases (obsolete) — Splinter Review
Most of the functions in this test script are copied from "test_conference.js".  Since we have other plan for moving these duplicates into "head.js", I will not touch others now.

Try: https://tbpl.mozilla.org/?tree=Try&rev=965e6f3bc47b
Attachment #8345796 - Flags: review?(htsai)
Sorry, remove unused bits only.  No change to effective lines.
Attachment #8345796 - Attachment is obsolete: true
Attachment #8345796 - Flags: review?(htsai)
Attachment #8345800 - Flags: review?(htsai)
Comment on attachment 8345800 [details] [diff] [review]
944262.patch-2 - test cases

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

Looks great, thanks for the test.
Attachment #8345800 - Flags: review?(htsai) → review+
https://hg.mozilla.org/mozilla-central/rev/f93d32e252e9
https://hg.mozilla.org/mozilla-central/rev/d65561224919
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C1/1.4 S1(20dec)
Whiteboard: [Fugu] [v1.2f-uplift-needed]
Hi styang ,pls help to merge this to v1.3
Flags: needinfo?(styang)
It's in v1.3.
Flags: needinfo?(styang)
Hi,steven.

We found this patch on master
not on v1.3
Please check it again

(In reply to Steven Yang [:styang] from comment #19)
> It's in v1.3.
Flags: needinfo?(styang)
1.3+ to make sure the fix be put on 1.3.
blocking-b2g: fugu+ → 1.3+
Flags: needinfo?(styang)
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap+
You need to log in before you can comment on or make changes to this bug.