935882 - [bluedroid] Support GetConnectedDevice API

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

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

Support GetConnectedDevice API

Comment 1

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

Attachment: Bug 935882 - [bluedroid] Support GetConnectedDevice API

Comment 2

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 8334559 [details] [diff] [review]
Bug 935882 - [bluedroid] Support GetConnectedDevice API

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

The main reason I r- is that I would like to propose using only one array to keep DOM requests which were sent via GetPairedDevices() and GetConnectedDevices().

::: dom/bluetooth/bluedroid/gonk/BluetoothServiceBluedroid.cpp
@@ +386,5 @@
>        BT_LOGR("No runnable to return");
>        return;
>      }
>  
> +    if (!sGetPairedDeviceRunnableArray.IsEmpty()) {

I don't think this mechanism would work. It's definitely possible that sGetPairedDeviceRunnableArray and sGetConnectedDeviceRunnableArray may both have elements. In that case we will fire the DOM request of GetPairedDevice() before GetConnectedDevice() anyway and it's clearly not right.

We should only use one of these two runnable array since we have already assumed that bluedroid will handle each request in sequence.

@@ +792,5 @@
> +    DispatchBluetoothReply(aRunnable, BluetoothValue(emptyArr), EmptyString());
> +    return NS_OK;
> +  }
> +
> +  nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;

Unused variable.

Comment 3

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

Attachment: Bug 935882 - [bluedroid] Support GetConnectedDevice API

Comment 4

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

Attachment: Bug 935882 - [bluedroid] Support GetConnectedDevice API

Comment 5

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 8335880 [details] [diff] [review]
Bug 935882 - [bluedroid] Support GetConnectedDevice API

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

r=me with nits addressed.

::: dom/bluetooth/bluedroid/gonk/BluetoothServiceBluedroid.cpp
@@ +812,5 @@
>  {
>    MOZ_ASSERT(NS_IsMainThread());
>  
> +  nsAutoString errorStr;
> +  BluetoothValue values = InfallibleTArray<BluetoothNamedValue>();

Variable 'values' is only used at line 827, so please move this into that if-block.

@@ +841,5 @@
> +    DispatchBluetoothReply(aRunnable, BluetoothValue(emptyArr), EmptyString());
> +    return NS_OK;
> +  }
> +
> +  nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;

Please remove unused variable 'runnable'.

Comment 6

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

Attachment: Bug 935882 - [bluedroid] Support GetConnectedDevice API, r=echou

Comment 7

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/integration/b2g-inbound/rev/861bcc802dc8

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/861bcc802dc8