Bug 892933: Replace AddServiceRecordsInternal by non-blocking implementation, r=echou,gyeh
authorThomas Zimmermann <tdz@users.sourceforge.net>
Fri, 02 Aug 2013 09:30:55 +0200
changeset 153395 73dcf35e07cf2e2a3b2e76c3985a9c78093181bf
parent 153394 92c6f27da2ce36edb919fd4441c83ab8c1898b06
child 153396 d3a0ac52d5d44452afc354567dc4ed527dd9d393
child 153397 989eeb2e71fffaeea5c8763ec918a5d66afb45ea
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersechou, gyeh
bugs892933
milestone25.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 892933: Replace AddServiceRecordsInternal by non-blocking implementation, r=echou,gyeh AddServiceRecordsInternal blocks the Bluetooth command thread while waiting for a DBus reply. This patch replaces the function by a DBus reply handler that runs asynchrounously on the DBus thread.
dom/bluetooth/linux/BluetoothDBusService.cpp
--- a/dom/bluetooth/linux/BluetoothDBusService.cpp
+++ b/dom/bluetooth/linux/BluetoothDBusService.cpp
@@ -353,68 +353,16 @@ UnpackObjectPathMessage(DBusMessage* aMs
         LOG_AND_FREE_DBUS_ERROR(&err);
       }
     } else {
       aValue = NS_ConvertUTF8toUTF16(object_path);
     }
   }
 }
 
-static void
-ExtractHandles(DBusMessage *aReply, nsTArray<uint32_t>& aOutHandles)
-{
-  uint32_t* handles = nullptr;
-  int len;
-
-  DBusError err;
-  dbus_error_init(&err);
-
-  if (dbus_message_get_args(aReply, &err,
-                            DBUS_TYPE_ARRAY, DBUS_TYPE_UINT32, &handles, &len,
-                            DBUS_TYPE_INVALID)) {
-    if (!handles) {
-      BT_WARNING("Null array in extract_handles");
-    } else {
-      for (int i = 0; i < len; ++i) {
-        aOutHandles.AppendElement(handles[i]);
-      }
-    }
-  } else {
-    LOG_AND_FREE_DBUS_ERROR(&err);
-  }
-}
-
-// static
-bool
-BluetoothDBusService::AddReservedServicesInternal(
-                                   const nsTArray<uint32_t>& aServices,
-                                   nsTArray<uint32_t>& aServiceHandlesContainer)
-{
-  MOZ_ASSERT(!NS_IsMainThread());
-
-  int length = aServices.Length();
-  if (length == 0) return false;
-
-  const uint32_t* services = aServices.Elements();
-  DBusMessage* reply =
-    dbus_func_args(gThreadConnection->GetConnection(),
-                   NS_ConvertUTF16toUTF8(sAdapterPath).get(),
-                   DBUS_ADAPTER_IFACE, "AddReservedServiceRecords",
-                   DBUS_TYPE_ARRAY, DBUS_TYPE_UINT32,
-                   &services, length, DBUS_TYPE_INVALID);
-
-  if (!reply) {
-    BT_WARNING("Null DBus message. Couldn't extract handles.");
-    return false;
-  }
-
-  ExtractHandles(reply, aServiceHandlesContainer);
-  return true;
-}
-
 void
 BluetoothDBusService::DisconnectAllAcls(const nsAString& aAdapterPath)
 {
   MOZ_ASSERT(!NS_IsMainThread());
 
   DBusMessage* reply =
     dbus_func_args(gThreadConnection->GetConnection(),
                    NS_ConvertUTF16toUTF8(aAdapterPath).get(),
@@ -1198,86 +1146,146 @@ public:
 
     NS_DispatchToMainThread(new PrepareProfileManagersRunnable());
   }
 
 private:
   const DBusObjectPathVTable* mAgentVTable;
 };
 
