Closed Bug 790739 Opened 7 years ago Closed 7 years ago

[b2g-bluetooth] Server socket connections for bluetooth

Categories

(Core :: DOM: Device Interfaces, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18
blocking-basecamp +

People

(Reporter: qdot, Assigned: qdot)

References

Details

(Whiteboard: [LOE:M])

Attachments

(3 files, 4 obsolete files)

Once a device is bonded, when it is powered on it will usually try to connect to whatever host it is bonded to (i.e. turning a bonded headset on when the phone is already on means the headset will automatically connect). To be able to support this, we need to be able to create and listen on server sockets related to services. This should just be a matter of adding a read watcher to the IO loop with a bit of extra logic to know when we should call read versus accept.
blocking-basecamp: --- → ?
Attachment #665237 - Attachment description: Patch 1 (v1) - Server Sockets for Bluetooth → Patch 1 (v1 - WIP) - Server Sockets for Bluetooth
Attachment #665279 - Attachment is obsolete: true
Attachment #666429 - Flags: review?(jones.chris.g)
Comment on attachment 666429 [details] [diff] [review]
Patch 1 (v3) - UnixSocket changes for server sockets

>diff --git a/ipc/unixsocket/UnixSocket.cpp b/ipc/unixsocket/UnixSocket.cpp

>+  /**
>+   * If true, do not requeue whatever task we're running
>+   */
>+  bool mCancelTask;
>+

Call this |mCurrentTaskIsCanceled|.

>+void
>+UnixSocketImpl::Accept()

>+    if (bind(mFd.get(), &addr, addr_sz)) {
>+#ifdef DEBUG
>+      LOG("...bind(%d) gave errno %d", mFd.get(), errno);
>+#endif

We shouldn't #ifdef DEBUG logging.

>+    EnqueueTask(1000, new SocketAcceptTask(this));

Magic constant.  Give it a symbolic name.

And below.

>+bool
>+UnixSocketImpl::SetNonblockFlags()
>+{
>+  int n = 1;
>+  setsockopt(mFd, SOL_SOCKET, SO_REUSEADDR, &n, sizeof(n));

Is this BT-specific?  I don't recognize this.  Document it.
Attachment #666429 - Flags: review?(jones.chris.g) → review+
Comment on attachment 666430 [details] [diff] [review]
Patch 2 (v3) - BluetoothUnixSocketConnector changes for server sockets

>diff --git a/dom/bluetooth/BluetoothUnixSocketConnector.cpp b/dom/bluetooth/BluetoothUnixSocketConnector.cpp

>+#undef LOG
>+#if defined(MOZ_WIDGET_GONK)
>+#include <android/log.h>
>+#define LOG(args...)  __android_log_print(ANDROID_LOG_INFO, "GonkDBus", args);
>+#else
>+#define BTDEBUG true
>+#define LOG(args...) if (BTDEBUG) printf(args);
>+#endif
>+

We should factor out this logging pattern into something reusable.
Work for followup.

>+int
>+BluetoothUnixSocketConnector::Create()

>+  default:

MOZ_NOT_REACHED()
Attachment #666430 - Flags: review?(jones.chris.g) → review+
Comment on attachment 666431 [details] [diff] [review]
Patch 3 (v3) - BluetoothService/BluetoothDBusService changes for server sockets

>diff --git a/dom/bluetooth/BluetoothHfpManager.cpp b/dom/bluetooth/BluetoothHfpManager.cpp

>+bool
>+BluetoothHfpManager::Listen()

>+  BluetoothService* bs = BluetoothService::Get();
>+  if (!bs) {
>+    NS_WARNING("BluetoothService not available!");
>+    return false;

When could this happen?  If it's a runtime error, we should report it
more loudly.

>diff --git a/dom/bluetooth/linux/BluetoothDBusService.cpp b/dom/bluetooth/linux/BluetoothDBusService.cpp
>index cecb383..e837d74 100644
>--- a/dom/bluetooth/linux/BluetoothDBusService.cpp
>+++ b/dom/bluetooth/linux/BluetoothDBusService.cpp
>@@ -261,6 +261,9 @@ public:
>       NS_WARNING("prepare adapter failed");
>       return NS_ERROR_FAILURE;
>     }
>+
>+    BluetoothHfpManager* m = BluetoothHfpManager::Get();
>+    m->Listen();
>     return NS_OK;
>   }
> 
>@@ -728,6 +731,29 @@ ExtractHandles(DBusMessage *aReply, nsTArray<uint32_t>& aOutHandles)
> 
> // static
> bool
>+BluetoothDBusService::AddServiceRecords(const nsAString& aAdapterPath,
>+                                        const char* serviceName,
>+                                        unsigned long long uuidMsb,
>+                                        unsigned long long uuidLsb,
>+                                        int channel)
>+{
>+  MOZ_ASSERT(!NS_IsMainThread());
>+
>+  DBusMessage *reply;
>+  reply = dbus_func_args(gThreadConnection->GetConnection(),
>+                         NS_ConvertUTF16toUTF8(aAdapterPath).get(),
>+                         DBUS_ADAPTER_IFACE, "AddRfcommServiceRecord",
>+                         DBUS_TYPE_STRING, &serviceName,
>+                         DBUS_TYPE_UINT64, &uuidMsb,
>+                         DBUS_TYPE_UINT64, &uuidLsb,
>+                         DBUS_TYPE_UINT16, &channel,
>+                         DBUS_TYPE_INVALID);
>+
>+  return reply ? dbus_returns_uint32(reply) : -1;
>+}
>+
>+// static
>+bool
> BluetoothDBusService::AddReservedServicesInternal(const nsAString& aAdapterPath,
>                                                   const nsTArray<uint32_t>& aServices,
>                                                   nsTArray<uint32_t>& aServiceHandlesContainer)
>@@ -2277,7 +2303,9 @@ public:
> 
>     nsString address = GetAddressFromObjectPath(mObjectPath);
>     nsString replyError;
>-    BluetoothUnixSocketConnector c(BluetoothSocketType::SCO, -1, mAuth, mEncrypt);
>+    BluetoothUnixSocketConnector* c =
>+      new BluetoothUnixSocketConnector(BluetoothSocketType::SCO, -1,
>+                                       mAuth, mEncrypt);
> 
>     BluetoothScoManager* sco = BluetoothScoManager::Get();
>     if (!mConsumer->ConnectSocket(c, NS_ConvertUTF16toUTF8(address).get())) {
>@@ -2302,16 +2330,16 @@ private:
>   bool mEncrypt;
> };
> 
>-class CreateBluetoothSocketRunnable : public nsRunnable
>+class ConnectBluetoothSocketRunnable : public nsRunnable
> {
> public:
>-  CreateBluetoothSocketRunnable(BluetoothReplyRunnable* aRunnable,
>-                                UnixSocketConsumer* aConsumer,
>-                                const nsAString& aObjectPath,
>-                                const nsAString& aServiceUUID,
>-                                BluetoothSocketType aType,
>-                                bool aAuth,
>-                                bool aEncrypt)
>+  ConnectBluetoothSocketRunnable(BluetoothReplyRunnable* aRunnable,
>+                                 UnixSocketConsumer* aConsumer,
>+                                 const nsAString& aObjectPath,
>+                                 const nsAString& aServiceUUID,
>+                                 BluetoothSocketType aType,
>+                                 bool aAuth,
>+                                 bool aEncrypt)
>     : mRunnable(dont_AddRef(aRunnable)),
>       mConsumer(aConsumer),
>       mObjectPath(aObjectPath),
>@@ -2325,14 +2353,14 @@ public:
>   nsresult
>   Run()
>   {
>-    NS_WARNING("Running create socket!\n");
>     MOZ_ASSERT(!NS_IsMainThread());
> 
>     nsString address = GetAddressFromObjectPath(mObjectPath);
>     int channel = GetDeviceServiceChannel(mObjectPath, mServiceUUID, 0x0004);
>     BluetoothValue v;
>     nsString replyError;
>-    BluetoothUnixSocketConnector c(mType, channel, mAuth, mEncrypt);
>+    BluetoothUnixSocketConnector* c =
>+      new BluetoothUnixSocketConnector(mType, channel, mAuth, mEncrypt);
>     if (!mConsumer->ConnectSocket(c, NS_ConvertUTF16toUTF8(address).get())) {
>       replyError.AssignLiteral("SocketConnectionError");
>       DispatchBluetoothReply(mRunnable, v, replyError);
>@@ -2395,7 +2423,7 @@ BluetoothDBusService::GetSocketViaService(const nsAString& aObjectPath,
>   }
>   nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;
> 
>-  nsRefPtr<nsRunnable> func(new CreateBluetoothSocketRunnable(runnable,
>+  nsRefPtr<nsRunnable> func(new ConnectBluetoothSocketRunnable(runnable,
>                                                               aConsumer,
>                                                               aObjectPath,
>                                                               aService, aType,
>@@ -2409,3 +2437,63 @@ BluetoothDBusService::GetSocketViaService(const nsAString& aObjectPath,
>   return NS_OK;
> }
> 
>+class ListenBluetoothSocketRunnable : public nsRunnable
>+{
>+public:
>+  ListenBluetoothSocketRunnable(UnixSocketConsumer* aConsumer,
>+                                int aChannel,
>+                                BluetoothSocketType aType,
>+                                bool aAuth,
>+                                bool aEncrypt)
>+    : mConsumer(aConsumer),
>+      mChannel(aChannel),
>+      mType(aType),
>+      mAuth(aAuth),
>+      mEncrypt(aEncrypt)

Make these

    : mConsumer(aConsumer)
    , mChannel(aChannel)
    , mType(aType)
    , mAuth(aAuth)
    , mEncrypt(aEncrypt)

Looks good!
Attachment #666431 - Flags: review?(jones.chris.g) → review+
https://hg.mozilla.org/mozilla-central/rev/aade44131d6c
https://hg.mozilla.org/mozilla-central/rev/307b15d6a63f
https://hg.mozilla.org/mozilla-central/rev/22011fdcca01

Should this have a test?
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.