Phonehub: Add additional eligibility checks to host phone

Previously only checked if the host phone is a PhoneHub host but
did not verify whether if it was a BetterTogether host and has a
valid Bluetooth address. This CL adds those checks.

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

chromeos/components/phonehub/feature_status_provider_impl.cc

@@ -29,9 +29,21 @@ using multidevice_setup::mojom::Feature;
 using multidevice_setup::mojom::FeatureState;
 using multidevice_setup::mojom::HostStatus;
 
-bool DoesDeviceSupportPhoneHubHost(const RemoteDeviceRef& device) {
-  return device.GetSoftwareFeatureState(SoftwareFeature::kPhoneHubHost) !=
-         SoftwareFeatureState::kNotSupported;
+bool IsEligiblePhoneHubHost(const RemoteDeviceRef& device) {
+  // Device must be capable of being a multi-device host.
+  if (device.GetSoftwareFeatureState(SoftwareFeature::kBetterTogetherHost) ==
+      SoftwareFeatureState::kNotSupported) {
+    return false;
+  }
+
+  if (device.GetSoftwareFeatureState(SoftwareFeature::kPhoneHubHost) ==
+      SoftwareFeatureState::kNotSupported) {
+    return false;
+  }
+
+  // Device must have a synced Bluetooth public address, which is used to
+  // bootstrap Phone Hub connections.
+  return !device.bluetooth_public_address().empty();
 }
 
 bool IsEligibleForFeature(
@@ -71,27 +83,12 @@ bool IsEligibleForFeature(
   // If there is a host device available, check if the device is eligible for
   // Phonehub.
   if (host_status.second.has_value())
-    return DoesDeviceSupportPhoneHubHost(*(host_status.second));
+    return IsEligiblePhoneHubHost(*(host_status.second));
 
   // Otherwise, check if there is any available remote device that is
   // eligible for Phonehub.
   for (const RemoteDeviceRef& device : remote_devices) {
-    // Device must be capable of being a multi-device host.
-    if (device.GetSoftwareFeatureState(SoftwareFeature::kBetterTogetherHost) ==
-        SoftwareFeatureState::kNotSupported) {
-      continue;
-    }
-
-    // Device must be capable of being a Phone Hub host.
-    if (!DoesDeviceSupportPhoneHubHost(device)) {
-      continue;
-    }
-
-    // Device must have a synced Bluetooth public address, which is used to
-    // bootstrap Phone Hub connections.
-    if (device.bluetooth_public_address().empty())
-      continue;
-
+    if (IsEligiblePhoneHubHost(device))
       return true;
   }
 

chromeos/components/phonehub/feature_status_provider_impl_unittest.cc

@@ -143,11 +143,13 @@ class FeatureStatusProviderImplTest : public testing::Test {
 
   void SetMultiDeviceState(HostStatus host_status,
                            FeatureState feature_state,
-                           bool supports_phone_hub) {
+                           bool supports_better_together_host,
+                           bool supports_phone_hub,
+                           bool has_bluetooth_address) {
     fake_multidevice_setup_client_.SetHostStatusWithDevice(std::make_pair(
-        host_status, CreatePhoneDevice(/*supports_better_together_host=*/true,
-                                       supports_phone_hub,
-                                       /*has_bluetooth_address=*/true)));
+        host_status,
+        CreatePhoneDevice(supports_better_together_host, supports_phone_hub,
+                          has_bluetooth_address)));
     fake_multidevice_setup_client_.SetFeatureState(Feature::kPhoneHub,
                                                    feature_state);
   }
@@ -327,14 +329,18 @@ TEST_F(FeatureStatusProviderImplTest, EligiblePhoneButNotSetUp) {
   SetEligibleSyncedDevices();
   SetMultiDeviceState(HostStatus::kEligibleHostExistsButNoHostSet,
                       FeatureState::kUnavailableNoVerifiedHost,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kEligiblePhoneButNotSetUp, GetStatus());
 }
 
 TEST_F(FeatureStatusProviderImplTest, NoEligiblePhones) {
   SetMultiDeviceState(HostStatus::kNoEligibleHosts,
                       FeatureState::kUnavailableNoVerifiedHost,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kNotEligibleForFeature, GetStatus());
 }
 
@@ -351,7 +357,23 @@ TEST_F(FeatureStatusProviderImplTest, MultiPhoneEligibility) {
                                       /*has_bluetooth_address=*/true)});
   SetMultiDeviceState(HostStatus::kEligibleHostExistsButNoHostSet,
                       FeatureState::kUnavailableNoVerifiedHost,
-                      /*supports_phone_hub=*/false);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/false,
+                      /*has_bluetooth_address=*/true);
+  EXPECT_EQ(FeatureStatus::kNotEligibleForFeature, GetStatus());
+
+  SetMultiDeviceState(HostStatus::kEligibleHostExistsButNoHostSet,
+                      FeatureState::kUnavailableNoVerifiedHost,
+                      /*supports_better_together_host=*/false,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
+  EXPECT_EQ(FeatureStatus::kNotEligibleForFeature, GetStatus());
+
+  SetMultiDeviceState(HostStatus::kEligibleHostExistsButNoHostSet,
+                      FeatureState::kUnavailableNoVerifiedHost,
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/false);
   EXPECT_EQ(FeatureStatus::kNotEligibleForFeature, GetStatus());
 
   // Simulate no host device connected and expect to detect one eligible host.
@@ -366,17 +388,23 @@ TEST_F(FeatureStatusProviderImplTest, PhoneSelectedAndPendingSetup) {
   SetMultiDeviceState(
       HostStatus::kHostSetLocallyButWaitingForBackendConfirmation,
       FeatureState::kUnavailableNoVerifiedHost,
-      /*supports_phone_hub=*/true);
+      /*supports_better_together_host=*/true,
+      /*supports_phone_hub=*/true,
+      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kPhoneSelectedAndPendingSetup, GetStatus());
 
   SetMultiDeviceState(HostStatus::kHostSetButNotYetVerified,
                       FeatureState::kUnavailableNoVerifiedHost,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kPhoneSelectedAndPendingSetup, GetStatus());
 
   SetMultiDeviceState(HostStatus::kHostVerified,
                       FeatureState::kNotSupportedByPhone,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kPhoneSelectedAndPendingSetup, GetStatus());
 }
 
