[CrOS Tether] Provide extra data to BleConnectionManager observers.

This CL adds an extra StateChangeDetail parameter to the
Observer::OnSecureChannelStatusChanged() function. This parameter is
currently unused, but it will be used by a future CL to better handle
connection retries.

Bug: 805218, 672263
Change-Id: I4dc80f324d74a6df54113b058dec0bb07459ae3f
Reviewed-on: https://chromium-review.googlesource.com/888038Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532510}

chromeos/components/tether/ble_connection_manager.cc

@@ -20,7 +20,32 @@ namespace chromeos {
 namespace tether {
 
 namespace {
+
 const char kTetherFeature[] = "magic_tether";
+
+std::string StateChangeDetailToString(
+    BleConnectionManager::StateChangeDetail state_change_detail) {
+  switch (state_change_detail) {
+    case BleConnectionManager::StateChangeDetail::STATE_CHANGE_DETAIL_NONE:
+      return "[none]";
+    case BleConnectionManager::StateChangeDetail::
+        STATE_CHANGE_DETAIL_COULD_NOT_ATTEMPT_CONNECTION:
+      return "[could not attempt connection]";
+    case BleConnectionManager::StateChangeDetail::
+        STATE_CHANGE_DETAIL_GATT_CONNECTION_WAS_ATTEMPTED:
+      return "[GATT connection was attempted]";
+    case BleConnectionManager::StateChangeDetail::
+        STATE_CHANGE_DETAIL_INTERRUPTED_BY_HIGHER_PRIORITY:
+      return "[attempt interrupted by higher priority]";
+    case BleConnectionManager::StateChangeDetail::
+        STATE_CHANGE_DETAIL_DEVICE_WAS_UNREGISTERED:
+      return "[device was unregistered]";
+    default:
+      NOTREACHED();
+      return std::string();
+  }
+}
+
 }  // namespace
 
 const int64_t BleConnectionManager::kAdvertisingTimeoutMillis = 12000;
@@ -158,13 +183,18 @@ void BleConnectionManager::ConnectionMetadata::OnSecureChannelStatusChanged(
   const cryptauth::SecureChannel::Status old_status_copy = old_status;
   const cryptauth::SecureChannel::Status new_status_copy = new_status;
 
+  StateChangeDetail state_change_detail =
+      StateChangeDetail::STATE_CHANGE_DETAIL_NONE;
+
   if (new_status == cryptauth::SecureChannel::Status::DISCONNECTED) {
     secure_channel_->RemoveObserver(this);
     secure_channel_.reset();
+    state_change_detail =
+        StateChangeDetail::STATE_CHANGE_DETAIL_GATT_CONNECTION_WAS_ATTEMPTED;
   }
 
   manager_->OnSecureChannelStatusChanged(device_id_, old_status_copy,
-                                         new_status_copy);
+                                         new_status_copy, state_change_detail);
 }
 
 void BleConnectionManager::ConnectionMetadata::OnMessageReceived(
@@ -177,7 +207,7 @@ void BleConnectionManager::ConnectionMetadata::OnMessageReceived(
     return;
   }
 
-  manager_->SendMessageReceivedEvent(device_id_, payload);
+  manager_->NotifyMessageReceived(device_id_, payload);
 }
 
 void BleConnectionManager::ConnectionMetadata::OnMessageSent(
@@ -187,7 +217,7 @@ void BleConnectionManager::ConnectionMetadata::OnMessageSent(
   PA_LOG(INFO) << "Message sent successfully to device with ID \""
                << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(device_id_)
                << "\"; message sequence number: " << sequence_number;
-  manager_->SendMessageSentEvent(sequence_number);
+  manager_->NotifyMessageSent(sequence_number);
 }
 
 void BleConnectionManager::ConnectionMetadata::
@@ -272,9 +302,10 @@ void BleConnectionManager::UnregisterRemoteDevice(
     if (status_before_disconnect !=
         cryptauth::SecureChannel::Status::DISCONNECTED) {
       // Send a status update for the disconnection.
-      SendSecureChannelStatusChangeEvent(
+      NotifySecureChannelStatusChanged(
           device_id_copy, status_before_disconnect,
-          cryptauth::SecureChannel::Status::DISCONNECTED);
+          cryptauth::SecureChannel::Status::DISCONNECTED,
+          StateChangeDetail::STATE_CHANGE_DETAIL_DEVICE_WAS_UNREGISTERED);
     }
   }
 
@@ -425,7 +456,9 @@ void BleConnectionManager::UpdateConnectionAttempts() {
                  << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(
                         device_id_to_stop)
                  << "\" interrupted by higher-priority connection.";
-    EndUnsuccessfulAttempt(device_id_to_stop);
+    EndUnsuccessfulAttempt(
+        device_id_to_stop,
+        StateChangeDetail::STATE_CHANGE_DETAIL_INTERRUPTED_BY_HIGHER_PRIORITY);
   }
 
   for (const auto& device_id : should_advertise_to) {
@@ -479,22 +512,24 @@ void BleConnectionManager::StartConnectionAttempt(
 
   // Send a "disconnected => connecting" update to alert clients that a
   // connection attempt for |device_id| is underway.
-  SendSecureChannelStatusChangeEvent(
+  NotifySecureChannelStatusChanged(
       device_id, cryptauth::SecureChannel::Status::DISCONNECTED,
-      cryptauth::SecureChannel::Status::CONNECTING);
+      cryptauth::SecureChannel::Status::CONNECTING,
+      StateChangeDetail::STATE_CHANGE_DETAIL_NONE);
 }
 
 void BleConnectionManager::EndUnsuccessfulAttempt(
-    const std::string& device_id) {
+    const std::string& device_id,
+    StateChangeDetail state_change_detail) {
   GetConnectionMetadata(device_id)->StopConnectionAttemptTimer();
   StopConnectionAttemptAndMoveToEndOfQueue(device_id);
   device_id_to_advertising_start_time_map_[device_id] = base::Time();
 
   // Send a "connecting => disconnected" update to alert clients that a
   // connection attempt for |device_id| has failed.
-  SendSecureChannelStatusChangeEvent(
+  NotifySecureChannelStatusChanged(
       device_id, cryptauth::SecureChannel::Status::CONNECTING,
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+      cryptauth::SecureChannel::Status::DISCONNECTED, state_change_detail);
 }
 
 void BleConnectionManager::StopConnectionAttemptAndMoveToEndOfQueue(
@@ -509,15 +544,19 @@ void BleConnectionManager::OnConnectionAttemptTimeout(
   PA_LOG(INFO) << "Connection attempt timeout - Device ID \""
                << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(device_id)
                << "\".";
-  EndUnsuccessfulAttempt(device_id);
+  EndUnsuccessfulAttempt(
+      device_id,
+      StateChangeDetail::STATE_CHANGE_DETAIL_COULD_NOT_ATTEMPT_CONNECTION);
   UpdateConnectionAttempts();
 }
 
 void BleConnectionManager::OnSecureChannelStatusChanged(
     const std::string& device_id,
     const cryptauth::SecureChannel::Status& old_status,
-    const cryptauth::SecureChannel::Status& new_status) {
-  SendSecureChannelStatusChangeEvent(device_id, old_status, new_status);
+    const cryptauth::SecureChannel::Status& new_status,
+    StateChangeDetail state_change_detail) {
+  NotifySecureChannelStatusChanged(device_id, old_status, new_status,
+                                   state_change_detail);
   UpdateConnectionAttempts();
 }
 
@@ -530,8 +569,8 @@ void BleConnectionManager::OnGattCharacteristicsNotAvailable(
   ad_hoc_ble_advertisement_->RequestGattServicesForDevice(device_id);
 }
 
-void BleConnectionManager::SendMessageReceivedEvent(std::string device_id,
-                                                    std::string payload) {
+void BleConnectionManager::NotifyMessageReceived(std::string device_id,
+                                                 std::string payload) {
   PA_LOG(INFO) << "Message received - Device ID: \""
                << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(device_id)
                << "\", Message: \"" << payload << "\".";
@@ -539,15 +578,17 @@ void BleConnectionManager::SendMessageReceivedEvent(std::string device_id,
     observer.OnMessageReceived(device_id, payload);
 }
 
-void BleConnectionManager::SendSecureChannelStatusChangeEvent(
+void BleConnectionManager::NotifySecureChannelStatusChanged(
     std::string device_id,
     cryptauth::SecureChannel::Status old_status,
-    cryptauth::SecureChannel::Status new_status) {
+    cryptauth::SecureChannel::Status new_status,
+    StateChangeDetail state_change_detail) {
   PA_LOG(INFO) << "Status change - Device ID: \""
                << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(device_id)
                << "\": " << cryptauth::SecureChannel::StatusToString(old_status)
-               << " => "
-               << cryptauth::SecureChannel::StatusToString(new_status);
+               << " => " << cryptauth::SecureChannel::StatusToString(new_status)
+               << ", State change detail: "
+               << StateChangeDetailToString(state_change_detail);
 
   if (new_status == cryptauth::SecureChannel::Status::CONNECTING) {
     device_id_to_advertising_start_time_map_[device_id] = clock_->Now();
@@ -558,11 +599,13 @@ void BleConnectionManager::SendSecureChannelStatusChangeEvent(
     RecordConnectionToAuthenticationDuration(device_id);
   }
 
-  for (auto& observer : observer_list_)
-    observer.OnSecureChannelStatusChanged(device_id, old_status, new_status);
+  for (auto& observer : observer_list_) {
+    observer.OnSecureChannelStatusChanged(device_id, old_status, new_status,
+                                          state_change_detail);
+  }
 }
 
-void BleConnectionManager::SendMessageSentEvent(int sequence_number) {
+void BleConnectionManager::NotifyMessageSent(int sequence_number) {
   for (auto& observer : observer_list_)
     observer.OnMessageSent(sequence_number);
 }

chromeos/components/tether/ble_connection_manager.h

@@ -64,12 +64,24 @@ class BleConnectionManager : public BleScanner::Observer {
  public:
   static std::string MessageTypeToString(const MessageType& reason);
 
+  // Extra data about a state change which is passed to observers in
+  // OnSecureChannelStatusChanged(). If no extra data applies to the state
+  // change, STATE_CHANGE_DETAIL_NONE is used.
+  enum class StateChangeDetail {
+    STATE_CHANGE_DETAIL_NONE,
+    STATE_CHANGE_DETAIL_COULD_NOT_ATTEMPT_CONNECTION,
+    STATE_CHANGE_DETAIL_GATT_CONNECTION_WAS_ATTEMPTED,
+    STATE_CHANGE_DETAIL_INTERRUPTED_BY_HIGHER_PRIORITY,
+    STATE_CHANGE_DETAIL_DEVICE_WAS_UNREGISTERED
+  };
+
   class Observer {
    public:
     virtual void OnSecureChannelStatusChanged(
         const std::string& device_id,
         const cryptauth::SecureChannel::Status& old_status,
-        const cryptauth::SecureChannel::Status& new_status) = 0;
+        const cryptauth::SecureChannel::Status& new_status,
+        StateChangeDetail state_change_detail) = 0;
 
     virtual void OnMessageReceived(const std::string& device_id,
                                    const std::string& payload) = 0;
@@ -124,12 +136,13 @@ class BleConnectionManager : public BleScanner::Observer {
       device::BluetoothDevice* bluetooth_device) override;
 
  protected:
-  void SendMessageReceivedEvent(std::string device_id, std::string payload);
-  void SendSecureChannelStatusChangeEvent(
+  void NotifyMessageReceived(std::string device_id, std::string payload);
+  void NotifySecureChannelStatusChanged(
       std::string device_id,
       cryptauth::SecureChannel::Status old_status,
-      cryptauth::SecureChannel::Status new_status);
-  void SendMessageSentEvent(int sequence_number);
+      cryptauth::SecureChannel::Status new_status,
+      StateChangeDetail state_change_detail);
+  void NotifyMessageSent(int sequence_number);
 
  private:
   friend class BleConnectionManagerTest;
@@ -197,14 +210,16 @@ class BleConnectionManager : public BleScanner::Observer {
   void UpdateAdvertisementQueue();
 
   void StartConnectionAttempt(const std::string& device_id);
-  void EndUnsuccessfulAttempt(const std::string& device_id);
+  void EndUnsuccessfulAttempt(const std::string& device_id,
+                              StateChangeDetail state_change_detail);
   void StopConnectionAttemptAndMoveToEndOfQueue(const std::string& device_id);
 
   void OnConnectionAttemptTimeout(const std::string& device_id);
   void OnSecureChannelStatusChanged(
       const std::string& device_id,
       const cryptauth::SecureChannel::Status& old_status,
-      const cryptauth::SecureChannel::Status& new_status);
+      const cryptauth::SecureChannel::Status& new_status,
+      StateChangeDetail state_change_detail);
   void OnGattCharacteristicsNotAvailable(const std::string& device_id);
 
   void SetTestDoubles(std::unique_ptr<base::Clock> test_clock,

chromeos/components/tether/connect_tethering_operation_unittest.cc

@@ -32,6 +32,8 @@ namespace {
 const char kTestSsid[] = "testSsid";
 const char kTestPassword[] = "testPassword";
 
+const size_t kMaxConnectionAttemptsPerDevice = 3;
+
 constexpr base::TimeDelta kConnectTetheringResponseTime =
     base::TimeDelta::FromSeconds(15);
 
@@ -276,21 +278,8 @@ TEST_F(ConnectTetheringOperationTest, TestCannotConnect) {
       .Times(0);
 
   // Simulate the device failing to connect.
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(), cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(), cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(), cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+  fake_ble_connection_manager_->SimulateFailedConnectionAttempts(
+      test_device_.GetDeviceId(), kMaxConnectionAttemptsPerDevice);
 
   // The maximum number of connection failures has occurred.
   EXPECT_TRUE(test_observer_->has_received_failure());

chromeos/components/tether/fake_ble_connection_manager.cc

@@ -34,7 +34,8 @@ FakeBleConnectionManager::~FakeBleConnectionManager() = default;
 
 void FakeBleConnectionManager::SetDeviceStatus(
     const std::string& device_id,
-    const cryptauth::SecureChannel::Status& status) {
+    const cryptauth::SecureChannel::Status& status,
+    BleConnectionManager::StateChangeDetail state_change_detail) {
   const auto iter = device_id_map_.find(device_id);
   DCHECK(iter != device_id_map_.end());
 
@@ -45,18 +46,31 @@ void FakeBleConnectionManager::SetDeviceStatus(
   }
 
   iter->second.status = status;
-  SendSecureChannelStatusChangeEvent(device_id, old_status, status);
+  NotifySecureChannelStatusChanged(device_id, old_status, status,
+                                   state_change_detail);
 }
 
 void FakeBleConnectionManager::ReceiveMessage(const std::string& device_id,
                                               const std::string& payload) {
   DCHECK(device_id_map_.find(device_id) != device_id_map_.end());
-  SendMessageReceivedEvent(device_id, payload);
+  NotifyMessageReceived(device_id, payload);
 }
 
 void FakeBleConnectionManager::SetMessageSent(int sequence_number) {
   DCHECK(sequence_number < next_sequence_number_);
-  SendMessageSentEvent(sequence_number);
+  NotifyMessageSent(sequence_number);
+}
+
+void FakeBleConnectionManager::SimulateFailedConnectionAttempts(
+    const std::string& device_id,
+    size_t num_attempts) {
+  for (size_t i = 0; i < num_attempts; ++i) {
+    SetDeviceStatus(device_id, cryptauth::SecureChannel::Status::CONNECTING,
+                    StateChangeDetail::STATE_CHANGE_DETAIL_NONE);
+    SetDeviceStatus(
+        device_id, cryptauth::SecureChannel::Status::DISCONNECTED,
+        StateChangeDetail::STATE_CHANGE_DETAIL_COULD_NOT_ATTEMPT_CONNECTION);
+  }
 }
 
 bool FakeBleConnectionManager::IsRegistered(const std::string& device_id) {

chromeos/components/tether/fake_ble_connection_manager.h

@@ -26,11 +26,19 @@ class FakeBleConnectionManager : public BleConnectionManager {
     std::string message;
   };
 
-  void SetDeviceStatus(const std::string& device_id,
-                       const cryptauth::SecureChannel::Status& status);
+  void SetDeviceStatus(
+      const std::string& device_id,
+      const cryptauth::SecureChannel::Status& status,
+      BleConnectionManager::StateChangeDetail state_change_detail);
   void ReceiveMessage(const std::string& device_id, const std::string& payload);
   void SetMessageSent(int sequence_number);
 
+  // Simulates |num_attempts| consecutive failed connection attempts for the
+  // device with ID |device_id|. Specifically, this function updates the
+  // device's status to CONNECTING then DISCONNECTED on each attempt.
+  void SimulateFailedConnectionAttempts(const std::string& device_id,
+                                        size_t num_attempts);
+
   std::vector<SentMessage>& sent_messages() { return sent_messages_; }
   // Returns -1 if no sequence numbers have been used yet.
   int last_sequence_number() { return next_sequence_number_ - 1; }

chromeos/components/tether/host_scanner_operation_unittest.cc

@@ -30,6 +30,8 @@ namespace tether {
 
 namespace {
 
+const size_t kMaxConnectionAttemptsPerDevice = 3;
+
 const char kDefaultCarrier[] = "Google Fi";
 
 constexpr base::TimeDelta kTetherAvailabilityResponseTime =
@@ -373,24 +375,8 @@ TEST_F(HostScannerOperationTest, TestMultipleDevices) {
       kTetherAvailabilityResponseTime, 2);
 
   // Simulate device 1 failing to connect.
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+  fake_ble_connection_manager_->SimulateFailedConnectionAttempts(
+      test_devices_[1].GetDeviceId(), kMaxConnectionAttemptsPerDevice);
 
   // The scan should still not be over, and no new scan results should have
   // come in.
@@ -398,24 +384,8 @@ TEST_F(HostScannerOperationTest, TestMultipleDevices) {
   EXPECT_EQ(2u, test_observer_->scanned_devices_so_far().size());
 
   // Simulate device 3 failing to connect.
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+  fake_ble_connection_manager_->SimulateFailedConnectionAttempts(
+      test_devices_[3].GetDeviceId(), kMaxConnectionAttemptsPerDevice);
 
   // The scan should still not be over, and no new scan results should have
   // come in.

chromeos/components/tether/keep_alive_operation_unittest.cc

@@ -22,6 +22,8 @@ namespace tether {
 
 namespace {
 
+const size_t kMaxConnectionAttemptsPerDevice = 3;
+
 constexpr base::TimeDelta kKeepAliveTickleResponseTime =
     base::TimeDelta::FromSeconds(3);
 
@@ -140,21 +142,8 @@ TEST_F(KeepAliveOperationTest, TestSendsKeepAliveTickleAndReceivesResponse) {
 
 TEST_F(KeepAliveOperationTest, TestCannotConnect) {
   // Simulate the device failing to connect.
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(), cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(), cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(), cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_device_.GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+  fake_ble_connection_manager_->SimulateFailedConnectionAttempts(
+      test_device_.GetDeviceId(), kMaxConnectionAttemptsPerDevice);
 
   // The maximum number of connection failures has occurred.
   EXPECT_TRUE(test_observer_->has_run_callback());

chromeos/components/tether/message_transfer_operation.cc

@@ -99,7 +99,8 @@ void MessageTransferOperation::Initialize() {
 void MessageTransferOperation::OnSecureChannelStatusChanged(
     const std::string& device_id,
     const cryptauth::SecureChannel::Status& old_status,
-    const cryptauth::SecureChannel::Status& new_status) {
+    const cryptauth::SecureChannel::Status& new_status,
+    BleConnectionManager::StateChangeDetail status_change_detail) {
   cryptauth::RemoteDevice* remote_device = GetRemoteDevice(device_id);
   if (!remote_device) {
     // If the device whose status has changed does not correspond to any of the
@@ -108,6 +109,9 @@ void MessageTransferOperation::OnSecureChannelStatusChanged(
     return;
   }
 
+  // TODO(khorimoto): Use |status_change_detail| to provide better retry
+  // handling. See https://crbug.com/805218.
+
   if (new_status == cryptauth::SecureChannel::Status::AUTHENTICATED) {
     StartTimerForDevice(*remote_device);
     OnDeviceAuthenticated(*remote_device);

chromeos/components/tether/message_transfer_operation.h

@@ -35,7 +35,8 @@ class MessageTransferOperation : public BleConnectionManager::Observer {
   void OnSecureChannelStatusChanged(
       const std::string& device_id,
       const cryptauth::SecureChannel::Status& old_status,
-      const cryptauth::SecureChannel::Status& new_status) override;
+      const cryptauth::SecureChannel::Status& new_status,
+      BleConnectionManager::StateChangeDetail status_change_detail) override;
   void OnMessageReceived(const std::string& device_id,
                          const std::string& payload) override;
   void OnMessageSent(int sequence_number) override {}

chromeos/components/tether/message_transfer_operation_unittest.cc

@@ -21,6 +21,8 @@ namespace tether {
 
 namespace {
 
+const size_t kMaxConnectionAttemptsPerDevice = 3;
+
 // Arbitrarily chosen value. The MessageType used in this test does not matter
 // except that it must be consistent throughout the test.
 const MessageType kTestMessageType = MessageType::TETHER_AVAILABILITY_REQUEST;
@@ -194,16 +196,20 @@ class MessageTransferOperationTest : public testing::Test {
 
     fake_ble_connection_manager_->SetDeviceStatus(
         remote_device.GetDeviceId(),
-        cryptauth::SecureChannel::Status::CONNECTING);
+        cryptauth::SecureChannel::Status::CONNECTING,
+        BleConnectionManager::StateChangeDetail::STATE_CHANGE_DETAIL_NONE);
     fake_ble_connection_manager_->SetDeviceStatus(
         remote_device.GetDeviceId(),
-        cryptauth::SecureChannel::Status::CONNECTED);
+        cryptauth::SecureChannel::Status::CONNECTED,
+        BleConnectionManager::StateChangeDetail::STATE_CHANGE_DETAIL_NONE);
     fake_ble_connection_manager_->SetDeviceStatus(
         remote_device.GetDeviceId(),
-        cryptauth::SecureChannel::Status::AUTHENTICATING);
+        cryptauth::SecureChannel::Status::AUTHENTICATING,
+        BleConnectionManager::StateChangeDetail::STATE_CHANGE_DETAIL_NONE);
     fake_ble_connection_manager_->SetDeviceStatus(
         remote_device.GetDeviceId(),
-        cryptauth::SecureChannel::Status::AUTHENTICATED);
+        cryptauth::SecureChannel::Status::AUTHENTICATED,
+        BleConnectionManager::StateChangeDetail::STATE_CHANGE_DETAIL_NONE);
   }
 
   base::MockTimer* GetTimerForDevice(
@@ -243,20 +249,26 @@ TEST_F(MessageTransferOperationTest, TestCannotConnectAndReachesRetryLimit) {
   // Try to connect and fail. The device should still be registered.
   fake_ble_connection_manager_->SetDeviceStatus(
       test_devices_[0].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
+      cryptauth::SecureChannel::Status::CONNECTING,
+      BleConnectionManager::StateChangeDetail::STATE_CHANGE_DETAIL_NONE);
   fake_ble_connection_manager_->SetDeviceStatus(
       test_devices_[0].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+      cryptauth::SecureChannel::Status::DISCONNECTED,
+      BleConnectionManager::StateChangeDetail::
+          STATE_CHANGE_DETAIL_COULD_NOT_ATTEMPT_CONNECTION);
   EXPECT_TRUE(fake_ble_connection_manager_->IsRegistered(
       test_devices_[0].GetDeviceId()));
 
   // Try and fail again. The device should still be registered.
   fake_ble_connection_manager_->SetDeviceStatus(
       test_devices_[0].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
+      cryptauth::SecureChannel::Status::CONNECTING,
+      BleConnectionManager::StateChangeDetail::STATE_CHANGE_DETAIL_NONE);
   fake_ble_connection_manager_->SetDeviceStatus(
       test_devices_[0].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+      cryptauth::SecureChannel::Status::DISCONNECTED,
+      BleConnectionManager::StateChangeDetail::
+          STATE_CHANGE_DETAIL_COULD_NOT_ATTEMPT_CONNECTION);
   EXPECT_TRUE(fake_ble_connection_manager_->IsRegistered(
       test_devices_[0].GetDeviceId()));
 
@@ -264,10 +276,13 @@ TEST_F(MessageTransferOperationTest, TestCannotConnectAndReachesRetryLimit) {
   // so the device should be unregistered.
   fake_ble_connection_manager_->SetDeviceStatus(
       test_devices_[0].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
+      cryptauth::SecureChannel::Status::CONNECTING,
+      BleConnectionManager::StateChangeDetail::STATE_CHANGE_DETAIL_NONE);
   fake_ble_connection_manager_->SetDeviceStatus(
       test_devices_[0].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+      cryptauth::SecureChannel::Status::DISCONNECTED,
+      BleConnectionManager::StateChangeDetail::
+          STATE_CHANGE_DETAIL_COULD_NOT_ATTEMPT_CONNECTION);
   EXPECT_FALSE(fake_ble_connection_manager_->IsRegistered(
       test_devices_[0].GetDeviceId()));
   VerifyOperationStartedAndFinished(true /* has_started */,
@@ -286,10 +301,13 @@ TEST_F(MessageTransferOperationTest, TestFailsThenConnects) {
   // Try to connect and fail. The device should still be registered.
   fake_ble_connection_manager_->SetDeviceStatus(
       test_devices_[0].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
+      cryptauth::SecureChannel::Status::CONNECTING,
+      BleConnectionManager::StateChangeDetail::STATE_CHANGE_DETAIL_NONE);
   fake_ble_connection_manager_->SetDeviceStatus(
       test_devices_[0].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+      cryptauth::SecureChannel::Status::DISCONNECTED,
+      BleConnectionManager::StateChangeDetail::
+          STATE_CHANGE_DETAIL_COULD_NOT_ATTEMPT_CONNECTION);
   EXPECT_TRUE(fake_ble_connection_manager_->IsRegistered(
       test_devices_[0].GetDeviceId()));
 
@@ -547,24 +565,8 @@ TEST_F(MessageTransferOperationTest, MultipleDevices) {
   // Fail 3 times to connect to |test_devices_[1]|.
   test_timer_factory_->set_device_id_for_next_timer(
       test_devices_[1].GetDeviceId());
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[1].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+  fake_ble_connection_manager_->SimulateFailedConnectionAttempts(
+      test_devices_[1].GetDeviceId(), kMaxConnectionAttemptsPerDevice);
   EXPECT_FALSE(operation_->HasDeviceAuthenticated(test_devices_[1]));
   EXPECT_FALSE(fake_ble_connection_manager_->IsRegistered(
       test_devices_[1].GetDeviceId()));
@@ -582,24 +584,8 @@ TEST_F(MessageTransferOperationTest, MultipleDevices) {
   // Fail 3 times to connect to |test_devices_[3]|.
   test_timer_factory_->set_device_id_for_next_timer(
       test_devices_[3].GetDeviceId());
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::CONNECTING);
-  fake_ble_connection_manager_->SetDeviceStatus(
-      test_devices_[3].GetDeviceId(),
-      cryptauth::SecureChannel::Status::DISCONNECTED);
+  fake_ble_connection_manager_->SimulateFailedConnectionAttempts(
+      test_devices_[3].GetDeviceId(), kMaxConnectionAttemptsPerDevice);
   EXPECT_FALSE(operation_->HasDeviceAuthenticated(test_devices_[3]));
   EXPECT_FALSE(fake_ble_connection_manager_->IsRegistered(
       test_devices_[3].GetDeviceId()));