STD: 1. Launch settings app and go into Bluetooth panel. 2. Turn Bluetooth on and try to pair with a remote device. 3. After pairing process completely, checking the name on the paired device list. Expected: The name of paired device should be show normally. Or the name should be updated quickly. Actual: It shows 'Unknown device'.

====== Log =======: I/Settings( 1932): at btc__refreshPairedDevicesInfo (app://settings.gaiamobile.org/js/modules/bluetooth/bluetooth_context.js:418:10)

Jamin will take a look first. Assign owner to him.

Current implementation return |adapter.getPairedDevices| with adapter's cached devices array that may contain no device name. bluetooth1 adopts DOMRequest to retrieve the latest properties of remote devices from stack. Possible fix is to 1) make |adapter.getPairedDevices| async call as bluetooth1 or 2) revise update of adapter's cached devices array to get the latest properties.

Comment on attachment 8569748 [details] [diff] [review] Append name of paired device to 'DevicePaired' BT signal (v1) Review of attachment 8569748 [details] [diff] [review]: ----------------------------------------------------------------- Please see my comment below. ::: dom/bluetooth2/bluedroid/BluetoothServiceBluedroid.cpp @@ +1674,5 @@ > > + // Retrieve device name from hash table of pairing device name and append it > + // to the properties array. > + if (bonded) { > + nsString deviceName; Move this section into https://dxr.mozilla.org/mozilla-central/source/dom/bluetooth2/bluedroid/BluetoothServiceBluedroid.cpp#1642 -- Also suggest to revise as following: nsString deviceName = EmptyString(); if (/* hashtable.get() */) { ... } BT_APPEND_NAMED_VALUE(propertiesArray, "Name", deviceName); @@ +1679,5 @@ > + if (sPairingNameTable.Get(aRemoteBdAddr, &deviceName)) { > + if (aStatus == STATUS_SUCCESS) { > + BT_APPEND_NAMED_VALUE(propertiesArray, "Name", deviceName); > + } > + sPairingNameTable.Remove(aRemoteBdAddr); When to remove the entry if pairing fails (i.e., bonded = false)?

(In reply to Ben Tian [:btian] from comment #5) > Comment on attachment 8569748 [details] [diff] [review] > Append name of paired device to 'DevicePaired' BT signal (v1) > > Review of attachment 8569748 [details] [diff] [review]: > @@ +1679,5 @@ > > + if (sPairingNameTable.Get(aRemoteBdAddr, &deviceName)) { > > + if (aStatus == STATUS_SUCCESS) { > > + BT_APPEND_NAMED_VALUE(propertiesArray, "Name", deviceName); > > + } > > + sPairingNameTable.Remove(aRemoteBdAddr); > > When to remove the entry if pairing fails (i.e., bonded = false)? #attachment 8569748 [details] [diff] [review] can't handle pairing fails correctly. Thank you for pointing it out to me. I've uploaded a new patch #attachment 8571228 [details] [diff] [review] to fix this issue. Thanks.

Comment on attachment 8571228 [details] [diff] [review] Append name of paired device to 'DevicePaired' BT signal (v3) Review of attachment 8571228 [details] [diff] [review]: ----------------------------------------------------------------- Please see my comment below. ::: dom/bluetooth2/bluedroid/BluetoothServiceBluedroid.cpp @@ +1665,5 @@ > BT_APPEND_NAMED_VALUE(propertiesArray, "Address", nsString(aRemoteBdAddr)); > BT_APPEND_NAMED_VALUE(propertiesArray, "Paired", bonded); > > + // Retrieve device name from hash table of pairing device name. > + nsString deviceName = EmptyString(); Remove the empty string assignment since |deviceName| is used only when it exists in hash table. @@ +1670,5 @@ > + if (sPairingNameTable.Get(aRemoteBdAddr, &deviceName)) { > + sPairingNameTable.Remove(aRemoteBdAddr); > + if (bonded) { > + // Append BD name to signal properties if the device is paired. > + BT_APPEND_NAMED_VALUE(propertiesArray, "Name", deviceName); We should also append device name into device's "PropertyChanged" signal in case the device created in "DeviceFound" doesn't get name before pairing. We may revise appending order to reuse the property array.

- Refine previous patch based on comment 9.

Comment on attachment 8571709 [details] [diff] [review] Append name of paired device to 'DevicePaired' and 'PropertyChanged' BT signal (v4) Review of attachment 8571709 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comment addressed. ::: dom/bluetooth2/BluetoothCommon.h @@ +88,5 @@ > + * Wrap literal name and value into a BluetoothNamedValue > + * and insert it to the array. > + */ > +#define BT_INSERT_NAMED_VALUE(array, index, name, value) \ > + array.InsertElementAt(index, BluetoothNamedValue(NS_LITERAL_STRING(name), \ nit: align the two '\' ::: dom/bluetooth2/bluedroid/BluetoothServiceBluedroid.cpp @@ +1651,5 @@ > } else { > sAdapterBondedAddressArray.RemoveElement(remoteBdAddr); > } > > // Update attribute BluetoothDevice.paired Revise comment: // Update BluetoothDevice attributes: paired and name @@ +1657,5 @@ > BT_APPEND_NAMED_VALUE(propertiesArray, "Paired", bonded); > > + // Retrieve device name from hash table of pairing device name. > + nsString deviceName; > + if (sPairingNameTable.Get(aRemoteBdAddr, &deviceName)) { Replace with MOZ_ASSERT here since |BluetoothAdatper::HandleDevicePaired| does so. == bool nameExists = sPairingNameTable.Get(aRemoteBdAddr, &deviceName); MOZ_ASSERT(nameExists); sPairingNameTable.Remove(aRemoteBdAddr); if (bonded) { // Append BD name to signal properties if the device is paired. BT_APPEND_NAMED_VALUE(propertiesArray, "Name", deviceName); }

- Refine previous patch based on #comment 11

No try server link since this patch is made for BlueDroid and tinderbox use BlueZ as hardware stack.

https://hg.mozilla.org/integration/b2g-inbound/rev/83f27620ef8e Thank Ben for reviewing the patch.

There is a defect in #attachment 8571821 [details] [diff] [review]. I'll upload another patch to fix it today. Sorry for the inconvenience caused

Comment on attachment 8571858 [details] [diff] [review] Fix the condition of if-statement of name updating when the Bluetoooth device paired. (v1) Review of attachment 8571858 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comment addressed. ::: dom/bluetooth2/bluedroid/BluetoothServiceBluedroid.cpp @@ +1659,3 @@ > // Retrieve device name from hash table of pairing device name. > nsString deviceName = EmptyString(); > bool nameExists = sPairingNameTable.Get(aRemoteBdAddr, &deviceName); Remove |nameExists| as following: nsString deviceName = EmptyString(); if (sPairingNameTable.Get(aRemoteBdAddr, &deviceName)) { sPairingNameTable.Remove(aRemoteBdAddr); } if (bonded && aStatus == STATUS_SUCCESS) { MOZ_ASSERT(!deviceName.IsEmpty()); BT_APPEND_NAMED_VALUE(propertiesArray, "Name", deviceName); }

One more thing: please also assert deviceName is not empty in [1]. [1] https://dxr.mozilla.org/mozilla-central/source/dom/bluetooth2/BluetoothAdapter.cpp#900 (In reply to Ben Tian [:btian] from comment #17) > Remove |nameExists| as following: > > nsString deviceName = EmptyString(); > if (sPairingNameTable.Get(aRemoteBdAddr, &deviceName)) { > sPairingNameTable.Remove(aRemoteBdAddr); > } > > if (bonded && aStatus == STATUS_SUCCESS) { > MOZ_ASSERT(!deviceName.IsEmpty()); > BT_APPEND_NAMED_VALUE(propertiesArray, "Name", deviceName); > }

(In reply to Ben Tian [:btian] from comment #19) > One more thing: please also assert deviceName is not empty in [1]. > > [1] > https://dxr.mozilla.org/mozilla-central/source/dom/bluetooth2/ > BluetoothAdapter.cpp#900 > > (In reply to Ben Tian [:btian] from comment #17) > > Remove |nameExists| as following: > > > > nsString deviceName = EmptyString(); > > if (sPairingNameTable.Get(aRemoteBdAddr, &deviceName)) { > > sPairingNameTable.Remove(aRemoteBdAddr); > > } > > > > if (bonded && aStatus == STATUS_SUCCESS) { > > MOZ_ASSERT(!deviceName.IsEmpty()); > > BT_APPEND_NAMED_VALUE(propertiesArray, "Name", deviceName); > > } A valid Bluetooth name is a UTF-8 encoding string which is up to 248 bytes in length. Some Bluetooth devices might allow users to set device name to an empty string. Therefore, I prefer to remove the MOZ_ASSERT statement in BluetoothAdapter::HandleDevicePaired(). I'll upload a new patch, please feel free to leave comments.

Comment on attachment 8572488 [details] [diff] [review] Fix the condition of if-statement of name updating when the Bluetoooth device paired. (v2) Review of attachment 8572488 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bluetooth2/BluetoothAdapter.cpp @@ -896,5 @@ > MOZ_ASSERT(arr.Length() == 3 && > arr[0].value().type() == BluetoothValue::TnsString && // Address > arr[1].value().type() == BluetoothValue::Tbool && // Paired > arr[2].value().type() == BluetoothValue::TnsString); // Name > - MOZ_ASSERT(!arr[0].value().get_nsString().IsEmpty() && Keep this assertion since it checks not name but address. ::: dom/bluetooth2/bluedroid/BluetoothServiceBluedroid.cpp @@ +1668,5 @@ > MOZ_ASSERT(nameExists); > + > + // According to Bluetooth Core Spec. v3.0 - Sec. 6.22, a valid Bluetooth > + // name is a UTF-8 encoding string which is up to 248 bytes in length. > + // e.g. An empty string is also a valid name. Revise comment to mention first thing first. // We don't assert |!deviceName.IsEmpty()| here since empty string is also a valid name. // According to ...

Comment on attachment 8572488 [details] [diff] [review] Fix the condition of if-statement of name updating when the Bluetoooth device paired. (v2) Review of attachment 8572488 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comment 22 addressed.

- revise previous patch based on #comment 23

- Remove the 'leave-open' keyword I added before. - Mark as RESOLVED FIXED since all patches have landed.