-static bool RegisterAgent(const DBusObjectPathVTable* aAgentVTable)
+class AddReservedServiceRecordsReplyHandler : public DBusReplyHandler
 {
-  MOZ_ASSERT(!NS_IsMainThread());
-
-  const char* agentPath = KEY_LOCAL_AGENT;
-  const char* capabilities = B2G_AGENT_CAPABILITIES;
-
-  // Local agent means agent for Adapter, not agent for Device. Some signals
-  // will be passed to local agent, some will be passed to device agent.
-  // For example, if a remote device would like to pair with us, then the
-  // signal will be passed to local agent. If we start pairing process with
-  // calling CreatePairedDevice, we'll get signal which should be passed to
-  // device agent.
-  if (!dbus_connection_register_object_path(gThreadConnection->GetConnection(),
-                                            KEY_LOCAL_AGENT,
-                                            aAgentVTable,
-                                            NULL)) {
-    BT_WARNING("%s: Can't register object path %s for agent!",
-               __FUNCTION__, KEY_LOCAL_AGENT);
-    return false;
+public:
+  void Handle(DBusMessage* aReply)
+  {
+    static const DBusObjectPathVTable sAgentVTable = {
+      NULL, AgentEventFilter, NULL, NULL, NULL, NULL
+    };
+
+    MOZ_ASSERT(!NS_IsMainThread()); // DBus thread
+
+    if (!aReply || (dbus_message_get_type(aReply) == DBUS_MESSAGE_TYPE_ERROR)) {
+      return;
+    }
+
+    // TODO/qdot: This needs to be held for the life of the bluetooth connection
+    // so we could clean it up. For right now though, we can throw it away.
+    nsTArray<uint32_t> handles;
+
+    ExtractHandles(aReply, handles);
+
+    if(!RegisterAgent(&sAgentVTable)) {
+      NS_WARNING("Failed to register agent");
+    }
   }
 
-  nsRefPtr<RegisterAgentReplyHandler> handler = new RegisterAgentReplyHandler(aAgentVTable);
-  MOZ_ASSERT(handler.get());
-
-  bool success = dbus_func_args_async(gThreadConnection->GetConnection(), -1,
-                                      RegisterAgentReplyHandler::Callback, handler.get(),
-                                      NS_ConvertUTF16toUTF8(sAdapterPath).get(),
-                                      DBUS_ADAPTER_IFACE, "RegisterAgent",
-                                      DBUS_TYPE_OBJECT_PATH, &agentPath,
-                                      DBUS_TYPE_STRING, &capabilities,
-                                      DBUS_TYPE_INVALID);
-  NS_ENSURE_TRUE(success, false);
-
-  handler.forget();
-
-  return true;
-}
+private:
+  void ExtractHandles(DBusMessage *aMessage, nsTArray<uint32_t>& aOutHandles)
+  {
+    DBusError error;
+    int length;
+    uint32_t* handles = nullptr;
+
+    dbus_error_init(&error);
+
+    bool success = dbus_message_get_args(aMessage, &error,
+                                         DBUS_TYPE_ARRAY,
+                                         DBUS_TYPE_UINT32,
+                                         &handles, &length,
+                                         DBUS_TYPE_INVALID);
+    if (success != TRUE) {
+      LOG_AND_FREE_DBUS_ERROR(&error);
+      return;
+    }
+
+    if (!handles) {
+      BT_WARNING("Null array in extract_handles");
+      return;
+    }
+
+    for (int i = 0; i < length; ++i) {
+      aOutHandles.AppendElement(handles[i]);
+    }
+  }
+
+  bool RegisterAgent(const DBusObjectPathVTable* aAgentVTable)
+  {
+    const char* agentPath = KEY_LOCAL_AGENT;
+    const char* capabilities = B2G_AGENT_CAPABILITIES;
+
+    nsRefPtr<RawDBusConnection> threadConnection = gThreadConnection;
+
+    if (!threadConnection.get()) {
+      BT_WARNING("%s: DBus connection has been closed.", __FUNCTION__);
+      return false;
+    }
+
+    // Local agent means agent for Adapter, not agent for Device. Some signals
+    // will be passed to local agent, some will be passed to device agent.
+    // For example, if a remote device would like to pair with us, then the
+    // signal will be passed to local agent. If we start pairing process with
+    // calling CreatePairedDevice, we'll get signal which should be passed to
+    // device agent.
+    if (!dbus_connection_register_object_path(threadConnection->GetConnection(),
+                                              KEY_LOCAL_AGENT,
+                                              aAgentVTable,
+                                              NULL)) {
+      BT_WARNING("%s: Can't register object path %s for agent!",
+                 __FUNCTION__, KEY_LOCAL_AGENT);
+      return false;
+    }
+
+    nsRefPtr<RegisterAgentReplyHandler> handler =
+      new RegisterAgentReplyHandler(aAgentVTable);
+    MOZ_ASSERT(handler.get());
+
+    bool success = dbus_func_args_async(threadConnection->GetConnection(), -1,
+                                        RegisterAgentReplyHandler::Callback,
+                                        handler.get(),
+                                        NS_ConvertUTF16toUTF8(sAdapterPath).get(),
+                                        DBUS_ADAPTER_IFACE, "RegisterAgent",
+                                        DBUS_TYPE_OBJECT_PATH, &agentPath,
+                                        DBUS_TYPE_STRING, &capabilities,
+                                        DBUS_TYPE_INVALID);
+    NS_ENSURE_TRUE(success, false);
+
+    handler.forget();
+
+    return true;
+  }
+};
 
 class PrepareAdapterRunnable : public nsRunnable
 {
 public:
   NS_IMETHOD Run()
   {
-    static const DBusObjectPathVTable sAgentVTable = {
-      NULL, AgentEventFilter, NULL, NULL, NULL, NULL
+    static const dbus_uint32_t sServices[] = {
+      BluetoothServiceClass::HANDSFREE_AG,
+      BluetoothServiceClass::HEADSET_AG,
+      BluetoothServiceClass::OBJECT_PUSH
     };
 
     MOZ_ASSERT(!NS_IsMainThread());
-    nsTArray<uint32_t> uuids;
-
-    uuids.AppendElement(BluetoothServiceClass::HANDSFREE_AG);
-    uuids.AppendElement(BluetoothServiceClass::HEADSET_AG);
-    uuids.AppendElement(BluetoothServiceClass::OBJECT_PUSH);
-
-    // TODO/qdot: This needs to be held for the life of the bluetooth connection
-    // so we could clean it up. For right now though, we can throw it away.
-    nsTArray<uint32_t> handles;
-
-    if (!BluetoothDBusService::AddReservedServicesInternal(uuids, handles)) {
-      NS_WARNING("Failed to add reserved services");
-#ifdef MOZ_WIDGET_GONK
-      return NS_ERROR_FAILURE;
-#endif
-    }
-
-    if(!RegisterAgent(&sAgentVTable)) {
-      NS_WARNING("Failed to register agent");
-      return NS_ERROR_FAILURE;
-    }
+
+    nsRefPtr<DBusReplyHandler> handler = new AddReservedServiceRecordsReplyHandler();
+    MOZ_ASSERT(handler.get());
+
+    const dbus_uint32_t* services = sServices;
+
+    bool success = dbus_func_args_async(gThreadConnection->GetConnection(), -1,
+                                        DBusReplyHandler::Callback, handler.get(),
+                                        NS_ConvertUTF16toUTF8(sAdapterPath).get(),
+                                        DBUS_ADAPTER_IFACE, "AddReservedServiceRecords",
+                                        DBUS_TYPE_ARRAY, DBUS_TYPE_UINT32,
+                                        &services, NS_ARRAY_LENGTH(sServices),
+                                        DBUS_TYPE_INVALID);
+    NS_ENSURE_TRUE(success, NS_ERROR_FAILURE);
+
+    handler.forget();
 
     return NS_OK;
   }
 };
 
 class PrepareAdapterTask : public nsRunnable
 {
 public:
@@ -2093,17 +2101,17 @@ BluetoothDBusService::SetProperty(Blueto
     NS_WARNING("Property type not handled!");
     dbus_message_unref(msg);
     return NS_ERROR_FAILURE;
   }
 
   DBusMessageIter value_iter, iter;
   dbus_message_iter_init_append(msg, &iter);
   char var_type[2] = {(char)type, '\0'};
-  if (!dbus_message_iter_open_container(&iter, DBUS_TYPE_VARIANT, 
+  if (!dbus_message_iter_open_container(&iter, DBUS_TYPE_VARIANT,
                                         var_type, &value_iter) ||
       !dbus_message_iter_append_basic(&value_iter, type, val) ||
       !dbus_message_iter_close_container(&iter, &value_iter)) {
     NS_WARNING("Could not append argument to method call!");
     dbus_message_unref(msg);
     return NS_ERROR_FAILURE;
   }