Commit 072b81e1 authored by Josh Nohle's avatar Josh Nohle Committed by Commit Bot

[Nearby] Add self to list of allowed contacts

Always include the logged-in account in the list of contacts uploaded
to the Nearby Share server. Also, mark this contact as "selected".

This automatically enables sharing with other devices logged in with
the same account. This self-sharing works in all- or selected-contacts
visibility modes. Previously, to enable self-sharing, a user would need
to manually add their own account to their list of contacts at
contacts.google.com, for instance.

Verified fix with manual testing.

Fixed: 1138679
Change-Id: Ib3a2137257e4d4c56a43011d747811f522d9ad40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2487244
Auto-Submit: Josh Nohle <nohle@chromium.org>
Commit-Queue: James Vecore <vecore@google.com>
Reviewed-by: default avatarJames Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#818943}
parent 3b5d7481
...@@ -12,10 +12,12 @@ std::unique_ptr<NearbyShareContactManager> ...@@ -12,10 +12,12 @@ std::unique_ptr<NearbyShareContactManager>
FakeNearbyShareContactManager::Factory::CreateInstance( FakeNearbyShareContactManager::Factory::CreateInstance(
PrefService* pref_service, PrefService* pref_service,
NearbyShareClientFactory* http_client_factory, NearbyShareClientFactory* http_client_factory,
NearbyShareLocalDeviceDataManager* local_device_data_manager) { NearbyShareLocalDeviceDataManager* local_device_data_manager,
const std::string& profile_user_name) {
latest_pref_service_ = pref_service; latest_pref_service_ = pref_service;
latest_http_client_factory_ = http_client_factory; latest_http_client_factory_ = http_client_factory;
latest_local_device_data_manager_ = local_device_data_manager; latest_local_device_data_manager_ = local_device_data_manager;
latest_profile_user_name_ = profile_user_name;
auto instance = std::make_unique<FakeNearbyShareContactManager>(); auto instance = std::make_unique<FakeNearbyShareContactManager>();
instances_.push_back(instance.get()); instances_.push_back(instance.get());
......
...@@ -51,18 +51,24 @@ class FakeNearbyShareContactManager : public NearbyShareContactManager { ...@@ -51,18 +51,24 @@ class FakeNearbyShareContactManager : public NearbyShareContactManager {
return latest_local_device_data_manager_; return latest_local_device_data_manager_;
} }
std::string latest_profile_user_name() const {
return latest_profile_user_name_;
}
private: private:
// NearbyShareContactManagerImpl::Factory: // NearbyShareContactManagerImpl::Factory:
std::unique_ptr<NearbyShareContactManager> CreateInstance( std::unique_ptr<NearbyShareContactManager> CreateInstance(
PrefService* pref_service, PrefService* pref_service,
NearbyShareClientFactory* http_client_factory, NearbyShareClientFactory* http_client_factory,
NearbyShareLocalDeviceDataManager* local_device_data_manager) override; NearbyShareLocalDeviceDataManager* local_device_data_manager,
const std::string& profile_user_name) override;
std::vector<FakeNearbyShareContactManager*> instances_; std::vector<FakeNearbyShareContactManager*> instances_;
PrefService* latest_pref_service_ = nullptr; PrefService* latest_pref_service_ = nullptr;
NearbyShareClientFactory* latest_http_client_factory_ = nullptr; NearbyShareClientFactory* latest_http_client_factory_ = nullptr;
NearbyShareLocalDeviceDataManager* latest_local_device_data_manager_ = NearbyShareLocalDeviceDataManager* latest_local_device_data_manager_ =
nullptr; nullptr;
std::string latest_profile_user_name_;
}; };
FakeNearbyShareContactManager(); FakeNearbyShareContactManager();
......
...@@ -61,6 +61,15 @@ std::vector<nearbyshare::proto::Contact> ContactRecordsToContacts( ...@@ -61,6 +61,15 @@ std::vector<nearbyshare::proto::Contact> ContactRecordsToContacts(
return contacts; return contacts;
} }
nearbyshare::proto::Contact CreateLocalContact(
const std::string& profile_user_name) {
nearbyshare::proto::Contact contact;
contact.mutable_identifier()->set_account_name(profile_user_name);
// Always consider your own account a selected contact.
contact.set_is_selected(true);
return contact;
}
// Creates a hex-encoded hash of the contact data, implicitly including the // Creates a hex-encoded hash of the contact data, implicitly including the
// allowlist, to be sent to the Nearby Share server. This hash is persisted and // allowlist, to be sent to the Nearby Share server. This hash is persisted and
// used to detect any changes to the user's contact list or allowlist since the // used to detect any changes to the user's contact list or allowlist since the
...@@ -138,13 +147,16 @@ std::unique_ptr<NearbyShareContactManager> ...@@ -138,13 +147,16 @@ std::unique_ptr<NearbyShareContactManager>
NearbyShareContactManagerImpl::Factory::Create( NearbyShareContactManagerImpl::Factory::Create(
PrefService* pref_service, PrefService* pref_service,
NearbyShareClientFactory* http_client_factory, NearbyShareClientFactory* http_client_factory,
NearbyShareLocalDeviceDataManager* local_device_data_manager) { NearbyShareLocalDeviceDataManager* local_device_data_manager,
const std::string& profile_user_name) {
if (test_factory_) { if (test_factory_) {
return test_factory_->CreateInstance(pref_service, http_client_factory, return test_factory_->CreateInstance(pref_service, http_client_factory,
local_device_data_manager); local_device_data_manager,
profile_user_name);
} }
return base::WrapUnique(new NearbyShareContactManagerImpl( return base::WrapUnique(new NearbyShareContactManagerImpl(
pref_service, http_client_factory, local_device_data_manager)); pref_service, http_client_factory, local_device_data_manager,
profile_user_name));
} }
// static // static
...@@ -158,10 +170,12 @@ NearbyShareContactManagerImpl::Factory::~Factory() = default; ...@@ -158,10 +170,12 @@ NearbyShareContactManagerImpl::Factory::~Factory() = default;
NearbyShareContactManagerImpl::NearbyShareContactManagerImpl( NearbyShareContactManagerImpl::NearbyShareContactManagerImpl(
PrefService* pref_service, PrefService* pref_service,
NearbyShareClientFactory* http_client_factory, NearbyShareClientFactory* http_client_factory,
NearbyShareLocalDeviceDataManager* local_device_data_manager) NearbyShareLocalDeviceDataManager* local_device_data_manager,
const std::string& profile_user_name)
: pref_service_(pref_service), : pref_service_(pref_service),
http_client_factory_(http_client_factory), http_client_factory_(http_client_factory),
local_device_data_manager_(local_device_data_manager), local_device_data_manager_(local_device_data_manager),
profile_user_name_(profile_user_name),
contact_download_and_upload_scheduler_( contact_download_and_upload_scheduler_(
NearbyShareSchedulerFactory::CreatePeriodicScheduler( NearbyShareSchedulerFactory::CreatePeriodicScheduler(
kContactDownloadPeriod, kContactDownloadPeriod,
...@@ -250,10 +264,20 @@ void NearbyShareContactManagerImpl::OnContactsDownloadSuccess( ...@@ -250,10 +264,20 @@ void NearbyShareContactManagerImpl::OnContactsDownloadSuccess(
NotifyContactsDownloaded(allowed_contact_ids, contacts); NotifyContactsDownloaded(allowed_contact_ids, contacts);
NotifyMojoObserverContactsDownloaded(allowed_contact_ids, contacts); NotifyMojoObserverContactsDownloaded(allowed_contact_ids, contacts);
// Only request a contacts upload if the contact list or allowlist has changed
// since the last successful upload.
std::vector<nearbyshare::proto::Contact> contacts_to_upload = std::vector<nearbyshare::proto::Contact> contacts_to_upload =
ContactRecordsToContacts(GetAllowedContacts(), contacts); ContactRecordsToContacts(GetAllowedContacts(), contacts);
// Enable cross-device self-share by adding your account to the list of
// contacts. It is also marked as a selected contact.
if (profile_user_name_.empty()) {
NS_LOG(WARNING) << __func__ << ": Profile user name is empty; could not "
<< "add self to list of contacts to upload.";
} else {
contacts_to_upload.push_back(CreateLocalContact(profile_user_name_));
}
// Only request a contacts upload if the contact list or allowlist has changed
// since the last successful upload.
std::string contact_upload_hash = ComputeHash(contacts_to_upload); std::string contact_upload_hash = ComputeHash(contacts_to_upload);
if (contact_upload_hash == if (contact_upload_hash ==
pref_service_->GetString( pref_service_->GetString(
......
...@@ -36,7 +36,7 @@ class PrefService; ...@@ -36,7 +36,7 @@ class PrefService;
// uploaded contact data, and after every contacts download, a subsequent upload // uploaded contact data, and after every contacts download, a subsequent upload
// request is made if we detect that the contact list or allowlist has changed // request is made if we detect that the contact list or allowlist has changed
// since the last successful upload. // since the last successful upload.
//
// In addition to supporting on-demand contact downloads, this implementation // In addition to supporting on-demand contact downloads, this implementation
// periodically checks in with the Nearby Share server to see if the user's // periodically checks in with the Nearby Share server to see if the user's
// contact list has changed since the last upload. // contact list has changed since the last upload.
...@@ -47,7 +47,8 @@ class NearbyShareContactManagerImpl : public NearbyShareContactManager { ...@@ -47,7 +47,8 @@ class NearbyShareContactManagerImpl : public NearbyShareContactManager {
static std::unique_ptr<NearbyShareContactManager> Create( static std::unique_ptr<NearbyShareContactManager> Create(
PrefService* pref_service, PrefService* pref_service,
NearbyShareClientFactory* http_client_factory, NearbyShareClientFactory* http_client_factory,
NearbyShareLocalDeviceDataManager* local_device_data_manager); NearbyShareLocalDeviceDataManager* local_device_data_manager,
const std::string& profile_user_name);
static void SetFactoryForTesting(Factory* test_factory); static void SetFactoryForTesting(Factory* test_factory);
protected: protected:
...@@ -55,7 +56,8 @@ class NearbyShareContactManagerImpl : public NearbyShareContactManager { ...@@ -55,7 +56,8 @@ class NearbyShareContactManagerImpl : public NearbyShareContactManager {
virtual std::unique_ptr<NearbyShareContactManager> CreateInstance( virtual std::unique_ptr<NearbyShareContactManager> CreateInstance(
PrefService* pref_service, PrefService* pref_service,
NearbyShareClientFactory* http_client_factory, NearbyShareClientFactory* http_client_factory,
NearbyShareLocalDeviceDataManager* local_device_data_manager) = 0; NearbyShareLocalDeviceDataManager* local_device_data_manager,
const std::string& profile_user_name) = 0;
private: private:
static Factory* test_factory_; static Factory* test_factory_;
...@@ -67,7 +69,8 @@ class NearbyShareContactManagerImpl : public NearbyShareContactManager { ...@@ -67,7 +69,8 @@ class NearbyShareContactManagerImpl : public NearbyShareContactManager {
NearbyShareContactManagerImpl( NearbyShareContactManagerImpl(
PrefService* pref_service, PrefService* pref_service,
NearbyShareClientFactory* http_client_factory, NearbyShareClientFactory* http_client_factory,
NearbyShareLocalDeviceDataManager* local_device_data_manager); NearbyShareLocalDeviceDataManager* local_device_data_manager,
const std::string& profile_user_name);
// NearbyShareContactsManager: // NearbyShareContactsManager:
void DownloadContacts() override; void DownloadContacts() override;
...@@ -98,6 +101,7 @@ class NearbyShareContactManagerImpl : public NearbyShareContactManager { ...@@ -98,6 +101,7 @@ class NearbyShareContactManagerImpl : public NearbyShareContactManager {
PrefService* pref_service_ = nullptr; PrefService* pref_service_ = nullptr;
NearbyShareClientFactory* http_client_factory_ = nullptr; NearbyShareClientFactory* http_client_factory_ = nullptr;
NearbyShareLocalDeviceDataManager* local_device_data_manager_ = nullptr; NearbyShareLocalDeviceDataManager* local_device_data_manager_ = nullptr;
std::string profile_user_name_;
std::unique_ptr<NearbyShareScheduler> contact_download_and_upload_scheduler_; std::unique_ptr<NearbyShareScheduler> contact_download_and_upload_scheduler_;
std::unique_ptr<NearbyShareContactDownloader> contact_downloader_; std::unique_ptr<NearbyShareContactDownloader> contact_downloader_;
mojo::RemoteSet<nearby_share::mojom::DownloadContactsObserver> observers_set_; mojo::RemoteSet<nearby_share::mojom::DownloadContactsObserver> observers_set_;
......
...@@ -34,6 +34,7 @@ const char kTestContactIdPrefix[] = "id_"; ...@@ -34,6 +34,7 @@ const char kTestContactIdPrefix[] = "id_";
const char kTestContactEmailPrefix[] = "email_"; const char kTestContactEmailPrefix[] = "email_";
const char kTestContactPhonePrefix[] = "phone_"; const char kTestContactPhonePrefix[] = "phone_";
const char kTestDefaultDeviceName[] = "Josh's Chromebook"; const char kTestDefaultDeviceName[] = "Josh's Chromebook";
const char kTestProfileUserName[] = "test@google.com";
// From nearby_share_contact_manager_impl.cc. // From nearby_share_contact_manager_impl.cc.
constexpr base::TimeDelta kContactDownloadPeriod = constexpr base::TimeDelta kContactDownloadPeriod =
...@@ -85,8 +86,10 @@ std::vector<nearbyshare::proto::ContactRecord> TestContactRecordList( ...@@ -85,8 +86,10 @@ std::vector<nearbyshare::proto::ContactRecord> TestContactRecordList(
} }
// Converts a list of ContactRecord protos, along with the allowlist, into a // Converts a list of ContactRecord protos, along with the allowlist, into a
// list of Contact protos. From nearby_share_contact_manager_impl.cc. // list of Contact protos. To enable self-sharing across devices, we expect the
std::vector<nearbyshare::proto::Contact> ContactRecordsToContacts( // local device to include itself in the contact list as an allowed contact.
// Partially from nearby_share_contact_manager_impl.cc.
std::vector<nearbyshare::proto::Contact> BuildContactListToUpload(
const std::set<std::string>& allowed_contact_ids, const std::set<std::string>& allowed_contact_ids,
const std::vector<nearbyshare::proto::ContactRecord>& contact_records) { const std::vector<nearbyshare::proto::ContactRecord>& contact_records) {
std::vector<nearbyshare::proto::Contact> contacts; std::vector<nearbyshare::proto::Contact> contacts;
...@@ -99,6 +102,13 @@ std::vector<nearbyshare::proto::Contact> ContactRecordsToContacts( ...@@ -99,6 +102,13 @@ std::vector<nearbyshare::proto::Contact> ContactRecordsToContacts(
contacts.push_back(contact); contacts.push_back(contact);
} }
} }
// Add self to list of contacts.
nearbyshare::proto::Contact contact;
contact.mutable_identifier()->set_account_name(kTestProfileUserName);
contact.set_is_selected(true);
contacts.push_back(contact);
return contacts; return contacts;
} }
...@@ -154,7 +164,8 @@ class NearbyShareContactManagerImplTest ...@@ -154,7 +164,8 @@ class NearbyShareContactManagerImplTest
&downloader_factory_); &downloader_factory_);
manager_ = NearbyShareContactManagerImpl::Factory::Create( manager_ = NearbyShareContactManagerImpl::Factory::Create(
&pref_service_, &http_client_factory_, &local_device_data_manager_); &pref_service_, &http_client_factory_, &local_device_data_manager_,
kTestProfileUserName);
manager_awaiter_ = manager_awaiter_ =
std::make_unique<nearby_share::mojom::ContactManagerAsyncWaiter>( std::make_unique<nearby_share::mojom::ContactManagerAsyncWaiter>(
manager_.get()); manager_.get());
...@@ -442,7 +453,7 @@ TEST_F(NearbyShareContactManagerImplTest, DownloadContacts_WithUpload) { ...@@ -442,7 +453,7 @@ TEST_F(NearbyShareContactManagerImplTest, DownloadContacts_WithUpload) {
// requested, which succeeds. // requested, which succeeds.
DownloadContacts(); DownloadContacts();
SucceedDownload(contact_records, allowlist, /*expect_upload=*/true); SucceedDownload(contact_records, allowlist, /*expect_upload=*/true);
FinishUpload(/*success=*/true, /*expected_contacts=*/ContactRecordsToContacts( FinishUpload(/*success=*/true, /*expected_contacts=*/BuildContactListToUpload(
allowlist, contact_records)); allowlist, contact_records));
// When contacts are downloaded again, we decect that contacts have not // When contacts are downloaded again, we decect that contacts have not
...@@ -462,7 +473,7 @@ TEST_F(NearbyShareContactManagerImplTest, ...@@ -462,7 +473,7 @@ TEST_F(NearbyShareContactManagerImplTest,
// requested, which succeeds. // requested, which succeeds.
DownloadContacts(); DownloadContacts();
SucceedDownload(contact_records, allowlist, /*expect_upload=*/true); SucceedDownload(contact_records, allowlist, /*expect_upload=*/true);
FinishUpload(/*success=*/true, /*expected_contacts=*/ContactRecordsToContacts( FinishUpload(/*success=*/true, /*expected_contacts=*/BuildContactListToUpload(
allowlist, contact_records)); allowlist, contact_records));
// When contacts are downloaded again, we decect that contacts have changed // When contacts are downloaded again, we decect that contacts have changed
...@@ -470,7 +481,7 @@ TEST_F(NearbyShareContactManagerImplTest, ...@@ -470,7 +481,7 @@ TEST_F(NearbyShareContactManagerImplTest,
contact_records = TestContactRecordList(/*num_contacts=*/4u); contact_records = TestContactRecordList(/*num_contacts=*/4u);
DownloadContacts(); DownloadContacts();
SucceedDownload(contact_records, allowlist, /*expect_upload=*/true); SucceedDownload(contact_records, allowlist, /*expect_upload=*/true);
FinishUpload(/*success=*/true, /*expected_contacts=*/ContactRecordsToContacts( FinishUpload(/*success=*/true, /*expected_contacts=*/BuildContactListToUpload(
allowlist, contact_records)); allowlist, contact_records));
} }
...@@ -485,7 +496,7 @@ TEST_F(NearbyShareContactManagerImplTest, ...@@ -485,7 +496,7 @@ TEST_F(NearbyShareContactManagerImplTest,
// requested, which succeeds. // requested, which succeeds.
DownloadContacts(); DownloadContacts();
SucceedDownload(contact_records, allowlist, /*expect_upload=*/true); SucceedDownload(contact_records, allowlist, /*expect_upload=*/true);
FinishUpload(/*success=*/true, /*expected_contacts=*/ContactRecordsToContacts( FinishUpload(/*success=*/true, /*expected_contacts=*/BuildContactListToUpload(
allowlist, contact_records)); allowlist, contact_records));
// When contacts are downloaded again, we decect that the allowlist has // When contacts are downloaded again, we decect that the allowlist has
...@@ -494,7 +505,7 @@ TEST_F(NearbyShareContactManagerImplTest, ...@@ -494,7 +505,7 @@ TEST_F(NearbyShareContactManagerImplTest,
SetAllowedContacts(allowlist, /*expect_allowlist_changed=*/true); SetAllowedContacts(allowlist, /*expect_allowlist_changed=*/true);
DownloadContacts(); DownloadContacts();
SucceedDownload(contact_records, allowlist, /*expect_upload=*/true); SucceedDownload(contact_records, allowlist, /*expect_upload=*/true);
FinishUpload(/*success=*/true, /*expected_contacts=*/ContactRecordsToContacts( FinishUpload(/*success=*/true, /*expected_contacts=*/BuildContactListToUpload(
allowlist, contact_records)); allowlist, contact_records));
} }
...@@ -513,7 +524,7 @@ TEST_F(NearbyShareContactManagerImplTest, DownloadContacts_RetryFailedUpload) { ...@@ -513,7 +524,7 @@ TEST_F(NearbyShareContactManagerImplTest, DownloadContacts_RetryFailedUpload) {
// requested, which succeeds. // requested, which succeeds.
DownloadContacts(); DownloadContacts();
SucceedDownload(contact_records, allowlist, /*expect_upload=*/true); SucceedDownload(contact_records, allowlist, /*expect_upload=*/true);
FinishUpload(/*success=*/true, /*expected_contacts=*/ContactRecordsToContacts( FinishUpload(/*success=*/true, /*expected_contacts=*/BuildContactListToUpload(
allowlist, contact_records)); allowlist, contact_records));
// When contacts are downloaded again, we decect that contacts have changed // When contacts are downloaded again, we decect that contacts have changed
...@@ -522,7 +533,7 @@ TEST_F(NearbyShareContactManagerImplTest, DownloadContacts_RetryFailedUpload) { ...@@ -522,7 +533,7 @@ TEST_F(NearbyShareContactManagerImplTest, DownloadContacts_RetryFailedUpload) {
DownloadContacts(); DownloadContacts();
SucceedDownload(contact_records, allowlist, /*expect_upload=*/true); SucceedDownload(contact_records, allowlist, /*expect_upload=*/true);
FinishUpload(/*success=*/false, FinishUpload(/*success=*/false,
/*expected_contacts=*/ContactRecordsToContacts(allowlist, /*expected_contacts=*/BuildContactListToUpload(allowlist,
contact_records)); contact_records));
// When contacts are downloaded again, we should continue to indicate that // When contacts are downloaded again, we should continue to indicate that
...@@ -532,7 +543,7 @@ TEST_F(NearbyShareContactManagerImplTest, DownloadContacts_RetryFailedUpload) { ...@@ -532,7 +543,7 @@ TEST_F(NearbyShareContactManagerImplTest, DownloadContacts_RetryFailedUpload) {
DownloadContacts(); DownloadContacts();
SucceedDownload(contact_records, allowlist, /*expect_upload=*/true); SucceedDownload(contact_records, allowlist, /*expect_upload=*/true);
FinishUpload(/*success=*/true, FinishUpload(/*success=*/true,
/*expected_contacts=*/ContactRecordsToContacts(allowlist, /*expected_contacts=*/BuildContactListToUpload(allowlist,
contact_records)); contact_records));
} }
...@@ -546,12 +557,12 @@ TEST_F(NearbyShareContactManagerImplTest, ContactUploadHash) { ...@@ -546,12 +557,12 @@ TEST_F(NearbyShareContactManagerImplTest, ContactUploadHash) {
SetAllowedContacts(allowlist, /*expect_allowlist_changed=*/true); SetAllowedContacts(allowlist, /*expect_allowlist_changed=*/true);
DownloadContacts(); DownloadContacts();
SucceedDownload(contact_records, allowlist, /*expect_upload=*/true); SucceedDownload(contact_records, allowlist, /*expect_upload=*/true);
FinishUpload(/*success=*/true, /*expected_contacts=*/ContactRecordsToContacts( FinishUpload(/*success=*/true, /*expected_contacts=*/BuildContactListToUpload(
allowlist, contact_records)); allowlist, contact_records));
// Hardcode expected contact upload hash to ensure that hashed value is // Hardcode expected contact upload hash to ensure that hashed value is
// consistent across process starts. // consistent across process starts.
EXPECT_EQ("DB408F2F01561308A97E9B6A1DB28536BEE33283D3E5B3842EFADAD4034E79DE", EXPECT_EQ("82A323B94B26BAED808E5FF1F83F11C795D598738522A0D307F1FE768BFEF286",
pref_service()->GetString( pref_service()->GetString(
prefs::kNearbySharingContactUploadHashPrefName)); prefs::kNearbySharingContactUploadHashPrefName));
} }
...@@ -240,7 +240,8 @@ NearbySharingServiceImpl::NearbySharingServiceImpl( ...@@ -240,7 +240,8 @@ NearbySharingServiceImpl::NearbySharingServiceImpl(
contact_manager_(NearbyShareContactManagerImpl::Factory::Create( contact_manager_(NearbyShareContactManagerImpl::Factory::Create(
prefs, prefs,
http_client_factory_.get(), http_client_factory_.get(),
local_device_data_manager_.get())), local_device_data_manager_.get(),
profile->GetProfileUserName())),
certificate_manager_(NearbyShareCertificateManagerImpl::Factory::Create( certificate_manager_(NearbyShareCertificateManagerImpl::Factory::Create(
local_device_data_manager_.get(), local_device_data_manager_.get(),
contact_manager_.get(), contact_manager_.get(),
......
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