Bug 903422: Replace AppendDeviceNameRunnable by non-blocking implementation, r=echou,gyeh
authorThomas Zimmermann <tdz@users.sourceforge.net>
Fri, 16 Aug 2013 11:48:42 +0200
changeset 142886 13064db19b63ccc7139eef92f532f8a4726b51cb
parent 142885 af1af7c82d0adad60b752c223119a5dfbc0eb32b
child 142887 ffaa22db60ffcf6670cb0a64a6dded04f890b014
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersechou, gyeh
bugs903422
milestone26.0a1
Bug 903422: Replace AppendDeviceNameRunnable by non-blocking implementation, r=echou,gyeh AppendDeviceNameRunnable currently block on the Bluetooth command thread for a DBus reply. This patch refactors the code to not block during the DBus call. The classes AppendDeviceNameHandler and AppendDeviceNameRunnable are being removed, and the DBus interaction is done directly in the DBus thread.
dom/bluetooth/linux/BluetoothDBusService.cpp
--- a/dom/bluetooth/linux/BluetoothDBusService.cpp
+++ b/dom/bluetooth/linux/BluetoothDBusService.cpp
@@ -838,108 +838,143 @@ GetPropertiesInternal(const nsAString& a
   if (!success) {
     BT_WARNING("Failed to get device properties");
     return false;
   }
 
   return true;
 }
 
-class AppendDeviceNameRunnable : public nsRunnable
+class GetPropertiesReplyHandler : public DBusReplyHandler
+{
+public:
+  GetPropertiesReplyHandler(const nsCString& aIface)
+    : mIface(aIface)
+  {
+    MOZ_ASSERT(!mIface.IsEmpty());
+  }
+
+  const nsCString& GetInterface() const
+  {
+    return mIface;
+  }
+
+private:
+  nsCString mIface;
+};
+
+class AppendDeviceNameReplyHandler: public GetPropertiesReplyHandler
 {
 public:
-  AppendDeviceNameRunnable(const BluetoothSignal& aSignal)
-    : mSignal(aSignal)
+  AppendDeviceNameReplyHandler(const nsCString& aIface,
+                               const nsString& aDevicePath,
+                               const BluetoothSignal& aSignal)
+    : GetPropertiesReplyHandler(aIface)
+    , mDevicePath(aDevicePath)
+    , mSignal(aSignal)
   {
+    MOZ_ASSERT(!mDevicePath.IsEmpty());
   }
 
-  nsresult Run()
+  void Handle(DBusMessage* aReply) MOZ_OVERRIDE
   {
-    MOZ_ASSERT(!NS_IsMainThread());
-
-    InfallibleTArray<BluetoothNamedValue>& arr =
-      mSignal.value().get_ArrayOfBluetoothNamedValue();
-    nsString devicePath = arr[0].value().get_nsString();
-
-    // Replace object path with device address
+    MOZ_ASSERT(!NS_IsMainThread()); // DBus thread
+
+    if (!aReply || (dbus_message_get_type(aReply) == DBUS_MESSAGE_TYPE_ERROR)) {
+      return;
+    }
+
+    // Get device properties from result of GetProperties
+
+    DBusError err;
+    dbus_error_init(&err);
+
+    BluetoothValue deviceProperties;
+
+    bool success = UnpackPropertiesMessage(aReply, &err, deviceProperties,
+                                           GetInterface().get());
+    if (!success) {
+      BT_WARNING("Failed to get device properties");
+      return;
+    }
+
+    // First we replace object path with device address.
+
     InfallibleTArray<BluetoothNamedValue>& parameters =
       mSignal.value().get_ArrayOfBluetoothNamedValue();
-    nsString address = GetAddressFromObjectPath(devicePath);
+    nsString address =
+      GetAddressFromObjectPath(mDevicePath);
     parameters[0].name().AssignLiteral("address");
     parameters[0].value() = address;
 
-    BluetoothValue prop;
-    if(!GetPropertiesInternal(devicePath, DBUS_DEVICE_IFACE, prop)) {
-      return NS_ERROR_FAILURE;
-    }
-
-    // Get device name from result of GetPropertiesInternal and append to
-    // original signal
+    // Then we append the device's name to the original signal's data.
+
     InfallibleTArray<BluetoothNamedValue>& properties =
-      prop.get_ArrayOfBluetoothNamedValue();
-   uint8_t i;
+      deviceProperties.get_ArrayOfBluetoothNamedValue();
+    InfallibleTArray<BluetoothNamedValue>::size_type i;
     for (i = 0; i < properties.Length(); i++) {
       if (properties[i].name().EqualsLiteral("Name")) {
         properties[i].name().AssignLiteral("name");
         parameters.AppendElement(properties[i]);
         break;
       }
     }
     MOZ_ASSERT_IF(i == properties.Length(), "failed to get device name");
 
     nsRefPtr<DistributeBluetoothSignalTask> task =
       new DistributeBluetoothSignalTask(mSignal);
     NS_DispatchToMainThread(task);
-
-    return NS_OK;
   }
 
 private:
+  nsString mDevicePath;
   BluetoothSignal mSignal;
 };
 
