953159 - FM radio tries to play even when there is no antenna

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[vasanth]

Sometimes FM tries to play even when headset is not connected.

To find whether antenna is available it checks (mHeadphoneState != SWITCH_STATE_OFF) [1]. When headset is not connected, sometimes mHeadphoneState is SWITCH_STATE_UNKNWON. So above check becomes true which is wrong

[1] http://mxr.mozilla.org/mozilla-central/source/dom/fmradio/FMRadio.cpp#237

Comment 1

[vasanth]

Attachment: handle-antenna-switch-state-unknown-error-in-fm.patch

Attached patch fixes this issue.

Hi Pin Zhang, 

I see you as the author of FMRadio.cpp. 
Can you please review this?

Comment 2

[Pin Zhang [:pzhang] (inactive)]

Comment on attachment 8351481 [details] [diff] [review]
handle-antenna-switch-state-unknown-error-in-fm.patch

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

::: dom/fmradio/FMRadio.cpp
@@ +234,5 @@
>  bool
>  FMRadio::AntennaAvailable() const
>  {
> +  return mHasInternalAntenna ? true : (mHeadphoneState != SWITCH_STATE_OFF) &&
> +      (mHeadphoneState != SWITCH_STATE_UNKNOWN);

Nits, we usually keep two spaces indention.

Comment 3

[vasanth]

Attachment: handle-antenna-switch-state-unknown-error-in-fm.patch

Addressed Nit

Comment 4

[Pin Zhang [:pzhang] (inactive)]

Comment on attachment 8351541 [details] [diff] [review]
handle-antenna-switch-state-unknown-error-in-fm.patch

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

r+, thanks!

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/integration/b2g-inbound/rev/564ec4c03762

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/564ec4c03762

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/17f677b7df0c

Comment 8

[KTucker [:KTucker][Inactive 3/4/2016]]

Test case has been added to Moztrap: https://moztrap.mozilla.org/manage/case/13743/