@@ -384,24 +412,32 @@ TEST_F(FeatureStatusProviderImplTest, Disabled) {
   SetEligibleSyncedDevices();
 
   SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kDisabledByUser,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kDisabled, GetStatus());
 
   SetMultiDeviceState(HostStatus::kHostVerified,
                       FeatureState::kUnavailableSuiteDisabled,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kDisabled, GetStatus());
 
   SetMultiDeviceState(HostStatus::kHostVerified,
                       FeatureState::kUnavailableTopLevelFeatureDisabled,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kDisabled, GetStatus());
 }
 
 TEST_F(FeatureStatusProviderImplTest, UnavailableBluetoothOff) {
   SetEligibleSyncedDevices();
   SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kEnabledByUser,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
 
   SetAdapterPoweredState(false);
   SetAdapterPresentState(false);
@@ -421,30 +457,40 @@ TEST_F(FeatureStatusProviderImplTest, TransitionBetweenAllStatuses) {
 
   SetMultiDeviceState(HostStatus::kNoEligibleHosts,
                       FeatureState::kUnavailableNoVerifiedHost,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kNotEligibleForFeature, GetStatus());
 
   SetMultiDeviceState(HostStatus::kEligibleHostExistsButNoHostSet,
                       FeatureState::kUnavailableNoVerifiedHost,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   SetEligibleSyncedDevices();
   EXPECT_EQ(FeatureStatus::kEligiblePhoneButNotSetUp, GetStatus());
   EXPECT_EQ(1u, GetNumObserverCalls());
 
   SetMultiDeviceState(HostStatus::kHostSetButNotYetVerified,
                       FeatureState::kNotSupportedByPhone,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kPhoneSelectedAndPendingSetup, GetStatus());
   EXPECT_EQ(2u, GetNumObserverCalls());
 
   SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kDisabledByUser,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kDisabled, GetStatus());
   EXPECT_EQ(3u, GetNumObserverCalls());
 
   SetAdapterPoweredState(false);
   SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kEnabledByUser,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kUnavailableBluetoothOff, GetStatus());
   EXPECT_EQ(4u, GetNumObserverCalls());
 
@@ -478,7 +524,9 @@ TEST_F(FeatureStatusProviderImplTest, TransitionBetweenAllStatuses) {
 TEST_F(FeatureStatusProviderImplTest, AttemptingConnection) {
   SetEligibleSyncedDevices();
   SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kEnabledByUser,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
   EXPECT_EQ(1u, GetNumObserverCalls());
 
@@ -490,7 +538,9 @@ TEST_F(FeatureStatusProviderImplTest, AttemptingConnection) {
 TEST_F(FeatureStatusProviderImplTest, AttemptedConnectionSuccessful) {
   SetEligibleSyncedDevices();
   SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kEnabledByUser,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
   EXPECT_EQ(1u, GetNumObserverCalls());
 
@@ -506,7 +556,9 @@ TEST_F(FeatureStatusProviderImplTest, AttemptedConnectionSuccessful) {
 TEST_F(FeatureStatusProviderImplTest, AttemptedConnectionFailed) {
   SetEligibleSyncedDevices();
   SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kEnabledByUser,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
   EXPECT_EQ(1u, GetNumObserverCalls());
 
@@ -522,7 +574,9 @@ TEST_F(FeatureStatusProviderImplTest, AttemptedConnectionFailed) {
 TEST_F(FeatureStatusProviderImplTest, LockScreenStatusUpdate) {
   SetEligibleSyncedDevices();
   SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kEnabledByUser,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
   EXPECT_EQ(1u, GetNumObserverCalls());
 
@@ -540,7 +594,9 @@ TEST_F(FeatureStatusProviderImplTest, LockScreenStatusUpdate) {
 TEST_F(FeatureStatusProviderImplTest, HandlePowerSuspend) {
   SetEligibleSyncedDevices();
   SetMultiDeviceState(HostStatus::kHostVerified, FeatureState::kEnabledByUser,
-                      /*supports_phone_hub=*/true);
+                      /*supports_better_together_host=*/true,
+                      /*supports_phone_hub=*/true,
+                      /*has_bluetooth_address=*/true);
   EXPECT_EQ(FeatureStatus::kEnabledButDisconnected, GetStatus());
   EXPECT_EQ(1u, GetNumObserverCalls());