Make Bluetooth modules independent from Setup module

RESOLVED FIXED in Firefox 44

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: tzimmermann, Assigned: tzimmermann)

Tracking

unspecified
FxOS-S10 (30Oct)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(7 attachments, 6 obsolete attachments)

8.24 KB, patch
brsun
: review+
Details | Diff | Splinter Review
17.46 KB, patch
tzimmermann
: review+
Details | Diff | Splinter Review
15.91 KB, patch
tzimmermann
: review+
Details | Diff | Splinter Review
16.43 KB, patch
tzimmermann
: review+
Details | Diff | Splinter Review
13.61 KB, patch
tzimmermann
: review+
Details | Diff | Splinter Review
18.15 KB, patch
tzimmermann
: review+
Details | Diff | Splinter Review
3.37 KB, patch
tzimmermann
: review+
Details | Diff | Splinter Review
The Bluetooth backend modules contain the methods |RegisterModuleCmd| and |UnregisterModuleCmd| to call into the Setup module and (un-)register themselves.

The dependency is non-obvious and not necessary. We should move the backend registering and unregistering into the profile mangers.
Created attachment 8670322 [details] [diff] [review]
[01] Bug 1211948: Add interface class for Setup module to Bluetooth backend interface
Attachment #8670322 - Flags: review?(shuang)
Created attachment 8670323 [details] [diff] [review]
[02] Bug 1211948: Introduce |enum BluetoothSetupServiceId|
Attachment #8670323 - Flags: review?(shuang)
Created attachment 8670324 [details] [diff] [review]
[03] Bug 1211948: Register Bluetooth A2DP module in A2DP manager
Attachment #8670324 - Flags: review?(shuang)
Created attachment 8670325 [details] [diff] [review]
[04] Bug 1211948: Register Bluetooth AVRCP module in AVRCP manager
Attachment #8670325 - Flags: review?(shuang)
Created attachment 8670327 [details] [diff] [review]
[05] Bug 1211948: Register Bluetooth GATT module in GATT manager
Attachment #8670327 - Flags: review?(joliu)
Created attachment 8670328 [details] [diff] [review]
[06] Bug 1211948: Register Bluetooth Handsfree module in Handsfree manager
Attachment #8670328 - Flags: review?(btian)
Created attachment 8670329 [details] [diff] [review]
[07] Bug 1211948: Remove registry interfaces from |BluetoothDaemonProtocol|
Attachment #8670329 - Flags: review?(shuang)

Comment 8

3 years ago
Comment on attachment 8670328 [details] [diff] [review]
[06] Bug 1211948: Register Bluetooth Handsfree module in Handsfree manager

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

r=me with questions addressed.

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
@@ +291,5 @@
>    void OnError(BluetoothStatus aStatus) override
>    {
> +    MOZ_ASSERT(NS_IsMainThread());
> +
> +    BT_WARNING("BluetoothSetupInterface::RegisterModule failed: %d", (int)aStatus);

Should we reset notification handler as nullptr when module registration failed?

@@ +355,5 @@
> +    }
> +    return;
> +  }
> +
> +  auto btInf = BluetoothInterface::GetInstance();

Is the 'auto' required for some reason or just to make the code less cluttered?
Attachment #8670328 - Flags: review?(btian) → review+
Comment on attachment 8670327 [details] [diff] [review]
[05] Bug 1211948: Register Bluetooth GATT module in GATT manager

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

::: dom/bluetooth/bluedroid/BluetoothDaemonGattInterface.cpp
@@ +2008,4 @@
>  BluetoothDaemonGattInterface::~BluetoothDaemonGattInterface()
>  { }
>  
> +#if 0

Hi Thomas,

Could you explain that why we need to keep these code blocks here and below?

Thanks,
Jocelyn
Hey!

