Bluetooth: macOS: Replacing BluetoothRemoteGattCharacteristicMac::StartNotifySession() by Bluetooth…

device/bluetooth/bluetooth_remote_gatt_characteristic_mac.mm

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "device/bluetooth/bluetooth_remote_gatt_characteristic_mac.h"
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ skipping 53 matching lines @@
   }
   return result;
 }
 }  // namespace
 
 BluetoothRemoteGattCharacteristicMac::BluetoothRemoteGattCharacteristicMac(
     BluetoothRemoteGattServiceMac* gatt_service,
     CBCharacteristic* cb_characteristic)
     : gatt_service_(gatt_service),
       cb_characteristic_(cb_characteristic, base::scoped_policy::RETAIN),
-      characteristic_value_read_or_write_in_progress_(false),
-      start_notifications_in_progress_(false),
-      weak_ptr_factory_(this) {
+      characteristic_value_read_or_write_in_progress_(false) {
   uuid_ = BluetoothAdapterMac::BluetoothUUIDWithCBUUID(
       [cb_characteristic_.get() UUID]);
   identifier_ = base::SysNSStringToUTF8(
       [NSString stringWithFormat:@"%s-%p", uuid_.canonical_value().c_str(),
                                  (void*)cb_characteristic_]);
 }
 
 BluetoothRemoteGattCharacteristicMac::~BluetoothRemoteGattCharacteristicMac() {
   if (!read_characteristic_value_callbacks_.first.is_null()) {
     std::pair<ValueCallback, ErrorCallback> callbacks;
     callbacks.swap(read_characteristic_value_callbacks_);
     callbacks.second.Run(BluetoothGattService::GATT_ERROR_FAILED);
   }
   if (!write_characteristic_value_callbacks_.first.is_null()) {
     std::pair<base::Closure, ErrorCallback> callbacks;
     callbacks.swap(write_characteristic_value_callbacks_);
     callbacks.second.Run(BluetoothGattService::GATT_ERROR_FAILED);
   }
+  if (!subscribe_to_notification_callback_.second.is_null()) {

[ortuno, 2017/01/18 01:22:02]

Do any tests fail if you remove this? I think the code in
~BluetoothRemoteGattCharacteristic should handle running the error callback.

[jlebel, 2017/01/18 14:42:32]

This code can be removed without breaking unit tests.

+    PendingNotifyCallback callbacks;
+    callbacks.swap(subscribe_to_notification_callback_);
+    callbacks.second.Run(BluetoothGattService::GATT_ERROR_FAILED);
+  }
 }
 
 std::string BluetoothRemoteGattCharacteristicMac::GetIdentifier() const {
   return identifier_;
 }
 
 BluetoothUUID BluetoothRemoteGattCharacteristicMac::GetUUID() const {
   return uuid_;
 }
 
@@ skipping 38 matching lines @@
 BluetoothRemoteGattCharacteristicMac::GetDescriptor(
     const std::string& identifier) const {
   auto searched_pair = gatt_descriptor_macs_.find(identifier);
   if (searched_pair == gatt_descriptor_macs_.end()) {
     return nullptr;
   }
   return static_cast<BluetoothRemoteGattDescriptor*>(
       searched_pair->second.get());
 }
 
-void BluetoothRemoteGattCharacteristicMac::StartNotifySession(
-    const NotifySessionCallback& callback,
-    const ErrorCallback& error_callback) {
-  if (IsNotifying()) {
-    VLOG(2) << "Already notifying. Creating notify session.";
-    std::unique_ptr<BluetoothGattNotifySession> notify_session(
-        new BluetoothGattNotifySession(weak_ptr_factory_.GetWeakPtr()));
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::Bind(callback, base::Passed(std::move(notify_session))));
-    return;
-  }
-
-  if (!SupportsNotificationsOrIndications()) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::Bind(error_callback,
-                   BluetoothRemoteGattService::GATT_ERROR_NOT_SUPPORTED));
-    return;
-  }
-
-  start_notify_session_callbacks_.push_back(
-      std::make_pair(callback, error_callback));
-
-  if (start_notifications_in_progress_) {
-    VLOG(2) << "Start Notifications already in progress. "
-            << "Request has been queued.";
-    return;
-  }
-
-  [GetCBPeripheral() setNotifyValue:YES
-                  forCharacteristic:cb_characteristic_.get()];
-  start_notifications_in_progress_ = true;
-}
-
-void BluetoothRemoteGattCharacteristicMac::StopNotifySession(
-    BluetoothGattNotifySession* session,
-    const base::Closure& callback) {
-  // TODO(http://crbug.com/633191): Remove this method and use the base version.
-  //   Instead, we should implement SubscribeToNotifications and
-  //   UnsubscribeFromNotifications.
-  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, callback);
-}
-
 void BluetoothRemoteGattCharacteristicMac::ReadRemoteCharacteristic(
     const ValueCallback& callback,
     const ErrorCallback& error_callback) {
   if (!IsReadable()) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(error_callback,
                    BluetoothRemoteGattService::GATT_ERROR_NOT_SUPPORTED));
     return;
   }
