Add unit testing for more org.bluez.Error.*

chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc

Index: chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc
diff --git a/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc b/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc
index 63f6b49ffee1f52251e86e3765a6685e8c0c0164..d15426e73fac7ace31d864744db45be76e390794 100644
--- a/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc
+++ b/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc
@@ -21,6 +21,16 @@ const int kHeartRateMeasurementNotificationIntervalMs = 2000;
 
 }  // namespace
 
+FakeBluetoothGattCharacteristicClient::DelayedCallback::DelayedCallback(
+    base::Closure callback,
+    size_t delay)
+    : callback_(callback), delay_(delay) {
+}
+
+FakeBluetoothGattCharacteristicClient::DelayedCallback::~DelayedCallback() {
+  return;

[armansito, 2014/10/01 20:08:22]

Return statement necessary?

[Marie Janssen, 2014/10/01 22:22:22]

Hmm.  I swore I had issues with the Chromium about an explicit destructor until
I added it. Done.

+}
+
 // static
 const char FakeBluetoothGattCharacteristicClient::
     kHeartRateMeasurementPathComponent[] = "char0000";
@@ -68,7 +78,9 @@ void FakeBluetoothGattCharacteristicClient::Properties::Set(
 
 FakeBluetoothGattCharacteristicClient::FakeBluetoothGattCharacteristicClient()
     : heart_rate_visible_(false),
+      authorized_(true),
       calories_burned_(0),
+      extra_requests_(0),
       weak_ptr_factory_(this) {
 }
 
@@ -120,8 +132,8 @@ void FakeBluetoothGattCharacteristicClient::ReadValue(
     const dbus::ObjectPath& object_path,
     const ValueCallback& callback,
     const ErrorCallback& error_callback) {
-  if (!IsHeartRateVisible()) {
-    error_callback.Run(kUnknownCharacteristicError, "");
+  if (!authorized_) {
+    error_callback.Run("org.bluez.Error.NotAuthorized", "Authenticate first");

[armansito, 2014/10/01 20:08:22]

"Authorize first"?

[Marie Janssen, 2014/10/01 22:22:22]

Done.

     return;
   }
 
@@ -137,9 +149,35 @@ void FakeBluetoothGattCharacteristicClient::ReadValue(
     return;
   }
 
-  std::vector<uint8> value;
-  value.push_back(0x06);  // Location is "foot".
-  callback.Run(value);
+  if (action_extra_requests_.find("ReadValue") !=
+      action_extra_requests_.end()) {
+    struct DelayedCallback* delayed = action_extra_requests_["ReadValue"];

[armansito, 2014/10/01 20:08:22]

struct keyword not necessary in C++

[Marie Janssen, 2014/10/01 22:22:22]

Done.

+    delayed->delay_--;
+    error_callback.Run("org.bluez.Error.InProgress",
+                       "Another read is currenty in progress");
+    if (delayed->delay_ == 0) {
+      delayed->callback_.Run();
+      action_extra_requests_.erase("ReadValue");
+      delete delayed;
+    }
+    return;
+  }
+  base::Closure completed_callback;
+  if (!IsHeartRateVisible()) {
+    completed_callback =
+        base::Bind(error_callback, kUnknownCharacteristicError, "");
+  } else {
+    std::vector<uint8> value;
+    value.push_back(0x06);  // Location is "foot".
+    completed_callback = base::Bind(callback, value);
+  }
+
+  if (extra_requests_ > 0) {
+    action_extra_requests_["ReadValue"] =
+        new DelayedCallback(completed_callback, extra_requests_);
+    return;
+  }
+  completed_callback.Run();
 }
 
 void FakeBluetoothGattCharacteristicClient::WriteValue(
@@ -147,6 +185,11 @@ void FakeBluetoothGattCharacteristicClient::WriteValue(
     const std::vector<uint8>& value,
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
+  if (!authorized_) {
+    error_callback.Run("org.bluez.Error.NotAuthorized", "Authenticate first");
+    return;
+  }
+
   if (!IsHeartRateVisible()) {
     error_callback.Run(kUnknownCharacteristicError, "");
     return;
@@ -159,21 +202,40 @@ void FakeBluetoothGattCharacteristicClient::WriteValue(
   }
 
   DCHECK(heart_rate_control_point_properties_.get());
-  if (value.size() != 1) {
-    error_callback.Run("org.bluez.Error.InvalidValueLength",
-                       "Invalid length for write");
-    return;
-  }
-  if (value[0] > 1) {
-    error_callback.Run("org.bluez.Error.Failed",
-                       "Invalid value given for write");
+  if (action_extra_requests_.find("WriteValue") !=
+      action_extra_requests_.end()) {
+    struct DelayedCallback* delayed = action_extra_requests_["WriteValue"];
+    delayed->delay_--;
+    error_callback.Run("org.bluez.Error.InProgress",
+                       "Another write is in progress");
+    if (delayed->delay_ == 0) {
+      delayed->callback_.Run();
+      action_extra_requests_.erase("WriteValue");
+      delete delayed;
+    }
     return;
   }
-
-  if (value[0] == 1)
+  base::Closure completed_callback;
+  if (value.size() != 1) {
+    completed_callback = base::Bind(error_callback,
+                                    "org.bluez.Error.InvalidValueLength",
+                                    "Invalid length for write");
+  } else if (value[0] > 1) {
+    completed_callback = base::Bind(error_callback,
+                                    "org.bluez.Error.Failed",
+                                    "Invalid value given for write");
+  } else if (value[0] == 1) {
+    // TODO(jamuraa): make this happen when the callback happens
     calories_burned_ = 0;
+    completed_callback = callback;
+  }
 
-  callback.Run();
+  if (extra_requests_ > 0) {
+    action_extra_requests_["WriteValue"] =
+        new DelayedCallback(completed_callback, extra_requests_);
+    return;
+  }
+  completed_callback.Run();
 }
 
 void FakeBluetoothGattCharacteristicClient::StartNotify(
@@ -332,6 +394,22 @@ void FakeBluetoothGattCharacteristicClient::HideHeartRateCharacteristics() {
   heart_rate_visible_ = false;
 }
 
+void FakeBluetoothGattCharacteristicClient::SetExtraProcessing(
+    size_t requests) {
+  extra_requests_ = requests;
+  if (extra_requests_ == 0) {
+    for (const auto& it : action_extra_requests_) {

[armansito, 2014/10/01 20:08:22]

Nice, I guess we should start adopting these C++11 idioms. That said, we should
try to make the rest of the code consistent with this, so maybe add a TODO to
use foreach loops elsewhere in this file?

[Marie Janssen, 2014/10/01 22:22:22]

This is the only foreach loop in this file.

+      it.second->callback_.Run();
+      delete it.second;
+    }
+    action_extra_requests_.clear();
+  }
+}
+
+size_t FakeBluetoothGattCharacteristicClient::GetExtraProcessing() const {
+  return extra_requests_;
+}
+
 dbus::ObjectPath
 FakeBluetoothGattCharacteristicClient::GetHeartRateMeasurementPath() const {
   return dbus::ObjectPath(heart_rate_measurement_path_);