1164314 - [BT Setting] Paired devices would appear in both 'Paired devices' and 'Devices in the area' sections

Status: RESOLVED FIXED

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

Description

[Jamin Liu [:jaliu][:Jamin]]

[Description]:
Paired devices would appear in both 'Paired devices' and 'Devices in the area' sections of BT setting.
If user click the paired device in 'Devices in the area' again, the device would be always pairing.

[Testing Steps]: 
1. Turn on bluetooth. 
2. Pair with a remote BT device. 
3. Operate on the remote device, tap to accept the pairing request.

[Expected Result]
The paired device is removed from 'Devices in the area'.

[Actual Result]
The paired devices appears in both 'Paired devices' and 'Devices in the area'

Comment 2

[Jamin Liu [:jaliu][:Jamin]]

Hi Ian,
Would you mind to take a look ?

Comment 3

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

It's really bad to see the problem. But I believe the problem is not happened before so that request regression window first. I will investigate in the same time.

Comment 4

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Bug 1156503 seems to have caused this issue

B2g-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150510095405
Gaia: 6089234ace8b294a8feef064387604bae16254e3
Gecko: 645a1e17de30
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

First Broken 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150510190405
Gaia: 6089234ace8b294a8feef064387604bae16254e3
Gecko: 152f16cf0602
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Last Working gaia / First Broken gecko - Issue DOES occur
Gaia: 6089234ace8b294a8feef064387604bae16254e3
Gecko: 152f16cf0602

First Broken gaia / Last Working gecko - Issue does NOT occur
Gaia: 6089234ace8b294a8feef064387604bae16254e3
Gecko: 645a1e17de30

Gecko Pushlog: http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=645a1e17de30&tochange=152f16cf0602

Comment 5

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

Ben, can you take a look at this please? This looks to have been caused by the work done on bug 1156503.

Comment 6

[Ben Tian (inactive)]

This is indeed regression by bug 1156503. I'll prepare a fix. Take this bug.

Comment 7

[Ben Tian (inactive)]

Set 2.2? since bug 1156503 is 2.2+.

Comment 8

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

Note: After investigation, the callback of |defaultAdapter.ondevicefound| will be trigger with the paired device. So that Gaia show(create) the found device in 'Devices in the area' list.

Comment 9

[howie [:howie]]

Hi Aaron, please help to triage this.

Comment 10

[Ben Tian (inactive)]

Attachment: Patch 1 (v1): Fire devicefound in RemoteDevicePropertiesNotification only during discovery

The patch fires devicefound to update device name ONLY during discovery.

Comment 11

[Aaron Wu]

Mark it as 2.2+

Comment 12

[Aaron Wu]

correct to 2.2+

Comment 13

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8605689 [details] [diff] [review]
Patch 1 (v1): Fire devicefound in RemoteDevicePropertiesNotification only during discovery

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

A few things need to confirm first, glad to review again after things clarified.

::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
@@ +2673,2 @@
>       */
> +    if (sAdapterDiscovering) {

Can you confirm this won't be executed when bluetooth is turned on?

@@ +2815,5 @@
>    }
>  #else
>    MOZ_ASSERT(NS_IsMainThread());
>  
> +  sAdapterDiscovering = aState;

Do you think we should also update |sAdapterDiscovering| in AdapterPropertiesNotification?

Comment 14

[Ben Tian (inactive)]