> ::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
> @@ +291,5 @@
> >    void OnError(BluetoothStatus aStatus) override
> >    {
> > +    MOZ_ASSERT(NS_IsMainThread());
> > +
> > +    BT_WARNING("BluetoothSetupInterface::RegisterModule failed: %d", (int)aStatus);
> 
> Should we reset notification handler as nullptr when module registration
> failed?

Sounds like a good idea. I don't think we do this now, but definitely makes sense. Will be part of the next version. 

> @@ +355,5 @@
> > +    }
> > +    return;
> > +  }
> > +
> > +  auto btInf = BluetoothInterface::GetInstance();
> 
> Is the 'auto' required for some reason or just to make the code less
> cluttered?

It's not strictly required. Other parts of Gecko already make use of 'auto' and I found it easier to read than long type names.
> 
> Could you explain that why we need to keep these code blocks here and below?

Oh, sorry about that. Sometimes I forget to clean up these blocks. :( Will be fixed in the next version.
Sorry for the late reply. I will check these patches tomorrow.
Comment on attachment 8670327 [details] [diff] [review]
[05] Bug 1211948: Register Bluetooth GATT module in GATT manager

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

Overall looks good to me, please clean up those legacy codes.
Thanks!

::: dom/bluetooth/bluedroid/BluetoothGattManager.cpp
@@ +416,5 @@
> +    : mInterface(aInterface)
> +    , mRes(aRes)
> +  { }
> +
> +  void OnError(BluetoothStatus aStatus) override

As Ben mentioned, we should clear the notification handler here.

@@ +566,5 @@
> +  }
> +
> +  // Set notification handler _before_ registering the module. It could
> +  // happen that we receive notifications, before the result handler runs.
> +  gattInterface->SetNotificationHandler(BluetoothGattManager::Get());

Makes sense.
I just found that in some notification handlers in GattMgr, we didn't properly check the sBluetoothGattInterface before using it.
I will open another bug to address them.
Attachment #8670327 - Flags: review?(joliu) → review+
Attachment #8670322 - Flags: review?(shuang) → review?(brsun)
Attachment #8670323 - Flags: review?(shuang) → review?(brsun)
Attachment #8670324 - Flags: review?(shuang) → review?(brsun)
Attachment #8670325 - Flags: review?(shuang) → review?(brsun)
Attachment #8670329 - Flags: review?(shuang) → review?(brsun)
Redirect review requests to Bruce.

Comment 15

3 years ago
Comment on attachment 8670322 [details] [diff] [review]
[01] Bug 1211948: Add interface class for Setup module to Bluetooth backend interface

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

Basically it looks good to me.

::: dom/bluetooth/bluedroid/BluetoothDaemonSetupInterface.h
@@ +101,5 @@
> +                        BluetoothSetupResultHandler* aRes) override;
> +
> +  void Configuration(const BluetoothConfigurationParameter* aParam,
> +                     uint8_t aLen,
> +                     BluetoothSetupResultHandler* aRes) override;

How about using |const nsTArray<BluetoothConfigurationParameter>& aParam| instead of using |const BluetoothConfigurationParameter* aParam| and |uint8_t aLen|?

The above comment is not a must if the corresponding modification (ex. |PackPDU|) is not so related to this bug. Handling it on another follow-up bug is fine as well.
Attachment #8670322 - Flags: review?(brsun) → review+

Comment 16

3 years ago
Comment on attachment 8670323 [details] [diff] [review]
[02] Bug 1211948: Introduce |enum BluetoothSetupServiceId|

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

Basically it looks good to me.

::: dom/bluetooth/common/BluetoothCommon.h
@@ +333,5 @@
> +  SETUP_SERVICE_ID_HANDSFREE,
> +  SETUP_SERVICE_ID_A2DP,
> +  SETUP_SERVICE_ID_HEALTH,
> +  SETUP_SERVICE_ID_AVRCP,
> +  SETUP_SERVICE_ID_GATT

There are four other modules (hf_client, map_client, avrcp-ctrl, a2dp_sink) defined by BlueZ. How about add corresponding enums for them as well to have a more complete list?
Attachment #8670323 - Flags: review?(brsun) → review+

Comment 17

