Bug 815325 - Patch 1: Turning bluetooth on and off multiple times causes restart, r=echou

dom/bluetooth/BluetoothService.cpp

@@ -54,6 +54,7 @@ namespace {
 StaticRefPtr<BluetoothService> gBluetoothService;
 
 bool gInShutdown = false;
+bool gToggleInProgress = false;
 
 bool
 IsMainProcess()
@@ -156,16 +157,29 @@ public:
   {
     MOZ_ASSERT(!NS_IsMainThread());
 
-    if (mEnabled) {
+    /*
-      if (NS_FAILED(gBluetoothService->StartInternal())) {
+     * mEnabled: expected status of bluetooth
-        NS_WARNING("Bluetooth service failed to start!");
+     * gBluetoothService->IsEnabled(): real status of bluetooth
-        mEnabled = !mEnabled;
+     *
+     * When two values are the same, we don't switch on/off bluetooth,
+     * but we still do ToggleBtAck task.
+     */
+    if (mEnabled == gBluetoothService->IsEnabled()) {
+      NS_WARNING("Bluetooth has already been enabled/disabled before.");
+    } else {
+      // Switch on/off bluetooth
+      if (mEnabled) {
+        if (NS_FAILED(gBluetoothService->StartInternal())) {
+          NS_WARNING("Bluetooth service failed to start!");
+          mEnabled = !mEnabled;
+        }
+      } else {
+        if (NS_FAILED(gBluetoothService->StopInternal())) {
+          NS_WARNING("Bluetooth service failed to stop!");
+          mEnabled = !mEnabled;
+        }
       }
     }
-    else if (NS_FAILED(gBluetoothService->StopInternal())) {
-      NS_WARNING("Bluetooth service failed to stop!");
-      mEnabled = !mEnabled;
-    }
 
     nsCOMPtr<nsIRunnable> ackTask = new BluetoothService::ToggleBtAck(mEnabled);
     if (NS_FAILED(NS_DispatchToMainThread(ackTask))) {
@@ -213,6 +227,12 @@ NS_IMPL_ISUPPORTS1(BluetoothService::StartupTask, nsISettingsServiceCallback);
 
 NS_IMPL_ISUPPORTS1(BluetoothService, nsIObserver)
 
+bool
+BluetoothService::IsToggling() const
+{
+  return gToggleInProgress;
+}
+
 BluetoothService::~BluetoothService()
 {
   Cleanup();
@@ -266,7 +286,6 @@ BluetoothService::Cleanup()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-
   nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
   if (obs &&
       (NS_FAILED(obs->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID)) ||
@@ -364,7 +383,6 @@ BluetoothService::StartStopBluetooth(bool aStart)
   }
 
   nsresult rv;
-
   if (!mBluetoothCommandThread) {
     MOZ_ASSERT(!gInShutdown);
 
@@ -385,13 +403,6 @@ BluetoothService::SetEnabled(bool aEnabled)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (aEnabled == mEnabled) {
-    // Nothing to do, maybe something failed.
-    return;
-  }
-
-  mEnabled = aEnabled;
-
   AutoInfallibleTArray<BluetoothParent*, 10> childActors;
   GetAllBluetoothActors(childActors);
 
@@ -401,10 +412,12 @@ BluetoothService::SetEnabled(bool aEnabled)
 
   if (aEnabled) {
     BluetoothManagerList::ForwardIterator iter(mLiveManagers);
-    BluetoothSignalObserverList* ol;
     nsString managerPath = NS_LITERAL_STRING("/");
 
-    // Re-register here after toggling due to table mBluetoothSignalObserverTable was cleared
+    /**
+     * Re-register managers since table mBluetoothSignalObserverTable was
+     * cleared after turned off bluetooth
+     */
     while (iter.HasMore()) {
       RegisterBluetoothSignalHandler(managerPath, (BluetoothSignalObserver*)iter.GetNext());
     }
@@ -412,19 +425,40 @@ BluetoothService::SetEnabled(bool aEnabled)
     mBluetoothSignalObserverTable.Clear();
   }
 
+  /**
+   * mEnabled: real status of bluetooth
+   * aEnabled: expected status of bluetooth
+   */
+  if (mEnabled == aEnabled) {
+    /**
+     * The process of toggling should be over here, so we set gToggleInProgress
+     * back to false here. Note that, we don't fire onenabled/ondisabled in
+     * this case.
+     */
+    NS_WARNING("Bluetooth has already been enabled/disabled before.\
+                Skip fire onenabled/ondisabled events here.");
+    gToggleInProgress = false;
+    return;
+  }
+
+  mEnabled = aEnabled;
+
+  // Fire onenabled/ondisabled event for each BluetoothManager
   BluetoothManagerList::ForwardIterator iter(mLiveManagers);
   while (iter.HasMore()) {
     if (NS_FAILED(iter.GetNext()->FireEnabledDisabledEvent(aEnabled))) {
       NS_WARNING("FireEnabledDisabledEvent failed!");
     }
   }
+
+  gToggleInProgress = false;
 }
 
 nsresult
 BluetoothService::HandleStartup()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(!mSettingsCheckInProgress);
+  MOZ_ASSERT(!gToggleInProgress);
 
   nsCOMPtr<nsISettingsService> settings =
     do_GetService("@mozilla.org/settingsService;1");
@@ -438,7 +472,7 @@ BluetoothService::HandleStartup()
   rv = settingsLock->Get(BLUETOOTH_ENABLED_SETTING, callback);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  mSettingsCheckInProgress = true;
+  gToggleInProgress = true;
   return NS_OK;
 }
 
@@ -447,18 +481,16 @@ BluetoothService::HandleStartupSettingsCheck(bool aEnable)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (!mSettingsCheckInProgress) {
-    // Somehow the enabled setting was changed before our first settings check
-    // completed. Don't do anything.
-    return NS_OK;
-  }
-
-  MOZ_ASSERT(!IsEnabled());
-
   if (aEnable) {
     return StartStopBluetooth(true);
   }
 
+  /*
+   * Since BLUETOOTH_ENABLED_SETTING is false, we don't have to turn on
+   * bluetooth here, and set gToggleInProgress back to false.
+   */
+  gToggleInProgress = false;
+
   return NS_OK;
 }
 
@@ -518,28 +550,14 @@ BluetoothService::HandleSettingsChanged(const nsAString& aData)
     return NS_ERROR_UNEXPECTED;
   }
 
-  if (mSettingsCheckInProgress) {
+  if (gToggleInProgress || value.toBoolean() == IsEnabled()) {
-    // Somehow the setting for bluetooth has been flipped before our first
-    // settings check completed. Flip this flag so that we ignore the result
-    // of that check whenever it finishes.
-    mSettingsCheckInProgress = false;
-  }
-
-  if (value.toBoolean() == IsEnabled()) {
     // Nothing to do here.
     return NS_OK;
   }
 
-  nsresult rv;
+  gToggleInProgress = true;
 
-  if (IsEnabled()) {
+  nsresult rv = StartStopBluetooth(value.toBoolean());
-    rv = StartStopBluetooth(false);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    return NS_OK;
-  }
-
-  rv = StartStopBluetooth(true);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;

dom/bluetooth/BluetoothService.h

@@ -303,12 +303,14 @@ public:
     return mEnabled;
   }
 
+  bool
+  IsToggling() const;
+
 protected:
   BluetoothService()
   : mEnabled(false)
-  , mSettingsCheckInProgress(false)
 #ifdef DEBUG
-    , mLastRequestedEnable(false)
+  , mLastRequestedEnable(false)
 #endif
   {
     mBluetoothSignalObserverTable.Init();
@@ -398,7 +400,6 @@ protected:
   BluetoothManagerList mLiveManagers;
 
   bool mEnabled;
-  bool mSettingsCheckInProgress;
 
 #ifdef DEBUG
   bool mLastRequestedEnable;

dom/bluetooth/linux/BluetoothDBusService.cpp

@@ -1428,6 +1428,16 @@ GetDefaultAdapterPath(BluetoothValue& aValue, nsString& aError)
   return NS_OK;
 }
 
+bool
+BluetoothDBusService::IsReady()
+{
+  if (!IsEnabled() || !mConnection || !gThreadConnection || IsToggling()) {
+    NS_WARNING("Bluetooth service is not ready yet!");
+    return false;
+  }
+  return true;
+}
+
 nsresult
 BluetoothDBusService::StartInternal()
 {
@@ -1607,14 +1617,17 @@ private:
 nsresult
 BluetoothDBusService::GetDefaultAdapterPathInternal(BluetoothReplyRunnable* aRunnable)
 {
-  if (!mConnection || !gThreadConnection) {
+  NS_ASSERTION(NS_IsMainThread(), "Must be called from main thread!");
-    NS_ERROR("Bluetooth service not started yet!");
+
-    return NS_ERROR_FAILURE;
+  if (!IsReady()) {
+    BluetoothValue v;
+    nsString errorStr;
+    errorStr.AssignLiteral("Bluetooth service is not ready yet!");
+    DispatchBluetoothReply(aRunnable, v, errorStr);
+    return NS_OK;
   }
 
-  NS_ASSERTION(NS_IsMainThread(), "Must be called from main thread!");
   nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;
-
   nsRefPtr<nsRunnable> func(new DefaultAdapterPropertiesRunnable(runnable));
   if (NS_FAILED(mBluetoothCommandThread->Dispatch(func, NS_DISPATCH_NORMAL))) {
     NS_WARNING("Cannot dispatch firmware loading task!");
@@ -1655,8 +1668,11 @@ nsresult
 BluetoothDBusService::StopDiscoveryInternal(const nsAString& aAdapterPath,
                                             BluetoothReplyRunnable* aRunnable)
 {
-  if (!mConnection) {
+  if (!IsReady()) {
-    NS_WARNING("Bluetooth service not started yet, no need to stop discovery.");
+    BluetoothValue v;
+    nsString errorStr;
+    errorStr.AssignLiteral("Bluetooth service is not ready yet!");
+    DispatchBluetoothReply(aRunnable, v, errorStr);
     return NS_OK;
   }
   return SendDiscoveryMessage(aAdapterPath, "StopDiscovery", aRunnable);
@@ -1666,9 +1682,12 @@ nsresult
 BluetoothDBusService::StartDiscoveryInternal(const nsAString& aAdapterPath,
                                              BluetoothReplyRunnable* aRunnable)
 {
-  if (!mConnection) {
+  if (!IsReady()) {
-    NS_WARNING("Bluetooth service not started yet, cannot start discovery!");
+    BluetoothValue v;
-    return NS_ERROR_FAILURE;
+    nsString errorStr;
+    errorStr.AssignLiteral("Bluetooth service is not ready yet!");
+    DispatchBluetoothReply(aRunnable, v, errorStr);
+    return NS_OK;
   }
   return SendDiscoveryMessage(aAdapterPath, "StartDiscovery", aRunnable);
 }
@@ -1877,12 +1896,15 @@ nsresult
 BluetoothDBusService::GetPairedDevicePropertiesInternal(const nsTArray<nsString>& aDeviceAddresses,
                                                         BluetoothReplyRunnable* aRunnable)
 {
-  if (!mConnection || !gThreadConnection) {
+  if (!IsReady()) {
-    NS_ERROR("Bluetooth service not started yet!");
+    BluetoothValue v;
-    return NS_ERROR_FAILURE;
+    nsString errorStr;
+    errorStr.AssignLiteral("Bluetooth service is not ready yet!");
+    DispatchBluetoothReply(aRunnable, v, errorStr);
+    return NS_OK;
   }
-  nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;
 
+  nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;
   nsRefPtr<nsRunnable> func(new BluetoothPairedDevicePropertiesRunnable(runnable, aDeviceAddresses));
   if (NS_FAILED(mBluetoothCommandThread->Dispatch(func, NS_DISPATCH_NORMAL))) {
     NS_WARNING("Cannot dispatch task!");
@@ -2501,9 +2523,12 @@ BluetoothDBusService::GetSocketViaService(const nsAString& aObjectPath,
                                           BluetoothReplyRunnable* aRunnable)
 {
   NS_ASSERTION(NS_IsMainThread(), "Must be called from main thread!");
-  if (!mConnection || !gThreadConnection) {
+  if (!IsReady()) {
-    NS_ERROR("Bluetooth service not started yet!");
+    BluetoothValue v;
-    return NS_ERROR_FAILURE;
+    nsString errorStr;
+    errorStr.AssignLiteral("Bluetooth service is not ready yet!");
+    DispatchBluetoothReply(aRunnable, v, errorStr);
+    return NS_OK;
   }
 
   nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;
@@ -2530,6 +2555,7 @@ BluetoothDBusService::GetScoSocket(const nsAString& aAddress,
                                    mozilla::ipc::UnixSocketConsumer* aConsumer)
 {
   NS_ASSERTION(NS_IsMainThread(), "Must be called from main thread!");
+
   if (!mConnection || !gThreadConnection) {
     NS_ERROR("Bluetooth service not started yet!");
     return NS_ERROR_FAILURE;

dom/bluetooth/linux/BluetoothDBusService.h

@@ -24,6 +24,8 @@ class BluetoothDBusService : public BluetoothService
                            , private mozilla::ipc::RawDBusConnection
 {
 public:
+  bool IsReady();
+
   virtual nsresult StartInternal();
 
   virtual nsresult StopInternal();