[CrOS Tether]: Create BleConnectionManager, which manages secure connections between the current de…

chromeos/components/tether/ble_connection_manager.h

Index: chromeos/components/tether/ble_connection_manager.h
diff --git a/chromeos/components/tether/ble_connection_manager.h b/chromeos/components/tether/ble_connection_manager.h
new file mode 100644
index 0000000000000000000000000000000000000000..40937def6bcc8f304bcdc034d12c66c042027e6e
--- /dev/null
+++ b/chromeos/components/tether/ble_connection_manager.h
@@ -0,0 +1,221 @@
+// Copyright 2017 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 CHROMEOS_COMPONENTS_TETHER_BLE_CONNECTION_MANAGER_H_
+#define CHROMEOS_COMPONENTS_TETHER_BLE_CONNECTION_MANAGER_H_
+
+#include <map>
+#include <set>
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/timer/timer.h"
+#include "chromeos/components/tether/ble_advertisement_device_queue.h"
+#include "chromeos/components/tether/ble_advertiser.h"
+#include "chromeos/components/tether/ble_scanner.h"
+#include "components/cryptauth/remote_device.h"
+#include "components/cryptauth/secure_channel.h"
+
+namespace cryptauth {
+class BluetoothThrottler;
+}  // namespace cryptauth
+
+namespace chromeos {
+
+namespace tether {
+
+// Manages connections to remote devices. To attempt a connection attempt, call
+// |RegisterRemoteDevice()| and register to observer status change events. Once

[Ryan Hansberry, 2017/02/14 18:27:28]

observe*

[Kyle Horimoto, 2017/02/14 22:54:46]

Done.

+// a device's status changes to |AUTHENTICATED|, messages can be sent and
+// received to/from that device via the |SendMessage()| function and
+// |OnMessageReceived()| observer callback.
+class BleConnectionManager : public BleScanner::Observer {
+ public:
+  enum class ConnectionReason {

[Ryan Hansberry, 2017/02/14 18:27:28]

We should just use the real proto constants.

[Kyle Horimoto, 2017/02/14 22:54:46]

Done.

+    TETHER_AVAILABILITY_REQUEST,
+    CONNECT_TETHERING_REQUEST,
+    KEEP_ALIVE_REQUEST,
+    DISCONNECT_REQUEST
+  };
+
+  static std::string ConnectionReasonToString(const ConnectionReason& reason);
+
+  class Observer {
+   public:
+    virtual void OnSecureChannelStatusChanged(
+        const cryptauth::RemoteDevice& remote_device,
+        const cryptauth::SecureChannel::Status& old_status,
+        const cryptauth::SecureChannel::Status& new_status) = 0;
+
+    virtual void OnMessageReceived(const cryptauth::RemoteDevice& remote_device,
+                                   const std::string& payload) = 0;
+  };
+
+  class Delegate {
+   public:
+    virtual std::unique_ptr<cryptauth::SecureChannel::Delegate>
+    CreateSecureChannelDelegate() = 0;
+  };
+
+  BleConnectionManager(
+      std::unique_ptr<Delegate> delegate,
+      scoped_refptr<device::BluetoothAdapter> adapter,
+      const LocalDeviceDataProvider* local_device_data_provider,
+      const cryptauth::RemoteBeaconSeedFetcher* remote_beacon_seed_fetcher,
+      cryptauth::BluetoothThrottler* bluetooth_throttler);
+  virtual ~BleConnectionManager();
+
+  // Registers |remote_device| for |connection_reason|. Once registered, this
+  // instance will continue to attempt to connect and authenticate to that
+  // device until the device is unregistered.
+  void RegisterRemoteDevice(const cryptauth::RemoteDevice& remote_device,
+                            const ConnectionReason& connection_reason);
+
+  // Unregisters |remote_device| for |connection_reason|. Once registered, a
+  // device will continue trying to connect until *ALL* of its
+  // |ConnectionReason|s have been unregistered.
+  void UnregisterRemoteDevice(const cryptauth::RemoteDevice& remote_device,
+                              const ConnectionReason& connection_reason);
+
+  // Sends |message| to |remote_device|. This function can only be called if the
+  // given device is authenticated.
+  void SendMessage(const cryptauth::RemoteDevice& remote_device,
+                   const std::string& message);
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  // BleScanner::Observer:
+  void OnReceivedAdvertisementFromDevice(
+      const std::string& device_address,
+      cryptauth::RemoteDevice remote_device) override;
+
+ private:
+  friend class BleConnectionManagerTest;
+
+  class ConnectionMetadata : public cryptauth::SecureChannel::Observer {

[Ryan Hansberry, 2017/02/14 18:27:28]

Does ConnectionMetadata need to be an inner class? If not, please break it out
into its own set of files.

[Ryan Hansberry, 2017/02/14 18:27:28]

This inner class is quite large; it needs documentation.

[Kyle Horimoto, 2017/02/14 22:54:46]

Yes, it should be an inner class. It is a bookkeeping class specifically
tailored to be used as the value of BleConnectionManager's
|device_to_metadata_map_| map. I don't think that this class should be visible
outside of the class to preserve encapsulation.

[Kyle Horimoto, 2017/02/14 22:54:46]

Done.

+   public:
+    ConnectionMetadata(const cryptauth::RemoteDevice remote_device,
+                       std::shared_ptr<base::Timer> timer);
+    ~ConnectionMetadata();
+
+    void RegisterConnectionReason(const ConnectionReason& connection_reason);
+    void UnregisterConnectionReason(const ConnectionReason& connection_reason);
+    bool HasReasonForConnection() const;
+
+    cryptauth::SecureChannel::Status GetStatus() const;
+
+    bool IsActivelyConnecting() const;
+    bool HasEstablishedConnection() const;
+    bool CanSendMessage() const;
+
+    using ConnectionAttemptTimeoutHandler =
+        base::Callback<void(const cryptauth::RemoteDevice& remote_device)>;
+    void StartConnectionAttemptTimeout(
+        const ConnectionAttemptTimeoutHandler& handler,
+        bool use_short_error_timeout);
+
+    void OnConnectionAttemptTimeout();
+
+    using SecureChannelStatusChangeHandler = base::Callback<void(

[Ryan Hansberry, 2017/02/14 18:27:28]

Is 'using' standard practice? I've only seen 'typedef'.

[Ryan Hansberry, 2017/02/14 18:27:28]

Please use '*Callback', not '*Handler'

[Kyle Horimoto, 2017/02/14 22:54:45]

Done.

[Kyle Horimoto, 2017/02/14 22:54:46]

Yep, pretty common. For example:
https://cs.chromium.org/chromium/src/device/bluetooth/bluetooth_adapter.h

+        const cryptauth::RemoteDevice& remote_device,
+        const cryptauth::SecureChannel::Status& old_status,
+        const cryptauth::SecureChannel::Status& new_status)>;
+    using MessageReceivedHandler =

[Ryan Hansberry, 2017/02/14 18:27:28]

Same here.

[Kyle Horimoto, 2017/02/14 22:54:45]

Done.

+        base::Callback<void(const cryptauth::RemoteDevice& remote_device,
+                            const std::string& payload)>;
+    void SetSecureChannel(
+        std::unique_ptr<cryptauth::SecureChannel> secure_channel,
+        const SecureChannelStatusChangeHandler& status_change_handler,
+        const MessageReceivedHandler message_received_handler);
+
+    void SendMessage(const std::string& payload);
+
+    // cryptauth::SecureChannel::Observer:
+    void OnSecureChannelStatusChanged(
+        cryptauth::SecureChannel* secure_channel,
+        const cryptauth::SecureChannel::Status& old_status,
+        const cryptauth::SecureChannel::Status& new_status) override;
+    void OnMessageReceived(cryptauth::SecureChannel* secure_channel,
+                           const std::string& feature,
+                           const std::string& payload) override;
+
+   private:
+    friend class BleConnectionManagerTest;
+
+    cryptauth::RemoteDevice remote_device_;
+    std::set<ConnectionReason> active_connection_reasons_;
+    std::shared_ptr<cryptauth::SecureChannel> secure_channel_;
+    std::shared_ptr<base::Timer> timer_;
+
+    SecureChannelStatusChangeHandler status_change_handler_;
+    MessageReceivedHandler message_received_handler_;
+    ConnectionAttemptTimeoutHandler timeout_handler_;
+
+    base::WeakPtrFactory<ConnectionMetadata> weak_ptr_factory_;
+  };
+
+  class TimerFactory {
+   public:
+    virtual std::unique_ptr<base::Timer> CreateTimer();
+  };
+
+  BleConnectionManager(
+      std::unique_ptr<Delegate> delegate,
+      scoped_refptr<device::BluetoothAdapter> adapter,
+      std::unique_ptr<BleScanner> ble_scanner,
+      std::unique_ptr<BleAdvertiser> ble_advertiser,
+      std::unique_ptr<BleAdvertisementDeviceQueue> device_queue,
+      std::unique_ptr<TimerFactory> timer_factory,
+      cryptauth::BluetoothThrottler* bluetooth_throttler);
+
+  std::shared_ptr<ConnectionMetadata> GetConnectionMetadata(
+      const cryptauth::RemoteDevice& remote_device);
+  std::shared_ptr<ConnectionMetadata> AddMetadataForDevice(
+      const cryptauth::RemoteDevice& remote_device);
+
+  void UpdateConnectionAttempts();
+  void UpdateAdvertisementQueue();
+
+  void StartConnectionAttempt(const cryptauth::RemoteDevice& remote_device);
+  void StopConnectionAttemptAndMoveToEndOfQueue(
+      const cryptauth::RemoteDevice& remote_device);
+
+  void OnConnectionAttemptTimeout(const cryptauth::RemoteDevice& remote_device);
+  void OnSecureChannelStatusChanged(
+      const cryptauth::RemoteDevice& remote_device,
+      const cryptauth::SecureChannel::Status& old_status,
+      const cryptauth::SecureChannel::Status& new_status);
+
+  void SendMessageReceivedEvent(const cryptauth::RemoteDevice& remote_device,
+                                const std::string& payload);
+  void SendSecureChannelStatusChangeEvent(
+      const cryptauth::RemoteDevice& remote_device,
+      const cryptauth::SecureChannel::Status& old_status,
+      const cryptauth::SecureChannel::Status& new_status);
+
+  std::unique_ptr<Delegate> delegate_;
+  scoped_refptr<device::BluetoothAdapter> adapter_;
+  std::unique_ptr<BleScanner> ble_scanner_;
+  std::unique_ptr<BleAdvertiser> ble_advertiser_;
+  std::unique_ptr<BleAdvertisementDeviceQueue> device_queue_;
+  std::unique_ptr<TimerFactory> timer_factory_;
+  cryptauth::BluetoothThrottler* bluetooth_throttler_;
+
+  std::map<cryptauth::RemoteDevice, std::shared_ptr<ConnectionMetadata>>
+      device_to_metadata_map_;
+
+  base::ObserverList<Observer> observer_list_;
+  base::WeakPtrFactory<BleConnectionManager> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(BleConnectionManager);
+};
+
+}  // namespace tether
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_TETHER_BLE_CONNECTION_MANAGER_H_