bluetooth: Subscribe to notifications

content/renderer/bluetooth/bluetooth_dispatcher.cc

Index: content/renderer/bluetooth/bluetooth_dispatcher.cc
diff --git a/content/renderer/bluetooth/bluetooth_dispatcher.cc b/content/renderer/bluetooth/bluetooth_dispatcher.cc
index 32323903c772a967d1725d25526dbd5b6a2e42ae..743e6a083f7be884c0103c9333398298391eac1d 100644
--- a/content/renderer/bluetooth/bluetooth_dispatcher.cc
+++ b/content/renderer/bluetooth/bluetooth_dispatcher.cc
@@ -14,6 +14,7 @@
 #include "third_party/WebKit/public/platform/WebPassOwnPtr.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothDevice.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothError.h"
+#include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothGATTCharacteristicInit.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothGATTRemoteServer.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothGATTService.h"
@@ -31,6 +32,8 @@ using blink::WebBluetoothScanFilter;
 using blink::WebRequestDeviceOptions;
 using blink::WebString;
 using blink::WebVector;
+using NotificationsRequestType =
+    content::BluetoothDispatcher::NotificationsRequestType;
 
 struct BluetoothPrimaryServiceRequest {
   BluetoothPrimaryServiceRequest(
@@ -62,6 +65,28 @@ struct BluetoothCharacteristicRequest {
   scoped_ptr<blink::WebBluetoothGetCharacteristicCallbacks> callbacks;
 };
 
+struct BluetoothNotificationsRequest {

[Jeffrey Yasskin, 2015/09/25 23:03:19]

Maybe "BluetoothNotificationStateChangeRequest"? Or comment what the class
means, since it's not just requesting the set of notifications.

[ortuno, 2015/09/29 22:47:36]

Done.

+  BluetoothNotificationsRequest(
+      const std::string characteristic_instance_id,
+      blink::WebBluetoothGATTCharacteristic* characteristic,
+      blink::WebBluetoothNotificationsCallbacks* callbacks,
+      NotificationsRequestType type)
+      : characteristic_instance_id(characteristic_instance_id),
+        characteristic(characteristic),
+        callbacks(callbacks),
+        type(type) {}
+  ~BluetoothNotificationsRequest() {}
+
+  const std::string characteristic_instance_id;
+  // Note that the characteristic object is owned by the execution context on
+  // the blink side which can destroy the object at any point. Since the
+  // object implements ActiveDOMObject we do get notify when it's being

[Jeffrey Yasskin, 2015/09/25 23:03:19]

Instead of "we get notified", can you mention the function that carries the
notification and nulls |characteristic|? You also don't need "Note that".
Otherwise, looks good.

[ortuno, 2015/09/29 22:47:36]

Done.

+  // destroyed, so we can remove any references to it.
+  blink::WebBluetoothGATTCharacteristic* characteristic;
+  scoped_ptr<blink::WebBluetoothNotificationsCallbacks> callbacks;
+  NotificationsRequestType type;
+};
+
 namespace content {
 
 namespace {
@@ -143,6 +168,12 @@ void BluetoothDispatcher::OnMessageReceived(const IPC::Message& msg) {
                       OnWriteValueSuccess);
   IPC_MESSAGE_HANDLER(BluetoothMsg_WriteCharacteristicValueError,
                       OnWriteValueError);
+  IPC_MESSAGE_HANDLER(BluetoothMsg_StartNotificationsSuccess,
+                      OnStartNotificationsSuccess)
+  IPC_MESSAGE_HANDLER(BluetoothMsg_StartNotificationsError,
+                      OnStartNotificationsError)
+  IPC_MESSAGE_HANDLER(BluetoothMsg_StopNotificationsSuccess,
+                      OnStopNotificationsSuccess)
   IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   DCHECK(handled) << "Unhandled message:" << msg.type();
@@ -225,10 +256,224 @@ void BluetoothDispatcher::writeValue(
       CurrentWorkerId(), request_id, characteristic_instance_id.utf8(), value));
 }
 
+void BluetoothDispatcher::startNotifications(
+    const blink::WebString& characteristic_instance_id,
+    blink::WebBluetoothGATTCharacteristic* characteristic,
+    blink::WebBluetoothNotificationsCallbacks* callbacks) {
+  int request_id = QueueNotificationRequest(characteristic_instance_id.utf8(),
+                                            characteristic, callbacks,
+                                            NotificationsRequestType::START);
+
+  // The Notification subscription's state can change after a request
+  // finishes. To avoid resolving with a soon-to-be-invalid state we queue
+  // requests.
+  if (PendingNotificationRequest(characteristic_instance_id.utf8())) {
+    return;
+  }
+
+  processStartNotificationsRequest(request_id);
+}
+
+void BluetoothDispatcher::stopNotifications(
+    const blink::WebString& characteristic_instance_id,
+    blink::WebBluetoothGATTCharacteristic* characteristic,
+    blink::WebBluetoothNotificationsCallbacks* callbacks) {
+  int request_id = QueueNotificationRequest(characteristic_instance_id.utf8(),
+                                            characteristic, callbacks,
+                                            NotificationsRequestType::STOP);
+  if (PendingNotificationRequest(characteristic_instance_id.utf8())) {
+    return;
+  }
+
+  processStopNotificationsRequest(request_id);
+}
+
+void BluetoothDispatcher::characteristicObjectRemoved(
+    const blink::WebString& characteristic_instance_id,
+    blink::WebBluetoothGATTCharacteristic* characteristic) {
+  // If there is a notification request pending then there are two posibilities:
+  //   1. The subscription is going to be active. If this is the case,
+  //      we null the characteristic object. startNotificationsSuccess will take
+  //      care of stopping the notifications.
+  //   2. The subscription is going to be inactive. If this is the case, we
+  //      don't need to to anything since the notification is stopped already.
+  if (PendingNotificationRequest(characteristic_instance_id.utf8())) {
+    for (IDMap<BluetoothNotificationsRequest, IDMapOwnPointer>::iterator iter(
+             &pending_notifications_requests_);
+         !iter.IsAtEnd(); iter.Advance()) {
+      iter.GetCurrentValue()->characteristic = nullptr;
+    }
+    return;
+  }
+
+  // If there are no notification requests pending then there are three
+  // possibilities after removing this characteristic:
+  //   1. The subscription was innactive already.

[Jeffrey Yasskin, 2015/09/25 23:03:19]

sp: innactive, and on the next line

[ortuno, 2015/09/29 22:47:36]

Done.

+  //   2. The subscription will become innactive.
+  //   3. The subscription will still be active.
+
+  if (!HasActiveNotificationSubscription(characteristic_instance_id.utf8())) {
+    return;
+  }
+
+  // For 2 and 3. We simply call processStopNotificationsRequest which will take
+  // care of stopping the notifications if the subscription becomes inactive.
+  processStopNotificationsRequest(QueueNotificationRequest(
+      characteristic_instance_id.utf8(), characteristic,
+      nullptr /* callbacks */, NotificationsRequestType::STOP));
+}
+
 void BluetoothDispatcher::WillStopCurrentWorkerThread() {
   delete this;
 }
 
+int BluetoothDispatcher::QueueNotificationRequest(
+    const std::string& characteristic_instance_id,
+    blink::WebBluetoothGATTCharacteristic* characteristic,
+    blink::WebBluetoothNotificationsCallbacks* callbacks,
+    NotificationsRequestType type) {
+  int request_id;
+  switch (type) {
+    case NotificationsRequestType::START:
+      request_id =
+          pending_notifications_requests_.Add(new BluetoothNotificationsRequest(

[Jeffrey Yasskin, 2015/09/25 23:03:19]

It looks like you don't actually need the switch here and can just
unconditionally add the BluetoothNotificationsRequest?

[ortuno, 2015/09/29 22:47:36]

Done.

+              characteristic_instance_id, characteristic, callbacks, type));
+      break;
+    case NotificationsRequestType::STOP:
+      request_id =
+          pending_notifications_requests_.Add(new BluetoothNotificationsRequest(
+              characteristic_instance_id, characteristic, callbacks, type));
+      break;
+  }
+  queued_notification_requests_[characteristic_instance_id].push(request_id);
+
+  return request_id;
+}
+
+void BluetoothDispatcher::ProcessQueuedNotificationRequests(int request_id) {
+  BluetoothNotificationsRequest* old_request =
+      pending_notifications_requests_.Lookup(request_id);
+
+  auto queue_iter = queued_notification_requests_.find(

[Jeffrey Yasskin, 2015/09/25 23:03:19]

Also CHECK(queue_iter != queued_notification_requests_.end()).

[ortuno, 2015/09/29 22:47:36]

Done.

+      old_request->characteristic_instance_id);
+
+  DCHECK(queue_iter->second.front() == request_id);

[Jeffrey Yasskin, 2015/09/25 23:03:20]

Alias std::queue<int>& request_queue = queue_iter->second;

[ortuno, 2015/09/29 22:47:36]

Done.

+
+  // Remove old request and clean map if necessary.
+  queue_iter->second.pop();
+  pending_notifications_requests_.Remove(request_id);
+  if (queue_iter->second.empty()) {
+    queued_notification_requests_.erase(queue_iter);
+    return;
+  }
+
+  int next_request_id = queue_iter->second.front();
+  BluetoothNotificationsRequest* next_request =
+      pending_notifications_requests_.Lookup(next_request_id);
+
+  switch (next_request->type) {
+    case NotificationsRequestType::START:
+      processStartNotificationsRequest(next_request_id);
+      return;
+    case NotificationsRequestType::STOP:
+      processStopNotificationsRequest(next_request_id);
+      return;
+  }
+}
+
+bool BluetoothDispatcher::PendingNotificationRequest(
+    const std::string& characteristic_instance_id) {
+  return queued_notification_requests_.find(characteristic_instance_id) !=
+             queued_notification_requests_.end() &&
+         (queued_notification_requests_[characteristic_instance_id].size() > 1);
+}
+
+bool BluetoothDispatcher::HasActiveNotificationSubscription(
+    const std::string& characteristic_instance_id) {
+  return active_notification_subscriptions_.find(characteristic_instance_id) !=

[Jeffrey Yasskin, 2015/09/25 23:03:19]

You can use ContainsKey() for this
(https://code.google.com/p/chromium/codesearch/#chromium/src/base/stl_util.h&s...)

[ortuno, 2015/09/29 22:47:36]

Done.

+         active_notification_subscriptions_.end();
+}
+
+void BluetoothDispatcher::AddToActiveNotificationSubscriptions(
+    const std::string& characteristic_instance_id,
+    blink::WebBluetoothGATTCharacteristic* characteristic) {
+  active_notification_subscriptions_[characteristic_instance_id].insert(
+      characteristic);
+}
+
+bool BluetoothDispatcher::RemoveFromActiveNotificationSubscriptions(
+    const std::string& characteristic_instance_id,
+    blink::WebBluetoothGATTCharacteristic* characteristic) {
+  auto active_map =
+      active_notification_subscriptions_.find(characteristic_instance_id);
+
+  if (active_map == active_notification_subscriptions_.end()) {
+    return false;
+  }
+
+  active_map->second.erase(characteristic);
+
+  if (active_map->second.empty()) {
+    active_notification_subscriptions_.erase(active_map);
+    DCHECK(!HasActiveNotificationSubscription(characteristic_instance_id));
+    return true;
+  }
+  return false;
+}
+
+void BluetoothDispatcher::processStartNotificationsRequest(int request_id) {
+  BluetoothNotificationsRequest* request =
+      pending_notifications_requests_.Lookup(request_id);
+  const std::string& characteristic_instance_id =
+      request->characteristic_instance_id;
+  blink::WebBluetoothGATTCharacteristic* characteristic =
+      request->characteristic;
+  blink::WebBluetoothNotificationsCallbacks* callbacks =
+      request->callbacks.get();
+
+  // If an object is already subscribed to notifications from the characteristic
+  // no need to send an IPC again.
+  if (HasActiveNotificationSubscription(characteristic_instance_id)) {
+    AddToActiveNotificationSubscriptions(characteristic_instance_id,
+                                         characteristic);
+    callbacks->onSuccess();
+
+    ProcessQueuedNotificationRequests(request_id);
+    return;
+  }
+  Send(new BluetoothHostMsg_StartNotifications(CurrentWorkerId(), request_id,
+                                               characteristic_instance_id));
+}
+
+void BluetoothDispatcher::processStopNotificationsRequest(int request_id) {
+  // The Notification subscription's state can change after a request
+  // finishes. To avoid resolving with a soon-to-be-invalid state we queue
+  // requests.
+  BluetoothNotificationsRequest* request =
+      pending_notifications_requests_.Lookup(request_id);
+  const std::string& characteristic_instance_id =
+      request->characteristic_instance_id;
+  blink::WebBluetoothGATTCharacteristic* characteristic =
+      request->characteristic;
+  blink::WebBluetoothNotificationsCallbacks* callbacks =
+      request->callbacks.get();
+
+  // If removing turns the subscription inactive then stop.
+  if (RemoveFromActiveNotificationSubscriptions(characteristic_instance_id,
+                                                characteristic)) {
+    Send(new BluetoothHostMsg_StopNotifications(CurrentWorkerId(), request_id,
+                                                characteristic_instance_id));
+    return;
+  }
+  // If the stop request comes from an object that was destroyed the
+  // callback was handled already. See OnStartNotificationsSuccess
+  // for more context.
+  if (callbacks != nullptr) {

[Jeffrey Yasskin, 2015/09/25 23:03:19]

Do you mean callbacks or characteristic here? If this is wrong, add a
TODO(crbug/NNNN) for a test that would have caught it.

[ortuno, 2015/09/29 22:47:36]

callbacks is correct. Added:

An object is destroyed 1) during a start request or 2) after a start
request. If 1), then OnStartNotificationsSuccess will queue a stop with
callback = nullptr and characteristic = nullptr which will eventually call
processStopNotificationsRequest. If 2), characteristic needs to be removed
from active_notifications_subscriptions so onCharacteristicObjectRemoved
will call processStopNotification with callback = nullptr only.

+    callbacks->onSuccess();
+  }
+  ProcessQueuedNotificationRequests(request_id);
+}
+
 void BluetoothDispatcher::OnRequestDeviceSuccess(
     int thread_id,
     int request_id,
@@ -386,4 +631,78 @@ void BluetoothDispatcher::OnWriteValueError(int thread_id,
   pending_write_value_requests_.Remove(request_id);
 }
 
+void BluetoothDispatcher::OnStartNotificationsSuccess(int thread_id,
+                                                      int request_id) {
+  DCHECK(pending_notifications_requests_.Lookup(request_id)) << request_id;
+
+  BluetoothNotificationsRequest* request =
+      pending_notifications_requests_.Lookup(request_id);
+
+  DCHECK(queued_notification_requests_[request->characteristic_instance_id]
+             .front() == request_id);
+
+  // We only send an IPC for inactive notifications.
+  DCHECK(
+      !HasActiveNotificationSubscription(request->characteristic_instance_id));
+
+  AddToActiveNotificationSubscriptions(request->characteristic_instance_id,
+                                       request->characteristic);
+
+  // The object requesting the notification could have been destroyed
+  // while waiting for the subscription. characteristicRemoved
+  // nulls the characteristic when the corresponding js object gets destroyed.
+  // Since there could be other characteristics waiting to subscribe, we queue
+  // a stop.
+  if (request->characteristic == nullptr) {
+    QueueNotificationRequest(
+        request->characteristic_instance_id, nullptr /* characteristic */,
+        nullptr /* callbacks */, NotificationsRequestType::STOP);
+  }
+
+  request->callbacks->onSuccess();
+
+  ProcessQueuedNotificationRequests(request_id);
+}
+
+void BluetoothDispatcher::OnStartNotificationsError(int thread_id,
+                                                    int request_id,
+                                                    WebBluetoothError error) {
+  DCHECK(pending_notifications_requests_.Lookup(request_id)) << request_id;
+
+  BluetoothNotificationsRequest* request =
+      pending_notifications_requests_.Lookup(request_id);
+
+  DCHECK(queued_notification_requests_[request->characteristic_instance_id]
+             .front() == request_id);
+
+  // We only send an IPC for inactive notifications.
+  DCHECK(
+      !HasActiveNotificationSubscription(request->characteristic_instance_id));
+
+  request->callbacks->onError(error);
+
+  ProcessQueuedNotificationRequests(request_id);
+}
+
+void BluetoothDispatcher::OnStopNotificationsSuccess(int thread_id,
+                                                     int request_id) {
+  DCHECK(pending_notifications_requests_.Lookup(request_id)) << request_id;
+
+  BluetoothNotificationsRequest* request =
+      pending_notifications_requests_.Lookup(request_id);
+
+  DCHECK(queued_notification_requests_[request->characteristic_instance_id]
+             .front() == request_id);
+
+  // We only send an IPC for inactive notifications.
+  DCHECK(
+      !HasActiveNotificationSubscription(request->characteristic_instance_id));
+
+  if (request->callbacks != nullptr) {
+    request->callbacks->onSuccess();
+  }
+
+  ProcessQueuedNotificationRequests(request_id);
+}
+
 }  // namespace content