1030912 - [B2G][Dialer][Bluetooth] While in a call the Speaker icon is shown instead of the Bluetooth Headset icon

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Josh Schmitt [Joshs]]

Attachment: log.txt

Description:
On first time call the speaker icon is shown instead of the Bluetooth Headset icon.

Repro Steps:
1) Update a Flame to 20140626000202
2) Open Settings > Bluetooth
3) Connect to a Bluetooth Headset
4) With a 2nd deivce call the DUT

Actual:
The speaker icon is displayed and the Bluetooth Headset icon is not

Expected:
The Bluetooth Headset icon is displayed while connected to a Bluetooth Headset

Environmental Variables:
Device: Flame 2.0
Build ID: 20140626000202
Gaia: 6a1373340b40fcfe901336bc9e80676e5f2ba979
Gecko: 82ef9bf64d87
Version: 32.0a2 (2.0)
Firmware Version: v122

User Agent: Mozilla/5.0 (Mobile; rv32.0) Gecko/32.0 Firefox/32.0

Notes:
Repro frequency: 100%
See attached: screenshot, logcat

Comment 1

[Josh Schmitt [Joshs]]

Attachment: 2014-06-26-15-02-27.png

Comment 2

[Josh Schmitt [Joshs]]

adding qawanted to check other branches/devices.

Comment 3

[Chris Kreinbring [:CKreinbring]]

The bug repros on Flame 2.1, Flame 2.0 and Buri 2.0

Flame 2.1
Build ID: 20140702040207
Gaia: 85e97290431ce6aa0a965421e84d6070cd899129
Gecko: 7075808c3306
Platform Version: 33.0a1
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Flame 2.0
Build ID: 20140702000201
Gaia: 3bfe47c58c959c42f5ffe0309b5380ea514ccd69
Gecko: f40e767ea283
Platform Version: 32.0a2
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Buri 2.0
Build ID: 20140703064557
Gaia: 26f82bf95d237272e66955bbca3323ccf8fcb74a
Gecko: 747a8c44ff44
Platform Version: 32.0a2 (2.0)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Actual result: After connecting to a Bluetooth audio device, the icon on the far right of the in-call screen will show a speaker for the first call and a headphone icon on subsequent calls.

--------------------------------------------------------------------------------------------------------

The bug does not repro on Flame 1.4

Build ID: 20140630000201
Gaia: aa896d5db1b4929f3bf31a0f4bb7de50530222a8
Gecko: 8cba60bc12ef
Platform Version: 30.0
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Actual result: After connecting to a Bluetooth audio device, the icon on the far right of the in-call screen will show a headphone icon on all calls.

Comment 4

[Joshua Mitchell (Inactive)]

issue is a regression but not nomming - issue seems cosmetic

Comment 5

[Jason Smith [:jsmith]]

Is the fact that a speaker icon, rather than a bluetooth icon, showing in the dialer app during a call with a bluetooth headset a cert issue or not?

Comment 6

[Beatriz Rodríguez [:brg]]

The behaviour must be consistent. The difference of the behaviour will block the certification. It seems that the headphone icon is the suitable option.

Comment 7

[Beatriz Rodríguez [:brg]]

Nominating accordingly.

Comment 8

[Anthony Ricaud (:rik)]

Attachment: headset-paired.png

When a headset is paired, the status bar shows a headset around the bluetooth icon. In attachment 8446704 [details], that headset is not present in the status bar. So I don't think this is a Dialer bug but rather a pairing issue.

Comment 9

[Doug Sherk (:drs) (inactive)]

I was able to repro this with the original STR on master.

Comment 10

[Doug Sherk (:drs) (inactive)]

This only repros with a new profile.

New Repro Steps:
1) Update a Flame to v2.0 or v2.1/master
2) Clear the profile using |NOFTU=1 make reset-gaia|. If you used B2G-flash-tool, it deletes your profile for you.
3) Open Settings > Bluetooth
4) Connect to a Bluetooth Headset
5) With a 2nd device call the DUT

Actual:
The speaker icon is displayed and the Bluetooth Headset icon is not

Expected:
The Bluetooth Headset icon is displayed while connected to a Bluetooth Headset

Comment 11

[Doug Sherk (:drs) (inactive)]

Tracked down to commit 7afce8a6e965a45f38988227828ccdfb315394f3

Comment 12

[Doug Sherk (:drs) (inactive)]

This is a regression. It affects every current branch because bug 990003 was uplifted to all of them.

