Commit 3c985325 authored by Richard Knoll's avatar Richard Knoll Committed by Commit Bot

De-duplicate sharing device candidates by name.

Disabling and re-enabling sync creates a new device id. This would lead
to multiple entries per physical device until the "old" device is not
considered active anymore.
This will de-duplicate devices based on their name and only show the
ones with the latest last sync time.

Bug: 978395
Change-Id: I85f4a9160722244113378a8a92aa9bf300aa2875
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1675430Reviewed-by: default avatarMichael van Ouwerkerk <mvanouwerkerk@chromium.org>
Commit-Queue: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Commit-Queue: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672069}
parent 9d78415f
......@@ -4,7 +4,9 @@
#include "chrome/browser/sharing/sharing_service.h"
#include <algorithm>
#include <map>
#include <unordered_set>
#include "base/time/time.h"
#include "chrome/browser/sharing/sharing_device_info.h"
......@@ -38,10 +40,21 @@ std::vector<SharingDeviceInfo> SharingService::GetDeviceCandidates(
const base::Time min_updated_time = base::Time::Now() - kDeviceExpiration;
// Sort the DeviceInfo vector so the most recently modified devices are first.
std::sort(all_devices.begin(), all_devices.end(),
[](const auto& device1, const auto& device2) {
return device1->last_updated_timestamp() >
device2->last_updated_timestamp();
});
std::unordered_set<std::string> device_names;
std::vector<SharingDeviceInfo> device_candidates;
for (const auto& device : all_devices) {
// If the current device is considered expired for our purposes, stop here
// since the next devices in the vector are at least as expired than this
// one.
if (device->last_updated_timestamp() < min_updated_time)
continue;
break;
auto synced_device = synced_devices.find(device->guid());
if (synced_device == synced_devices.end())
......@@ -51,9 +64,13 @@ std::vector<SharingDeviceInfo> SharingService::GetDeviceCandidates(
if ((device_capabilities & required_capabilities) != required_capabilities)
continue;
device_candidates.emplace_back(device->guid(), device->client_name(),
device->last_updated_timestamp(),
device_capabilities);
// Only insert the first occurrence of each device name.
auto inserted = device_names.insert(device->client_name());
if (inserted.second) {
device_candidates.emplace_back(device->guid(), device->client_name(),
device->last_updated_timestamp(),
device_capabilities);
}
}
// TODO(knollr): Remove devices from |sync_prefs_| that are in
......
......@@ -127,3 +127,26 @@ TEST_F(SharingServiceTest, GetDeviceCandidates_MissingRequirements) {
EXPECT_TRUE(candidates.empty());
}
TEST_F(SharingServiceTest, GetDeviceCandidates_DuplicateDeviceNames) {
// Add first device.
std::string id1 = base::GenerateGUID();
std::unique_ptr<syncer::DeviceInfo> device_info_1 = CreateFakeDeviceInfo(id1);
device_info_tracker_.Add(device_info_1.get());
sync_prefs_->SetSyncDevice(id1, CreateFakeSyncDevice());
// Advance time for a bit to create a newer device.
scoped_task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(10));
// Add second device.
std::string id2 = base::GenerateGUID();
std::unique_ptr<syncer::DeviceInfo> device_info_2 = CreateFakeDeviceInfo(id2);
device_info_tracker_.Add(device_info_2.get());
sync_prefs_->SetSyncDevice(id2, CreateFakeSyncDevice());
std::vector<SharingDeviceInfo> candidates =
sharing_service_->GetDeviceCandidates(kNoCapabilities);
ASSERT_EQ(1u, candidates.size());
EXPECT_EQ(id2, candidates[0].guid());
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment