Update FeatureStatusProvider to check ConnectionManager status

FeatureStatusProvider will now also be notified if there is a change
with the local and remote host connection.

Bug: 1106937
Test: unit_tests
Change-Id: I8d2cc98beb5453e71e084935b88f6ab51a308b36
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2389105
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804175}

chromeos/components/phonehub/feature_status_provider_impl.cc

@@ -113,11 +113,14 @@ bool IsFeatureDisabledByUser(FeatureState feature_state) {
 
 FeatureStatusProviderImpl::FeatureStatusProviderImpl(
     device_sync::DeviceSyncClient* device_sync_client,
-    multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client)
+    multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client,
+    ConnectionManager* connection_manager)
     : device_sync_client_(device_sync_client),
-      multidevice_setup_client_(multidevice_setup_client) {
+      multidevice_setup_client_(multidevice_setup_client),
+      connection_manager_(connection_manager) {
   device_sync_client_->AddObserver(this);
   multidevice_setup_client_->AddObserver(this);
+  connection_manager_->AddObserver(this);
 
   device::BluetoothAdapterFactory::Get()->GetAdapter(
       base::BindOnce(&FeatureStatusProviderImpl::OnBluetoothAdapterReceived,
@@ -129,6 +132,7 @@ FeatureStatusProviderImpl::FeatureStatusProviderImpl(
 FeatureStatusProviderImpl::~FeatureStatusProviderImpl() {
   device_sync_client_->RemoveObserver(this);
   multidevice_setup_client_->RemoveObserver(this);
+  connection_manager_->RemoveObserver(this);
   if (bluetooth_adapter_)
     bluetooth_adapter_->RemoveObserver(this);
 }
@@ -180,6 +184,10 @@ void FeatureStatusProviderImpl::OnBluetoothAdapterReceived(
     UpdateStatus();
 }
 
+void FeatureStatusProviderImpl::OnStatusChanged() {
+  UpdateStatus();
+}
+
 void FeatureStatusProviderImpl::UpdateStatus() {
   DCHECK(status_.has_value());
 
@@ -217,8 +225,15 @@ FeatureStatus FeatureStatusProviderImpl::ComputeStatus() {
   if (!IsBluetoothOn())
     return FeatureStatus::kUnavailableBluetoothOff;
 
-  // TODO(khorimoto): Return different statuses based on whether we have an
-  // active connection.
+  switch (connection_manager_->GetStatus()) {
+    case ConnectionManager::Status::kDisconnected:
+      return FeatureStatus::kEnabledButDisconnected;
+    case ConnectionManager::Status::kConnecting:
+      return FeatureStatus::kEnabledAndConnecting;
+    case ConnectionManager::Status::kConnected:
+      return FeatureStatus::kEnabledAndConnected;
+  }
+
   return FeatureStatus::kEnabledButDisconnected;
 }
 

chromeos/components/phonehub/feature_status_provider_impl.h

@@ -7,6 +7,7 @@
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "chromeos/components/phonehub/connection_manager.h"
 #include "chromeos/components/phonehub/feature_status_provider.h"
 #include "chromeos/services/device_sync/public/cpp/device_sync_client.h"
 #include "chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
@@ -23,11 +24,13 @@ class FeatureStatusProviderImpl
     : public FeatureStatusProvider,
       public device_sync::DeviceSyncClient::Observer,
       public multidevice_setup::MultiDeviceSetupClient::Observer,
-      public device::BluetoothAdapter::Observer {
+      public device::BluetoothAdapter::Observer,
+      public ConnectionManager::Observer {
  public:
   FeatureStatusProviderImpl(
       device_sync::DeviceSyncClient* device_sync_client,
-      multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client);
+      multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client,
+      ConnectionManager* connection_manager);
   ~FeatureStatusProviderImpl() override;
 
  private:
@@ -60,8 +63,12 @@ class FeatureStatusProviderImpl
   void AdapterPoweredChanged(device::BluetoothAdapter* adapter,
                              bool powered) override;
 
+  // ConnectionManager::Observer:
+  void OnStatusChanged() override;
+
   device_sync::DeviceSyncClient* device_sync_client_;
   multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client_;
+  ConnectionManager* connection_manager_;
 
   scoped_refptr<device::BluetoothAdapter> bluetooth_adapter_;
   base::Optional<FeatureStatus> status_;

chromeos/components/phonehub/feature_status_provider_impl_unittest.cc

@@ -8,6 +8,7 @@
 
 #include "base/test/task_environment.h"
 #include "chromeos/components/multidevice/remote_device_test_util.h"
+#include "chromeos/components/phonehub/fake_connection_manager.h"
 #include "chromeos/services/device_sync/public/cpp/fake_device_sync_client.h"
 #include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h"
 #include "device/bluetooth/bluetooth_adapter_factory.h"
@@ -101,7 +102,8 @@ class FeatureStatusProviderImplTest : public testing::Test {
     device::BluetoothAdapterFactory::SetAdapterForTesting(mock_adapter_);
 
     provider_ = std::make_unique<FeatureStatusProviderImpl>(
-        &fake_device_sync_client_, &fake_multidevice_setup_client_);
+        &fake_device_sync_client_, &fake_multidevice_setup_client_,
+        &fake_connection_manager_);
     provider_->AddObserver(&fake_observer_);
   }
 
@@ -159,6 +161,10 @@ class FeatureStatusProviderImplTest : public testing::Test {
     impl->AdapterPoweredChanged(mock_adapter_.get(), powered);
   }
 
+  void SetConnectionStatus(ConnectionManager::Status status) {
+    fake_connection_manager_.SetStatus(status);
+  }
+
   FeatureStatus GetStatus() const { return provider_->GetStatus(); }
 
   size_t GetNumObserverCalls() const { return fake_observer_.num_calls(); }
@@ -171,6 +177,7 @@ class FeatureStatusProviderImplTest : public testing::Test {
 
   device_sync::FakeDeviceSyncClient fake_device_sync_client_;
   multidevice_setup::FakeMultiDeviceSetupClient fake_multidevice_setup_client_;
+  FakeConnectionManager fake_connection_manager_;
 
   scoped_refptr<testing::NiceMock<device::MockBluetoothAdapter>> mock_adapter_;
 
@@ -324,7 +331,7 @@ TEST_F(FeatureStatusProviderImplTest, UnavailableBluetoothOff) {
   EXPECT_EQ(FeatureStatus::kUnavailableBluetoothOff, GetStatus());
 }
 
-TEST_F(FeatureStatusProviderImplTest, TransitionBetweenStatuses) {
+TEST_F(FeatureStatusProviderImplTest, TransitionBetweenAllStatuses) {
   EXPECT_EQ(FeatureStatus::kNotEligibleForFeature, GetStatus());
 
   SetMultiDeviceState(HostStatus::kEligibleHostExistsButNoHostSet,
@@ -350,6 +357,59 @@ TEST_F(FeatureStatusProviderImplTest, TransitionBetweenStatuses) {
   SetAdapterPoweredState(true);
   EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
   EXPECT_EQ(5u, GetNumObserverCalls());
+
+  SetConnectionStatus(ConnectionManager::Status::kConnecting);
+  EXPECT_EQ(FeatureStatus::kEnabledAndConnecting, GetStatus());
+  EXPECT_EQ(6u, GetNumObserverCalls());
+
+  SetConnectionStatus(ConnectionManager::Status::kConnected);
+  EXPECT_EQ(FeatureStatus::kEnabledAndConnected, GetStatus());
+  EXPECT_EQ(7u, GetNumObserverCalls());
+
+  SetConnectionStatus(ConnectionManager::Status::kDisconnected);
+  EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
+  EXPECT_EQ(8u, GetNumObserverCalls());
+}
+
+TEST_F(FeatureStatusProviderImplTest, AttemptingConnection) {
+  SetEligibleSyncedDevices();
+  SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kEnabledByUser);
+  EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
+  EXPECT_EQ(1u, GetNumObserverCalls());
+
+  SetConnectionStatus(ConnectionManager::Status::kConnecting);
+  EXPECT_EQ(FeatureStatus::kEnabledAndConnecting, GetStatus());
+  EXPECT_EQ(2u, GetNumObserverCalls());
+}
+
+TEST_F(FeatureStatusProviderImplTest, AttemptedConnectionSuccessful) {
+  SetEligibleSyncedDevices();
+  SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kEnabledByUser);
+  EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
+  EXPECT_EQ(1u, GetNumObserverCalls());
+
+  SetConnectionStatus(ConnectionManager::Status::kConnecting);
+  EXPECT_EQ(FeatureStatus::kEnabledAndConnecting, GetStatus());
+  EXPECT_EQ(2u, GetNumObserverCalls());
+
+  SetConnectionStatus(ConnectionManager::Status::kConnected);
+  EXPECT_EQ(FeatureStatus::kEnabledAndConnected, GetStatus());
+  EXPECT_EQ(3u, GetNumObserverCalls());
+}
+
+TEST_F(FeatureStatusProviderImplTest, AttemptedConnectionFailed) {
+  SetEligibleSyncedDevices();
+  SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kEnabledByUser);
+  EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
+  EXPECT_EQ(1u, GetNumObserverCalls());
+
+  SetConnectionStatus(ConnectionManager::Status::kConnecting);
+  EXPECT_EQ(FeatureStatus::kEnabledAndConnecting, GetStatus());
+  EXPECT_EQ(2u, GetNumObserverCalls());
+
+  SetConnectionStatus(ConnectionManager::Status::kDisconnected);
+  EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
+  EXPECT_EQ(3u, GetNumObserverCalls());
 }
 
 }  // namespace phonehub

chromeos/components/phonehub/phone_hub_manager_impl.cc

@@ -29,7 +29,8 @@ PhoneHubManagerImpl::PhoneHubManagerImpl(
                                                   secure_channel_client)),
       feature_status_provider_(std::make_unique<FeatureStatusProviderImpl>(
           device_sync_client,
-          multidevice_setup_client)),
+          multidevice_setup_client,
+          connection_manager_.get())),
       find_my_device_controller_(
           std::make_unique<FindMyDeviceControllerImpl>()),
       notification_access_manager_(