From 57a7a7d68663b364ff5ba61d0bd34289041da9ee Mon Sep 17 00:00:00 2001
From: Michal Bistyga <m.bistyga@samsung.com>
Date: Mon, 27 Nov 2017 14:20:25 +0100
Subject: [PATCH] [Bluetooth] Fix bug with ConnectStateChangeListener

ConnectStateChangeListener's onConnected callback has been called even when connection failed.

[Verification] 1) automatic bluetooth tests have pass rate 100%
2) All manual tests related to add*/removeConnectStateChangeListener functions passed.

Change-Id: Ic765cc6710c64f82132c96f4b76edcd62bc69305
Signed-off-by: Michal Bistyga <m.bistyga@samsung.com>
Signed-off-by: Piotr Kosko <p.kosko@samsung.com>
---
 src/bluetooth/bluetooth_le_device.cc | 84 ++++++++++++++++++----------
 src/bluetooth/bluetooth_le_device.h  |  6 +-
 2 files changed, 59 insertions(+), 31 deletions(-)

diff --git a/src/bluetooth/bluetooth_le_device.cc b/src/bluetooth/bluetooth_le_device.cc
index c10ebc81..b0f6413a 100644
--- a/src/bluetooth/bluetooth_le_device.cc
+++ b/src/bluetooth/bluetooth_le_device.cc
@@ -426,22 +426,63 @@ void BluetoothLEDevice::GetServiceAllUuids(const picojson::value& data, picojson
 void BluetoothLEDevice::GattConnectionState(int result, bool connected, const char* remote_address,
                                             void* user_data) {
   ScopeLogger("%s connected: %d", remote_address, connected);
-  auto le_device = static_cast<BluetoothLEDevice*>(user_data);
-
-  if (!le_device) {
+  if (!user_data) {
     LoggerE("user_data is NULL");
     return;
   }
+  BluetoothLEDevice* le_device = static_cast<BluetoothLEDevice*>(user_data);
+
+  // check state change success
+  PlatformResult operation_success = ValidateConnectionChange(result);
+  if (operation_success) {
+    if (connected) { // connect success
+      le_device->is_connected_.insert(remote_address);
+    } else { // disconnect success
+      le_device->CleanClientInfo(remote_address);
+    }
+  }
 
-  if (connected) {
-    le_device->is_connected_.insert(remote_address);
+  // trigger connect/disconnect callback (success or error)
+  le_device->TriggerConnectCallback(remote_address, operation_success);
+
+  // trigger state change listener, if operation succeed
+  if (operation_success) {
+    le_device->TriggerConnectStateChangeListener(remote_address, connected);
+  }
+}
+
+PlatformResult BluetoothLEDevice::ValidateConnectionChange(int err_code) {
+  ScopeLogger();
+  if (BT_ERROR_NONE != err_code) {
+    return LogAndCreateResult(
+        ErrorCode::UNKNOWN_ERR, "Failed to get connection state",
+        ("GattConnectionState error: %d (%s)", err_code, get_error_message(err_code)));
+  }
+  return PlatformResult(ErrorCode::NO_ERROR);
+}
+
+void BluetoothLEDevice::CleanClientInfo(const char* remote_address) {
+  ScopeLogger();
+  is_connected_.erase(remote_address);
+  // inform that this device is not connected anymore
+  service_.TryDestroyClient(remote_address);
+}
+
+void BluetoothLEDevice::TriggerConnectCallback(const char* remote_address, PlatformResult result) {
+  ScopeLogger();
+  auto it = connecting_.find(remote_address);
+  if (connecting_.end() != it) {
+    instance_.AsyncResponse(it->second, result);
+    connecting_.erase(it);
   } else {
-    le_device->is_connected_.erase(remote_address);
-    // inform that this device is not connected anymore
-    le_device->service_.TryDestroyClient(remote_address);
+    LoggerW("Given address is not in waiting connections list");
   }
+}
 
-  if (le_device->is_listener_set_) {
+void BluetoothLEDevice::TriggerConnectStateChangeListener(const char* remote_address,
+                                                          bool connected) {
+  ScopeLogger();
+  if (is_listener_set_) {
     picojson::value value = picojson::value(picojson::object());
     picojson::object* data_obj = &value.get<picojson::object>();
     if (connected) {
@@ -451,29 +492,12 @@ void BluetoothLEDevice::GattConnectionState(int result, bool connected, const ch
       LoggerD("OnDisconnected");
       data_obj->insert(std::make_pair(kAction, picojson::value(kOnDisconnected)));
     }
-
     data_obj->insert(std::make_pair(kDeviceAddress, picojson::value(remote_address)));
-
-    le_device->instance_.FireEvent(kConnectChangeEvent, value);
-  }
-
-  auto it = le_device->connecting_.find(remote_address);
-  if (le_device->connecting_.end() == it) {
-    LoggerW("Given address is not in waiting connections list");
-    return;
-  }
-
-  PlatformResult ret = PlatformResult(ErrorCode::NO_ERROR);
-  if (BT_ERROR_NONE != result) {
-    ret = LogAndCreateResult(
-        ErrorCode::UNKNOWN_ERR, "Failed to get connection state",
-        ("GattConnectionState error: %d (%s)", result, get_error_message(result)));
+    instance_.FireEvent(kConnectChangeEvent, value);
+  } else {
+    LoggerD("Listener is not registered - ignoring event");
   }
-
-  le_device->instance_.AsyncResponse(it->second, ret);
-
-  le_device->connecting_.erase(it);
-}
+};
 
 }  // namespace bluetooth
 }  // namespace extension
diff --git a/src/bluetooth/bluetooth_le_device.h b/src/bluetooth/bluetooth_le_device.h
index 2715bcf1..e2a161ae 100644
--- a/src/bluetooth/bluetooth_le_device.h
+++ b/src/bluetooth/bluetooth_le_device.h
@@ -53,7 +53,11 @@ class BluetoothLEDevice {
  private:
   static void GattConnectionState(int result, bool connected, const char* remote_address,
                                   void* user_data);
-
+  bool IsInConnecting(const char* remote_address);
+  static common::PlatformResult ValidateConnectionChange(int err_code);
+  void TriggerConnectCallback(const char* remote_address, common::PlatformResult result);
+  void CleanClientInfo(const char* remote_address);
+  void TriggerConnectStateChangeListener(const char* remote_address, bool connected);
   BluetoothInstance& instance_;
   BluetoothGATTService& service_;
   std::unordered_map<std::string, double> connecting_;
-- 
2.34.1