Commit b3c4e563 authored by Josh Nohle's avatar Josh Nohle Committed by Commit Bot

[Nearby] Set a default device name

When the Nearby Share service starts or if the service observes that the
device name was set to an empty string, set the device name to a default
value of the form "<profile name>'s <device>."

Bug: b/154863679
Change-Id: I4700efd0e01d64f281c0c70a382b0c86e0c06997
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2406534
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: default avatarJames Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#806460}
parent dbe9347e
...@@ -62,6 +62,9 @@ std::string FakeNearbyShareLocalDeviceDataManager::GetId() { ...@@ -62,6 +62,9 @@ std::string FakeNearbyShareLocalDeviceDataManager::GetId() {
base::Optional<std::string> base::Optional<std::string>
FakeNearbyShareLocalDeviceDataManager::GetDeviceName() const { FakeNearbyShareLocalDeviceDataManager::GetDeviceName() const {
if (!device_name_ || device_name_->empty())
return base::nullopt;
return device_name_; return device_name_;
} }
...@@ -77,7 +80,14 @@ base::Optional<std::string> FakeNearbyShareLocalDeviceDataManager::GetIconUrl() ...@@ -77,7 +80,14 @@ base::Optional<std::string> FakeNearbyShareLocalDeviceDataManager::GetIconUrl()
void FakeNearbyShareLocalDeviceDataManager::SetDeviceName( void FakeNearbyShareLocalDeviceDataManager::SetDeviceName(
const std::string& name) { const std::string& name) {
if (device_name_ == name)
return;
device_name_ = name; device_name_ = name;
NotifyLocalDeviceDataChanged(
/*did_device_name_change=*/true,
/*did_full_name_change=*/false,
/*did_icon_url_change=*/false);
} }
void FakeNearbyShareLocalDeviceDataManager::DownloadDeviceData() { void FakeNearbyShareLocalDeviceDataManager::DownloadDeviceData() {
...@@ -96,6 +106,29 @@ void FakeNearbyShareLocalDeviceDataManager::UploadCertificates( ...@@ -96,6 +106,29 @@ void FakeNearbyShareLocalDeviceDataManager::UploadCertificates(
upload_certificates_calls_.emplace_back(std::move(certificates), upload_certificates_calls_.emplace_back(std::move(certificates),
std::move(callback)); std::move(callback));
} }
void FakeNearbyShareLocalDeviceDataManager::SetFullName(
const base::Optional<std::string>& full_name) {
if (full_name_ == full_name)
return;
full_name_ = full_name;
NotifyLocalDeviceDataChanged(
/*did_device_name_change=*/false,
/*did_full_name_change=*/true,
/*did_icon_url_change=*/false);
}
void FakeNearbyShareLocalDeviceDataManager::SetIconUrl(
const base::Optional<std::string>& icon_url) {
if (icon_url_ == icon_url)
return;
icon_url_ = icon_url;
NotifyLocalDeviceDataChanged(
/*did_device_name_change=*/false,
/*did_full_name_change=*/false,
/*did_icon_url_change=*/true);
}
void FakeNearbyShareLocalDeviceDataManager::OnStart() {} void FakeNearbyShareLocalDeviceDataManager::OnStart() {}
......
...@@ -90,17 +90,9 @@ class FakeNearbyShareLocalDeviceDataManager ...@@ -90,17 +90,9 @@ class FakeNearbyShareLocalDeviceDataManager
std::vector<nearbyshare::proto::PublicCertificate> certificates, std::vector<nearbyshare::proto::PublicCertificate> certificates,
UploadCompleteCallback callback) override; UploadCompleteCallback callback) override;
// Make protected observer-notification methods from base class public in this
// fake class.
using NearbyShareLocalDeviceDataManager::NotifyLocalDeviceDataChanged;
void SetId(const std::string& id) { id_ = id; } void SetId(const std::string& id) { id_ = id; }
void SetFullName(const base::Optional<std::string>& full_name) { void SetFullName(const base::Optional<std::string>& full_name);
full_name_ = full_name; void SetIconUrl(const base::Optional<std::string>& icon_url);
}
void SetIconUrl(const base::Optional<std::string>& icon_url) {
icon_url_ = icon_url;
}
size_t num_download_device_data_calls() const { size_t num_download_device_data_calls() const {
return num_download_device_data_calls_; return num_download_device_data_calls_;
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/system/sys_info.h" #include "base/system/sys_info.h"
#include "build/build_config.h"
// For profile name retrieval: // For profile name retrieval:
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
...@@ -38,8 +39,13 @@ base::Optional<std::string> GetNameFromProfile(Profile* profile) { ...@@ -38,8 +39,13 @@ base::Optional<std::string> GetNameFromProfile(Profile* profile) {
return base::nullopt; return base::nullopt;
name = base::UTF16ToUTF8(user->GetDisplayName()); name = base::UTF16ToUTF8(user->GetDisplayName());
#else // !defined(OS_CHROMEOS) #elif defined(OS_WIN)
ProfileAttributesEntry* entry; // TODO(https://crbug.com/1127603): The non-Chrome OS strategy below caused
// Nearby Share service unit tests to crash on Windows trybots when we tried
// to integrate this into the Nearby Share service.
name = "First Last";
#else // !defined(OS_CHROMEOS) && !defined(OS_WIN)
ProfileAttributesEntry* entry = nullptr;
if (!g_browser_process->profile_manager() if (!g_browser_process->profile_manager()
->GetProfileAttributesStorage() ->GetProfileAttributesStorage()
.GetProfileAttributesWithPath(profile->GetPath(), &entry)) { .GetProfileAttributesWithPath(profile->GetPath(), &entry)) {
...@@ -47,7 +53,7 @@ base::Optional<std::string> GetNameFromProfile(Profile* profile) { ...@@ -47,7 +53,7 @@ base::Optional<std::string> GetNameFromProfile(Profile* profile) {
} }
name = base::UTF16ToUTF8(entry->GetLocalProfileName()); name = base::UTF16ToUTF8(entry->GetLocalProfileName());
#endif // defined(OS_CHROMEOS) #endif
return name.empty() ? base::nullopt : base::make_optional(name); return name.empty() ? base::nullopt : base::make_optional(name);
} }
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/system/sys_info.h" #include "base/system/sys_info.h"
#include "base/test/bind_test_util.h" #include "base/test/bind_test_util.h"
#include "build/build_config.h"
#include "chrome/browser/nearby_sharing/nearby_share_default_device_name.h" #include "chrome/browser/nearby_sharing/nearby_share_default_device_name.h"
#include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
...@@ -82,7 +83,14 @@ TEST(NearbyShareDefaultDeviceNameTest, DefaultDeviceName) { ...@@ -82,7 +83,14 @@ TEST(NearbyShareDefaultDeviceNameTest, DefaultDeviceName) {
EXPECT_EQ(std::string(kFakeNameFromProfile) + "'s " + EXPECT_EQ(std::string(kFakeNameFromProfile) + "'s " +
base::UTF16ToUTF8(ui::GetChromeOSDeviceName()), base::UTF16ToUTF8(ui::GetChromeOSDeviceName()),
*device_name); *device_name);
#else // !defined(OS_CHROMEOS) #elif defined(OS_WIN)
std::string expected_model_name = GetModelNameBlocking();
if (expected_model_name.empty()) {
EXPECT_TRUE(device_name->rfind("First Last's ", 0) == 0);
} else {
EXPECT_EQ("First Last's " + expected_model_name, device_name);
}
#else // !defined(OS_CHROMEOS) && !defined(OS_WIN)
std::string expected_model_name = GetModelNameBlocking(); std::string expected_model_name = GetModelNameBlocking();
if (expected_model_name.empty()) { if (expected_model_name.empty()) {
EXPECT_TRUE( EXPECT_TRUE(
......
...@@ -123,10 +123,6 @@ TEST_F(NearbyShareSettingsTest, GetAndSetDeviceName) { ...@@ -123,10 +123,6 @@ TEST_F(NearbyShareSettingsTest, GetAndSetDeviceName) {
EXPECT_EQ("d", nearby_share_settings_.GetDeviceName()); EXPECT_EQ("d", nearby_share_settings_.GetDeviceName());
EXPECT_EQ("uncalled", observer_.device_name); EXPECT_EQ("uncalled", observer_.device_name);
local_device_data_manager_.NotifyLocalDeviceDataChanged(
/*did_device_name_change=*/true,
/*did_full_name_change=*/false,
/*did_icon_url_change=*/false);
FlushMojoMessages(); FlushMojoMessages();
EXPECT_EQ("d", observer_.device_name); EXPECT_EQ("d", observer_.device_name);
......
...@@ -28,6 +28,7 @@ ...@@ -28,6 +28,7 @@
#include "chrome/browser/nearby_sharing/local_device_data/nearby_share_local_device_data_manager_impl.h" #include "chrome/browser/nearby_sharing/local_device_data/nearby_share_local_device_data_manager_impl.h"
#include "chrome/browser/nearby_sharing/logging/logging.h" #include "chrome/browser/nearby_sharing/logging/logging.h"
#include "chrome/browser/nearby_sharing/nearby_connections_manager.h" #include "chrome/browser/nearby_sharing/nearby_connections_manager.h"
#include "chrome/browser/nearby_sharing/nearby_share_default_device_name.h"
#include "chrome/browser/nearby_sharing/paired_key_verification_runner.h" #include "chrome/browser/nearby_sharing/paired_key_verification_runner.h"
#include "chrome/browser/nearby_sharing/transfer_metadata.h" #include "chrome/browser/nearby_sharing/transfer_metadata.h"
#include "chrome/browser/nearby_sharing/transfer_metadata_builder.h" #include "chrome/browser/nearby_sharing/transfer_metadata_builder.h"
...@@ -243,6 +244,8 @@ NearbySharingServiceImpl::NearbySharingServiceImpl( ...@@ -243,6 +244,8 @@ NearbySharingServiceImpl::NearbySharingServiceImpl(
contact_manager_->Start(); contact_manager_->Start();
certificate_manager_->Start(); certificate_manager_->Start();
} }
SetDefaultDeviceNameIfEmpty();
} }
NearbySharingServiceImpl::~NearbySharingServiceImpl() { NearbySharingServiceImpl::~NearbySharingServiceImpl() {
...@@ -927,6 +930,8 @@ void NearbySharingServiceImpl::OnDeviceNameChanged( ...@@ -927,6 +930,8 @@ void NearbySharingServiceImpl::OnDeviceNameChanged(
NS_LOG(VERBOSE) << __func__ << ": Nearby sharing device name changed to " NS_LOG(VERBOSE) << __func__ << ": Nearby sharing device name changed to "
<< device_name; << device_name;
// TODO(vecore): handle device name change // TODO(vecore): handle device name change
SetDefaultDeviceNameIfEmpty();
} }
void NearbySharingServiceImpl::OnAllowedContactsChanged( void NearbySharingServiceImpl::OnAllowedContactsChanged(
...@@ -3154,3 +3159,28 @@ void NearbySharingServiceImpl::SetInHighVisibility( ...@@ -3154,3 +3159,28 @@ void NearbySharingServiceImpl::SetInHighVisibility(
observer.OnHighVisibilityChanged(in_high_visibility); observer.OnHighVisibilityChanged(in_high_visibility);
} }
} }
void NearbySharingServiceImpl::SetDefaultDeviceNameIfEmpty() {
if (local_device_data_manager_->GetDeviceName() || !profile_)
return;
GetNearbyShareDefaultDeviceName(
profile_,
base::BindOnce(&NearbySharingServiceImpl::OnDefaultDeviceNameFetched,
weak_ptr_factory_.GetWeakPtr()));
}
void NearbySharingServiceImpl::OnDefaultDeviceNameFetched(
const base::Optional<std::string>& default_device_name) {
// Check that the device name wasn't set while the default device name was
// being generated.
if (local_device_data_manager_->GetDeviceName())
return;
if (!default_device_name) {
NS_LOG(ERROR) << __func__ << ": Could not generate default device name.";
return;
}
local_device_data_manager_->SetDeviceName(*default_device_name);
}
...@@ -297,6 +297,10 @@ class NearbySharingServiceImpl ...@@ -297,6 +297,10 @@ class NearbySharingServiceImpl
NearbyConnectionsManager::ConnectionsStatus status); NearbyConnectionsManager::ConnectionsStatus status);
void SetInHighVisibility(bool in_high_visibility); void SetInHighVisibility(bool in_high_visibility);
void SetDefaultDeviceNameIfEmpty();
void OnDefaultDeviceNameFetched(
const base::Optional<std::string>& default_device_name);
Profile* profile_; Profile* profile_;
std::unique_ptr<NearbyConnectionsManager> nearby_connections_manager_; std::unique_ptr<NearbyConnectionsManager> nearby_connections_manager_;
NearbyProcessManager* process_manager_; NearbyProcessManager* process_manager_;
......
...@@ -39,6 +39,7 @@ ...@@ -39,6 +39,7 @@
#include "chrome/browser/nearby_sharing/mock_nearby_process_manager.h" #include "chrome/browser/nearby_sharing/mock_nearby_process_manager.h"
#include "chrome/browser/nearby_sharing/mock_nearby_sharing_decoder.h" #include "chrome/browser/nearby_sharing/mock_nearby_sharing_decoder.h"
#include "chrome/browser/nearby_sharing/nearby_connections_manager.h" #include "chrome/browser/nearby_sharing/nearby_connections_manager.h"
#include "chrome/browser/nearby_sharing/nearby_share_default_device_name.h"
#include "chrome/browser/nearby_sharing/proto/rpc_resources.pb.h" #include "chrome/browser/nearby_sharing/proto/rpc_resources.pb.h"
#include "chrome/browser/notifications/notification_display_service_factory.h" #include "chrome/browser/notifications/notification_display_service_factory.h"
#include "chrome/browser/notifications/notification_display_service_tester.h" #include "chrome/browser/notifications/notification_display_service_tester.h"
...@@ -59,6 +60,10 @@ ...@@ -59,6 +60,10 @@
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/idle/scoped_set_idle_state.h" #include "ui/base/idle/scoped_set_idle_state.h"
#if defined(OS_CHROMEOS)
#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
#include "components/user_manager/scoped_user_manager.h"
#endif // defined(OS_CHROMEOS)
using ::testing::_; using ::testing::_;
using testing::AtLeast; using testing::AtLeast;
...@@ -183,6 +188,10 @@ namespace { ...@@ -183,6 +188,10 @@ namespace {
constexpr base::TimeDelta kDelta = base::TimeDelta::FromMilliseconds(100); constexpr base::TimeDelta kDelta = base::TimeDelta::FromMilliseconds(100);
const char kProfileName[] = "profile_name";
#if defined(OS_CHROMEOS)
const char kUserDisplayName[] = "user_display_name";
#endif // defined(OS_CHROMEOS)
const char kServiceId[] = "NearbySharing"; const char kServiceId[] = "NearbySharing";
const char kDeviceName[] = "test_device_name"; const char kDeviceName[] = "test_device_name";
const nearby_share::mojom::ShareTargetType kDeviceType = const nearby_share::mojom::ShareTargetType kDeviceType =
...@@ -355,7 +364,7 @@ class NearbySharingServiceImplTest : public testing::Test { ...@@ -355,7 +364,7 @@ class NearbySharingServiceImplTest : public testing::Test {
device::BluetoothAdapterFactory::SetAdapterForTesting( device::BluetoothAdapterFactory::SetAdapterForTesting(
mock_bluetooth_adapter_); mock_bluetooth_adapter_);
service_ = CreateService("name"); service_ = CreateService();
SetFakeFastInitiationManagerFactory(/*should_succeed_on_start=*/true); SetFakeFastInitiationManagerFactory(/*should_succeed_on_start=*/true);
EXPECT_CALL(mock_nearby_process_manager(), EXPECT_CALL(mock_nearby_process_manager(),
...@@ -387,9 +396,21 @@ class NearbySharingServiceImplTest : public testing::Test { ...@@ -387,9 +396,21 @@ class NearbySharingServiceImplTest : public testing::Test {
FastInitiationManager::Factory::SetFactoryForTesting(nullptr); FastInitiationManager::Factory::SetFactoryForTesting(nullptr);
} }
std::unique_ptr<NearbySharingServiceImpl> CreateService( std::unique_ptr<NearbySharingServiceImpl> CreateService() {
const std::string& profile_name) { profile_ = profile_manager_.CreateTestingProfile(kProfileName);
profile_ = profile_manager_.CreateTestingProfile(profile_name);
// Perform additional profile setup for Chrome OS. This is necessary for
// constructing a default device name.
#if defined(OS_CHROMEOS)
chromeos::FakeChromeUserManager* user_manager =
new chromeos::FakeChromeUserManager();
scoped_user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
base::WrapUnique(user_manager));
user_manager->AddUser(AccountId::FromUserEmail(kProfileName));
user_manager->SaveUserDisplayName(AccountId::FromUserEmail(kProfileName),
base::UTF8ToUTF16(kUserDisplayName));
#endif // defined(OS_CHROMEOS)
fake_nearby_connections_manager_ = new FakeNearbyConnectionsManager(); fake_nearby_connections_manager_ = new FakeNearbyConnectionsManager();
notification_tester_ = notification_tester_ =
std::make_unique<NotificationDisplayServiceTester>(profile_); std::make_unique<NotificationDisplayServiceTester>(profile_);
...@@ -406,8 +427,9 @@ class NearbySharingServiceImplTest : public testing::Test { ...@@ -406,8 +427,9 @@ class NearbySharingServiceImplTest : public testing::Test {
->SetDownloadManagerDelegateForTesting( ->SetDownloadManagerDelegateForTesting(
std::make_unique<ChromeDownloadManagerDelegate>(profile_)); std::make_unique<ChromeDownloadManagerDelegate>(profile_));
// Allow the posted task to fetch the BluetoothAdapter to finish. // Allow the posted tasks to fetch the BluetoothAdapter and set the default
base::RunLoop().RunUntilIdle(); // device name to finish.
task_environment_.RunUntilIdle();
return service; return service;
} }
...@@ -840,6 +862,9 @@ class NearbySharingServiceImplTest : public testing::Test { ...@@ -840,6 +862,9 @@ class NearbySharingServiceImplTest : public testing::Test {
ui::ScopedSetIdleState idle_state_{ui::IDLE_STATE_IDLE}; ui::ScopedSetIdleState idle_state_{ui::IDLE_STATE_IDLE};
TestingProfileManager profile_manager_{TestingBrowserProcess::GetGlobal()}; TestingProfileManager profile_manager_{TestingBrowserProcess::GetGlobal()};
Profile* profile_ = nullptr; Profile* profile_ = nullptr;
#if defined(OS_CHROMEOS)
std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
#endif // defined(OS_CHROMEOS)
sync_preferences::TestingPrefServiceSyncable prefs_; sync_preferences::TestingPrefServiceSyncable prefs_;
FakeNearbyConnectionsManager* fake_nearby_connections_manager_ = nullptr; FakeNearbyConnectionsManager* fake_nearby_connections_manager_ = nullptr;
FakeNearbyShareLocalDeviceDataManager::Factory FakeNearbyShareLocalDeviceDataManager::Factory
...@@ -3486,3 +3511,39 @@ TEST_F(NearbySharingServiceImplTest, ShutdownCallsObservers) { ...@@ -3486,3 +3511,39 @@ TEST_F(NearbySharingServiceImplTest, ShutdownCallsObservers) {
// Prevent a double shutdown. // Prevent a double shutdown.
service_.reset(); service_.reset();
} }
TEST_F(NearbySharingServiceImplTest, SetDefaultDeviceName) {
// The default device name is set in the ctor because the device name was not
// yet set.
base::Optional<std::string> expected_device_name;
base::RunLoop run_loop;
GetNearbyShareDefaultDeviceName(
profile_,
base::BindLambdaForTesting([&run_loop, &expected_device_name](
const base::Optional<std::string>& name) {
expected_device_name = std::move(name);
run_loop.Quit();
}));
run_loop.Run();
ASSERT_TRUE(local_device_data_manager()->GetDeviceName());
EXPECT_EQ(*expected_device_name,
*local_device_data_manager()->GetDeviceName());
// If the device name is cleared, the service notices and sets the default
// device name.
local_device_data_manager()->SetDeviceName(/*device_name=*/std::string());
EXPECT_FALSE(local_device_data_manager()->GetDeviceName());
// Allow the observer notifications to propagate, and allow the posted task to
// set the default device name to finish.
// Propagate first device name change notification: device name empty.
service_->FlushMojoForTesting();
// Run tasks to generate default device name.
task_environment_.RunUntilIdle();
// Propagate second device name change notification: default device name set;
// should be no-op.
service_->FlushMojoForTesting();
ASSERT_TRUE(local_device_data_manager()->GetDeviceName());
EXPECT_EQ(*expected_device_name,
*local_device_data_manager()->GetDeviceName());
}
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