(In reply to Shawn Huang [:shawnjohnjr] from comment #13)
> Can you confirm this won't be executed when bluetooth is turned on?

It's not executed since |RemoteDevicePropertiesNotification| triggered by |GetPairedDevices| fails the condition [1].
[1] https://dxr.mozilla.org/mozilla-central/source/dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp#2657

> Do you think we should also update |sAdapterDiscovering| in
> AdapterPropertiesNotification?

I think it's redundant since discovery is always set OFF before bluetooth is off [2].

===
[2]
GeckoBluetooth: OnError: BluetoothSocketInterface::Accept failed: 1
GeckoBluetooth: DiscoveryStateChangedNotification: 0
GeckoBluetooth: DiscoveryStateChangedNotification: 0    <-------- discovery stops
GeckoBluetooth: AdapterStateChangedNotification: BT_STATE: 0  <-- bluetooth is OFF

Comment 15

[Ben Tian (inactive)]

Attachment: [final] Patch 1: Fire devicefound in RemoteDevicePropertiesNotification only during discovery, r=shuang

Correct comment that |RemoteDevicePropertiesNotification| is called when 'BT is turning on.' In this case devicefound wouldn't be fired since |sAdapterDiscovering| is false.

Comment 16

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8605758 [details] [diff] [review]
[final] Patch 1: Fire devicefound in RemoteDevicePropertiesNotification only during discovery, r=shuang

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

Code looks good. Thanks for taking care of this bug.

Comment 17

[Ben Tian (inactive)]

Comment on attachment 8605758 [details] [diff] [review]
[final] Patch 1: Fire devicefound in RemoteDevicePropertiesNotification only during discovery, r=shuang

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e1454dd187c

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/8b9eeefbba70

Comment 19

[Ben Tian (inactive)]

Attachment: (v2.2) Patch 1: Fire devicefound in RemoteDevicePropertiesNotification only during discovery, r=shuang

Request qa-approval for 2.2 patch.

Eric, the fix relates to BT discovery and pairing. Please refer to comment 0 & bug 1156503, and try different discovery/pairing orders. Let me know for any side effect. Thanks.

Comment 20

[Eric Chang [:ericcc] [:echang]]

Let me try this on FLAME-KK & NEXUS-5-L.

Comment 21

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

https://hg.mozilla.org/mozilla-central/rev/8b9eeefbba70

Comment 22

[Eric Chang [:ericcc] [:echang]]

--nexus-5-l building failed(either with/without patch), will try that tomorrow.

...
collect2: error: ld returned 1 exit status
make[7]: *** [/home/ericcc/repo/mozilla-b2g/B2G-1164314-NEXUS-5-L/objdir-gecko/security/nss/cmd/shlibsign/shlibsign] Error 1
make[6]: *** [libs-nss/cmd/shlibsign] Error 2
make[6]: *** Waiting for unfinished jobs....
pk11.c: In function 'ListModule':
pk11.c:612:16: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
      if(reason < numDisableReasonStr) {
                ^
keystuff.c: In function 'UpdateRNG':
keystuff.c:96:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     while (count < sizeof randbuf) {
                  ^
ytab.c: In function 'Pk11Install_yyparse':
ytab.c:219:5: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
In function 'SECItemToHex',
    inlined from 'PrintKey.isra.2' at certutil.c:790:21,
    inlined from 'ListKeysInSlot' at certutil.c:860:10:
certutil.c:755:13: warning: call to 'sprintf' declared with attribute warning: sprintf is often misused; please use snprintf [enabled by default]
      sprintf(dst, "%02x", *src++);
             ^
certutil.c: In function 'getObjectClass':
certutil.c:2035:12: warning: call to 'sprintf' declared with attribute warning: sprintf is often misused; please use snprintf [enabled by default]
     sprintf(buf, "0x%lx", classType);
            ^
install.c: In function 'rm_dash_r.part.0':
install.c:834:20: warning: call to 'sprintf' declared with attribute warning: sprintf is often misused; please use snprintf [enabled by default]
             sprintf(filename, "%s/%s", path, entry->name);
                    ^
install-ds.c: In function 'Pk11Install_PlatformName_GetVerString':
install-ds.c:614:10: warning: call to 'sprintf' declared with attribute warning: sprintf is often misused; please use snprintf [enabled by default]
   sprintf(buf, "%s.", _this->verString[i]);
          ^
cd builtins; make libs
cts.c: In function 'CTS_DecryptUpdate':
cts.c:226:24: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     tmp =  (fullblocks < blocksize*2) ? cts->iv :
                        ^
gcm.c: In function 'gcm_getX':
gcm.c:223:13: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (len != blocksize) {
             ^
bfind.c: In function 'builtins_attrmatch':
bfind.c:122:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
      (len == a->ulValueLen) && 
           ^
dh.c: In function 'DH_Derive':
...

Comment 23

[Eric Chang [:ericcc] [:echang]]

Attachment: NEXUS-5-L_j1_error.log

$BRANCH=v2.2 ./config.sh nexus-5-l
Error log from $./build.sh gecko -j1

Comment 24

[Eric Chang [:ericcc] [:echang]]

Comment on attachment 8606136 [details] [diff] [review]
(v2.2) Patch 1: Fire devicefound in RemoteDevicePropertiesNotification only during discovery, r=shuang

Patch on FLAME-KK works as expected, discover-ability, renaming, pairing/un-pairing, device list ... etc. are fine.

Not testing on NEXUS-5-L, unable to build at this moment.

Comment 25

[Ben Tian (inactive)]

Comment on attachment 8606136 [details] [diff] [review]
(v2.2) Patch 1: Fire devicefound in RemoteDevicePropertiesNotification only during discovery, r=shuang

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  1156503
User impact if declined: 
  Paired devices appear in both "Paired Devices" and "Devices in the area"
Testing completed: 
  treeherder (comment 17) and qa test (comment 24)
Risk to taking this patch (and alternatives if risky): 
  None
String or UUID changes made by this patch:
  None

Comment 26

[Josh Cheng [:josh]]

Hi Eric,
Please also verify on 2.2 after patch land there.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/40b38e97cd28

Comment 28

[Eric Chang [:ericcc] [:echang]]

Hi Norry, Please help to check this on both FLAME-KK & NEXUS-5-L, thank you.

Comment 29

[Verson Xiong (Leave from Mozilla)]

Attachment: Verify.mp4

This bug has been verified as pass on latest build of Flame v2.2&3.0,Nexus5 v2.2&3.0 by the STR in Comment 0.
Actually Result：The paired device is removed from 'Devices in the area'.
Reproduce rate:0/5
See attachment:verify.mp4

Device:Flame v2.2 build(Pass)
Build ID               20150521002508
Gaia Revision          bc42fbc12d622bffd7e8afcb8d56f8a1d9773c60
Gaia Date              2015-05-20 22:32:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6e4eaf59efda
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150521.040918
Firmware Date          Thu May 21 04:09:27 EDT 2015
Bootloader             L1TC000118D0

Device:Flame v3.0 build(Pass)
Build ID               20150521160241
Gaia Revision          1126d8bee559f7cde675df2fcc6c196da9cfeba1
Gaia Date              2015-05-21 21:23:56
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3e737d30f842
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150521.193021
Firmware Date          Thu May 21 19:30:31 EDT 2015
Bootloader             L1TC000118D0

Device:Nexus 5 v2.2 build(Pass)
Build ID               20150521002508
Gaia Revision          bc42fbc12d622bffd7e8afcb8d56f8a1d9773c60
Gaia Date              2015-05-20 22:32:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6e4eaf59efda
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150521.040628
Firmware Date          Thu May 21 04:06:43 EDT 2015
Bootloader             HHZ12f

Device:Nexus 5 v3.0 build(Pass)
Build ID               20150521160241
Gaia Revision          1126d8bee559f7cde675df2fcc6c196da9cfeba1
Gaia Date              2015-05-21 21:23:56
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3e737d30f842
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150521.193039
Firmware Date          Thu May 21 19:30:54 EDT 2015
Bootloader             HHZ12f