Bug 768306 - Patch 1: Fix error handling in unpacking DBus replies, set up DBus blocking call handling thread; r=mrbkap
--- a/dom/bluetooth/BluetoothService.cpp
+++ b/dom/bluetooth/BluetoothService.cpp
@@ -17,44 +17,47 @@
#include "mozilla/Services.h"
#include "mozilla/LazyIdleThread.h"
#include "mozilla/Util.h"
using namespace mozilla;
USING_BLUETOOTH_NAMESPACE
-nsRefPtr<BluetoothService> gBluetoothService;
-nsCOMPtr<nsIThread> gToggleBtThread;
-int gPendingInitCount = 0;
-bool gInShutdown = false;
+static nsRefPtr<BluetoothService> gBluetoothService;
+static bool gInShutdown = false;
NS_IMPL_ISUPPORTS1(BluetoothService, nsIObserver)
class ToggleBtAck : public nsRunnable
{
public:
+ ToggleBtAck(bool aEnabled) :
+ mEnabled(aEnabled)
+ {
+ }
+
NS_IMETHOD Run()
{
MOZ_ASSERT(NS_IsMainThread());
- gPendingInitCount--;
-
- if (gPendingInitCount) {
- return NS_OK;
+
+ if (!mEnabled || gInShutdown) {
+ nsCOMPtr<nsIThread> t;
+ gBluetoothService->mBluetoothCommandThread.swap(t);
+ t->Shutdown();
}
if (gInShutdown) {
gBluetoothService = nullptr;
}
- nsCOMPtr<nsIThread> t;
- gToggleBtThread.swap(t);
- t->Shutdown();
return NS_OK;
}
+
+ bool mEnabled;
};
class ToggleBtTask : public nsRunnable
{
public:
ToggleBtTask(bool aEnabled,
BluetoothReplyRunnable* aRunnable)
: mEnabled(aEnabled),
@@ -77,17 +80,17 @@ public:
if (NS_FAILED(gBluetoothService->StopInternal())) {
replyError.AssignLiteral("Bluetooth service not available - We should never reach this point!");
}
}
// Always has to be called since this is where we take care of our reference
// count for runnables. If there's an error, replyError won't be empty, so
// consider our status flipped.
- nsCOMPtr<nsIRunnable> ackTask = new ToggleBtAck();
+ nsCOMPtr<nsIRunnable> ackTask = new ToggleBtAck(mEnabled);
if (NS_FAILED(NS_DispatchToMainThread(ackTask))) {
NS_WARNING("Failed to dispatch to main thread!");
}
if (!mRunnable) {
return NS_OK;
}
@@ -163,27 +166,26 @@ BluetoothService::StartStopBluetooth(Blu
{
MOZ_ASSERT(NS_IsMainThread());
// If we're shutting down, bail early.
if (gInShutdown && aStart) {
NS_ERROR("Start called while in shutdown!");
return NS_ERROR_FAILURE;
}
- if (!gToggleBtThread) {
- nsresult rv = NS_NewNamedThread("BluetoothCtrl",
- getter_AddRefs(gToggleBtThread));
+ if (!mBluetoothCommandThread) {
+ nsresult rv = NS_NewNamedThread("BluetoothCmd",
+ getter_AddRefs(mBluetoothCommandThread));
NS_ENSURE_SUCCESS(rv, rv);
}
nsCOMPtr<nsIRunnable> r = new ToggleBtTask(aStart, aResultRunnable);
- if (NS_FAILED(gToggleBtThread->Dispatch(r, NS_DISPATCH_NORMAL))) {
+ if (NS_FAILED(mBluetoothCommandThread->Dispatch(r, NS_DISPATCH_NORMAL))) {
NS_WARNING("Cannot dispatch firmware loading task!");
return NS_ERROR_FAILURE;
}
- gPendingInitCount++;
return NS_OK;
}
nsresult
BluetoothService::Start(BluetoothReplyRunnable* aResultRunnable)
{
return StartStopBluetooth(aResultRunnable, true);
}
--- a/dom/bluetooth/BluetoothService.h
+++ b/dom/bluetooth/BluetoothService.h
@@ -187,16 +187,29 @@ public:
const nsAString& aDeviceAddress,
nsAString& aDevicePath) = 0;
virtual int
GetDeviceServiceChannelInternal(const nsAString& aObjectPath,
const nsAString& aPattern,
int aAttributeId) = 0;
+ /**
+ * Due to the fact that some operations require multiple calls, a
+ * CommandThread is created that can run blocking, platform-specific calls
+ * where either no asynchronous equivilent exists, or else where multiple
+ * asynchronous calls would require excessive runnable bouncing between main
+ * thread and IO thread.
+ *
+ * For instance, when we retrieve an Adapter object, we would like it to come
+ * with all of its properties filled in and registered as an agent, which
+ * requires a minimum of 3 calls to platform specific code on some platforms.
+ *
+ */
+ nsCOMPtr<nsIThread> mBluetoothCommandThread;
protected:
BluetoothService()
{
mBluetoothSignalObserverTable.Init();
}
virtual ~BluetoothService()
{
--- a/dom/bluetooth/linux/BluetoothDBusService.cpp
+++ b/dom/bluetooth/linux/BluetoothDBusService.cpp
@@ -144,125 +144,135 @@ public:
NS_WARNING("BluetoothService not available!");
return NS_ERROR_FAILURE;
}
return bs->DistributeSignal(mSignal);
}
};
bool
-IsDBusMessageError(DBusMessage* aMsg, nsAString& aError)
+IsDBusMessageError(DBusMessage* aMsg, DBusError* aErr, nsAString& aErrorStr)
{
+ if(aErr && dbus_error_is_set(aErr)) {
+ aErrorStr = NS_ConvertUTF8toUTF16(aErr->message);
+ LOG_AND_FREE_DBUS_ERROR(aErr);
+ return true;
+ }
+
DBusError err;
dbus_error_init(&err);
if (dbus_message_get_type(aMsg) == DBUS_MESSAGE_TYPE_ERROR) {
const char* error_msg;
if (!dbus_message_get_args(aMsg, &err, DBUS_TYPE_STRING,
&error_msg, DBUS_TYPE_INVALID) ||
!error_msg) {
if (dbus_error_is_set(&err)) {
- aError = NS_ConvertUTF8toUTF16(err.message);
+ aErrorStr = NS_ConvertUTF8toUTF16(err.message);
LOG_AND_FREE_DBUS_ERROR(&err);
return true;
+ } else {
+ aErrorStr.AssignLiteral("Unknown Error");
+ return true;
}
} else {
- aError = NS_ConvertUTF8toUTF16(error_msg);
+ aErrorStr = NS_ConvertUTF8toUTF16(error_msg);
return true;
}
}
return false;
}
void
DispatchBluetoothReply(BluetoothReplyRunnable* aRunnable,
- const BluetoothValue& aValue, const nsAString& aError)
+ const BluetoothValue& aValue, const nsAString& aErrorStr)
{
// Reply will be deleted by the runnable after running on main thread
BluetoothReply* reply;
- if (!aError.IsEmpty()) {
- nsString err(aError);
+ if (!aErrorStr.IsEmpty()) {
+ nsString err(aErrorStr);
reply = new BluetoothReply(BluetoothReplyError(err));
- }
- else {
+ } else {
reply = new BluetoothReply(BluetoothReplySuccess(aValue));
}
aRunnable->SetReply(reply);
if (NS_FAILED(NS_DispatchToMainThread(aRunnable))) {
NS_WARNING("Failed to dispatch to main thread!");
}
}
void
-UnpackObjectPathMessage(DBusMessage* aMsg, BluetoothValue& aValue,
- nsAString& aErrorStr)
+UnpackObjectPathMessage(DBusMessage* aMsg, DBusError* aErr,
+ BluetoothValue& aValue, nsAString& aErrorStr)
{
DBusError err;
dbus_error_init(&err);
- if (!IsDBusMessageError(aMsg, aErrorStr)) {
+ if (!IsDBusMessageError(aMsg, aErr, aErrorStr)) {
NS_ASSERTION(dbus_message_get_type(aMsg) == DBUS_MESSAGE_TYPE_METHOD_RETURN,
"Got dbus callback that's not a METHOD_RETURN!");
const char* object_path;
if (!dbus_message_get_args(aMsg, &err, DBUS_TYPE_OBJECT_PATH,
&object_path, DBUS_TYPE_INVALID) ||
!object_path) {
if (dbus_error_is_set(&err)) {
aErrorStr = NS_ConvertUTF8toUTF16(err.message);
LOG_AND_FREE_DBUS_ERROR(&err);
}
} else {
aValue = NS_ConvertUTF8toUTF16(object_path);
}
}
}
-typedef void (*UnpackFunc)(DBusMessage*, BluetoothValue&, nsAString&);
+typedef void (*UnpackFunc)(DBusMessage*, DBusError*, BluetoothValue&, nsAString&);
void
RunDBusCallback(DBusMessage* aMsg, void* aBluetoothReplyRunnable,
UnpackFunc aFunc)
{
MOZ_ASSERT(!NS_IsMainThread());
nsRefPtr<BluetoothReplyRunnable> replyRunnable =
dont_AddRef(static_cast< BluetoothReplyRunnable* >(aBluetoothReplyRunnable));
NS_ASSERTION(replyRunnable, "Callback reply runnable is null!");
nsString replyError;
BluetoothValue v;
- aFunc(aMsg, v, replyError);
+ aFunc(aMsg, nsnull, v, replyError);
DispatchBluetoothReply(replyRunnable, v, replyError);
}
void
GetObjectPathCallback(DBusMessage* aMsg, void* aBluetoothReplyRunnable)
{
- RunDBusCallback(aMsg, aBluetoothReplyRunnable, UnpackObjectPathMessage);
+ RunDBusCallback(aMsg, aBluetoothReplyRunnable,
+ UnpackObjectPathMessage);
}
void
-UnpackVoidMessage(DBusMessage* aMsg, BluetoothValue& aValue,
+UnpackVoidMessage(DBusMessage* aMsg, DBusError* aErr, BluetoothValue& aValue,
nsAString& aErrorStr)
{
DBusError err;
dbus_error_init(&err);
- if (!IsDBusMessageError(aMsg, aErrorStr) &&
+ if (!IsDBusMessageError(aMsg, aErr, aErrorStr) &&
dbus_message_get_type(aMsg) == DBUS_MESSAGE_TYPE_METHOD_RETURN &&
!dbus_message_get_args(aMsg, &err, DBUS_TYPE_INVALID)) {
if (dbus_error_is_set(&err)) {
aErrorStr = NS_ConvertUTF8toUTF16(err.message);
LOG_AND_FREE_DBUS_ERROR(&err);
}
}
}
void
GetVoidCallback(DBusMessage* aMsg, void* aBluetoothReplyRunnable)
{
- RunDBusCallback(aMsg, aBluetoothReplyRunnable, UnpackVoidMessage);
+ RunDBusCallback(aMsg, aBluetoothReplyRunnable,
+ UnpackVoidMessage);
}
bool
GetProperty(DBusMessageIter aIter, Properties* aPropertyTypes,
int aPropertyTypeLen, int* aPropIndex,
InfallibleTArray<BluetoothNamedValue>& aProperties)
{
DBusMessageIter prop_val, array_val_iter;
@@ -376,72 +386,83 @@ ParseProperties(DBusMessageIter* aIter,
NS_WARNING("Can't create property!");
return;
}
} while (dbus_message_iter_next(&dict));
aValue = props;
}
-void UnpackPropertiesMessage(DBusMessage* aMsg, BluetoothValue& aValue,
- nsAString& aErrorStr, Properties* aPropertyTypes,
- const int aPropertyTypeLen)
+void
+UnpackPropertiesMessage(DBusMessage* aMsg, DBusError* aErr,
+ BluetoothValue& aValue, nsAString& aErrorStr,
+ Properties* aPropertyTypes,
+ const int aPropertyTypeLen)
{
- if (!IsDBusMessageError(aMsg, aErrorStr) &&
+ if (!IsDBusMessageError(aMsg, aErr, aErrorStr) &&
dbus_message_get_type(aMsg) == DBUS_MESSAGE_TYPE_METHOD_RETURN) {
DBusMessageIter iter;
if (!dbus_message_iter_init(aMsg, &iter)) {
aErrorStr.AssignLiteral("Cannot create dbus message iter!");
} else {
ParseProperties(&iter, aValue, aErrorStr, aPropertyTypes,
aPropertyTypeLen);
}
}
}
-void UnpackAdapterPropertiesMessage(DBusMessage* aMsg, BluetoothValue& aValue,
- nsAString& aErrorStr)
+void
+UnpackAdapterPropertiesMessage(DBusMessage* aMsg, DBusError* aErr,
+ BluetoothValue& aValue,
+ nsAString& aErrorStr)
{
- UnpackPropertiesMessage(aMsg, aValue, aErrorStr,
+ UnpackPropertiesMessage(aMsg, aErr, aValue, aErrorStr,
sAdapterProperties,
ArrayLength(sAdapterProperties));
}
-void UnpackDevicePropertiesMessage(DBusMessage* aMsg, BluetoothValue& aValue,
- nsAString& aErrorStr)
+void
+UnpackDevicePropertiesMessage(DBusMessage* aMsg, DBusError* aErr,
+ BluetoothValue& aValue,
+ nsAString& aErrorStr)
{
- UnpackPropertiesMessage(aMsg, aValue, aErrorStr,
+ UnpackPropertiesMessage(aMsg, aErr, aValue, aErrorStr,
sDeviceProperties,
ArrayLength(sDeviceProperties));
}
-void UnpackManagerPropertiesMessage(DBusMessage* aMsg, BluetoothValue& aValue,
- nsAString& aErrorStr)
+void
+UnpackManagerPropertiesMessage(DBusMessage* aMsg, DBusError* aErr,
+ BluetoothValue& aValue,
+ nsAString& aErrorStr)
{
- UnpackPropertiesMessage(aMsg, aValue, aErrorStr,
+ UnpackPropertiesMessage(aMsg, aErr, aValue, aErrorStr,
sManagerProperties,
ArrayLength(sManagerProperties));
}
void
GetManagerPropertiesCallback(DBusMessage* aMsg, void* aBluetoothReplyRunnable)
{
- RunDBusCallback(aMsg, aBluetoothReplyRunnable, UnpackManagerPropertiesMessage);
+ RunDBusCallback(aMsg, aBluetoothReplyRunnable,
+ UnpackManagerPropertiesMessage);
}
void
GetAdapterPropertiesCallback(DBusMessage* aMsg, void* aBluetoothReplyRunnable)
{
- RunDBusCallback(aMsg, aBluetoothReplyRunnable, UnpackAdapterPropertiesMessage);
+ RunDBusCallback(aMsg, aBluetoothReplyRunnable,
+ UnpackAdapterPropertiesMessage);
}
void
GetDevicePropertiesCallback(DBusMessage* aMsg, void* aBluetoothReplyRunnable)
{
- RunDBusCallback(aMsg, aBluetoothReplyRunnable, UnpackDevicePropertiesMessage);
+ RunDBusCallback(aMsg, aBluetoothReplyRunnable,
+ UnpackDevicePropertiesMessage);
}
static DBusCallback sBluetoothDBusPropCallbacks[] =
{
GetManagerPropertiesCallback,
GetAdapterPropertiesCallback,
GetDevicePropertiesCallback
};
@@ -606,17 +627,16 @@ BluetoothDBusService::StartInternal()
return NS_ERROR_FAILURE;
}
if (mConnection) {
return NS_OK;
}
if (NS_FAILED(EstablishDBusConnection())) {
- NS_WARNING("Cannot start DBus connection!");
StopDBus();
return NS_ERROR_FAILURE;
}
DBusError err;
dbus_error_init(&err);
// Set which messages will be processed by this dbus connection.
--- a/ipc/dbus/DBusUtils.cpp
+++ b/ipc/dbus/DBusUtils.cpp
@@ -194,38 +194,39 @@ DBusMessage * dbus_func_args_timeout_val
if (!dbus_message_append_args_valist(msg, first_arg_type, args)) {
LOG("Could not append argument to method call!");
goto done;
}
/* Make the call. */
reply = dbus_connection_send_with_reply_and_block(conn, msg, timeout_ms, err);
if (!return_error && dbus_error_is_set(err)) {
- //LOG_AND_FREE_DBUS_ERROR_WITH_MSG(err, msg);
+ LOG_AND_FREE_DBUS_ERROR_WITH_MSG(err, msg);
}
done:
if (!return_error) {
free(err);
}
if (msg) dbus_message_unref(msg);
return reply;
}
DBusMessage * dbus_func_args_timeout(DBusConnection *conn,
int timeout_ms,
+ DBusError* err,
const char *path,
const char *ifc,
const char *func,
int first_arg_type,
...) {
DBusMessage *ret;
va_list lst;
va_start(lst, first_arg_type);
- ret = dbus_func_args_timeout_valist(conn, timeout_ms, NULL,
+ ret = dbus_func_args_timeout_valist(conn, timeout_ms, err,
path, ifc, func,
first_arg_type, lst);
va_end(lst);
return ret;
}
DBusMessage * dbus_func_args(DBusConnection *conn,
const char *path,
--- a/ipc/dbus/DBusUtils.h
+++ b/ipc/dbus/DBusUtils.h
@@ -85,16 +85,17 @@ DBusMessage* dbus_func_args_error(DBusC
const char* path,
const char* ifc,
const char* func,
int first_arg_type,
...);
DBusMessage* dbus_func_args_timeout(DBusConnection* conn,
int timeout_ms,
+ DBusError* err,
const char* path,
const char* ifc,
const char* func,
int first_arg_type,
...);
DBusMessage* dbus_func_args_timeout_valist(DBusConnection* conn,
int timeout_ms,
--- a/ipc/dbus/RawDBusConnection.h
+++ b/ipc/dbus/RawDBusConnection.h
@@ -26,15 +26,19 @@ class RawDBusConnection
{
static void release(DBusConnection* ptr);
};
public:
RawDBusConnection();
~RawDBusConnection();
nsresult EstablishDBusConnection();
+ DBusConnection* GetConnection() {
+ return mConnection;
+ }
+protected:
Scoped<ScopedDBusConnectionPtrTraits> mConnection;
};
}
}
#endif