[chromecast][bt] Refactor LE and GATT interfaces

Adds some of the methods previously defined only in the
LeScanManagerImpl and GattClientManagerImpl as part of the abstract
interfaces. Making this change helps keeping the Bluetooth logic
consistent between Cast and Fuchsia platforms.

Bug: b/161948793
Test: bluetooth unittests
Change-Id: I24f4de56bcc6c564cfdf2d6a8ed9bca899fb9b4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2333567Reviewed-by: Luke Halliwell (slow) <halliwell@chromium.org>
Commit-Queue: Marzena Dell'Aquila <mdellaquila@google.com>
Cr-Commit-Position: refs/heads/master@{#794307}

chromecast/device/bluetooth/le/gatt_client_manager.h

@@ -6,18 +6,29 @@
 #define CHROMECAST_DEVICE_BLUETOOTH_LE_GATT_CLIENT_MANAGER_H_
 
 #include <map>
+#include <memory>
 #include <set>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/single_thread_task_runner.h"
 #include "chromecast/public/bluetooth/bluetooth_types.h"
 
+namespace base {
+class SingleThreadTaskRunner;
+}  // namespace base
+
 namespace chromecast {
+namespace bluetooth_v2_shlib {
+class GattClient;
+}  // namespace bluetooth_v2_shlib
+
 namespace bluetooth {
 
+class BluetoothManagerPlatform;
+class LeScanManager;
 class RemoteCharacteristic;
 class RemoteDevice;
 class RemoteService;
@@ -55,6 +66,17 @@ class GattClientManager {
     virtual ~Observer() = default;
   };
 
+  static std::unique_ptr<GattClientManager> Create(
+      bluetooth_v2_shlib::GattClient* gatt_client,
+      BluetoothManagerPlatform* bluetooth_manager,
+      LeScanManager* le_scan_manager);
+
+  virtual ~GattClientManager() = default;
+
+  virtual void Initialize(
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) = 0;
+  virtual void Finalize() = 0;
+
   virtual void AddObserver(Observer* o) = 0;
   virtual void RemoveObserver(Observer* o) = 0;
 
@@ -87,13 +109,27 @@ class GattClientManager {
   // Note that these devices might not be connected.
   virtual void NotifyBonded(const bluetooth_v2_shlib::Addr& addr) = 0;
 
+  // Returns true if |addr| corresponds to a connected BLE device.
+  virtual bool IsConnectedLeDevice(const bluetooth_v2_shlib::Addr& addr) = 0;
+
+  // Enable or disable GATT client connectability. Returns |true| if successful
+  // otherwise |false|.
+  virtual bool SetGattClientConnectable(bool connectable) = 0;
+
+  // Disconnect all connected devices. Callback will return |true| if all
+  // devices are disconnected, otherwise false.
+  // When disabling GATT client, caller should call
+  // SetGattClientConnectable(false) before calling DisconnectAll so that
+  // upcoming GATT client connections can also be blocked.
+  using StatusCallback = base::OnceCallback<void(bool)>;
+  virtual void DisconnectAll(StatusCallback cb) = 0;
+
   // TODO(bcf): Deprecated. Should be removed now that this class may be used
   // from any thread.
   virtual scoped_refptr<base::SingleThreadTaskRunner> task_runner() = 0;
 
  protected:
   GattClientManager() = default;
-  virtual ~GattClientManager() = default;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(GattClientManager);

chromecast/device/bluetooth/le/gatt_client_manager_impl.cc

@@ -52,6 +52,14 @@ constexpr base::TimeDelta GattClientManagerImpl::kConnectTimeout;
 constexpr base::TimeDelta GattClientManagerImpl::kDisconnectTimeout;
 constexpr base::TimeDelta GattClientManagerImpl::kReadRemoteRssiTimeout;
 
+// static
+std::unique_ptr<GattClientManager> GattClientManager::Create(
+    bluetooth_v2_shlib::GattClient* gatt_client,
+    BluetoothManagerPlatform* bluetooth_manager,
+    LeScanManager* le_scan_manager) {
+  return std::make_unique<GattClientManagerImpl>(gatt_client);
+}
+
 GattClientManagerImpl::GattClientManagerImpl(
     bluetooth_v2_shlib::GattClient* gatt_client)
     : gatt_client_(gatt_client),
@@ -67,12 +75,16 @@ GattClientManagerImpl::~GattClientManagerImpl() {}
 void GattClientManagerImpl::Initialize(
     scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) {
   io_task_runner_ = std::move(io_task_runner);
+  InitializeOnIoThread();
+}
+
+void GattClientManagerImpl::InitializeOnIoThread() {
+  MAKE_SURE_IO_THREAD(InitializeOnIoThread);
+  gatt_client_->SetDelegate(this);
 }
 
 void GattClientManagerImpl::Finalize() {
-  io_task_runner_->PostTask(
-      FROM_HERE, base::BindOnce(&GattClientManagerImpl::FinalizeOnIoThread,
-                                std::move(weak_factory_)));
+  FinalizeOnIoThread();
 }
 
 void GattClientManagerImpl::AddObserver(Observer* o) {
@@ -588,10 +600,10 @@ void GattClientManagerImpl::OnReadRemoteRssiTimeout(
   RUN_ON_IO_THREAD(OnReadRemoteRssi, addr, false /* status */, 0 /* rssi */);
 }
 
-// static
-void GattClientManagerImpl::FinalizeOnIoThread(
-    std::unique_ptr<base::WeakPtrFactory<GattClientManagerImpl>> weak_factory) {
-  weak_factory->InvalidateWeakPtrs();
+void GattClientManagerImpl::FinalizeOnIoThread() {
+  MAKE_SURE_IO_THREAD(FinalizeOnIoThread);
+  weak_factory_->InvalidateWeakPtrs();
+  gatt_client_->SetDelegate(nullptr);
 }
 
 }  // namespace bluetooth

chromecast/device/bluetooth/le/gatt_client_manager_impl.h

@@ -40,15 +40,15 @@ class GattClientManagerImpl
   static constexpr base::TimeDelta kReadRemoteRssiTimeout =
       base::TimeDelta::FromSeconds(10);
 
-  using StatusCallback = base::OnceCallback<void(bool)>;
-
   explicit GattClientManagerImpl(bluetooth_v2_shlib::GattClient* gatt_client);
   ~GattClientManagerImpl() override;
 
-  void Initialize(scoped_refptr<base::SingleThreadTaskRunner> io_task_runner);
-  void Finalize();
+  void InitializeOnIoThread();
 
   // GattClientManager implementation:
+  void Initialize(
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) override;
+  void Finalize() override;
   void AddObserver(Observer* o) override;
   void RemoveObserver(Observer* o) override;
   void GetDevice(
@@ -60,6 +60,9 @@ class GattClientManagerImpl
   void GetNumConnected(base::OnceCallback<void(size_t)> cb) const override;
   void NotifyConnect(const bluetooth_v2_shlib::Addr& addr) override;
   void NotifyBonded(const bluetooth_v2_shlib::Addr& addr) override;
+  bool IsConnectedLeDevice(const bluetooth_v2_shlib::Addr& addr) override;
+  bool SetGattClientConnectable(bool connectable) override;
+  void DisconnectAll(StatusCallback cb) override;
   scoped_refptr<base::SingleThreadTaskRunner> task_runner() override;
 
   // Add a Connect or Disconnect request to the queue. |is_connect| is true for
@@ -72,20 +75,6 @@ class GattClientManagerImpl
   // serially.
   void EnqueueReadRemoteRssiRequest(const bluetooth_v2_shlib::Addr& addr);
 
-  // Enable or disable GATT client connectability. Returns |true| if successful
-  // otherwise |false|.
-  bool SetGattClientConnectable(bool connectable);
-
-  // Disconnect all connected devices. Callback will return |true| if all
-  // devices are disconnected, otherwise false.
-  // When disabling GATT client, caller should call
-  // SetGattClientConnectable(false) before calling DisconnectAll so that
-  // upcoming GATT client connections can also be blocked.
-  void DisconnectAll(StatusCallback cb);
-
-  // True if it is a connected BLE device. Must be called on IO task runner.
-  bool IsConnectedLeDevice(const bluetooth_v2_shlib::Addr& addr);
-
   // TODO(bcf): Should be private and passed into objects which need it (e.g.
   // RemoteDevice, RemoteCharacteristic).
   bluetooth_v2_shlib::GattClient* gatt_client() const { return gatt_client_; }
@@ -141,9 +130,7 @@ class GattClientManagerImpl
   void OnDisconnectTimeout(const bluetooth_v2_shlib::Addr& addr);
   void OnReadRemoteRssiTimeout(const bluetooth_v2_shlib::Addr& addr);
 
-  static void FinalizeOnIoThread(
-      std::unique_ptr<base::WeakPtrFactory<GattClientManagerImpl>>
-          weak_factory);
+  void FinalizeOnIoThread();
 
   bluetooth_v2_shlib::GattClient* const gatt_client_;
 

chromecast/device/bluetooth/le/le_scan_manager.h

@@ -7,15 +7,26 @@
 
 #include <list>
 #include <map>
+#include <memory>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
 #include "chromecast/device/bluetooth/le/le_scan_result.h"
 #include "chromecast/device/bluetooth/le/scan_filter.h"
 
+namespace base {
+class SingleThreadTaskRunner;
+}  // namespace base
+
 namespace chromecast {
+namespace bluetooth_v2_shlib {
+class LeScannerImpl;
+}  // namespace bluetooth_v2_shlib
+
 namespace bluetooth {
+class BluetoothManagerPlatform;
 
 class LeScanManager {
  public:
@@ -44,6 +55,16 @@ class LeScanManager {
     DISALLOW_COPY_AND_ASSIGN(ScanHandle);
   };
 
+  static std::unique_ptr<LeScanManager> Create(
+      BluetoothManagerPlatform* bluetooth_manager,
+      bluetooth_v2_shlib::LeScannerImpl* le_scanner);
+
+  virtual ~LeScanManager() = default;
+
+  virtual void Initialize(
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) = 0;
+  virtual void Finalize() = 0;
+
   // Request a handle to enable BLE scanning. Can be called on any thread. |cb|
   // returns a handle. As long is there is at least one handle in existence, BLE
   // scanning will be enabled. Returns nullptr if failed to enable scanning.
@@ -71,7 +92,6 @@ class LeScanManager {
 
  protected:
   LeScanManager() = default;
-  virtual ~LeScanManager() = default;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(LeScanManager);

chromecast/device/bluetooth/le/le_scan_manager_impl.cc

@@ -46,6 +46,13 @@ const int kMaxMessagesInQueue = 5;
 // static
 constexpr int LeScanManagerImpl::kMaxScanResultEntries;
 
+// static
+std::unique_ptr<LeScanManager> LeScanManager::Create(
+    BluetoothManagerPlatform* bluetooth_manager,
+    bluetooth_v2_shlib::LeScannerImpl* le_scanner) {
+  return std::make_unique<LeScanManagerImpl>(le_scanner);
+}
+
 class LeScanManagerImpl::ScanHandleImpl : public LeScanManager::ScanHandle {
  public:
   explicit ScanHandleImpl(LeScanManagerImpl* manager, int32_t id)
@@ -70,10 +77,16 @@ LeScanManagerImpl::~LeScanManagerImpl() = default;
 void LeScanManagerImpl::Initialize(
     scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) {
   io_task_runner_ = std::move(io_task_runner);
+  InitializeOnIoThread();
 }
 
 void LeScanManagerImpl::Finalize() {}
 
+void LeScanManagerImpl::InitializeOnIoThread() {
+  MAKE_SURE_IO_THREAD(InitializeOnIoThread);
+  le_scanner_->SetDelegate(this);
+}
+
 void LeScanManagerImpl::AddObserver(Observer* observer) {
   observers_->AddObserver(observer);
 }

chromecast/device/bluetooth/le/le_scan_manager_impl.h

@@ -31,10 +31,10 @@ class LeScanManagerImpl : public LeScanManager,
 
   static constexpr int kMaxScanResultEntries = 1024;
 
-  void Initialize(scoped_refptr<base::SingleThreadTaskRunner> io_task_runner);
-  void Finalize();
-
   // LeScanManager implementation:
+  void Initialize(
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) override;
+  void Finalize() override;
   void AddObserver(Observer* o) override;
   void RemoveObserver(Observer* o) override;
   void RequestScan(RequestScanCallback cb) override;
@@ -53,6 +53,8 @@ class LeScanManagerImpl : public LeScanManager,
   void OnScanResult(const bluetooth_v2_shlib::LeScanner::ScanResult&
                         scan_result_shlib) override;
 
+  void InitializeOnIoThread();
+
   // Returns a list of all BLE scan results. The results are sorted by RSSI.
   // Must be called on |io_task_runner|.
   std::vector<LeScanResult> GetScanResultsInternal(

chromecast/device/bluetooth/le/mock_gatt_client_manager.h

@@ -7,7 +7,9 @@
 
 #include <vector>
 
+#include "base/memory/scoped_refptr.h"
 #include "base/observer_list.h"
+#include "base/single_thread_task_runner.h"
 #include "chromecast/device/bluetooth/le/gatt_client_manager.h"
 #include "chromecast/device/bluetooth/le/mock_remote_device.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -18,34 +20,59 @@ namespace bluetooth {
 class MockGattClientManager : public GattClientManager {
  public:
   MockGattClientManager();
-  ~MockGattClientManager();
+  ~MockGattClientManager() override;
 
   void AddObserver(Observer* o) override { observers_.AddObserver(o); }
   void RemoveObserver(Observer* o) override { observers_.RemoveObserver(o); }
 
-  MOCK_METHOD1(
+  MOCK_METHOD(scoped_refptr<RemoteDevice>,
               GetDevice,
-      scoped_refptr<RemoteDevice>(const bluetooth_v2_shlib::Addr& addr));
+              (const bluetooth_v2_shlib::Addr& addr));
   void GetDevice(
       const bluetooth_v2_shlib::Addr& addr,
       base::OnceCallback<void(scoped_refptr<RemoteDevice>)> cb) override {
     std::move(cb).Run(GetDevice(addr));
   }
 
-  MOCK_METHOD1(
+  MOCK_METHOD(scoped_refptr<RemoteDevice>,
               GetDeviceSync,
-      scoped_refptr<RemoteDevice>(const bluetooth_v2_shlib::Addr& addr));
+              (const bluetooth_v2_shlib::Addr& addr),
+              (override));
 
-  MOCK_METHOD0(GetConnectedDevices, std::vector<scoped_refptr<RemoteDevice>>());
+  MOCK_METHOD(std::vector<scoped_refptr<RemoteDevice>>,
+              GetConnectedDevices,
+              ());
   void GetConnectedDevices(GetConnectDevicesCallback cb) {
     std::move(cb).Run(GetConnectedDevices());
   }
 
-  MOCK_CONST_METHOD1(GetNumConnected,
-                     void(base::OnceCallback<void(size_t)> cb));
-  MOCK_METHOD1(NotifyConnect, void(const bluetooth_v2_shlib::Addr& addr));
-  MOCK_METHOD1(NotifyBonded, void(const bluetooth_v2_shlib::Addr& addr));
-  MOCK_METHOD0(task_runner, scoped_refptr<base::SingleThreadTaskRunner>());
+  MOCK_METHOD(void,
+              Initialize,
+              (scoped_refptr<base::SingleThreadTaskRunner> io_task_runner),
+              (override));
+  MOCK_METHOD(void, Finalize, (), (override));
+  MOCK_METHOD(void,
+              GetNumConnected,
+              (base::OnceCallback<void(size_t)> cb),
+              (const, override));
+  MOCK_METHOD(void,
+              NotifyConnect,
+              (const bluetooth_v2_shlib::Addr& addr),
+              (override));
+  MOCK_METHOD(void,
+              NotifyBonded,
+              (const bluetooth_v2_shlib::Addr& addr),
+              (override));
+  MOCK_METHOD(scoped_refptr<base::SingleThreadTaskRunner>,
+              task_runner,
+              (),
+              (override));
+  MOCK_METHOD(bool,
+              IsConnectedLeDevice,
+              (const bluetooth_v2_shlib::Addr& addr),
+              (override));
+  MOCK_METHOD(bool, SetGattClientConnectable, (bool connectable), (override));
+  MOCK_METHOD(void, DisconnectAll, (StatusCallback cb), (override));
 
   base::ObserverList<Observer>::Unchecked observers_;
 };

chromecast/device/bluetooth/le/mock_le_scan_manager.h

@@ -7,6 +7,8 @@
 
 #include <vector>
 
+#include "base/memory/scoped_refptr.h"
+#include "base/single_thread_task_runner.h"
 #include "chromecast/device/bluetooth/le/le_scan_manager.h"
 #include "chromecast/device/bluetooth/le/scan_filter.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -34,19 +36,24 @@ class MockLeScanManager : public LeScanManager {
     observer_ = nullptr;
   }
 
-  MOCK_METHOD0(RequestScan, std::unique_ptr<ScanHandle>());
+  MOCK_METHOD(void,
+              Initialize,
+              (scoped_refptr<base::SingleThreadTaskRunner> io_task_runner),
+              (override));
+  MOCK_METHOD(void, Finalize, (), (override));
+  MOCK_METHOD(std::unique_ptr<ScanHandle>, RequestScan, ());
   void RequestScan(RequestScanCallback cb) override {
     std::move(cb).Run(RequestScan());
   }
 
-  MOCK_METHOD1(
+  MOCK_METHOD(std::vector<LeScanResult>,
               GetScanResults,
-      std::vector<LeScanResult>(base::Optional<ScanFilter> scan_filter));
+              (base::Optional<ScanFilter> scan_filter));
   void GetScanResults(GetScanResultsCallback cb,
                       base::Optional<ScanFilter> scan_filter) override {
     std::move(cb).Run(GetScanResults(std::move(scan_filter)));
   }
-  MOCK_METHOD0(ClearScanResults, void());
+  MOCK_METHOD(void, ClearScanResults, (), (override));
 
   Observer* observer_ = nullptr;
 };