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

RESOLVED FIXED in Firefox 41

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jaliu, Assigned: ben.tian)

Tracking

({regression})

unspecified
2.2 S12 (15may)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.2+, firefox39 wontfix, firefox40 wontfix, firefox41 fixed, b2g-v2.2 verified, b2g-master verified)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
[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'
(Reporter)

Updated

4 years ago
See Also: → 1163967
(Reporter)

Updated

4 years ago
Duplicate of this bug: 1163967
(Reporter)

Comment 2

4 years ago
Hi Ian,
Would you mind to take a look ?
Flags: needinfo?(iliu)
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.
Assignee: nobody → iliu
Flags: needinfo?(iliu)
Status: NEW → ASSIGNED
QA Contact: jmercado
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
Blocks: 1156503
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Ben, can you take a look at this please? This looks to have been caused by the work done on bug 1156503.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?(btian)
(Assignee)

Comment 6

4 years ago
This is indeed regression by bug 1156503. I'll prepare a fix. Take this bug.
Assignee: iliu → btian
Flags: needinfo?(btian)
(Assignee)

Comment 7

4 years ago
Set 2.2? since bug 1156503 is 2.2+.
blocking-b2g: --- → 2.2?
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

4 years ago
Hi Aaron, please help to triage this.
Component: Gaia::Settings → Gaia::Bluetooth
Flags: needinfo?(awu)
(Assignee)

Comment 10

4 years ago
The patch fires devicefound to update device name ONLY during discovery.
Attachment #8605689 - Flags: review?(shuang)

Comment 11

4 years ago
Mark it as 2.2+
blocking-b2g: 2.2? → 2.1+
Flags: needinfo?(awu)

Comment 12

4 years ago
correct to 2.2+
blocking-b2g: 2.1+ → 2.2+
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?
Attachment #8605689 - Flags: review?(shuang)
(Assignee)

Comment 14

4 years ago
(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
(Assignee)

Comment 15

4 years ago
Correct comment that |RemoteDevicePropertiesNotification| is called when 'BT is turning on.' In this case devicefound wouldn't be fired since |sAdapterDiscovering| is false.
Attachment #8605689 - Attachment is obsolete: true
Attachment #8605758 - Flags: review?(shuang)
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.
Attachment #8605758 - Flags: review?(shuang) → review+
(Assignee)

Comment 17

4 years ago
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
Attachment #8605758 - Attachment description: Patch 1 (v2): Fire devicefound in RemoteDevicePropertiesNotification only during discovery → [final] Patch 1: Fire devicefound in RemoteDevicePropertiesNotification only during discovery, r=shuang
(Assignee)

Comment 19

4 years ago
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.
Attachment #8606136 - Flags: qa-approval?(echang)
Let me try this on FLAME-KK & NEXUS-5-L.
https://hg.mozilla.org/mozilla-central/rev/8b9eeefbba70
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S12 (15may)
--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':
...
$BRANCH=v2.2 ./config.sh nexus-5-l
Error log from $./build.sh gecko -j1
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.
Attachment #8606136 - Flags: qa-approval?(echang) → qa-approval+
(Assignee)

Comment 25

4 years ago
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
Attachment #8606136 - Flags: approval-mozilla-b2g37?

Updated

4 years ago
Attachment #8606136 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Hi Eric,
Please also verify on 2.2 after patch land there.
Flags: needinfo?(echang)
Keywords: verifyme
(Assignee)

Updated

4 years ago
See Also: → 1166636
Hi Norry, Please help to check this on both FLAME-KK & NEXUS-5-L, thank you.
Flags: needinfo?(echang) → needinfo?(fan.luo)
Posted video 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
Flags: needinfo?(fan.luo)
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.