Josh Schmitt [Joshs] Reporter
	Comment 2 • 10 years ago

adding qawanted to check other branches/devices.

	Chris Kreinbring [:CKreinbring]
	Comment 3 • 10 years ago

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.

	Joshua Mitchell (Inactive)
	Comment 4 • 10 years ago

issue is a regression but not nomming - issue seems cosmetic

	Jason Smith [:jsmith]
	Comment 5 • 10 years ago

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?

	Beatriz Rodríguez [:brg]
	Comment 6 • 10 years ago

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

	Beatriz Rodríguez [:brg]
	Comment 7 • 10 years ago

Nominating accordingly.

	Anthony Ricaud (:rik)
	Comment 8 • 10 years ago

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.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 9 • 10 years ago

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

	Doug Sherk (:drs) (inactive) Assignee
	Comment 10 • 10 years ago

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

	Doug Sherk (:drs) (inactive) Assignee
	Comment 11 • 10 years ago

Tracked down to commit 7afce8a6e965a45f38988227828ccdfb315394f3

	Doug Sherk (:drs) (inactive) Assignee
	Comment 12 • 10 years ago

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

	Anthony Ricaud (:rik)
	Comment 13 • 10 years ago

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

	Doug Sherk (:drs) (inactive) Assignee
	Comment 14 • 10 years ago

(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.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 15 • 10 years ago

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.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 16 • 10 years ago

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

	Etienne Segonzac (:etienne)
	Comment 17 • 10 years ago

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?

	Doug Sherk (:drs) (inactive) Assignee
	Comment 18 • 10 years ago

(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.

	Etienne Segonzac (:etienne)
	Comment 19 • 10 years ago

(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?

	Doug Sherk (:drs) (inactive) Assignee
	Comment 20 • 10 years ago

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.

	Etienne Segonzac (:etienne)
	Comment 21 • 10 years ago

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.

	iliu@mozilla.com, ianliu.moz@gmail.com
	Comment 22 • 10 years ago

(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.

	iliu@mozilla.com, ianliu.moz@gmail.com
	Comment 23 • 10 years ago

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.:)

	iliu@mozilla.com, ianliu.moz@gmail.com
	Comment 24 • 10 years ago

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.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 25 • 10 years ago

(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.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 26 • 10 years ago

Added test and merged:

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

	Doug Sherk (:drs) (inactive) Assignee
	Comment 27 • 10 years ago

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.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 28 • 10 years ago

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.

	Vance Chen [:vchen][skype:vance.lucida][vchen@mozilla.com]
	Comment 29 • 10 years ago

(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

	Ryan VanderMeulen [:RyanVM]
	Comment 30 • 10 years ago

Needs a branch patch for v1.4 uplift.

	Ryan VanderMeulen [:RyanVM]
	Comment 31 • 10 years ago

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

	Doug Sherk (:drs) (inactive) Assignee
	Comment 32 • 10 years ago

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

	SandKing
	Comment 35 • 10 years ago

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