[Eche] Override host feature status via PhoneHub message.

In PhoneHub, grab incoming Eche FeatureStatus from the phone,
and persist it to prefs. Consume this pref within the general
FeatureStateManager and apply it to Eche on the Chromebook
like so:
  * if the phone returns "unspecified", continue with prior
    behavior (consider Eche supported if kEcheHost is either
    supported or enabled).
  * otherwise, entirely dismiss the value of kEcheHost:
    * if the phone returns "supported", "enabled", or
      "prohibited by policy", consider Eche supported.
    * if the phone returns "unsupported" or "failed
      attestation", consider Eche not supported.

Bug: b:246842111
Change-Id: I782988291ce0ec285899d142bac9377a0baa383c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3914082
Reviewed-by: Jon Mann <jonmann@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1052160}

ash/components/phonehub/phone_hub_manager_impl.cc

@@ -130,7 +130,8 @@ PhoneHubManagerImpl::PhoneHubManagerImpl(
           notification_processor_.get(),
           multidevice_setup_client,
           phone_model_.get(),
-          recent_apps_interaction_handler_.get())),
+          recent_apps_interaction_handler_.get(),
+          pref_service)),
       tether_controller_(
           std::make_unique<TetherControllerImpl>(phone_model_.get(),
                                                  user_action_recorder_.get(),

ash/components/phonehub/phone_status_processor.cc

@@ -16,9 +16,11 @@
 #include "ash/components/phonehub/recent_apps_interaction_handler.h"
 #include "ash/components/phonehub/screen_lock_manager_impl.h"
 #include "ash/constants/ash_features.h"
+#include "ash/services/multidevice_setup/public/cpp/prefs.h"
 #include "ash/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
 #include "base/containers/flat_set.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/prefs/pref_service.h"
 
 namespace ash {
 namespace phonehub {
@@ -192,7 +194,8 @@ PhoneStatusProcessor::PhoneStatusProcessor(
     NotificationProcessor* notification_processor_,
     MultiDeviceSetupClient* multidevice_setup_client,
     MutablePhoneModel* phone_model,
-    RecentAppsInteractionHandler* recent_apps_interaction_handler)
+    RecentAppsInteractionHandler* recent_apps_interaction_handler,
+    PrefService* pref_service)
     : do_not_disturb_controller_(do_not_disturb_controller),
       feature_status_provider_(feature_status_provider),
       message_receiver_(message_receiver),
@@ -202,7 +205,8 @@ PhoneStatusProcessor::PhoneStatusProcessor(
       notification_processor_(notification_processor_),
       multidevice_setup_client_(multidevice_setup_client),
       phone_model_(phone_model),
-      recent_apps_interaction_handler_(recent_apps_interaction_handler) {
+      recent_apps_interaction_handler_(recent_apps_interaction_handler),
+      pref_service_(pref_service) {
   DCHECK(do_not_disturb_controller_);
   DCHECK(feature_status_provider_);
   DCHECK(message_receiver_);
@@ -211,6 +215,7 @@ PhoneStatusProcessor::PhoneStatusProcessor(
   DCHECK(notification_processor_);
   DCHECK(multidevice_setup_client_);
   DCHECK(phone_model_);
+  DCHECK(pref_service_);
 
   message_receiver_->AddObserver(this);
   feature_status_provider_->AddObserver(this);
@@ -283,6 +288,9 @@ void PhoneStatusProcessor::SetReceivedPhoneStatusModelStates(
   if (features::IsEcheSWAEnabled()) {
     recent_apps_interaction_handler_->set_user_states(
         GetUserStates(phone_properties.user_states()));
+
+    SetEcheFeatureStatusReceivedFromPhoneHub(
+        phone_properties.eche_feature_status());
   }
 
   multidevice_feature_access_manager_->SetFeatureSetupRequestSupportedInternal(
@@ -300,6 +308,38 @@ void PhoneStatusProcessor::MaybeSetPhoneModelName(
   phone_model_->SetPhoneName(base::UTF8ToUTF16(remote_device->name()));
 }
 
+void PhoneStatusProcessor::SetEcheFeatureStatusReceivedFromPhoneHub(
+    proto::FeatureStatus eche_feature_status) {
+  auto eche_support_received_from_phone_hub =
+      ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kNotSpecified;
+  if (eche_feature_status == proto::FeatureStatus::FEATURE_STATUS_SUPPORTED ||
+      eche_feature_status == proto::FeatureStatus::FEATURE_STATUS_ENABLED ||
+      eche_feature_status ==
+          proto::FeatureStatus::FEATURE_STATUS_PROHIBITED_BY_POLICY) {
+    eche_support_received_from_phone_hub =
+        ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kSupported;
+  } else if (eche_feature_status ==
+                 proto::FeatureStatus::FEATURE_STATUS_UNSUPPORTED ||
+             eche_feature_status ==
+                 proto::FeatureStatus::FEATURE_STATUS_ATTESTATION_FAILED) {
+    eche_support_received_from_phone_hub =
+        ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kNotSupported;
+  } else if (eche_feature_status ==
+             proto::FeatureStatus::FEATURE_STATUS_UNSPECIFIED) {
+    eche_support_received_from_phone_hub =
+        ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kNotSpecified;
+  } else {
+    NOTREACHED();
+    eche_support_received_from_phone_hub =
+        ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kNotSpecified;
+  }
+
+  pref_service_->SetInteger(
+      ash::multidevice_setup::
+          kEcheOverriddenSupportReceivedFromPhoneHubPrefName,
+      static_cast<int>(eche_support_received_from_phone_hub));
+}
+
 void PhoneStatusProcessor::OnFeatureStatusChanged() {
   // Reset phone model instance when but still keep the phone's name.
   if (feature_status_provider_->GetStatus() !=

ash/components/phonehub/phone_status_processor.h

@@ -12,8 +12,9 @@
 #include "ash/components/phonehub/proto/phonehub_api.pb.h"
 #include "ash/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
 
-namespace ash {
-namespace phonehub {
+class PrefService;
+
+namespace ash::phonehub {
 
 using ::google::protobuf::RepeatedPtrField;
 
@@ -42,7 +43,8 @@ class PhoneStatusProcessor
       NotificationProcessor* notification_processor_,
       multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client,
       MutablePhoneModel* phone_model,
-      RecentAppsInteractionHandler* recent_apps_interaction_handler);
+      RecentAppsInteractionHandler* recent_apps_interaction_handler,
+      PrefService* pref_service);
   ~PhoneStatusProcessor() override;
 
   PhoneStatusProcessor(const PhoneStatusProcessor&) = delete;
@@ -76,7 +78,8 @@ class PhoneStatusProcessor
   void MaybeSetPhoneModelName(
       const absl::optional<multidevice::RemoteDeviceRef>& remote_device);
 
-  void SetDoNotDisturbState(proto::NotificationMode mode);
+  void SetEcheFeatureStatusReceivedFromPhoneHub(
+      proto::FeatureStatus eche_feature_status);
 
   DoNotDisturbController* do_not_disturb_controller_;
   FeatureStatusProvider* feature_status_provider_;
@@ -88,9 +91,9 @@ class PhoneStatusProcessor
   multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client_;
   MutablePhoneModel* phone_model_;
   RecentAppsInteractionHandler* recent_apps_interaction_handler_;
+  PrefService* pref_service_;
 };
 
-}  // namespace phonehub
-}  // namespace ash
+}  // namespace ash::phonehub
 
 #endif  // ASH_COMPONENTS_PHONEHUB_PHONE_STATUS_PROCESSOR_H_

ash/components/phonehub/phone_status_processor_unittest.cc

@@ -25,10 +25,12 @@
 #include "ash/components/phonehub/proto/phonehub_api.pb.h"
 #include "ash/constants/ash_features.h"
 #include "ash/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h"
+#include "ash/services/multidevice_setup/public/cpp/prefs.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "chromeos/ash/components/multidevice/remote_device_test_util.h"
+#include "components/prefs/testing_pref_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/image/image.h"
 
@@ -93,6 +95,9 @@ class PhoneStatusProcessorTest : public testing::Test {
         std::make_unique<multidevice_setup::FakeMultiDeviceSetupClient>();
     fake_recent_apps_interaction_handler_ =
         std::make_unique<FakeRecentAppsInteractionHandler>();
+
+    multidevice_setup::RegisterFeaturePrefs(pref_service_.registry());
+
     scoped_feature_list_.InitWithFeatures(
         /*enabled_features=*/{features::kEcheSWA,
                               features::kPhoneHubCameraRoll},
@@ -107,7 +112,7 @@ class PhoneStatusProcessorTest : public testing::Test {
         fake_multidevice_feature_access_manager_.get(),
         fake_screen_lock_manager_.get(), fake_notification_processor_.get(),
         fake_multidevice_setup_client_.get(), mutable_phone_model_.get(),
-        fake_recent_apps_interaction_handler_.get());
+        fake_recent_apps_interaction_handler_.get(), &pref_service_);
   }
 
   void InitializeNotificationProto(proto::Notification* notification,
@@ -133,6 +138,14 @@ class PhoneStatusProcessorTest : public testing::Test {
     notification->set_shared_image("123");
   }
 
+  ash::multidevice_setup::EcheSupportReceivedFromPhoneHub
+  GetEcheSupportReceivedFromPhoneHub() {
+    return static_cast<ash::multidevice_setup::EcheSupportReceivedFromPhoneHub>(
+        pref_service_.GetInteger(
+            ash::multidevice_setup::
+                kEcheOverriddenSupportReceivedFromPhoneHubPrefName));
+  }
+
   base::test::ScopedFeatureList scoped_feature_list_;
   multidevice::RemoteDeviceRef test_remote_device_;
   std::unique_ptr<FakeDoNotDisturbController> fake_do_not_disturb_controller_;
@@ -149,6 +162,7 @@ class PhoneStatusProcessorTest : public testing::Test {
       fake_multidevice_setup_client_;
   std::unique_ptr<FakeRecentAppsInteractionHandler>
       fake_recent_apps_interaction_handler_;
+  TestingPrefServiceSimple pref_service_;
   std::unique_ptr<PhoneStatusProcessor> phone_status_processor_;
 };
 
@@ -287,6 +301,8 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusSnapshotUpdate) {
   proto::FeatureSetupConfig* feature_setup_config =
       expected_phone_properties->mutable_feature_setup_config();
   feature_setup_config->set_feature_setup_request_supported(true);
+  expected_phone_properties->set_eche_feature_status(
+      proto::FeatureStatus::FEATURE_STATUS_SUPPORTED);
 
   expected_phone_properties->add_user_states();
   proto::UserState* mutable_user_state =
@@ -342,6 +358,9 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusSnapshotUpdate) {
   EXPECT_EQ(PhoneStatusModel::MobileStatus::kSimWithReception,
             phone_status_model->mobile_status());
 
+  EXPECT_EQ(ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kSupported,
+            GetEcheSupportReceivedFromPhoneHub());
+
   // Change feature status to disconnected.
   fake_feature_status_provider_->SetStatus(
       FeatureStatus::kEnabledButDisconnected);
@@ -388,6 +407,8 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusUpdate) {
   proto::FeatureSetupConfig* feature_setup_config =
       expected_phone_properties->mutable_feature_setup_config();
   feature_setup_config->set_feature_setup_request_supported(false);
+  expected_phone_properties->set_eche_feature_status(
+      proto::FeatureStatus::FEATURE_STATUS_SUPPORTED);
 
   expected_phone_properties->add_user_states();
   proto::UserState* mutable_user_state =
@@ -439,6 +460,9 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusUpdate) {
   EXPECT_EQ(PhoneStatusModel::MobileStatus::kSimWithReception,
             phone_status_model->mobile_status());
 
+  EXPECT_EQ(ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kSupported,
+            GetEcheSupportReceivedFromPhoneHub());
+
   std::vector<RecentAppsInteractionHandler::UserState> user_states =
       fake_recent_apps_interaction_handler_->user_states();
   EXPECT_EQ(1u, user_states[0].user_id);
@@ -483,6 +507,9 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusUpdate) {
   EXPECT_EQ(PhoneStatusModel::MobileStatus::kSimWithReception,
             phone_status_model->mobile_status());
 
+  EXPECT_EQ(ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kSupported,
+            GetEcheSupportReceivedFromPhoneHub());
+
   // Change feature status to disconnected.
   fake_feature_status_provider_->SetStatus(
       FeatureStatus::kEnabledButDisconnected);
@@ -606,5 +633,59 @@ TEST_F(PhoneStatusProcessorTest, NotificationAccess) {
   EXPECT_EQ(2u, fake_notification_manager_->num_notifications());
 }
 
+TEST_F(PhoneStatusProcessorTest, EcheFeatureStatus) {
+  fake_multidevice_setup_client_->SetHostStatusWithDevice(
+      std::make_pair(HostStatus::kHostVerified, test_remote_device_));
+  CreatePhoneStatusProcessor();
+
+  fake_feature_status_provider_->SetStatus(FeatureStatus::kEnabledAndConnected);
+  fake_multidevice_setup_client_->SetFeatureState(
+      Feature::kPhoneHubNotifications, FeatureState::kEnabledByUser);
+
+  auto expected_phone_properties = std::make_unique<proto::PhoneProperties>();
+  expected_phone_properties->set_eche_feature_status(
+      proto::FeatureStatus::FEATURE_STATUS_UNSPECIFIED);
+
+  proto::PhoneStatusUpdate expected_update;
+  expected_update.set_allocated_properties(expected_phone_properties.release());
+
+  fake_message_receiver_->NotifyPhoneStatusUpdateReceived(expected_update);
+  EXPECT_EQ(
+      ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kNotSpecified,
+      GetEcheSupportReceivedFromPhoneHub());
+
+  expected_update.mutable_properties()->set_eche_feature_status(
+      proto::FeatureStatus::FEATURE_STATUS_SUPPORTED);
+  fake_message_receiver_->NotifyPhoneStatusUpdateReceived(expected_update);
+  EXPECT_EQ(ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kSupported,
+            GetEcheSupportReceivedFromPhoneHub());
+
+  expected_update.mutable_properties()->set_eche_feature_status(
+      proto::FeatureStatus::FEATURE_STATUS_ENABLED);
+  fake_message_receiver_->NotifyPhoneStatusUpdateReceived(expected_update);
+  EXPECT_EQ(ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kSupported,
+            GetEcheSupportReceivedFromPhoneHub());
+
+  expected_update.mutable_properties()->set_eche_feature_status(
+      proto::FeatureStatus::FEATURE_STATUS_PROHIBITED_BY_POLICY);
+  fake_message_receiver_->NotifyPhoneStatusUpdateReceived(expected_update);
+  EXPECT_EQ(ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kSupported,
+            GetEcheSupportReceivedFromPhoneHub());
+
+  expected_update.mutable_properties()->set_eche_feature_status(
+      proto::FeatureStatus::FEATURE_STATUS_UNSUPPORTED);
+  fake_message_receiver_->NotifyPhoneStatusUpdateReceived(expected_update);
+  EXPECT_EQ(
+      ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kNotSupported,
+      GetEcheSupportReceivedFromPhoneHub());
+
+  expected_update.mutable_properties()->set_eche_feature_status(
+      proto::FeatureStatus::FEATURE_STATUS_ATTESTATION_FAILED);
+  fake_message_receiver_->NotifyPhoneStatusUpdateReceived(expected_update);
+  EXPECT_EQ(
+      ash::multidevice_setup::EcheSupportReceivedFromPhoneHub::kNotSupported,
+      GetEcheSupportReceivedFromPhoneHub());
+}
+
 }  // namespace phonehub
 }  // namespace ash

ash/components/phonehub/proto/phonehub_api.proto

@@ -130,6 +130,20 @@ enum FindMyDeviceCapability {
   NOT_ALLOWED = 1;
 }
 
+enum FeatureStatus {
+  FEATURE_STATUS_UNSPECIFIED = 0;
+  FEATURE_STATUS_UNSUPPORTED = 1;
+  FEATURE_STATUS_SUPPORTED = 2;
+  FEATURE_STATUS_ENABLED = 3;
+
+  // Feature is supported, but disabled by enterprise policy setting.
+  FEATURE_STATUS_PROHIBITED_BY_POLICY = 4;
+
+  // Feature is supported but requires attestation, and the connected device
+  // can not be verified.
+  FEATURE_STATUS_ATTESTATION_FAILED = 5;
+}
+
 // Information about the phone whether is secured with a PIN, pattern or
 // password.
 enum ScreenLockState {
@@ -202,7 +216,9 @@ message PhoneProperties {
 
   FeatureSetupConfig feature_setup_config = 19;
 
-  // Next ID: 20
+  FeatureStatus eche_feature_status = 20;
+
+  // Next ID: 21
 }
 
 message UserState {

ash/services/multidevice_setup/feature_state_manager_impl.cc

@@ -264,6 +264,11 @@ FeatureStateManagerImpl::FeatureStateManagerImpl(
                             base::Unretained(this)));
   }
 
+  registrar_.Add(
+      kEcheOverriddenSupportReceivedFromPhoneHubPrefName,
+      base::BindRepeating(&FeatureStateManagerImpl::OnPrefValueChanged,
+                          base::Unretained(this)));
+
   // Prime the cache. Since this is the initial computation, it does not
   // represent a true change of feature state values, so observers should not be
   // notified.
@@ -495,17 +500,37 @@ bool FeatureStateManagerImpl::HasBeenActivatedByPhone(
     multidevice::SoftwareFeatureState feature_state =
         host_device.GetSoftwareFeatureState(pair.second);
 
-    if (feature_state == multidevice::SoftwareFeatureState::kEnabled) {
-      return true;
-    }
-
     // Edge Case: Eche is considered activated on the host when Phone Hub is
-    // enabled and Eche's state is kSupported or kEnabled.
+    // enabled and either:
+    // * if phone did not specify Eche state via PhoneHub status message:
+    //   * Eche's state is kSupported or kEnabled.
+    // * if phone did specify Eche state via PhoneHub status message:
+    //   * use the specified Eche state.
     if (feature == mojom::Feature::kEche) {
-      return feature_state == multidevice::SoftwareFeatureState::kSupported &&
-             host_device.GetSoftwareFeatureState(
-                 multidevice::SoftwareFeature::kPhoneHubHost) ==
-                 multidevice::SoftwareFeatureState::kEnabled;
+      if (host_device.GetSoftwareFeatureState(
+              multidevice::SoftwareFeature::kPhoneHubHost) !=
+          multidevice::SoftwareFeatureState::kEnabled) {
+        return false;
+      }
+
+      EcheSupportReceivedFromPhoneHub eche_support_received_from_phone_hub =
+          static_cast<EcheSupportReceivedFromPhoneHub>(
+              pref_service_->GetInteger(
+                  kEcheOverriddenSupportReceivedFromPhoneHubPrefName));
+      switch (eche_support_received_from_phone_hub) {
+        case EcheSupportReceivedFromPhoneHub::kNotSpecified:
+          return feature_state ==
+                     multidevice::SoftwareFeatureState::kSupported ||
+                 feature_state == multidevice::SoftwareFeatureState::kEnabled;
+        case EcheSupportReceivedFromPhoneHub::kNotSupported:
+          return false;
+        case EcheSupportReceivedFromPhoneHub::kSupported:
+          return true;
+      };
+    }
+
+    if (feature_state == multidevice::SoftwareFeatureState::kEnabled) {
+      return true;
     }
 
     // Edge Case: features with global states are considered activated on host