components/arc: implement multi advertising

components/arc/bluetooth/arc_bluetooth_bridge.h

 // 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.
 
 #ifndef COMPONENTS_ARC_BLUETOOTH_ARC_BLUETOOTH_BRIDGE_H_
 #define COMPONENTS_ARC_BLUETOOTH_ARC_BLUETOOTH_BRIDGE_H_
 
 #include <stdint.h>
 
 #include <memory>
 #include <string>
 #include <unordered_map>
 #include <vector>
 
 #include "base/callback.h"
 #include "components/arc/arc_bridge_service.h"
 #include "components/arc/arc_service.h"
 #include "components/arc/common/bluetooth.mojom.h"
 #include "components/arc/instance_holder.h"
 #include "device/bluetooth/bluetooth_adapter.h"
 #include "device/bluetooth/bluetooth_adapter_factory.h"
+#include "device/bluetooth/bluetooth_advertisement.h"
 #include "device/bluetooth/bluetooth_device.h"
 #include "device/bluetooth/bluetooth_discovery_session.h"
 #include "device/bluetooth/bluetooth_local_gatt_service.h"
 #include "device/bluetooth/bluetooth_remote_gatt_characteristic.h"
 #include "device/bluetooth/bluetooth_remote_gatt_descriptor.h"
 #include "device/bluetooth/bluetooth_remote_gatt_service.h"
 #include "device/bluetooth/bluez/bluetooth_adapter_bluez.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
 namespace arc {
@@ skipping 227 matching lines @@
   // Delete a local service.
   void DeleteService(int32_t service_handle,
                      const DeleteServiceCallback& callback) override;
   // Send value indication to a remote device.
   void SendIndication(int32_t attribute_handle,
                       mojom::BluetoothAddressPtr address,
                       bool confirm,
                       mojo::Array<uint8_t> value,
                       const SendIndicationCallback& callback) override;
 
+  // Set up or disable multiple advertising.
+  void EnableAdvertising(

[rkc, 2016/08/18 00:06:09]

These functions are very, vague. Can we add comments to clarify them?

A few questions that come up are, what exactly does EnableAdvertising do? Does
it create an advertisement? If so, this should be clearly documented. What
happens if min/max interval is specified to be different than a previous call?
(on Chrome OS we won't ever really support per advertisement intervals).

+      int32_t min_interval,
+      int32_t max_interval,
+      device::BluetoothAdvertisement::AdvertisementType adv_type,
+      int32_t channel_map,
+      int32_t tx_power,
+      int32_t timeout_s,
+      const EnableAdvertisingCallback& callback) override;
+  void SetAdvertisingData(
+      int32_t adv_handle,
+      bool include_name,
+      bool include_tx_power,
+      int32_t appearance,

[rkc, 2016/08/18 00:06:10]

Document what |appearance| means.

+      mojo::Array<mojom::BluetoothAdvertisingDataPtr> adv_data,
+      const SetAdvertisingDataCallback& callback) override;
+  void DisableAdvertising(int32_t adv_handle,

[rkc, 2016/08/18 00:06:10]

Document what this really does. Does it forget all created advertisements? Does
it just pause advertising?

+                          const DisableAdvertisingCallback& callback) override;
+
   // Chrome observer callbacks
   void OnPoweredOn(
       const base::Callback<void(mojom::BluetoothAdapterState)>& callback) const;
   void OnPoweredOff(
       const base::Callback<void(mojom::BluetoothAdapterState)>& callback) const;
   void OnPoweredError(
       const base::Callback<void(mojom::BluetoothAdapterState)>& callback) const;
   void OnDiscoveryStarted(
       std::unique_ptr<device::BluetoothDiscoverySession> session);
   void OnDiscoveryStopped();
@@ skipping 26 matching lines @@
   void OnStopLEListenError(
       const StopLEListenCallback& callback,
       device::BluetoothAdvertisement::ErrorCode error_code);
 
   void OnGattNotifyStartDone(
       const RegisterForGattNotificationCallback& callback,
       const std::string char_string_id,
       std::unique_ptr<device::BluetoothGattNotifySession> notify_session);
 
  private:
+  // Structure to do bookkeeping for advertisements registered via
+  // multi advertisement across API calls.
+  struct InstanceAdvertisement {

[rkc, 2016/08/18 00:06:10]

s/InstanceAdvertisement/AdvertisementInstance

+    bool in_use;

[Luis Héctor Chávez, 2016/08/18 04:29:13]

data members should go at the end:
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

+    device::BluetoothAdvertisement::AdvertisementType adv_type;
+    scoped_refptr<device::BluetoothAdvertisement> advertisement;
+
+    void Clear() {
+      advertisement = nullptr;
+      in_use = false;
+    }
+  };
+
   mojo::Array<mojom::BluetoothPropertyPtr> GetDeviceProperties(
       mojom::BluetoothPropertyType type,
       device::BluetoothDevice* device) const;
   mojo::Array<mojom::BluetoothPropertyPtr> GetAdapterProperties(
       mojom::BluetoothPropertyType type) const;
   mojo::Array<mojom::BluetoothAdvertisingDataPtr> GetAdvertisingData(
       device::BluetoothDevice* device) const;
 
   void SendCachedDevicesFound() const;
   bool HasBluetoothInstance() const;
@@ skipping 28 matching lines @@
 
   // Common code for OnCharacteristicWriteRequest and OnDescriptorWriteRequest
   template <class LocalGattAttribute>
   void OnGattAttributeWriteRequest(const device::BluetoothDevice* device,
                                    const LocalGattAttribute* attribute,
                                    const std::vector<uint8_t>& value,
                                    int offset,
                                    const base::Closure& success_callback,
                                    const ErrorCallback& error_callback);
 
+  // Callbacks for managing multiple advertisements
+  void OnRegisterAdvertisementDone(
+      const SetAdvertisingDataCallback& callback,
+      int32_t adv_handle,
+      scoped_refptr<device::BluetoothAdvertisement> advertisement);
+  void OnRegisterAdvertisementError(
+      const SetAdvertisingDataCallback& callback,
+      int32_t adv_handle,
+      device::BluetoothAdvertisement::ErrorCode error_code);
+  void OnUnregisterAdvertisementDone(const DisableAdvertisingCallback& callback,
+                                     int32_t adv_handle);
+  void OnUnregisterAdvertisementError(
+      const DisableAdvertisingCallback& callback,
+      int32_t adv_handle,
+      device::BluetoothAdvertisement::ErrorCode error_code);
+
   bool CalledOnValidThread();
 
   mojo::Binding<mojom::BluetoothHost> binding_;
 
   scoped_refptr<bluez::BluetoothAdapterBlueZ> bluetooth_adapter_;
   scoped_refptr<device::BluetoothAdvertisement> advertisment_;
   std::unique_ptr<device::BluetoothDiscoverySession> discovery_session_;
   std::unordered_map<std::string,
                      std::unique_ptr<device::BluetoothGattNotifySession>>
       notification_session_;
   // Map from Android int handle to Chrome (BlueZ) string identifier.
   std::unordered_map<int32_t, std::string> gatt_identifier_;
   // Map from Chrome (BlueZ) string identifier to android int handle.
   std::unordered_map<std::string, int32_t> gatt_handle_;
   // Store last GattCharacteristic added to each GattService for GattServer.
   std::unordered_map<int32_t, int32_t> last_characteristic_;
   // Monotonically increasing value to use as handle to give to Android side.
   int32_t gatt_server_attribute_next_handle_ = 0;
   // Keeps track of all devices which initiated a GATT connection to us.
   std::unordered_set<std::string> gatt_connection_cache_;
 
+  // Copied from Android at system/bt/stack/btm/btm_ble_int.h
+  // https://goo.gl/k7PM6u

[puthik_chromium, 2016/08/17 23:22:04]

This is wrong comment

+  static constexpr uint16_t kMaxAdvertisement = 5;

[rkc, 2016/08/18 00:06:10]

s/kMaxAdvertisement/kMaxAdvertisements

+  // Holds advertising data registered by the instance.
+  InstanceAdvertisement advertisements_[kMaxAdvertisement];

[rkc, 2016/08/18 00:06:09]

Instead of having this as a static array and needing an 'in_use' argument, just
make this,
std::array<uinque_ptr<InstanceAdvertisement>, kMaxAdvertisement>
advertisements_;

Then use advertisements_[handle] == nullptr as an in use check.

+
   base::ThreadChecker thread_checker_;
 
   // WeakPtrFactory to use for callbacks.
   base::WeakPtrFactory<ArcBluetoothBridge> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcBluetoothBridge);
 };
 
 }  // namespace arc
 
 #endif  // COMPONENTS_ARC_BLUETOOTH_ARC_BLUETOOTH_BRIDGE_H_