[Multidevice] Remove devices that have been inactive for >30 days from

the eligible list.

Bug: 923594
Change-Id: I00f5efb6681e381cedb8938c09554a8796933583
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612264
Commit-Queue: Shane Fitzpatrick <shanefitz@google.com>
Reviewed-by: Josh Nohle <nohle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840857}

chromeos/services/multidevice_setup/eligible_host_devices_provider_impl.cc

@@ -14,6 +14,10 @@ namespace chromeos {
 
 namespace multidevice_setup {
 
+// static
+constexpr base::TimeDelta
+    EligibleHostDevicesProviderImpl::kInactiveDeviceThresholdInDays;
+
 // static
 EligibleHostDevicesProviderImpl::Factory*
     EligibleHostDevicesProviderImpl::Factory::test_factory_ = nullptr;
@@ -119,6 +123,35 @@ void EligibleHostDevicesProviderImpl::OnGetDevicesActivityStatus(
                                       std::move(device_activity_status_ptr)});
   }
 
+  // Remove inactive devices. A device is inactive if it has a
+  // last_activity_time before some defined threshold.
+  base::Time now = base::Time::Now();
+  eligible_active_devices_from_last_sync_.erase(
+      std::remove_if(
+          eligible_active_devices_from_last_sync_.begin(),
+          eligible_active_devices_from_last_sync_.end(),
+          [&id_to_activity_status_map,
+           &now](const multidevice::DeviceWithConnectivityStatus& device) {
+            auto it = id_to_activity_status_map.find(
+                device.remote_device.instance_id());
+
+            if (it == id_to_activity_status_map.end()) {
+              return false;
+            }
+
+            base::Time last_activity_time =
+                std::get<1>(*it)->last_activity_time;
+
+            // Do not filter out devices whose last activity time was not set by
+            // the server.
+            if (last_activity_time == base::Time()) {
+              return false;
+            }
+
+            return now - last_activity_time > kInactiveDeviceThresholdInDays;
+          }),
+      eligible_active_devices_from_last_sync_.end());
+
   // Sort the list preferring online devices, then last activity time, then
   // last update time.
   std::sort(

chromeos/services/multidevice_setup/eligible_host_devices_provider_impl.h

@@ -20,6 +20,9 @@ class EligibleHostDevicesProviderImpl
     : public EligibleHostDevicesProvider,
       public device_sync::DeviceSyncClient::Observer {
  public:
+  static constexpr base::TimeDelta kInactiveDeviceThresholdInDays =
+      base::TimeDelta::FromDays(30);
+
   class Factory {
    public:
     static std::unique_ptr<EligibleHostDevicesProvider> Create(

chromeos/services/multidevice_setup/eligible_host_devices_provider_impl_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "base/stl_util.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/time/time_override.h"
 #include "chromeos/components/multidevice/remote_device_test_util.h"
 #include "chromeos/components/multidevice/software_feature.h"
 #include "chromeos/components/multidevice/software_feature_state.h"
@@ -180,6 +181,11 @@ TEST_P(MultiDeviceSetupEligibleHostDevicesProviderImplTest,
   fake_device_sync_client()->set_synced_devices(devices);
   fake_device_sync_client()->NotifyNewDevicesSynced();
 
+  // Set current time so that no devices are filtered out based on their last
+  // activity time
+  base::subtle::ScopedTimeClockOverrides time_now_override(
+      []() { return base::Time::FromTimeT(20000); }, nullptr, nullptr);
+
   std::vector<device_sync::mojom::DeviceActivityStatusPtr>
       device_activity_statuses;
   device_activity_statuses.emplace_back(
@@ -254,6 +260,67 @@ TEST_P(MultiDeviceSetupEligibleHostDevicesProviderImplTest,
   }
 }
 
+TEST_P(MultiDeviceSetupEligibleHostDevicesProviderImplTest,
+       GetDevicesActivityStatusRemovesInactiveDevices) {
+  if (!use_get_devices_activity_status()) {
+    return;
+  }
+
+  SetBitsOnTestDevices();
+
+  base::subtle::ScopedTimeClockOverrides time_now_override(
+      []() {
+        return base::Time() +
+               EligibleHostDevicesProviderImpl::kInactiveDeviceThresholdInDays +
+               base::TimeDelta::FromDays(1000);
+      },
+      nullptr, nullptr);
+
+  multidevice::RemoteDeviceRefList devices{test_devices()[0], test_devices()[1],
+                                           test_devices()[2], test_devices()[3],
+                                           test_devices()[4]};
+  fake_device_sync_client()->set_synced_devices(devices);
+  fake_device_sync_client()->NotifyNewDevicesSynced();
+
+  std::vector<device_sync::mojom::DeviceActivityStatusPtr>
+      device_activity_statuses;
+  device_activity_statuses.emplace_back(
+      device_sync::mojom::DeviceActivityStatus::New(
+          test_devices()[0].instance_id(), base::Time(),
+          cryptauthv2::ConnectivityStatus::ONLINE));
+  device_activity_statuses.emplace_back(
+      device_sync::mojom::DeviceActivityStatus::New(
+          test_devices()[1].instance_id(),
+          base::Time::Now() -
+              EligibleHostDevicesProviderImpl::kInactiveDeviceThresholdInDays -
+              base::TimeDelta::FromDays(1),
+          cryptauthv2::ConnectivityStatus::OFFLINE));
+  device_activity_statuses.emplace_back(
+      device_sync::mojom::DeviceActivityStatus::New(
+          test_devices()[2].instance_id(),
+          base::Time::Now() -
+              EligibleHostDevicesProviderImpl::kInactiveDeviceThresholdInDays -
+              base::TimeDelta::FromDays(10),
+          cryptauthv2::ConnectivityStatus::ONLINE));
+  device_activity_statuses.emplace_back(
+      device_sync::mojom::DeviceActivityStatus::New(
+          test_devices()[3].instance_id(),
+          base::Time::Now() -
+              EligibleHostDevicesProviderImpl::kInactiveDeviceThresholdInDays,
+          cryptauthv2::ConnectivityStatus::ONLINE));
+
+  fake_device_sync_client()->InvokePendingGetDevicesActivityStatusCallback(
+      device_sync::mojom::NetworkRequestResult::kSuccess,
+      std::move(device_activity_statuses));
+
+  multidevice::DeviceWithConnectivityStatusList eligible_active_devices =
+      provider()->GetEligibleActiveHostDevices();
+
+  EXPECT_EQ(2u, eligible_active_devices.size());
+  EXPECT_EQ(test_devices()[3], eligible_active_devices[0].remote_device);
+  EXPECT_EQ(test_devices()[0], eligible_active_devices[1].remote_device);
+}
+
 TEST_P(MultiDeviceSetupEligibleHostDevicesProviderImplTest,
        GetDevicesActivityStatusFailedRequest) {
   if (!use_get_devices_activity_status()) {