Commit c8529ce8 authored by Jordy Greenblatt's avatar Jordy Greenblatt Committed by Commit Bot

[CrOS MultiDevice] Provide host name to 'Chromebook added' notification

The new strings for settings (added in
https://chromium-review.googlesource.com/c/chromium/src/+/1252915)
include a notification title that contains the host device name. This
CL provides it as an argument to
MultiDeviceNotificationPresenter::OnNewChromebookAddedForExistingUser
and updates the references to that function. Several of these are for
tests (unit and manual).

Screenshot of the new notification: http://screen/fzqJUOtSgHV

Bug: 890521
Change-Id: If3586ea526866530a35b35d80957c57d3c83fe5c
Reviewed-on: https://chromium-review.googlesource.com/c/1252338
Commit-Queue: Jordy Greenblatt <jordynass@chromium.org>
Reviewed-by: default avatarTom Sepez <tsepez@chromium.org>
Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596058}
parent dd2470cc
...@@ -121,16 +121,18 @@ void MultiDeviceNotificationPresenter::OnConnectedHostSwitchedForExistingUser( ...@@ -121,16 +121,18 @@ void MultiDeviceNotificationPresenter::OnConnectedHostSwitchedForExistingUser(
const std::string& new_host_device_name) { const std::string& new_host_device_name) {
base::string16 title = l10n_util::GetStringFUTF16( base::string16 title = l10n_util::GetStringFUTF16(
IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_HOST_SWITCHED_TITLE, IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_HOST_SWITCHED_TITLE,
base::UTF8ToUTF16(new_host_device_name)); base::ASCIIToUTF16(new_host_device_name));
base::string16 message = l10n_util::GetStringUTF16( base::string16 message = l10n_util::GetStringUTF16(
IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_HOST_SWITCHED_MESSAGE); IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_HOST_SWITCHED_MESSAGE);
ShowNotification(Status::kExistingUserHostSwitchedNotificationVisible, title, ShowNotification(Status::kExistingUserHostSwitchedNotificationVisible, title,
message); message);
} }
void MultiDeviceNotificationPresenter::OnNewChromebookAddedForExistingUser() { void MultiDeviceNotificationPresenter::OnNewChromebookAddedForExistingUser(
base::string16 title = l10n_util::GetStringUTF16( const std::string& new_host_device_name) {
IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_NEW_CHROMEBOOK_ADDED_TITLE); base::string16 title = l10n_util::GetStringFUTF16(
IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_NEW_CHROMEBOOK_ADDED_TITLE,
base::ASCIIToUTF16(new_host_device_name));
base::string16 message = l10n_util::GetStringUTF16( base::string16 message = l10n_util::GetStringUTF16(
IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_NEW_CHROMEBOOK_ADDED_MESSAGE); IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_NEW_CHROMEBOOK_ADDED_MESSAGE);
ShowNotification(Status::kExistingUserNewChromebookNotificationVisible, title, ShowNotification(Status::kExistingUserNewChromebookNotificationVisible, title,
......
...@@ -61,7 +61,8 @@ class ASH_EXPORT MultiDeviceNotificationPresenter ...@@ -61,7 +61,8 @@ class ASH_EXPORT MultiDeviceNotificationPresenter
void OnPotentialHostExistsForNewUser() override; void OnPotentialHostExistsForNewUser() override;
void OnConnectedHostSwitchedForExistingUser( void OnConnectedHostSwitchedForExistingUser(
const std::string& new_host_device_name) override; const std::string& new_host_device_name) override;
void OnNewChromebookAddedForExistingUser() override; void OnNewChromebookAddedForExistingUser(
const std::string& new_host_device_name) override;
// SessionObserver: // SessionObserver:
void OnUserSessionAdded(const AccountId& account_id) override; void OnUserSessionAdded(const AccountId& account_id) override;
......
...@@ -184,7 +184,8 @@ class MultiDeviceNotificationPresenterTest : public NoSessionAshTestBase { ...@@ -184,7 +184,8 @@ class MultiDeviceNotificationPresenterTest : public NoSessionAshTestBase {
void ShowExistingUserNewChromebookNotification() { void ShowExistingUserNewChromebookNotification() {
EXPECT_TRUE(fake_multidevice_setup_->delegate().is_bound()); EXPECT_TRUE(fake_multidevice_setup_->delegate().is_bound());
fake_multidevice_setup_->delegate()->OnNewChromebookAddedForExistingUser(); fake_multidevice_setup_->delegate()->OnNewChromebookAddedForExistingUser(
kTestHostDeviceName);
InvokePendingMojoCalls(); InvokePendingMojoCalls();
} }
...@@ -285,8 +286,9 @@ class MultiDeviceNotificationPresenterTest : public NoSessionAshTestBase { ...@@ -285,8 +286,9 @@ class MultiDeviceNotificationPresenterTest : public NoSessionAshTestBase {
break; break;
case MultiDeviceNotificationPresenter::Status:: case MultiDeviceNotificationPresenter::Status::
kExistingUserNewChromebookNotificationVisible: kExistingUserNewChromebookNotificationVisible:
title = l10n_util::GetStringUTF16( title = l10n_util::GetStringFUTF16(
IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_NEW_CHROMEBOOK_ADDED_TITLE); IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_NEW_CHROMEBOOK_ADDED_TITLE,
base::ASCIIToUTF16(kTestHostDeviceName));
message = l10n_util::GetStringUTF16( message = l10n_util::GetStringUTF16(
IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_NEW_CHROMEBOOK_ADDED_MESSAGE); IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_NEW_CHROMEBOOK_ADDED_MESSAGE);
break; break;
......
...@@ -156,7 +156,8 @@ void AccountStatusChangeDelegateNotifierImpl::CheckForMultiDeviceEvents( ...@@ -156,7 +156,8 @@ void AccountStatusChangeDelegateNotifierImpl::CheckForMultiDeviceEvents(
CheckForNewUserPotentialHostExistsEvent(host_status_with_device); CheckForNewUserPotentialHostExistsEvent(host_status_with_device);
CheckForExistingUserHostSwitchedEvent(host_status_with_device, CheckForExistingUserHostSwitchedEvent(host_status_with_device,
host_device_id_before_update); host_device_id_before_update);
CheckForExistingUserChromebookAddedEvent(host_device_id_before_update); CheckForExistingUserChromebookAddedEvent(host_status_with_device,
host_device_id_before_update);
} }
void AccountStatusChangeDelegateNotifierImpl:: void AccountStatusChangeDelegateNotifierImpl::
...@@ -207,6 +208,7 @@ void AccountStatusChangeDelegateNotifierImpl:: ...@@ -207,6 +208,7 @@ void AccountStatusChangeDelegateNotifierImpl::
void AccountStatusChangeDelegateNotifierImpl:: void AccountStatusChangeDelegateNotifierImpl::
CheckForExistingUserChromebookAddedEvent( CheckForExistingUserChromebookAddedEvent(
const HostStatusProvider::HostStatusWithDevice& host_status_with_device,
const base::Optional<std::string>& host_device_id_before_update) { const base::Optional<std::string>& host_device_id_before_update) {
// The Chromebook added event requires that a set host was found by the // The Chromebook added event requires that a set host was found by the
// update, i.e. there was no host before the host status update but afterward // update, i.e. there was no host before the host status update but afterward
...@@ -214,7 +216,8 @@ void AccountStatusChangeDelegateNotifierImpl:: ...@@ -214,7 +216,8 @@ void AccountStatusChangeDelegateNotifierImpl::
if (!host_device_id_from_most_recent_update_ || host_device_id_before_update) if (!host_device_id_from_most_recent_update_ || host_device_id_before_update)
return; return;
delegate()->OnNewChromebookAddedForExistingUser(); delegate()->OnNewChromebookAddedForExistingUser(
host_status_with_device.host_device()->name());
pref_service_->SetInt64(kExistingUserChromebookAddedPrefName, pref_service_->SetInt64(kExistingUserChromebookAddedPrefName,
clock_->Now().ToJavaTime()); clock_->Now().ToJavaTime());
} }
......
...@@ -86,6 +86,7 @@ class AccountStatusChangeDelegateNotifierImpl ...@@ -86,6 +86,7 @@ class AccountStatusChangeDelegateNotifierImpl
const HostStatusProvider::HostStatusWithDevice& host_status_with_device, const HostStatusProvider::HostStatusWithDevice& host_status_with_device,
const base::Optional<std::string>& host_device_id_before_update); const base::Optional<std::string>& host_device_id_before_update);
void CheckForExistingUserChromebookAddedEvent( void CheckForExistingUserChromebookAddedEvent(
const HostStatusProvider::HostStatusWithDevice& host_status_with_device,
const base::Optional<std::string>& host_device_id_before_update); const base::Optional<std::string>& host_device_id_before_update);
// Loads data from previous session using PrefService. // Loads data from previous session using PrefService.
......
...@@ -28,7 +28,8 @@ void FakeAccountStatusChangeDelegate::OnConnectedHostSwitchedForExistingUser( ...@@ -28,7 +28,8 @@ void FakeAccountStatusChangeDelegate::OnConnectedHostSwitchedForExistingUser(
++num_existing_user_host_switched_events_handled_; ++num_existing_user_host_switched_events_handled_;
} }
void FakeAccountStatusChangeDelegate::OnNewChromebookAddedForExistingUser() { void FakeAccountStatusChangeDelegate::OnNewChromebookAddedForExistingUser(
const std::string& new_host_device_name) {
++num_existing_user_chromebook_added_events_handled_; ++num_existing_user_chromebook_added_events_handled_;
} }
......
...@@ -36,7 +36,8 @@ class FakeAccountStatusChangeDelegate ...@@ -36,7 +36,8 @@ class FakeAccountStatusChangeDelegate
void OnPotentialHostExistsForNewUser() override; void OnPotentialHostExistsForNewUser() override;
void OnConnectedHostSwitchedForExistingUser( void OnConnectedHostSwitchedForExistingUser(
const std::string& new_host_device_name) override; const std::string& new_host_device_name) override;
void OnNewChromebookAddedForExistingUser() override; void OnNewChromebookAddedForExistingUser(
const std::string& new_host_device_name) override;
private: private:
size_t num_new_user_events_handled_ = 0u; size_t num_new_user_events_handled_ = 0u;
......
...@@ -251,7 +251,8 @@ void MultiDeviceSetupImpl::TriggerEventForDebugging( ...@@ -251,7 +251,8 @@ void MultiDeviceSetupImpl::TriggerEventForDebugging(
kTestDeviceNameForDebugNotification); kTestDeviceNameForDebugNotification);
break; break;
case mojom::EventTypeForDebugging::kExistingUserNewChromebookAdded: case mojom::EventTypeForDebugging::kExistingUserNewChromebookAdded:
delegate->OnNewChromebookAddedForExistingUser(); delegate->OnNewChromebookAddedForExistingUser(
kTestDeviceNameForDebugNotification);
break; break;
default: default:
NOTREACHED(); NOTREACHED();
......
...@@ -94,14 +94,13 @@ interface AccountStatusChangeDelegate { ...@@ -94,14 +94,13 @@ interface AccountStatusChangeDelegate {
// Callback which indicates that the currently-connected MultiDevice host has // Callback which indicates that the currently-connected MultiDevice host has
// changed. This likely means that the user has changed MultiDevice settings // changed. This likely means that the user has changed MultiDevice settings
// on another device. This function is only called if the current user has // on another device. This function is only called if the current user has
// already set up MultiDevice features. Note: |new_host_device_name| is // already set up MultiDevice features.
// expected to be a UTF-8 string.
OnConnectedHostSwitchedForExistingUser(string new_host_device_name); OnConnectedHostSwitchedForExistingUser(string new_host_device_name);
// Callback which indicates that a new Chromebook was added to the account of // Callback which indicates that a new Chromebook was added to the account of
// the current user. This function is only called if the current user has // the current user. This function is only called if the current user has
// already set up MultiDevice features. // already set up MultiDevice features.
OnNewChromebookAddedForExistingUser(); OnNewChromebookAddedForExistingUser(string new_host_device_name);
}; };
interface HostStatusObserver { interface HostStatusObserver {
......
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