Comment 13

[Anthony Ricaud (:rik)]

Does this reproduce without NOFTU=1 ? If it doesn't, then it's probably not a release blocker.

Comment 14

[Doug Sherk (:drs) (inactive)]

(In reply to Anthony Ricaud (:rik) from comment #13)
> Does this reproduce without NOFTU=1 ? If it doesn't, then it's probably not
> a release blocker.

Yes, I only wrote that because it makes bisecting faster since the FTU setting doesn't matter for reproing.

Comment 15

[Doug Sherk (:drs) (inactive)]

The problem was that BluetoothHelper was being loaded when _bluetooth (pointer to window.navigator.mozBluetooth) was null. This is only loaded once on initialization, so we have to re-check it later, when it's not null. Patch coming.

Comment 16

[Doug Sherk (:drs) (inactive)]

Attachment: Make BluetoothHelper check later if window.navigator.mozBluetooth isn't loaded.

PR: https://github.com/mozilla-b2g/gaia/pull/21590

Comment 17

[Etienne Segonzac (:etienne)]

Comment on attachment 8453840 [details] [diff] [review]
Make BluetoothHelper check later if window.navigator.mozBluetooth isn't loaded.

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

::: apps/callscreen/js/index.js
@@ +9,5 @@
>  });
>  
>  // Don't keep an audio channel open when the callscreen is not displayed
>  document.addEventListener('visibilitychange', function visibilitychanged() {
> +  CallsHandler.setup();

debug ?
(if not I have further suggestions)

::: shared/js/bluetooth_helper.js
@@ +25,5 @@
> +    _isLoading = true;
> +
> +    var req = _bluetooth.getDefaultAdapter();
> +    req.onsuccess = function() {
> +      _isReady = true;

shouldn't we |_isLoading = false| here too?

Comment 18

[Doug Sherk (:drs) (inactive)]

(In reply to Etienne Segonzac (:etienne) from comment #17)
> debug ?
> (if not I have further suggestions)

No, this was intentionally. I wasn't happy with this since it should get queued up by the getConnectedDevices() call anyways. I didn't think it was worth investigating more but if you have another suggestion or you think I should get it working that way, I'd be fine with that.

> ::: shared/js/bluetooth_helper.js
> @@ +25,5 @@
> > +    _isLoading = true;
> > +
> > +    var req = _bluetooth.getDefaultAdapter();
> > +    req.onsuccess = function() {
> > +      _isReady = true;
> 
> shouldn't we |_isLoading = false| here too?

Yeah, but once _isReady is set, it's not used anywhere. I'll add this in on the next iteration anyways.

Comment 19

[Etienne Segonzac (:etienne)]

(In reply to Doug Sherk (:drs) from comment #18)
> (In reply to Etienne Segonzac (:etienne) from comment #17)
> > debug ?
> > (if not I have further suggestions)
> 
> No, this was intentionally. I wasn't happy with this since it should get
> queued up by the getConnectedDevices() call anyways. I didn't think it was
> worth investigating more but if you have another suggestion or you think I
> should get it working that way, I'd be fine with that.

* I'd like to extract the BT setup in it's own method if it's the only setup that needs to run on visibility change
* And can we do it only when !document.hidden?

Comment 20

[Doug Sherk (:drs) (inactive)]

Attachment: Make BluetoothHelper listen for mozBluetooth events.

PR: https://github.com/mozilla-b2g/gaia/pull/21636

Okay, sorry, that patch was crap. This is much cleaner. Not sure why I didn't just check if mozBluetooth fires any events before.

Etienne, do you think I should ask Ian for review again? I'm not sure how comfortable you are with this code.

Comment 21

[Etienne Segonzac (:etienne)]

Comment on attachment 8454589 [details] [diff] [review]
Make BluetoothHelper listen for mozBluetooth events.

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

We're so used to workarounds that sometime we forget about clean solutions :)

But yes, I think Ian should have a look.

Comment 22

[[e-mail], [e-mail]]

(In reply to Doug Sherk (:drs) from comment #18)
> (In reply to Etienne Segonzac (:etienne) from comment #17)
> > debug ?
> > (if not I have further suggestions)
> 
> No, this was intentionally. I wasn't happy with this since it should get
> queued up by the getConnectedDevices() call anyways. I didn't think it was
> worth investigating more but if you have another suggestion or you think I
> should get it working that way, I'd be fine with that.
> 
> > ::: shared/js/bluetooth_helper.js
> > @@ +25,5 @@
> > > +    _isLoading = true;
> > > +
> > > +    var req = _bluetooth.getDefaultAdapter();
> > > +    req.onsuccess = function() {
> > > +      _isReady = true;
> > 
> > shouldn't we |_isLoading = false| here too?
> 
> Yeah, but once _isReady is set, it's not used anywhere. I'll add this in on
> the next iteration anyways.

The flag '_isLoading' of first patch is looking like a semaphore to avoid other request doing getDefaultAdapter() again. This is not a good idea to have a flag in API wrapper. And the naming of flag '_isLoading' is not accurate enough to figure out its intent.

Comment 23

[[e-mail], [e-mail]]

Comment on attachment 8454589 [details] [diff] [review]
Make BluetoothHelper listen for mozBluetooth events.

The patch is okay for me. And please also add a test for 'adapteradded' event to call _getAdapter() while receiving the event. That would be perfect.:)

Comment 24

[[e-mail], [e-mail]]

For the paired headset reconnection, I remember the reconnection is triggered while the bluetooth.js of settings app is running. Is the patch really fixing the bug here? Please make sure the problem is happened while the paired headset is connected, and the status bar icon is indicating connected state. The speaker/bluetooth-headset icon of call screen should be consistent with paired headset status.

Comment 25

[Doug Sherk (:drs) (inactive)]

(In reply to Ian Liu [:ianliu] from comment #24)
> For the paired headset reconnection, I remember the reconnection is
> triggered while the bluetooth.js of settings app is running. Is the patch
> really fixing the bug here? Please make sure the problem is happened while
> the paired headset is connected, and the status bar icon is indicating
> connected state. The speaker/bluetooth-headset icon of call screen should be
> consistent with paired headset status.

The problem is within the callscreen app, which is started on boot. Its instance of BluetoothHelper is also initialized during the app's startup. We get an "error getting bluetooth adapter" message in logcat during this time.

Comment 26

[Doug Sherk (:drs) (inactive)]

Added test and merged:

https://github.com/mozilla-b2g/gaia/commit/343272f78ce6fdf0c4ccf2a7d2f7568b8f01df12

Comment 27

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8454589 [details] [diff] [review]
Make BluetoothHelper listen for mozBluetooth events.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 990003
[User impact] if declined: The first time a user makes a call with a Bluetooth headset, the callscreen will show a speaker icon instead of a Bluetooth icon.
[Testing completed]: I tested it.
[Risk to taking this patch] (and alternatives if risky): Low.
[String changes made]: None.

Comment 28

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8454589 [details] [diff] [review]
Make BluetoothHelper listen for mozBluetooth events.

I spoke with drs about his testing. 1.4+ and 2.0+

ni on Vance to comment on whether it's worth taking this fix on 1.3.

Comment 29

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

(In reply to Lawrence Mandel [:lmandel] from comment #28)
> Comment on attachment 8454589 [details] [diff] [review]
> Make BluetoothHelper listen for mozBluetooth events.
> 
> I spoke with drs about his testing. 1.4+ and 2.0+
> 
> ni on Vance to comment on whether it's worth taking this fix on 1.3.

I will inform partner about this bug and let them decide whether to cherry-pick this one or not

Thanks

Vance

Comment 30

[Ryan VanderMeulen [:RyanVM]]

Needs a branch patch for v1.4 uplift.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/da86ed17bb6905eacca91db2302e575702b43d80

Comment 32

[Doug Sherk (:drs) (inactive)]

v1.4: https://github.com/mozilla-b2g/gaia/commit/621d152f89347c79619aa909ad62cc2ac9d3ab5b

Comment 35

[SandKing]

This issue has been verified successfully on Flame2.0&2.1
Verify video:"verify_1030912.mp4".

Flame2.0 build
Gaia-Rev        824a61cccec4c69be9a86ad5cb629a1f61fa142f
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/acde07cb4e4d
Build-ID        20141125040209
Version         36.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141125.113029
FW-Date         Tue Nov 25 11:30:43 EST 2014
Bootloader      L1TC00011880

Flame2.1 build:
Gaia-Rev        db2e84860f5a7cc334464618c6ea9e92ff82e9dd
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/211eae88f119
Build-ID        20141126001202
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141126.033519
FW-Date         Wed Nov 26 03:35:30 EST 2014
Bootloader      L1TC00011880

Comment 36

[SandKing]

Attachment: Verified video: verify_1030912.mp4