@@ skipping 42 matching lines @@
         FROM_HERE,
         base::Bind(&BluetoothRemoteGattCharacteristicMac::DidWriteValue,
                    base::Unretained(this), nil));
   }
 }
 
 void BluetoothRemoteGattCharacteristicMac::SubscribeToNotifications(
     BluetoothRemoteGattDescriptor* ccc_descriptor,
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
-  // TODO(http://crbug.com/633191): Implement this method
-  NOTIMPLEMENTED();
+  DCHECK(subscribe_to_notification_callback_.first.is_null());
+  DCHECK(subscribe_to_notification_callback_.second.is_null());
+  subscribe_to_notification_callback_ =
+      std::make_pair(callback, error_callback);
+  [GetCBPeripheral() setNotifyValue:YES
+                  forCharacteristic:cb_characteristic_.get()];
 }
 
 void BluetoothRemoteGattCharacteristicMac::UnsubscribeFromNotifications(
     BluetoothRemoteGattDescriptor* ccc_descriptor,
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
   // TODO(http://crbug.com/633191): Implement this method
   NOTIMPLEMENTED();
 }
 
@@ skipping 58 matching lines @@
     BluetoothGattService::GattErrorCode error_code =
         BluetoothDeviceMac::GetGattErrorCodeFromNSError(error);
     callbacks.second.Run(error_code);
     return;
   }
   callbacks.first.Run();
 }
 
 void BluetoothRemoteGattCharacteristicMac::DidUpdateNotificationState(
     NSError* error) {
-  std::vector<std::pair<NotifySessionCallback, ErrorCallback>>
-      reentrant_safe_callbacks;
-  reentrant_safe_callbacks.swap(start_notify_session_callbacks_);
-  start_notifications_in_progress_ = false;
+  PendingNotifyCallback reentrant_safe_callbacks;
+  reentrant_safe_callbacks.swap(subscribe_to_notification_callback_);
   if (error) {
     VLOG(1) << "Bluetooth error while modifying notification state for "
                "characteristic, domain: "
             << base::SysNSStringToUTF8(error.domain)
             << ", error code: " << error.code << ", localized description: "
             << base::SysNSStringToUTF8(error.localizedDescription);
     BluetoothGattService::GattErrorCode error_code =
         BluetoothDeviceMac::GetGattErrorCodeFromNSError(error);
-    for (const auto& callback : reentrant_safe_callbacks) {
-      callback.second.Run(error_code);
+    if (!reentrant_safe_callbacks.second.is_null()) {
+      reentrant_safe_callbacks.second.Run(error_code);
     }
     return;
   }
-  for (const auto& callback : reentrant_safe_callbacks) {
-    callback.first.Run(base::MakeUnique<BluetoothGattNotifySession>(
-        weak_ptr_factory_.GetWeakPtr()));
+  if (!reentrant_safe_callbacks.first.is_null()) {
+    reentrant_safe_callbacks.first.Run();
   }
 }
 
 void BluetoothRemoteGattCharacteristicMac::DidDiscoverDescriptors() {
   DCHECK(!is_discovery_complete_);
   std::unordered_set<std::string> descriptor_identifier_to_remove;
   for (const auto& iter : gatt_descriptor_macs_) {
     descriptor_identifier_to_remove.insert(iter.first);
   }
 
@@ skipping 76 matching lines @@
               pair) {
         return pair.second->GetCBDescriptor() == cb_descriptor;
       });
   if (found == gatt_descriptor_macs_.end()) {
     return nullptr;
   } else {
     return found->second.get();
   }
 }
 }  // namespace device.