979370 - [Bluetooth] Move BT thread into BlueZ backend

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Thomas Zimmermann [:tzimmermann] [:tdz]]

The BT thread is only necessary for the BlueZ backend. We should remove it from the generic code BluetoothService and move it into BluetoothDBusService.

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [01] Bug 979370: Move Bluetooth thread into BlueZ backend

Comment 2

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [02] Bug 979370: Split |BluetoothService::StartStopBluetooth|

Comment 3

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [03] Bug 979370: Remove |ToggleBtTask|

Comment 4

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [04] Bug 979370: Remove |BluetoothService::IsEnabledInternal|

Comment 5

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

Comment on attachment 8386717 [details] [diff] [review]
[01] Bug 979370: Move Bluetooth thread into BlueZ backend

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

Looks good to me. This r+ is actually based on that I know other patches will do the cleanup on stuff like removing ToggleBtTask.

Comment 6

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

Hi Thomas,

Sorry for being a little late. I've been attending media work week this week. I'll find some time to review your patches. :)

Comment 7

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

Comment on attachment 8386718 [details] [diff] [review]
[02] Bug 979370: Split |BluetoothService::StartStopBluetooth|

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

Looks good.

Comment 8

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

Comment on attachment 8386719 [details] [diff] [review]
[03] Bug 979370: Remove |ToggleBtTask|

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

r=me with nits picked. Thanks.

::: dom/bluetooth/BluetoothService.cpp
@@ +465,5 @@
>    }
>  
>    mAdapterAddedReceived = false;
>  
> +  /* When IsEnabled() is false, we don't switch on Bluetooth but we still

nit: we don't switch /off/ Bluetooth

Comment 9

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

Comment on attachment 8386720 [details] [diff] [review]
[04] Bug 979370: Remove |BluetoothService::IsEnabledInternal|

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

Looks good. Thanks.

Comment 10

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [03] Bug 979370: Remove |ToggleBtTask| (v2)

Changes to v1:

  - fixed comment in |BluetoothService::StopBluetooth|

Comment 11

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Thank you very much, Eric!

https://hg.mozilla.org/integration/b2g-inbound/rev/a77f6548e510
https://hg.mozilla.org/integration/b2g-inbound/rev/55032b8c9281
https://hg.mozilla.org/integration/b2g-inbound/rev/30f9202e4f92
https://hg.mozilla.org/integration/b2g-inbound/rev/4a581a135772

https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=4a581a135772

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/a77f6548e510
https://hg.mozilla.org/mozilla-central/rev/55032b8c9281
https://hg.mozilla.org/mozilla-central/rev/30f9202e4f92
https://hg.mozilla.org/mozilla-central/rev/4a581a135772