Bug 1019372 - Patch 1/6: [bluetooth2] Remove unneccessary variable sIsBtEnabled to avoid racing issue, r=echou
authorBen Tian <btian@mozilla.com>
Tue, 03 Jun 2014 11:40:42 +0800
changeset 205979 d45fe00e7dcd09e60da3e550cc4974bc55597dd0
parent 205978 e6554692037712f460d050fdff7e0df7b1afc296
child 205980 76fc983a381374af7f904582b9eb4062562c7c2b
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersechou
bugs1019372
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1019372 - Patch 1/6: [bluetooth2] Remove unneccessary variable sIsBtEnabled to avoid racing issue, r=echou
dom/bluetooth2/bluedroid/BluetoothServiceBluedroid.cpp
--- a/dom/bluetooth2/bluedroid/BluetoothServiceBluedroid.cpp
+++ b/dom/bluetooth2/bluedroid/BluetoothServiceBluedroid.cpp
@@ -28,44 +28,53 @@
 #include "BluetoothUtils.h"
 #include "BluetoothUuid.h"
 #include "mozilla/dom/bluetooth/BluetoothTypes.h"
 #include "mozilla/ipc/UnixSocket.h"
 #include "mozilla/StaticMutex.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/unused.h"
 
+#define ENSURE_BLUETOOTH_IS_READY(runnable, result)                    \
+  do {                                                                 \
+    if (!sBtInterface || !IsEnabled()) {                               \
+      NS_NAMED_LITERAL_STRING(errorStr, "Bluetooth is not ready");     \
+      DispatchBluetoothReply(runnable, BluetoothValue(), errorStr);    \
+      return result;                                                   \
+    }                                                                  \
+  } while(0)
+
 using namespace mozilla;
 using namespace mozilla::ipc;
 USING_BLUETOOTH_NAMESPACE
 
 /**
  *  Static variables
  */
 static bluetooth_device_t* sBtDevice;
 static const bt_interface_t* sBtInterface;
 static bool sAdapterDiscoverable = false;
-static bool sIsBtEnabled = false;
 static nsString sAdapterBdAddress;
 static nsString sAdapterBdName;
 static uint32_t sAdapterDiscoverableTimeout;
 static InfallibleTArray<nsString> sAdapterBondedAddressArray;
 static InfallibleTArray<BluetoothNamedValue> sRemoteDevicesPack;
 static nsTArray<nsRefPtr<BluetoothProfileController> > sControllerArray;
 static nsTArray<nsRefPtr<BluetoothReplyRunnable> > sBondingRunnableArray;
 static nsTArray<nsRefPtr<BluetoothReplyRunnable> > sChangeDiscoveryRunnableArray;
 static nsTArray<nsRefPtr<BluetoothReplyRunnable> > sGetDeviceRunnableArray;
 static nsTArray<nsRefPtr<BluetoothReplyRunnable> > sSetPropertyRunnableArray;
 static nsTArray<nsRefPtr<BluetoothReplyRunnable> > sUnbondingRunnableArray;
 static nsTArray<int> sRequestedDeviceCountArray;
 
 /**
  *  Classes only used in this file
  */
-class DistributeBluetoothSignalTask : public nsRunnable {
+class DistributeBluetoothSignalTask MOZ_FINAL : public nsRunnable
+{
 public:
   DistributeBluetoothSignalTask(const BluetoothSignal& aSignal) :
     mSignal(aSignal)
   {
   }
 
   NS_IMETHOD
   Run()
@@ -79,22 +88,19 @@ public:
 
     return NS_OK;
   }
 
 private:
   BluetoothSignal mSignal;
 };
 
-class SetupAfterEnabledTask : public nsRunnable
+class SetupAfterEnabledTask MOZ_FINAL : public nsRunnable
 {
 public:
-  SetupAfterEnabledTask()
-  { }
-
   NS_IMETHOD
   Run()
   {
     MOZ_ASSERT(NS_IsMainThread());
 
     // Bluetooth just enabled, clear profile controllers and runnable arrays.
     sControllerArray.Clear();
     sBondingRunnableArray.Clear();
@@ -130,22 +136,19 @@ public:
     if (!opp || !opp->Listen()) {
       BT_LOGR("Fail to start BluetoothOppManager listening");
     }
 
     return NS_OK;
   }
 };
 
