1016716 - [Tarako] Turning off the bluetooth should cancel any incoming request automatically

Status: VERIFIED FIXED

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

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

1. on a computer request bluetooth transfer
2. before the bluetooth overlay comes up, turn off blue tooth

Expected: no bluetooth overlay
Actual: bluetooth overlay comes up and if you try to dismiss the overlay it will still partially show at the top until next reboot.

See video : http://www.youtube.com/watch?v=rmiT7N57r5g

Note:
1. 1.4 is not affected with the graphics defect; the overlay still does appear.

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Gaia      c8f6ba7aa9a694f5f691555a0d049f5630cdde3d
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/bde42427e332
BuildID   20140527014007
Version   28.1
ro.build.version.incremental=eng.cltbld.20140527.053404
ro.build.date=Tue May 27 05:34:12 EDT 2014
Tarako

Comment 2

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

Looks like a timing issue. That the pairing request event is coming before a user turn Bluetooth off.

Comment 3

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

Hi Naoki, is the pairing dialog able to close in this case on v1.3t?

Comment 4

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Yes, it is a timing case; so it is an edge case.  This is why I did not nom it.

For Tarako, no.  The screen is not fully dismissed and part of the overlay seems to be persistant at the very top as seen in the video until reboot.  Closing the apps will still show part of the overlay.

Comment 5

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

Yes, I'm able to reproduce the issue. After the issue is happened, sometimes the system will be crash. But the issue is not existed in the code base of master. There is an error log as following while the issue is happened.

log: E/GeckoConsole(  470): [JavaScript Error: "NS_ERROR_FAILURE: " {file: "app://bluetooth.gaiamobile.org/shared/js/bluetooth_helper.js" line: 63}]

It looks like the referenced variable '_adapter' is no longer for access while Bluetooth is turned off. I have tried to check 'if (_adapter)' in this case. The '_adapter' is still existed. I will try to use observation for Bluetooth on/off in somewhere. Then, return the function call directly if Bluetooth is turned off. But, I think it might be some race condition here. CC Gecko::Bluetooth devs too.

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Do you think this is important to raise as a 1.3T blocker?  If you do, please nom the bug for 1.3T?

Comment 7

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

I think this is a blocking issue. Nominating 1.3T+..

Comment 8

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

Attachment: pull request 19980

Observe `mozBluetooth.ondisabled`, settings key `bluetooth.enabled` event to close pairing dialog while Bluetooth is just turned off.

Arthur, could you please help to review the pull request? Thanks.

Comment 9

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

(In reply to Ian Liu [:ianliu] from comment #7)
> I think this is a blocking issue. Nominating 1.3T+..
There is a way to reproduce the issue. Once the pairing dialog is showing, pull down the notification center, click quick settings to turn off Bluetooth. Then, a user is able to click 'close' or 'pair' button while Bluetooth is in disabled mode. On v1.3T, the dialog is no longer to close, sometimes the system is crashing after function call run on useless adapter. Since the issue is easy to reproduce as above mention, I'm nominating to 1.3T. NeedInfo Steven.

On v1.4/master, there is no blocking issue on the UI flow. But it's better to close the pairing dialog while Bluetooth is disabled. Looks like it's a long issue that we have not tracked on it.

Comment 10

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8433881 [details] [review]
pull request 19980

Looks good to me. r=me.

Comment 11

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

Thanks for Arthur's reviewing effort. Since the patch is landed on gaia/v1.3t, we can close the issue now.

Gaia/v1.3t: c7d82808c695537fc258accb4217c58eb8160c48

Comment 12

[Michael Henretty [:mikehenrty][:mhenretty]]

Do we need to do anything for 1.4?

Comment 13

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

I think it's worth and needed to give a patch for master/2.0. We really no need to let a user to confirm pairing request while Bluetooth is off.

For v1.4, the code base of pairing flow is different from master/2.0/v1.3t. It will need some more change to make it work. The issue *graphics defect* is only blocking v1.3t.

Comment 14

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

Attachment: pull request 20279

Arthur, 

I would like to land the patch for master/2.0. Beside of additional unit test, the pull request is same with pr 19980. Could you please help to review it? Thanks.

Comment 15

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8437559 [details] [review]
pull request 20279

r=me with the comment addressed. Please restore the created spies and stubs in the teardown function.

Comment 16

[Arthur Chen [:arthurcc][inactive after 6/30]]

BTW, please remember to edit the commit message as it is going to be checked in to master instead of the tarako branch.

Comment 17

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

(In reply to Arthur Chen [:arthurcc] from comment #15)
> Comment on attachment 8437559 [details] [review]
> pull request 20279
> 
> r=me with the comment addressed. Please restore the created spies and stubs
> in the teardown function.

Per offline discussion with Arthur: Since this.sinon will be restored for each test, I think we don't need to create a variable to catch the return value for the restore work.

Comment 18

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

I have updated the commit message. Thanks for the reminder.:)

Gaia/master:  96e9c34d2e44036de6be5c25186194218ac0e50b

Comment 19

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

Comment on attachment 8437559 [details] [review]
pull request 20279

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Defect
[User impact] if declined: A user is able to respond the confirmation while Bluetooth is off. Then, it will run and call the method of adapter. But the adapter is useless while Bluetooth is off already. Some of the platform might occur unexpected issue.
[Testing completed]: Done and passed.
[Risk to taking this patch] (and alternatives if risky): Low.
[String changes made]: None.

Comment 20

[bhavana bajaj [:bajaj]]

Requesting QA verification on 2.0 once this lands.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

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

Treating comment 13 as a wontfix for v1.4.

Comment 22

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Tarako 1.3, Flame 2.1 and 2.0:

Result: The pairing request does not show up if the user turns off Bluetooth.

Tarako 1.3

Device: Tarako 1.3
BuildID: 20141027014003
Gaia: 265776c72f3be59ca6915a0c7d83bfdd42281f26
Gecko: 2d419d110f66
Version: 28.1 (Unknown)
Firmware: SP6821a-Gonk-4.0-5-12
User Agent: User Agent: Mozilla/5.0 (Mobile; rv:28.1) Gecko/28.1 Firefox/28.1

Flame 2.1 

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141028001203
Gaia: a0174f7166745256aaca1cb3aa9f894033fbffa6
Gecko: 43bda3541f6b
Gonk: 6e51d9216901d39d192d9e6dd86a5e15b0641a89
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.0

Device: Flame 2.0 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141028000202
Gaia: 5e532a84e762b1bb6231756182cf1465671a061e
Gecko: 124f0bed1700
Gonk: 6e51d9216901d39d192d9e6dd86a5e15b0641a89
Version: 32.0 (2.0)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0