-class AppendDeviceNameHandler : public nsRunnable
+static void
+AppendDeviceName(BluetoothSignal& aSignal)
 {
-public:
-  AppendDeviceNameHandler(const BluetoothSignal& aSignal)
-    : mSignal(aSignal)
-  {
+  BluetoothValue v = aSignal.value();
+  if (v.type() != BluetoothValue::TArrayOfBluetoothNamedValue ||
+      v.get_ArrayOfBluetoothNamedValue().Length() == 0) {
+    BT_WARNING("Invalid argument type for AppendDeviceNameRunnable");
+    return;
+  }
+  const InfallibleTArray<BluetoothNamedValue>& arr =
+    v.get_ArrayOfBluetoothNamedValue();
+
+  // Device object path should be put in the first element
+  if (!arr[0].name().EqualsLiteral("path") ||
+       arr[0].value().type() != BluetoothValue::TnsString) {
+    BT_WARNING("Invalid object path for AppendDeviceNameRunnable");
+    return;
   }
 
-  nsresult Run()
-  {
-    MOZ_ASSERT(NS_IsMainThread());
-    BluetoothService* bs = BluetoothService::Get();
-    NS_ENSURE_TRUE(bs, NS_ERROR_FAILURE);
-
-    BluetoothValue v = mSignal.value();
-    if (v.type() != BluetoothValue::TArrayOfBluetoothNamedValue ||
-        v.get_ArrayOfBluetoothNamedValue().Length() == 0) {
-      NS_WARNING("Invalid argument type for AppendDeviceNameHandler");
-      return NS_ERROR_FAILURE;
-    }
-    const InfallibleTArray<BluetoothNamedValue>& arr =
-      v.get_ArrayOfBluetoothNamedValue();
-
-    // Device object path should be put in the first element
-    if (!arr[0].name().EqualsLiteral("path") ||
-        arr[0].value().type() != BluetoothValue::TnsString) {
-      NS_WARNING("Invalid object path for AppendDeviceNameHandler");
-      return NS_ERROR_FAILURE;
-    }
-
-    nsRefPtr<nsRunnable> func(new AppendDeviceNameRunnable(mSignal));
-    bs->DispatchToCommandThread(func);
-
-    return NS_OK;
+  nsString devicePath = arr[0].value().get_nsString();
+
+  nsRefPtr<RawDBusConnection> threadConnection = gThreadConnection;
+
+  if (!threadConnection.get()) {
+    BT_WARNING("%s: DBus connection has been closed.", __FUNCTION__);
+    return;
   }
 
-private:
-  BluetoothSignal mSignal;
-};
+  nsRefPtr<AppendDeviceNameReplyHandler> handler =
+    new AppendDeviceNameReplyHandler(nsCString(DBUS_DEVICE_IFACE),
+                                     devicePath, aSignal);
+  bool success = dbus_func_args_async(threadConnection->GetConnection(), 1000,
+                                      AppendDeviceNameReplyHandler::Callback,
+                                      handler.get(),
+                                      NS_ConvertUTF16toUTF8(devicePath).get(),
+                                      DBUS_DEVICE_IFACE, "GetProperties",
+                                      DBUS_TYPE_INVALID);
+  NS_ENSURE_TRUE_VOID(success);
+
+  handler.forget();
+}
 
 static DBusHandlerResult
 AgentEventFilter(DBusConnection *conn, DBusMessage *msg, void *data)
 {
   if (dbus_message_get_type(msg) != DBUS_MESSAGE_TYPE_METHOD_CALL) {
     BT_WARNING("%s: agent handler not interested (not a method call).\n",
                __FUNCTION__);
     return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
@@ -952,17 +987,16 @@ AgentEventFilter(DBusConnection *conn, D
                        dbus_message_get_path(msg),
                        dbus_message_get_member(msg));
 
   nsString signalPath = NS_ConvertUTF8toUTF16(dbus_message_get_path(msg));
   nsString signalName = NS_ConvertUTF8toUTF16(dbus_message_get_member(msg));
   nsString errorStr;
   BluetoothValue v;
   InfallibleTArray<BluetoothNamedValue> parameters;
-  nsRefPtr<nsRunnable> handler;
   bool isPairingReq = false;
   BluetoothSignal signal(signalName, signalPath, v);
   char *objectPath;
 
   // The following descriptions of each signal are retrieved from:
   //
   // http://maemo.org/api_refs/5.0/beta/bluez/agent.html
   //
@@ -1127,21 +1161,20 @@ AgentEventFilter(DBusConnection *conn, D
   if (isPairingReq) {
     sPairingReqTable.Put(
       GetAddressFromObjectPath(NS_ConvertUTF8toUTF16(objectPath)), msg);
 
     // Increase ref count here because we need this message later.
     // It'll be unrefed when set*Internal() is called.
     dbus_message_ref(msg);
 
-    handler = new AppendDeviceNameHandler(signal);
+    AppendDeviceName(signal);
   } else {
-    handler = new DistributeBluetoothSignalTask(signal);
+    NS_DispatchToMainThread(new DistributeBluetoothSignalTask(signal));
   }
-  NS_DispatchToMainThread(handler);
 
   return DBUS_HANDLER_RESULT_HANDLED;
 
 handle_error:
   BT_WARNING(NS_ConvertUTF16toUTF8(errorStr).get());
   return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }