service: Fix IBluetoothLowEnergy signatures to return bool

Changed the IBluetoothLowEnergy API signatures that can
synchronously fail to return bool to report synchronous errors. Also
fixed a bug with lambda capture by value in advertising API
implementation.

Bug: 24245347
Change-Id: I9ce4bb44d155d74ba38916e2834d7e93bf49301c

6 files changed, 84 insertions, 60 deletions

diff --git a/system/service/client/main.cpp b/system/service/client/main.cpp
index 52dd181429..617d8f909d 100644
--- a/system/service/client/main.cpp
+++ b/system/service/client/main.cpp
@@ -17,6 +17,8 @@
 #include <iostream>
 #include <string>
 
+#include <base/at_exit.h>
+#include <base/command_line.h>
 #include <base/logging.h>
 #include <base/macros.h>
 #include <base/strings/string_split.h>
@@ -256,9 +258,9 @@ void HandleRegisterBLE(IBluetooth* bt_iface, const vector<string>& args) {
     return;
   }
 
-  ble_iface->RegisterClient(new CLIBluetoothLowEnergyCallback());
-  ble_registering = true;
-  PrintCommandStatus(true);
+  bool status = ble_iface->RegisterClient(new CLIBluetoothLowEnergyCallback());
+  ble_registering = status;
+  PrintCommandStatus(status);
 }
 
 void HandleUnregisterBLE(IBluetooth* bt_iface, const vector<string>& args) {
@@ -361,8 +363,9 @@ void HandleStartAdv(IBluetooth* bt_iface, const vector<string>& args) {
 
   bluetooth::AdvertiseData scan_rsp;
 
-  ble_iface->StartMultiAdvertising(ble_client_if.load(),
-                                   adv_data, scan_rsp, settings);
+  bool status = ble_iface->StartMultiAdvertising(ble_client_if.load(),
+                                                 adv_data, scan_rsp, settings);
+  PrintCommandStatus(status);
 }
 
 void HandleStopAdv(IBluetooth* bt_iface, const vector<string>& args) {
@@ -377,7 +380,8 @@ void HandleStopAdv(IBluetooth* bt_iface, const vector<string>& args) {
     return;
   }
 
-  ble_iface->StopMultiAdvertising(ble_client_if.load());
+  bool status = ble_iface->StopMultiAdvertising(ble_client_if.load());
+  PrintCommandStatus(status);
 }
 
 void HandleHelp(IBluetooth* bt_iface, const vector<string>& args);
@@ -441,7 +445,16 @@ class BluetoothDeathRecipient : public android::IBinder::DeathRecipient {
   DISALLOW_COPY_AND_ASSIGN(BluetoothDeathRecipient);
 };
 
-int main() {
+int main(int argc, char* argv[]) {
+  base::AtExitManager exit_manager;
+  base::CommandLine::Init(argc, argv);
+  logging::LoggingSettings log_settings;
+
+  if (!logging::InitLogging(log_settings)) {
+    LOG(ERROR) << "Failed to set up logging";
+    return EXIT_FAILURE;
+  }
+
   sp<IBluetooth> bt_iface = IBluetooth::getClientInterface();
   if (!bt_iface.get()) {
     LOG(ERROR) << "Failed to obtain handle on IBluetooth";
diff --git a/system/service/ipc/binder/IBluetoothLowEnergy.cpp b/system/service/ipc/binder/IBluetoothLowEnergy.cpp
index e9c0a43d14..1060170213 100644
--- a/system/service/ipc/binder/IBluetoothLowEnergy.cpp
+++ b/system/service/ipc/binder/IBluetoothLowEnergy.cpp
@@ -49,7 +49,11 @@ status_t BnBluetoothLowEnergy::onTransact(
   switch (code) {
   case REGISTER_CLIENT_TRANSACTION: {
     sp<IBinder> callback = data.readStrongBinder();
-    RegisterClient(interface_cast<IBluetoothLowEnergyCallback>(callback));
+    bool result = RegisterClient(
+        interface_cast<IBluetoothLowEnergyCallback>(callback));
+
+    reply->writeInt32(result);
+
     return android::NO_ERROR;
   }
   case UNREGISTER_CLIENT_TRANSACTION: {
@@ -70,14 +74,18 @@ status_t BnBluetoothLowEnergy::onTransact(
     std::unique_ptr<AdvertiseSettings> adv_settings =
         CreateAdvertiseSettingsFromParcel(data);
 
-    StartMultiAdvertising(client_if, *adv_data, *scan_rsp, *adv_settings);
+    bool result = StartMultiAdvertising(
+        client_if, *adv_data, *scan_rsp, *adv_settings);
+
+    reply->writeInt32(result);
 
     return android::NO_ERROR;
   }
   case STOP_MULTI_ADVERTISING_TRANSACTION: {
     int client_if = data.readInt32();
+    bool result = StopMultiAdvertising(client_if);
 
-    StopMultiAdvertising(client_if);
+    reply->writeInt32(result);
 
     return android::NO_ERROR;
   }
@@ -93,7 +101,7 @@ BpBluetoothLowEnergy::BpBluetoothLowEnergy(const sp<IBinder>& impl)
     : BpInterface<IBluetoothLowEnergy>(impl) {
 }
 
-void BpBluetoothLowEnergy::RegisterClient(
+bool BpBluetoothLowEnergy::RegisterClient(
       const sp<IBluetoothLowEnergyCallback>& callback) {
   Parcel data, reply;
 
@@ -102,6 +110,8 @@ void BpBluetoothLowEnergy::RegisterClient(
 
   remote()->transact(IBluetoothLowEnergy::REGISTER_CLIENT_TRANSACTION,
                      data, &reply);
+
+  return reply.readInt32();
 }
 
 void BpBluetoothLowEnergy::UnregisterClient(int client_if) {
@@ -123,7 +133,7 @@ void BpBluetoothLowEnergy::UnregisterAll() {
                      data, &reply);
 }
 
-void BpBluetoothLowEnergy::StartMultiAdvertising(
+bool BpBluetoothLowEnergy::StartMultiAdvertising(
     int client_if,
     const AdvertiseData& advertise_data,
     const AdvertiseData& scan_response,
@@ -138,9 +148,11 @@ void BpBluetoothLowEnergy::StartMultiAdvertising(
 
   remote()->transact(IBluetoothLowEnergy::START_MULTI_ADVERTISING_TRANSACTION,
                      data, &reply);
+
+  return reply.readInt32();
 }
 
-void BpBluetoothLowEnergy::StopMultiAdvertising(int client_if) {
+bool BpBluetoothLowEnergy::StopMultiAdvertising(int client_if) {
   Parcel data, reply;
 
   data.writeInterfaceToken(IBluetoothLowEnergy::getInterfaceDescriptor());
@@ -148,6 +160,8 @@ void BpBluetoothLowEnergy::StopMultiAdvertising(int client_if) {
 
   remote()->transact(IBluetoothLowEnergy::STOP_MULTI_ADVERTISING_TRANSACTION,
                      data, &reply);
+
+  return reply.readInt32();
 }
 
 IMPLEMENT_META_INTERFACE(BluetoothLowEnergy, IBluetoothLowEnergy::kServiceName);
diff --git a/system/service/ipc/binder/IBluetoothLowEnergy.h b/system/service/ipc/binder/IBluetoothLowEnergy.h
index 8ba8564b1d..cd9d3027e1 100644
--- a/system/service/ipc/binder/IBluetoothLowEnergy.h
+++ b/system/service/ipc/binder/IBluetoothLowEnergy.h
@@ -65,17 +65,17 @@ class IBluetoothLowEnergy : public android::IInterface {
     NUM_HW_TRACK_FILTERS_AVAILABLE,
   };
 
-  virtual void RegisterClient(
+  virtual bool RegisterClient(
       const android::sp<IBluetoothLowEnergyCallback>& callback) = 0;
   virtual void UnregisterClient(int client_if) = 0;
   virtual void UnregisterAll() = 0;
 
-  virtual void StartMultiAdvertising(
+  virtual bool StartMultiAdvertising(
       int client_if,
       const bluetooth::AdvertiseData& advertise_data,
       const bluetooth::AdvertiseData& scan_response,
       const bluetooth::AdvertiseSettings& settings) = 0;
-  virtual void StopMultiAdvertising(int client_if) = 0;
+  virtual bool StopMultiAdvertising(int client_if) = 0;
 
   // TODO(armansito): Complete the API definition.
 
@@ -107,16 +107,16 @@ class BpBluetoothLowEnergy : public android::BpInterface<IBluetoothLowEnergy> {
   virtual ~BpBluetoothLowEnergy() = default;
 
   // IBluetoothLowEnergy overrides:
-  void RegisterClient(
+  bool RegisterClient(
       const android::sp<IBluetoothLowEnergyCallback>& callback) override;
   void UnregisterClient(int client_if) override;
   void UnregisterAll() override;
-  void StartMultiAdvertising(
+  bool StartMultiAdvertising(
       int client_if,
       const bluetooth::AdvertiseData& advertise_data,
       const bluetooth::AdvertiseData& scan_response,
       const bluetooth::AdvertiseSettings& settings) override;
-  void StopMultiAdvertising(int client_if) override;
+  bool StopMultiAdvertising(int client_if) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(BpBluetoothLowEnergy);
diff --git a/system/service/ipc/binder/bluetooth_low_energy_binder_server.cpp b/system/service/ipc/binder/bluetooth_low_energy_binder_server.cpp
index 95f271213d..6cc8d1087b 100644
--- a/system/service/ipc/binder/bluetooth_low_energy_binder_server.cpp
+++ b/system/service/ipc/binder/bluetooth_low_energy_binder_server.cpp
@@ -31,13 +31,13 @@ BluetoothLowEnergyBinderServer::BluetoothLowEnergyBinderServer(
 BluetoothLowEnergyBinderServer::~BluetoothLowEnergyBinderServer() {
 }
 
-void BluetoothLowEnergyBinderServer::RegisterClient(
+bool BluetoothLowEnergyBinderServer::RegisterClient(
     const android::sp<IBluetoothLowEnergyCallback>& callback) {
   VLOG(2) << __func__;
   bluetooth::LowEnergyClientFactory* ble_factory =
       adapter_->GetLowEnergyClientFactory();
 
-  RegisterClientBase(callback, ble_factory);
+  return RegisterClientBase(callback, ble_factory);
 }
 
 void BluetoothLowEnergyBinderServer::UnregisterClient(int client_if) {
@@ -50,31 +50,25 @@ void BluetoothLowEnergyBinderServer::UnregisterAll() {
   UnregisterAllBase();
 }
 
-void BluetoothLowEnergyBinderServer::StartMultiAdvertising(
+bool BluetoothLowEnergyBinderServer::StartMultiAdvertising(
     int client_if,
     const bluetooth::AdvertiseData& advertise_data,
     const bluetooth::AdvertiseData& scan_response,
     const bluetooth::AdvertiseSettings& settings) {
-  VLOG(2) << __func__;
+  VLOG(2) << __func__ << " client_if: " << client_if;
   std::lock_guard<std::mutex> lock(*maps_lock());
 
   auto client = GetLEClient(client_if);
   if (!client) {
     LOG(ERROR) << "Unknown client_if: " << client_if;
-    return;
-  }
-
-  auto cb = GetLECallback(client_if);
-  if (!cb.get()) {
-    LOG(ERROR) << "Client was unregistered - client_if: " << client_if;
-    return;
+    return false;
   }
 
   // Create a weak pointer and pass that to the callback to prevent a potential
   // use after free.
-  bluetooth::AdvertiseSettings settings_copy = settings;
   android::wp<BluetoothLowEnergyBinderServer> weak_ptr_to_this(this);
-  auto callback = [&](bluetooth::BLEStatus status) {
+  auto settings_copy = settings;
+  auto callback = [=](bluetooth::BLEStatus status) {
     auto sp_to_this = weak_ptr_to_this.promote();
     if (!sp_to_this.get()) {
       VLOG(2) << "BluetoothLowEnergyBinderServer was deleted";
@@ -83,60 +77,62 @@ void BluetoothLowEnergyBinderServer::StartMultiAdvertising(
 
     std::lock_guard<std::mutex> lock(*maps_lock());
 
+    auto cb = GetLECallback(client_if);
+    if (!cb.get()) {
+      VLOG(1) << "Client was removed before callback: " << client_if;
+      return;
+    }
+
     cb->OnMultiAdvertiseCallback(status, true /* is_start */, settings_copy);
   };
 
-  if (client->StartAdvertising(
-      settings, advertise_data, scan_response, callback))
-    return;
-
-  LOG(ERROR) << "Failed to initiate call to start advertising";
+  if (!client->StartAdvertising(
+      settings, advertise_data, scan_response, callback)) {
+    LOG(ERROR) << "Failed to initiate call to start advertising";
+    return false;
+  }
 
-  // Notify error in oneway callback.
-  cb->OnMultiAdvertiseCallback(
-      bluetooth::BLE_STATUS_FAILURE, true /* is_start */, settings_copy);
+  return true;
 }
 
-void BluetoothLowEnergyBinderServer::StopMultiAdvertising(int client_if) {
+bool BluetoothLowEnergyBinderServer::StopMultiAdvertising(int client_if) {
   VLOG(2) << __func__;
   std::lock_guard<std::mutex> lock(*maps_lock());
 
   auto client = GetLEClient(client_if);
   if (!client) {
     LOG(ERROR) << "Unknown client_if: " << client_if;
-    return;
-  }
-
-  auto cb = GetLECallback(client_if);
-  if (!cb.get()) {
-    LOG(ERROR) << "Client was unregistered - client_if: " << client_if;
-    return;
+    return false;
   }
 
   // Create a weak pointer and pass that to the callback to prevent a potential
   // use after free.
   android::wp<BluetoothLowEnergyBinderServer> weak_ptr_to_this(this);
-  bluetooth::AdvertiseSettings settings_copy = client->settings();
-  auto callback = [&](bluetooth::BLEStatus status) {
+  auto settings_copy = client->settings();
+  auto callback = [=](bluetooth::BLEStatus status) {
     auto sp_to_this = weak_ptr_to_this.promote();
     if (!sp_to_this.get()) {
       VLOG(2) << "BluetoothLowEnergyBinderServer was deleted";
       return;
     }
 
+    auto cb = GetLECallback(client_if);
+    if (!cb.get()) {
+      VLOG(2) << "Client was unregistered - client_if: " << client_if;
+      return;
+    }
+
     std::lock_guard<std::mutex> lock(*maps_lock());
 
     cb->OnMultiAdvertiseCallback(status, false /* is_start */, settings_copy);
   };
 
-  if (client->StopAdvertising(callback))
-    return;
-
-  LOG(ERROR) << "Failed to initiate call to start advertising";
+  if (!client->StopAdvertising(callback)) {
+    LOG(ERROR) << "Failed to initiate call to start advertising";
+    return false;
+  }
 
-  // Notify error in oneway callback.
-  cb->OnMultiAdvertiseCallback(
-      bluetooth::BLE_STATUS_FAILURE, false/* is_start */, settings_copy);
+  return true;
 }
 
 android::sp<IBluetoothLowEnergyCallback>
diff --git a/system/service/ipc/binder/bluetooth_low_energy_binder_server.h b/system/service/ipc/binder/bluetooth_low_energy_binder_server.h
index e491a9dcd1..7cb7fb257e 100644
--- a/system/service/ipc/binder/bluetooth_low_energy_binder_server.h
+++ b/system/service/ipc/binder/bluetooth_low_energy_binder_server.h
@@ -41,16 +41,16 @@ class BluetoothLowEnergyBinderServer : public BnBluetoothLowEnergy,
   ~BluetoothLowEnergyBinderServer() override;
 
   // IBluetoothLowEnergy overrides:
-  void RegisterClient(
+  bool RegisterClient(
       const android::sp<IBluetoothLowEnergyCallback>& callback) override;
   void UnregisterClient(int client_if) override;
   void UnregisterAll() override;
-  void StartMultiAdvertising(
+  bool StartMultiAdvertising(
       int client_if,
       const bluetooth::AdvertiseData& advertise_data,
       const bluetooth::AdvertiseData& scan_response,
       const bluetooth::AdvertiseSettings& settings) override;
-  void StopMultiAdvertising(int client_if) override;
+  bool StopMultiAdvertising(int client_if) override;
 
  private:
   // Returns a pointer to the IBluetoothLowEnergyCallback instance associated
diff --git a/system/service/main.cpp b/system/service/main.cpp
index 93eb25fc09..90b7c9a1fb 100644
--- a/system/service/main.cpp
+++ b/system/service/main.cpp
@@ -13,6 +13,7 @@
 //  See the License for the specific language governing permissions and
 //  limitations under the License.
 //
+
 #include <base/at_exit.h>
 #include <base/command_line.h>
 #include <base/files/scoped_file.h>