-class CleanupTask : public nsRunnable
+class CleanupTask MOZ_FINAL : public nsRunnable
 {
 public:
-  CleanupTask()
-  { }
-
   NS_IMETHOD
   Run()
   {
     MOZ_ASSERT(NS_IsMainThread());
 
     // Cleanup bluetooth interfaces after BT state becomes BT_STATE_OFF.
     BluetoothHfpManager::DeinitHfpInterface();
     BluetoothA2dpManager::DeinitA2dpInterface();
@@ -265,55 +268,53 @@ PlayStatusStringToControlPlayStatus(cons
     playStatus = ControlPlayStatus::PLAYSTATUS_REV_SEEK;
   } else if (aPlayStatus.EqualsLiteral("ERROR")) {
     playStatus = ControlPlayStatus::PLAYSTATUS_ERROR;
   }
 
   return playStatus;
 }
 
-static bool
-IsReady()
-{
-  if (!sBtInterface || !sIsBtEnabled) {
-    BT_LOGR("Warning! Bluetooth Service is not ready");
-    return false;
-  }
-  return true;
-}
-
+/**
+ *  Bluedroid HAL callback functions
+ *
+ *  Several callbacks are dispatched to main thread to avoid racing issues.
+ */
 static void
 AdapterStateChangeCallback(bt_state_t aStatus)
 {
   MOZ_ASSERT(!NS_IsMainThread());
 
-  BT_LOGR("BT_STATE %d", aStatus);
+  BT_LOGR("BT_STATE: %d", aStatus);
+
+  bool isBtEnabled = (aStatus == BT_STATE_ON);
 
-  sIsBtEnabled = (aStatus == BT_STATE_ON);
-
-  if (!sIsBtEnabled && NS_FAILED(NS_DispatchToMainThread(new CleanupTask()))) {
+  if (!isBtEnabled &&
+      NS_FAILED(NS_DispatchToMainThread(new CleanupTask()))) {
     BT_WARNING("Failed to dispatch to main thread!");
+    return;
   }
 
   nsRefPtr<nsRunnable> runnable =
-    new BluetoothService::ToggleBtAck(sIsBtEnabled);
+    new BluetoothService::ToggleBtAck(isBtEnabled);
   if (NS_FAILED(NS_DispatchToMainThread(runnable))) {
     BT_WARNING("Failed to dispatch to main thread!");
     return;
   }
 
-  if (sIsBtEnabled &&
+  if (isBtEnabled &&
       NS_FAILED(NS_DispatchToMainThread(new SetupAfterEnabledTask()))) {
     BT_WARNING("Failed to dispatch to main thread!");
+    return;
   }
 }
 
 /**
  * AdapterPropertiesCallback will be called after enable() but before
- * AdapterStateChangeCallback sIsBtEnabled get updated. At that moment, both
+ * AdapterStateChangeCallback is called. At that moment, both
  * BluetoothManager/BluetoothAdapter does not register observer yet.
  */
 static void
 AdapterPropertiesCallback(bt_status_t aStatus, int aNumProperties,
                           bt_property_t *aProperties)
 {
   MOZ_ASSERT(!NS_IsMainThread());
 
@@ -704,22 +705,26 @@ EnableInternal()
   BluetoothA2dpManager::InitA2dpInterface();
   return sBtInterface->enable();
 }
 
 static nsresult
 StartStopGonkBluetooth(bool aShouldEnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
+
   NS_ENSURE_TRUE(sBtInterface, NS_ERROR_FAILURE);
 
-  if (sIsBtEnabled == aShouldEnable) {
+  BluetoothService* bs = BluetoothService::Get();
+  NS_ENSURE_TRUE(bs, NS_ERROR_FAILURE);
+
+  if (bs->IsEnabled() == aShouldEnable) {
     // Keep current enable status
     nsRefPtr<nsRunnable> runnable =
-      new BluetoothService::ToggleBtAck(sIsBtEnabled);
+      new BluetoothService::ToggleBtAck(aShouldEnable);
     if (NS_FAILED(NS_DispatchToMainThread(runnable))) {
       BT_WARNING("Failed to dispatch to main thread!");
     }
     return NS_OK;
   }
 
   int ret = aShouldEnable ? EnableInternal() : sBtInterface->disable();
   NS_ENSURE_TRUE(ret == BT_STATUS_SUCCESS, NS_ERROR_FAILURE);
@@ -851,21 +856,17 @@ BluetoothServiceBluedroid::GetAdaptersIn
 }
 
 nsresult
 BluetoothServiceBluedroid::GetConnectedDevicePropertiesInternal(
   uint16_t aServiceUuid, BluetoothReplyRunnable* aRunnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (!IsReady()) {
-    NS_NAMED_LITERAL_STRING(errorStr, "Bluetooth service is not ready yet!");
-    DispatchBluetoothReply(aRunnable, BluetoothValue(), errorStr);
-    return NS_OK;
-  }
+  ENSURE_BLUETOOTH_IS_READY(aRunnable, NS_OK);
 
   BluetoothProfileManagerBase* profile =
     BluetoothUuidHelper::GetBluetoothProfileManager(aServiceUuid);
   if (!profile) {
     InfallibleTArray<BluetoothNamedValue> emptyArr;
     DispatchBluetoothReply(aRunnable, emptyArr,
                            NS_LITERAL_STRING(ERR_UNKNOWN_PROFILE));
     return NS_OK;
@@ -905,21 +906,17 @@ BluetoothServiceBluedroid::GetConnectedD
 }
 
 nsresult
 BluetoothServiceBluedroid::GetPairedDevicePropertiesInternal(
   const nsTArray<nsString>& aDeviceAddress, BluetoothReplyRunnable* aRunnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (!IsReady()) {
-    NS_NAMED_LITERAL_STRING(errorStr, "Bluetooth service is not ready yet!");
-    DispatchBluetoothReply(aRunnable, BluetoothValue(), errorStr);
-    return NS_OK;
-  }
+  ENSURE_BLUETOOTH_IS_READY(aRunnable, NS_OK);
 
   int requestedDeviceCount = aDeviceAddress.Length();
   if (requestedDeviceCount == 0) {
     InfallibleTArray<BluetoothNamedValue> emptyArr;
     DispatchBluetoothReply(aRunnable, BluetoothValue(emptyArr), EmptyString());
     return NS_OK;
   }
 
@@ -942,22 +939,18 @@ BluetoothServiceBluedroid::GetPairedDevi
 }
 
 nsresult
 BluetoothServiceBluedroid::StartDiscoveryInternal(
   BluetoothReplyRunnable* aRunnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (!IsReady()) {
-    NS_NAMED_LITERAL_STRING(errorStr, "Bluetooth service is not ready yet!");
-    DispatchBluetoothReply(aRunnable, BluetoothValue(), errorStr);
+  ENSURE_BLUETOOTH_IS_READY(aRunnable, NS_OK);
 
-    return NS_OK;
-  }
   int ret = sBtInterface->start_discovery();
   if (ret != BT_STATUS_SUCCESS) {
     ReplyStatusError(aRunnable, ret, NS_LITERAL_STRING("StartDiscovery"));
 
     return NS_OK;
   }
 
   sChangeDiscoveryRunnableArray.AppendElement(aRunnable);
@@ -965,21 +958,17 @@ BluetoothServiceBluedroid::StartDiscover
 }
 
 nsresult
 BluetoothServiceBluedroid::StopDiscoveryInternal(
   BluetoothReplyRunnable* aRunnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (!IsReady()) {
-    NS_NAMED_LITERAL_STRING(errorStr, "Bluetooth service is not ready yet!");
-    DispatchBluetoothReply(aRunnable, BluetoothValue(), errorStr);
-    return NS_OK;
-  }
+  ENSURE_BLUETOOTH_IS_READY(aRunnable, NS_OK);
 
   int ret = sBtInterface->cancel_discovery();
   if (ret != BT_STATUS_SUCCESS) {
     ReplyStatusError(aRunnable, ret, NS_LITERAL_STRING("StopDiscovery"));
     return NS_OK;
   }
 
   sChangeDiscoveryRunnableArray.AppendElement(aRunnable);
@@ -989,22 +978,17 @@ BluetoothServiceBluedroid::StopDiscovery
 
 nsresult
 BluetoothServiceBluedroid::SetProperty(BluetoothObjectType aType,
                                        const BluetoothNamedValue& aValue,
                                        BluetoothReplyRunnable* aRunnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (!IsReady()) {
-    NS_NAMED_LITERAL_STRING(errorStr, "Bluetooth service is not ready yet!");
-    DispatchBluetoothReply(aRunnable, BluetoothValue(), errorStr);
-
-    return NS_OK;
-  }
+  ENSURE_BLUETOOTH_IS_READY(aRunnable, NS_OK);
 
   const nsString propName = aValue.name();
   bt_property_t prop;
   bt_scan_mode_t scanMode;
   nsCString str;
 
   // For Bluedroid, it's necessary to check property name for SetProperty
   if (propName.EqualsLiteral("Name")) {
@@ -1067,21 +1051,17 @@ BluetoothServiceBluedroid::UpdateSdpReco
 
 nsresult
 BluetoothServiceBluedroid::CreatePairedDeviceInternal(
   const nsAString& aDeviceAddress, int aTimeout,
   BluetoothReplyRunnable* aRunnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (!IsReady()) {
-    NS_NAMED_LITERAL_STRING(errorStr, "Bluetooth service is not ready yet!");
-    DispatchBluetoothReply(aRunnable, BluetoothValue(), errorStr);
-    return NS_OK;
-  }
+  ENSURE_BLUETOOTH_IS_READY(aRunnable, NS_OK);
 
   bt_bdaddr_t remoteAddress;
   StringToBdAddressType(aDeviceAddress, &remoteAddress);
 
   int ret = sBtInterface->create_bond(&remoteAddress);
   if (ret != BT_STATUS_SUCCESS) {
     ReplyStatusError(aRunnable, ret, NS_LITERAL_STRING("CreatedPairedDevice"));
   } else {
@@ -1092,21 +1072,17 @@ BluetoothServiceBluedroid::CreatePairedD
 }
 
 nsresult
 BluetoothServiceBluedroid::RemoveDeviceInternal(
   const nsAString& aDeviceAddress, BluetoothReplyRunnable* aRunnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (!IsReady()) {
-    NS_NAMED_LITERAL_STRING(errorStr, "Bluetooth service is not ready yet!");
-    DispatchBluetoothReply(aRunnable, BluetoothValue(), errorStr);
-    return NS_OK;
-  }
+  ENSURE_BLUETOOTH_IS_READY(aRunnable, NS_OK);
 
   bt_bdaddr_t remoteAddress;
   StringToBdAddressType(aDeviceAddress, &remoteAddress);
 
   int ret = sBtInterface->remove_bond(&remoteAddress);
   if (ret != BT_STATUS_SUCCESS) {
     ReplyStatusError(aRunnable, ret,
                      NS_LITERAL_STRING("RemoveDevice"));
@@ -1119,21 +1095,17 @@ BluetoothServiceBluedroid::RemoveDeviceI
 
 bool
 BluetoothServiceBluedroid::SetPinCodeInternal(
   const nsAString& aDeviceAddress, const nsAString& aPinCode,
   BluetoothReplyRunnable* aRunnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (!IsReady()) {
-    NS_NAMED_LITERAL_STRING(errorStr, "Bluetooth service is not ready yet!");
-    DispatchBluetoothReply(aRunnable, BluetoothValue(), errorStr);
-    return false;
-  }
+  ENSURE_BLUETOOTH_IS_READY(aRunnable, false);
 
   bt_bdaddr_t remoteAddress;
   StringToBdAddressType(aDeviceAddress, &remoteAddress);
 
   int ret = sBtInterface->pin_reply(
       &remoteAddress, true, aPinCode.Length(),
       (bt_pin_code_t*)NS_ConvertUTF16toUTF8(aPinCode).get());
 
@@ -1156,21 +1128,17 @@ BluetoothServiceBluedroid::SetPasskeyInt
 
 bool
 BluetoothServiceBluedroid::SetPairingConfirmationInternal(
   const nsAString& aDeviceAddress, bool aConfirm,
   BluetoothReplyRunnable* aRunnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (!IsReady()) {
-    NS_NAMED_LITERAL_STRING(errorStr, "Bluetooth service is not ready yet!");
-    DispatchBluetoothReply(aRunnable, BluetoothValue(), errorStr);
-    return false;
-  }
+  ENSURE_BLUETOOTH_IS_READY(aRunnable, false);
 
   bt_bdaddr_t remoteAddress;
   StringToBdAddressType(aDeviceAddress, &remoteAddress);
 
   int ret = sBtInterface->ssp_reply(&remoteAddress, (bt_ssp_variant_t)0,
                                     aConfirm, 0);
   if (ret != BT_STATUS_SUCCESS) {
     ReplyStatusError(aRunnable, ret,