Implement BluetoothRemoteGattCharacteristicWin::StartNotifySession and related unit tests

device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc

Index: device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc
index d98c0ea8d2fcb7f5709672d03305dee3076abfe7..34d464bdc2f9db7ab842a5273a01dc7cef3c4714 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc
@@ -6,10 +6,24 @@
 
 #include "base/bind.h"
 #include "device/bluetooth/bluetooth_adapter_win.h"
+#include "device/bluetooth/bluetooth_gatt_notify_session_win.h"
 #include "device/bluetooth/bluetooth_remote_gatt_descriptor_win.h"
 #include "device/bluetooth/bluetooth_remote_gatt_service_win.h"
 #include "device/bluetooth/bluetooth_task_manager_win.h"
 
+namespace {
+
+// Function to be registered to OS to monitor Bluetooth LE GATT event.
+void OnGetGattEventWin(BTH_LE_GATT_EVENT_TYPE type,
+                       PVOID event_parameter,
+                       PVOID context) {
+  device::BluetoothRemoteGattCharacteristicWin* characteristic =
+      (device::BluetoothRemoteGattCharacteristicWin*)context;
+  characteristic->OnGetRemoteGattEvent(type, event_parameter);
+}
+
+}  // namespace
+
 namespace device {
 
 BluetoothRemoteGattCharacteristicWin::BluetoothRemoteGattCharacteristicWin(
@@ -21,6 +35,9 @@ BluetoothRemoteGattCharacteristicWin::BluetoothRemoteGattCharacteristicWin(
       ui_task_runner_(ui_task_runner),
       characteristic_added_notified_(false),
       characteristic_value_read_or_write_in_progress_(false),
+      number_of_active_notify_sessions_(0),
+      gatt_event_registeration_in_progress_(false),
+      gatt_event_handle_(NULL),
       weak_ptr_factory_(this) {
   DCHECK(ui_task_runner_->RunsTasksOnCurrentThread());
   DCHECK(parent_service_);
@@ -41,6 +58,11 @@ BluetoothRemoteGattCharacteristicWin::BluetoothRemoteGattCharacteristicWin(
 BluetoothRemoteGattCharacteristicWin::~BluetoothRemoteGattCharacteristicWin() {
   DCHECK(ui_task_runner_->RunsTasksOnCurrentThread());
 
+  if (gatt_event_handle_ != NULL) {
+    task_manager_->UnregisterGattCharacteristicValueChangedEvent(
+        gatt_event_handle_);
+    gatt_event_handle_ = NULL;
+  }
   parent_service_->GetWinAdapter()->NotifyGattCharacteristicRemoved(this);
 }
 
@@ -105,8 +127,7 @@ BluetoothRemoteGattCharacteristicWin::GetPermissions() const {
 }
 
 bool BluetoothRemoteGattCharacteristicWin::IsNotifying() const {
-  NOTIMPLEMENTED();
-  return false;
+  return gatt_event_handle_ != NULL;
 }
 
 std::vector<BluetoothGattDescriptor*>
@@ -140,8 +161,33 @@ bool BluetoothRemoteGattCharacteristicWin::UpdateValue(
 void BluetoothRemoteGattCharacteristicWin::StartNotifySession(
     const NotifySessionCallback& callback,
     const ErrorCallback& error_callback) {
-  NOTIMPLEMENTED();
-  error_callback.Run(BluetoothGattService::GATT_ERROR_NOT_SUPPORTED);
+  DCHECK(ui_task_runner_->RunsTasksOnCurrentThread());
+
+  if (IsNotifying()) {
+    scoped_ptr<BluetoothGattNotifySessionWin> notify_session(
+        new BluetoothGattNotifySessionWin(weak_ptr_factory_.GetWeakPtr()));
+    ui_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(callback, base::Passed(std::move(notify_session))));
+    number_of_active_notify_sessions_++;
+    return;
+  }
+
+  if (gatt_event_registeration_in_progress_) {
+    start_notify_session_callbacks_.push_back(
+        std::make_pair(callback, error_callback));
+    return;
+  }
+
+  start_notify_session_callbacks_.push_back(
+      std::make_pair(callback, error_callback));
+  task_manager_->PostRegisterGattCharacteristicValueChangedEvent(
+      parent_service_->GetServicePath(), characteristic_info_.get(),
+      base::Bind(
+          &BluetoothRemoteGattCharacteristicWin::GattEventRegistrationCallback,
+          weak_ptr_factory_.GetWeakPtr()),
+      &OnGetGattEventWin, (PVOID) this);
+  gatt_event_registeration_in_progress_ = true;
 }
 
 void BluetoothRemoteGattCharacteristicWin::ReadRemoteCharacteristic(
@@ -195,6 +241,29 @@ void BluetoothRemoteGattCharacteristicWin::WriteRemoteCharacteristic(
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
+void BluetoothRemoteGattCharacteristicWin::OnGetRemoteGattEvent(
+    BTH_LE_GATT_EVENT_TYPE type,
+    PVOID event_parameter) {
+  if (type == CharacteristicValueChangedEvent) {
+    BLUETOOTH_GATT_VALUE_CHANGED_EVENT* event =
+        (BLUETOOTH_GATT_VALUE_CHANGED_EVENT*)event_parameter;
+    PBTH_LE_GATT_CHARACTERISTIC_VALUE new_value_win =
+        event->CharacteristicValue;
+    scoped_ptr<std::vector<uint8_t>> new_value(
+        new std::vector<uint8_t>(new_value_win->DataSize));
+    for (ULONG i = 0; i < new_value_win->DataSize; i++)
+      new_value->at(i) = new_value_win->Data[i];

[ortuno, 2016/03/08 00:05:36]

nit: You can just use the [] operator: new_value.get()[i]

[gogerald1, 2016/03/08 19:22:23]

Acknowledged.

[ortuno, 2016/03/09 16:53:59]

Rather than just saying "Ackowledged", you should mention the reasons as to why
you didn't follow the comment.

[gogerald1, 2016/03/10 16:10:54]

I suppose nit means optional, ->at(i) looks neat and easy understand, anyway,
I've changed it to (*new_value)[i]. 

[ortuno, 2016/03/14 01:37:37]

Yes nit means optional. But it's important you communicate why you don't do it,
that way we can both learn something :)

[scheib, 2016/03/14 17:54:46]

nit == nitpick == a small, possibly trivial, problem
(http://www.merriam-webster.com/dictionary/nit). It should not be used to imply
optional though.

Reviewers should explicitly state anything that is optional, and authors should
reply to all comments.

+
+    ui_task_runner_->PostTask(FROM_HERE,
+                              base::Bind(&BluetoothRemoteGattCharacteristicWin::
+                                             OnGattCharacteristicValueChanged,
+                                         weak_ptr_factory_.GetWeakPtr(),
+                                         base::Passed(std::move(new_value))));
+    return;
+  }
+  NOTREACHED();
+}
+
 void BluetoothRemoteGattCharacteristicWin::Update() {
   DCHECK(ui_task_runner_->RunsTasksOnCurrentThread());
 
@@ -209,6 +278,23 @@ uint16_t BluetoothRemoteGattCharacteristicWin::GetAttributeHandle() const {
   return characteristic_info_->AttributeHandle;
 }
 
+void BluetoothRemoteGattCharacteristicWin::StopNotifySession() {
+  DCHECK(ui_task_runner_->RunsTasksOnCurrentThread());
+
+  if (!IsNotifying())
+    return;
+
+  if (number_of_active_notify_sessions_ > 1) {
+    number_of_active_notify_sessions_--;
+    return;
+  }
+
+  task_manager_->UnregisterGattCharacteristicValueChangedEvent(
+      gatt_event_handle_);
+  gatt_event_handle_ = NULL;
+  number_of_active_notify_sessions_ = 0;
+}
+
 void BluetoothRemoteGattCharacteristicWin::OnGetIncludedDescriptorsCallback(
     scoped_ptr<BTH_LE_GATT_DESCRIPTOR> descriptors,
     uint16_t num,
@@ -333,9 +419,41 @@ BluetoothRemoteGattCharacteristicWin::HRESULTToGattErrorCode(HRESULT hr) {
     case ERROR_INVALID_USER_BUFFER:
     case E_BLUETOOTH_ATT_INVALID_ATTRIBUTE_VALUE_LENGTH:
       return BluetoothGattService::GATT_ERROR_INVALID_LENGTH;
+    case E_BLUETOOTH_ATT_REQUEST_NOT_SUPPORTED:
+      return BluetoothGattService::GATT_ERROR_NOT_SUPPORTED;
     default:
       return BluetoothGattService::GATT_ERROR_FAILED;
   }
 }
 
+void BluetoothRemoteGattCharacteristicWin::OnGattCharacteristicValueChanged(
+    scoped_ptr<std::vector<uint8_t>> new_value) {
+  DCHECK(ui_task_runner_->RunsTasksOnCurrentThread());
+
+  characteristic_value_.assign(new_value->begin(), new_value->end());
+  parent_service_->GetWinAdapter()->NotifyGattCharacteristicValueChanged(
+      this, characteristic_value_);
+}
+
+void BluetoothRemoteGattCharacteristicWin::GattEventRegistrationCallback(
+    BLUETOOTH_GATT_EVENT_HANDLE event_handle,
+    HRESULT hr) {
+  DCHECK(ui_task_runner_->RunsTasksOnCurrentThread());
+
+  gatt_event_registeration_in_progress_ = false;
+  std::vector<std::pair<NotifySessionCallback, ErrorCallback>> callbacks;
+  callbacks.swap(start_notify_session_callbacks_);
+  if (SUCCEEDED(hr)) {
+    gatt_event_handle_ = event_handle;
+    for (const auto& callback : callbacks) {
+      callback.first.Run(make_scoped_ptr(
+          new BluetoothGattNotifySessionWin(weak_ptr_factory_.GetWeakPtr())));
+      number_of_active_notify_sessions_++;
+    }
+  } else {
+    for (const auto& callback : callbacks)
+      callback.second.Run(HRESULTToGattErrorCode(hr));
+  }
+}
+
 }  // namespace device.