3 years ago
Comment on attachment 8670324 [details] [diff] [review]
[03] Bug 1211948: Register Bluetooth A2DP module in A2DP manager

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

Basically it looks good to me.

::: dom/bluetooth/bluedroid/BluetoothA2dpManager.cpp
@@ +99,5 @@
>    { }
>  
>    void OnError(BluetoothStatus aStatus) override
>    {
> +    BT_WARNING("BluetoothSetupInterface::RegisterModule failed: %d",

Suggest to add some words to indicate which module fails to be registered.

@@ +161,4 @@
>  void
>  BluetoothA2dpManager::InitA2dpInterface(BluetoothProfileResultHandler* aRes)
>  {
> +  if (sBtA2dpInterface) {

Suppose |sBtA2dpInterface| can only be accessed on the main thread, suggest to add an assertion to check the running thread.

@@ +161,5 @@
>  void
>  BluetoothA2dpManager::InitA2dpInterface(BluetoothProfileResultHandler* aRes)
>  {
> +  if (sBtA2dpInterface) {
> +    BT_LOGR("Bluetooth A2DP manager is already initalized.");

nit: "manager" or "interface"?

@@ +168,2 @@
>      if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch A2DP runnable");

nit: A2DP "Init" runnable

@@ +181,2 @@
>      if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch A2DP OnError runnable");

nit: "OnError" or "Init"?

@@ +191,5 @@
> +    // that calls the profile result handler.
> +    nsRefPtr<nsRunnable> r =
> +      new InitProfileResultHandlerRunnable(aRes, NS_ERROR_FAILURE);
> +    if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch A2DP OnError runnable");

ditto

@@ +204,5 @@
> +    // that calls the profile result handler.
> +    nsRefPtr<nsRunnable> r =
> +      new InitProfileResultHandlerRunnable(aRes, NS_ERROR_FAILURE);
> +    if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch A2DP OnError runnable");

ditto

@@ +280,5 @@
>    { }
>  
>    void OnError(BluetoothStatus aStatus) override
>    {
> +    BT_WARNING("BluetoothSetupInterface::UnregisterModule failed: %d",

Suggest to add some words to indicate which module fails to be unregistered.

@@ +334,5 @@
>  // static
>  void
>  BluetoothA2dpManager::DeinitA2dpInterface(BluetoothProfileResultHandler* aRes)
>  {
> +  if (!sBtA2dpInterface) {

Suppose |sBtA2dpInterface| can only be accessed on the main thread, suggest to add an assertion to check the running thread.

@@ +335,5 @@
>  void
>  BluetoothA2dpManager::DeinitA2dpInterface(BluetoothProfileResultHandler* aRes)
>  {
> +  if (!sBtA2dpInterface) {
> +    BT_LOGR("Bluetooth A2DP manager has not been initalized.");

nit: "manager" or "interface"?

@@ +339,5 @@
> +    BT_LOGR("Bluetooth A2DP manager has not been initalized.");
> +    nsRefPtr<nsRunnable> r =
> +      new DeinitProfileResultHandlerRunnable(aRes, NS_OK);
> +    if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch A2DP runnable");

nit: A2DP "Deinit" runnable

@@ +355,2 @@
>      if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch A2DP OnError runnable");

nit: "OnError" or "Deinit"?

@@ +365,5 @@
> +    // that calls the profile result handler.
> +    nsRefPtr<nsRunnable> r =
> +      new DeinitProfileResultHandlerRunnable(aRes, NS_ERROR_FAILURE);
> +    if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch A2DP OnError runnable");

ditto

::: dom/bluetooth/bluedroid/BluetoothDaemonA2dpInterface.cpp
@@ -316,5 @@
> -
> -    // Clear notification handler _after_ module has been
> -    // unregistered. While unregistering the module, we might
> -    // still receive notifications.
> -    mModule->SetNotificationHandler(nullptr);

There is no such cleanup behavior while doing unregistering module. I think it might still be needed after the refinement of setup module.
Attachment #8670324 - Flags: review?(brsun) → review+

Comment 18

3 years ago
Comment on attachment 8670325 [details] [diff] [review]
[04] Bug 1211948: Register Bluetooth AVRCP module in AVRCP manager

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

Basically it looks good to me.

::: dom/bluetooth/bluedroid/BluetoothAvrcpManager.cpp
@@ +135,5 @@
>    void OnError(BluetoothStatus aStatus) override
>    {
> +    MOZ_ASSERT(NS_IsMainThread());
> +
> +    BT_WARNING("BluetoothSetupInterface::RegisterModule failed: %d",

Suggest to add some words to indicate which module fails to be registered.

@@ +202,4 @@
>  void
>  BluetoothAvrcpManager::InitAvrcpInterface(BluetoothProfileResultHandler* aRes)
>  {
> +  if (sBtAvrcpInterface) {

Suppose |sBtAvrcpInterface| can only be accessed on the main thread, suggest to add an assertion to check the running thread.

@@ +202,5 @@
>  void
>  BluetoothAvrcpManager::InitAvrcpInterface(BluetoothProfileResultHandler* aRes)
>  {
> +  if (sBtAvrcpInterface) {
> +    BT_LOGR("Bluetooth AVRCP manager is already initalized.");

nit: "manager" or "interface"?

@@ +209,2 @@
>      if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch AVRCP runnable");

nit: AVRCP "Init" runnable

@@ +222,2 @@
>      if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch AVRCP OnError runnable");

nit: "OnError" or "Init"?

@@ +232,5 @@
> +    // that calls the profile result handler.
> +    nsRefPtr<nsRunnable> r =
> +      new InitProfileResultHandlerRunnable(aRes, NS_ERROR_FAILURE);
> +    if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch AVRCP OnError runnable");

ditto

@@ +245,5 @@
> +    // that calls the profile result handler.
> +    nsRefPtr<nsRunnable> r =
> +      new InitProfileResultHandlerRunnable(aRes, NS_ERROR_FAILURE);
> +    if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch AVRCP OnError runnable");

ditto

@@ +302,5 @@
>    { }
>  
>    void OnError(BluetoothStatus aStatus) override
>    {
> +    BT_WARNING("BluetoothSetupInterface::UnregisterModule failed: %d",

Suggest to add some words to indicate which module fails to be unregistered.

@@ +356,5 @@
>  // static
>  void
>  BluetoothAvrcpManager::DeinitAvrcpInterface(BluetoothProfileResultHandler* aRes)
>  {
> +  if (!sBtAvrcpInterface) {

Suppose |sBtAvrcpInterface| can only be accessed on the main thread, suggest to add an assertion to check the running thread.

@@ +357,5 @@
>  void
>  BluetoothAvrcpManager::DeinitAvrcpInterface(BluetoothProfileResultHandler* aRes)
>  {
> +  if (!sBtAvrcpInterface) {
> +    BT_LOGR("Bluetooth AVRCP manager has not been initalized.");

nit: "manager" or "interface"?

@@ +361,5 @@
> +    BT_LOGR("Bluetooth AVRCP manager has not been initalized.");
> +    nsRefPtr<nsRunnable> r =
> +      new DeinitProfileResultHandlerRunnable(aRes, NS_OK);
> +    if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch AVRCP runnable");

nit: AVRCP "Deinit" runnable

@@ +377,2 @@
>      if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch AVRCP OnError runnable");

nit: "OnError" or "Deinit"?

@@ +387,5 @@
> +    // that calls the profile result handler.
> +    nsRefPtr<nsRunnable> r =
> +      new DeinitProfileResultHandlerRunnable(aRes, NS_ERROR_FAILURE);
> +    if (NS_FAILED(NS_DispatchToMainThread(r))) {
> +      BT_LOGR("Failed to dispatch AVRCP OnError runnable");

ditto

::: dom/bluetooth/bluedroid/BluetoothDaemonAvrcpInterface.cpp
@@ -889,5 @@
> -
> -    // Clear notification handler _after_ module has been
> -    // unregistered. While unregistering the module, we might
> -    // still receive notifications.
> -    mModule->SetNotificationHandler(nullptr);

There is no such cleanup behavior while doing unregistering module. I think it might still be needed after the refinement of setup module.
Attachment #8670325 - Flags: review?(brsun) → review+

Comment 19

3 years ago
Comment on attachment 8670329 [details] [diff] [review]
[07] Bug 1211948: Remove registry interfaces from |BluetoothDaemonProtocol|

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

LGTM
Attachment #8670329 - Flags: review?(brsun) → review+
Hi

> > +  void Configuration(const BluetoothConfigurationParameter* aParam,
> > +                     uint8_t aLen,
> > +                     BluetoothSetupResultHandler* aRes) override;
> 
> How about using |const nsTArray<BluetoothConfigurationParameter>& aParam|
> instead of using |const BluetoothConfigurationParameter* aParam| and
> |uint8_t aLen|?

We don't call this anyway ATM. The current interface is not bound to a container type, which is good. I'm not really against using a container, but |nsTArray| might be too restrictive.
> @@ +333,5 @@
> > +  SETUP_SERVICE_ID_HANDSFREE,
> > +  SETUP_SERVICE_ID_A2DP,
> > +  SETUP_SERVICE_ID_HEALTH,
> > +  SETUP_SERVICE_ID_AVRCP,
> > +  SETUP_SERVICE_ID_GATT
> 
> There are four other modules (hf_client, map_client, avrcp-ctrl, a2dp_sink)
> defined by BlueZ. How about add corresponding enums for them as well to have
> a more complete list?

Good point. I'll add this in the next iteration.
Created attachment 8674834 [details] [diff] [review]
[02] Bug 1211948: Introduce |enum BluetoothSetupServiceId| (v2)

Changes since v1:

  - include all services in enum
  - conversion LUT is now uint8_t[]
Attachment #8670323 - Attachment is obsolete: true
Attachment #8674834 - Flags: review+
> >  BluetoothA2dpManager::InitA2dpInterface(BluetoothProfileResultHandler* aRes)
> >  {
> > +  if (sBtA2dpInterface) {
> > +    BT_LOGR("Bluetooth A2DP manager is already initalized.");
> 
> nit: "manager" or "interface"?

I now changed this to "interface", although we're actually initializing both of them, sort of.

> @@ +168,2 @@
> >      if (NS_FAILED(NS_DispatchToMainThread(r))) {
> > +      BT_LOGR("Failed to dispatch A2DP runnable");
> 
> nit: A2DP "Init" runnable

Here: "Init"

> @@ +181,2 @@
> >      if (NS_FAILED(NS_DispatchToMainThread(r))) {
> > +      BT_LOGR("Failed to dispatch A2DP OnError runnable");
> 
> nit: "OnError" or "Init"?

Here and below: "OnError".

> @@ +280,5 @@
> >    { }
> >  
> >    void OnError(BluetoothStatus aStatus) override
> >    {
> > +    BT_WARNING("BluetoothSetupInterface::UnregisterModule failed: %d",
> 
> Suggest to add some words to indicate which module fails to be unregistered.
> 
> @@ +334,5 @@
> >  // static
> >  void
> >  BluetoothA2dpManager::DeinitA2dpInterface(BluetoothProfileResultHandler* aRes)
> >  {
> > +  if (!sBtA2dpInterface) {
> 
> Suppose |sBtA2dpInterface| can only be accessed on the main thread, suggest
> to add an assertion to check the running thread.
> 
> @@ +335,5 @@
> >  void
> >  BluetoothA2dpManager::DeinitA2dpInterface(BluetoothProfileResultHandler* aRes)
> >  {
> > +  if (!sBtA2dpInterface) {
> > +    BT_LOGR("Bluetooth A2DP manager has not been initalized.");
> 
> nit: "manager" or "interface"?

I'll change these strings like the ones for init.

> ::: dom/bluetooth/bluedroid/BluetoothDaemonA2dpInterface.cpp
> @@ -316,5 @@
> > -
> > -    // Clear notification handler _after_ module has been
> > -    // unregistered. While unregistering the module, we might
> > -    // still receive notifications.
> > -    mModule->SetNotificationHandler(nullptr);
> 
> There is no such cleanup behavior while doing unregistering module. I think
> it might still be needed after the refinement of setup module.

Good point. I added this code to the A2DP manager.
(In reply to Bruce Sun [:brsun] from comment #18)
> Comment on attachment 8670325 [details] [diff] [review]
> [04] Bug 1211948: Register Bluetooth AVRCP module in AVRCP manager

I applied the same cleanups as for [03].
Created attachment 8674855 [details] [diff] [review]
[03] Bug 1211948: Register Bluetooth A2DP module in A2DP manager (v2)

Changes since v1:

  - stricter tests for |NS_IsMainThread|
  - string cleanups
  - unset notification handler on errors and deinit
Attachment #8670324 - Attachment is obsolete: true
Attachment #8674855 - Flags: review+
Created attachment 8674856 [details] [diff] [review]
[04] Bug 1211948: Register Bluetooth AVRCP module in AVRCP manager (v2)

Changes since v1:

  - stricter tests for |NS_IsMainThread|
  - string cleanups
  - unset notification handler on errors and deinit
Attachment #8670325 - Attachment is obsolete: true
Attachment #8674856 - Flags: review+
Created attachment 8674858 [details] [diff] [review]
[05] Bug 1211948: Register Bluetooth GATT module in GATT manager (v2)

Changes since v1:

  - removed 'if 0' blocks
  - stricter tests for |NS_IsMainThread|
  - string cleanups
  - unset notification handler on errors and deinit
Attachment #8670327 - Attachment is obsolete: true
Attachment #8674858 - Flags: review+
Created attachment 8674859 [details] [diff] [review]
[06] Bug 1211948: Register Bluetooth Handsfree module in Handsfree manager (v2)

Changes since v1:

  - stricter tests for |NS_IsMainThread|
  - string cleanups
  - unset notification handler on errors and deinit
Attachment #8670328 - Attachment is obsolete: true
Attachment #8674859 - Flags: review+
Created attachment 8674861 [details] [diff] [review]
[07] Bug 1211948: Remove registry interfaces from |BluetoothDaemonProtocol| (v2)

Changes since v1:

  - rebased on [06]
Attachment #8674861 - Flags: review+
Attachment #8670329 - Attachment is obsolete: true
Hi Thomas,

It seems like there are still some if 0 blocks introduced by patch 5(https://hg.mozilla.org/mozilla-central/rev/53a3b3a2f6f8), for example, https://dxr.mozilla.org/mozilla-central/source/dom/bluetooth/bluedroid/BluetoothDaemonGattInterface.cpp?case=true&from=BluetoothDaemonGattInterface.cpp#2011.

Are they needed? If not, could you open a bug and remove them?

Thanks,
Jocelyn
Flags: needinfo?(tzimmermann)
See Also: → bug 1228946
(In reply to Jocelyn Liu [:jocelyn] [:joliu] from comment #33)
> Hi Thomas,
> 
> It seems like there are still some if 0 blocks introduced by patch
> 5(https://hg.mozilla.org/mozilla-central/rev/53a3b3a2f6f8), for example,
> https://dxr.mozilla.org/mozilla-central/source/dom/bluetooth/bluedroid/
> BluetoothDaemonGattInterface.cpp?case=true&from=BluetoothDaemonGattInterface.
> cpp#2011.
> 
> Are they needed? If not, could you open a bug and remove them?

I'm so sorry! I keep the old code around while writing these patches, but I'm notoriously bad at finding and removing it afterwards. These blocks should never be needed. I opened bug 1228946 and will provide a patch ASAP.
Flags: needinfo?(tzimmermann)
You need to log in before you can comment on or make changes to this bug.