bluetooth: Make in progress errors consistent across android

device/bluetooth/bluetooth_remote_gatt_characteristic.cc

 // 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.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "device/bluetooth/bluetooth_device.h"
 #include "device/bluetooth/bluetooth_gatt_notify_session.h"
 #include "device/bluetooth/bluetooth_remote_gatt_descriptor.h"
 
 namespace device {
 
 BluetoothRemoteGattCharacteristic::BluetoothRemoteGattCharacteristic()
     : weak_ptr_factory_(this) {}
 
 BluetoothRemoteGattCharacteristic::~BluetoothRemoteGattCharacteristic() {
   while (!pending_notify_commands_.empty()) {
@@ skipping 57 matching lines @@
                  GetWeakPtr(),
                  base::Bind(error_callback,
                             BluetoothRemoteGattService::GATT_ERROR_FAILED)));
 
   pending_notify_commands_.push(std::unique_ptr<NotifySessionCommand>(command));
   if (pending_notify_commands_.size() == 1) {
     command->Execute();
   }
 }
 
+void BluetoothRemoteGattCharacteristic::ReadRemoteCharacteristic(

[scheib, 2017/04/21 23:33:10]

Consider keeping all logic in the base classes, never referring to
PendingGattOperation in the platform specific sub-classes. Ideally the sub class
___Impl methods do the minimal OS specific work to perform the task and then
fire back the callbacks.

The methods on device would check and set the pending_gatt_operation_ state, and
wrap callbacks so that when the sub-classes Run them they clear
pending_gatt_operation_.

There may be benefit from having generic methods

something roughly like:

BluetoothRemoteGattCharacteristic::ReadRemoteCharacteristic(callback,
error_callback) {
  if (ReserveGattOperationOrRunError(error_callback)) {
    ReadRemoteCharacteristicImpl(
        MakeValueCallbackAfterGattOperation(callback),
        MakeErrorCallbackAfterGattOperation(error_callback));
  }

where MakeValueCallbackAfterGattOperation come from BluetoothDevice and
generically return a callback that wraps another callback and clears
pending_gatt_operation_ before running the inner callback.

[ortuno, 2017/04/28 03:47:12]

I was exploring something like that at some point albeit only at the
characteristic level. I ended up abandoning that approach because it could
affect reentrancy and we have no tests for it. Since this is somewhat orthogonal
I'll do it in a separate CL before continuing this.

+    const ValueCallback& callback,
+    const ErrorCallback& error_callback) {
+  if (HasPendingGattOperation()) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(error_callback,
+                   BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS));
+    return;
+  }
+  ReadRemoteCharacteristicImpl(callback, error_callback);
+}
+
+void BluetoothRemoteGattCharacteristic::WriteRemoteCharacteristic(
+    const std::vector<uint8_t>& value,
+    const base::Closure& callback,
+    const ErrorCallback& error_callback) {
+  if (HasPendingGattOperation()) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(error_callback,
+                   BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS));
+    return;
+  }
+  WriteRemoteCharacteristicImpl(value, callback, error_callback);
+}
+
 void BluetoothRemoteGattCharacteristic::ExecuteStartNotifySession(
     NotifySessionCallback callback,
     ErrorCallback error_callback,
     NotifySessionCommand::Type previous_command_type,
     NotifySessionCommand::Result previous_command_result,
     BluetoothRemoteGattService::GattErrorCode previous_command_error_code) {
   // If the command that was resolved immediately before this command was run,
   // this command should be resolved with the same result.
   if (previous_command_type == NotifySessionCommand::COMMAND_START) {
     if (previous_command_result == NotifySessionCommand::RESULT_SUCCESS) {
@@ skipping 123 matching lines @@
   error_callback.Run(error);
 
   pending_notify_commands_.pop();
   if (!pending_notify_commands_.empty()) {
     pending_notify_commands_.front()->Execute(
         NotifySessionCommand::COMMAND_START, NotifySessionCommand::RESULT_ERROR,
         error);
   }
 }
 
+bool BluetoothRemoteGattCharacteristic::HasPendingGattOperation() {
+  return GetService()->GetDevice()->HasPendingGattOperation();
+}
+void BluetoothRemoteGattCharacteristic::SetPendingGattOperation(bool pending) {
+  GetService()->GetDevice()->SetPendingGattOperation(pending);
+}
+
 void BluetoothRemoteGattCharacteristic::StopNotifySession(
     BluetoothGattNotifySession* session,
     const base::Closure& callback) {
   NotifySessionCommand* command = new NotifySessionCommand(
       base::Bind(&BluetoothRemoteGattCharacteristic::ExecuteStopNotifySession,
                  GetWeakPtr(), session, callback),
       base::Bind(&BluetoothRemoteGattCharacteristic::CancelStopNotifySession,
                  GetWeakPtr(), callback));
 
   pending_notify_commands_.push(std::unique_ptr<NotifySessionCommand>(command));
@@ skipping 110 matching lines @@
 
   pending_notify_commands_.pop();
   if (!pending_notify_commands_.empty()) {
     pending_notify_commands_.front()->Execute(
         NotifySessionCommand::COMMAND_STOP, NotifySessionCommand::RESULT_ERROR,
         error);
   }
 }
 
 }  // namespace device