Skip to content

Commit

Permalink
Wi-Fi Sync: Show announcement notification on first unlock
Browse files Browse the repository at this point in the history
Existing users of the multidevice suite will have Wi-Fi Sync set to
be disabled by default when it is first.  This adds a notification to
announce the feature to these users on the first unlock after the
feature is available.

Bug: 1117619
Change-Id: Ia580ed2b4f99561a7295e585bc3cfa592ee5f1b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2630733
Commit-Queue: Jon Mann <jonmann@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844301}
  • Loading branch information
Jon Mann authored and Chromium LUCI CQ committed Jan 16, 2021
1 parent 95aaa5c commit 695ff57
Show file tree
Hide file tree
Showing 18 changed files with 350 additions and 69 deletions.
6 changes: 6 additions & 0 deletions ash/ash_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -2668,6 +2668,12 @@ This file contains the strings for ash.
<message name="IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_NEW_CHROME_DEVICE_ADDED_MESSAGE" desc="Message shown as part of the notification shown to a user that has already completed the MultiDevice setup flow logging into a new Chrome device that all their Chrome devices are MultiDevice enabled.">
This <ph name="DEVICE_NAME">$1<ex>Chromebook</ex></ph> and your phone will connect automatically
</message>
<message name="IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_TITLE" desc="Title of the notification shown to a user that is newly eligible for Wi-Fi Sync.">
Turn on Wi-Fi Sync
</message>
<message name="IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_MESSAGE" desc="Message shown as part of the notification shown to a user that has become eligible to enable Wi-Fi Sync.">
Wi-Fi networks will be shared between your phone and <ph name="DEVICE_NAME">$1<ex>Chromebook</ex></ph>
</message>

<!-- Window teleportation warning dialog -->
<message name="IDS_ASH_TELEPORT_WARNING_TITLE" desc="The title of the dialog which warns user about oddities which can be seen when a window gets moved to another user desktop.">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
77739c07b3a18f3b0a727267e02a489e466a048f
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
77739c07b3a18f3b0a727267e02a489e466a048f
79 changes: 52 additions & 27 deletions ash/multi_device_setup/multi_device_notification_presenter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,13 @@ const char kNotifierMultiDevice[] = "ash.multi_device_setup";
} // namespace

// static
const char MultiDeviceNotificationPresenter::kNotificationId[] =
const char MultiDeviceNotificationPresenter::kSetupNotificationId[] =
"cros_multi_device_setup_notification_id";

// static
const char MultiDeviceNotificationPresenter::kWifiSyncNotificationId[] =
"cros_wifi_sync_announcement_notification_id";

// static
std::string
MultiDeviceNotificationPresenter::GetNotificationDescriptionForLogging(
Expand Down Expand Up @@ -96,7 +100,7 @@ void MultiDeviceNotificationPresenter::OnPotentialHostExistsForNewUser() {
base::string16 message = l10n_util::GetStringFUTF16(
IDS_ASH_MULTI_DEVICE_SETUP_NEW_USER_POTENTIAL_HOST_EXISTS_MESSAGE,
ui::GetChromeOSDeviceName());
ShowNotification(Status::kNewUserNotificationVisible, title, message);
ShowSetupNotification(Status::kNewUserNotificationVisible, title, message);
}

void MultiDeviceNotificationPresenter::OnNoLongerNewUser() {
Expand All @@ -113,8 +117,8 @@ void MultiDeviceNotificationPresenter::OnConnectedHostSwitchedForExistingUser(
base::string16 message = l10n_util::GetStringFUTF16(
IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_HOST_SWITCHED_MESSAGE,
ui::GetChromeOSDeviceName());
ShowNotification(Status::kExistingUserHostSwitchedNotificationVisible, title,
message);
ShowSetupNotification(Status::kExistingUserHostSwitchedNotificationVisible,
title, message);
}

void MultiDeviceNotificationPresenter::OnNewChromebookAddedForExistingUser(
Expand All @@ -125,13 +129,22 @@ void MultiDeviceNotificationPresenter::OnNewChromebookAddedForExistingUser(
base::string16 message = l10n_util::GetStringFUTF16(
IDS_ASH_MULTI_DEVICE_SETUP_EXISTING_USER_NEW_CHROME_DEVICE_ADDED_MESSAGE,
ui::GetChromeOSDeviceName());
ShowNotification(Status::kExistingUserNewChromebookNotificationVisible, title,
message);
ShowSetupNotification(Status::kExistingUserNewChromebookNotificationVisible,
title, message);
}

void MultiDeviceNotificationPresenter::OnBecameEligibleForWifiSync() {
base::string16 title =
l10n_util::GetStringUTF16(IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_TITLE);
base::string16 message = l10n_util::GetStringFUTF16(
IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_MESSAGE,
ui::GetChromeOSDeviceName());
ShowNotification(kWifiSyncNotificationId, title, message);
}

void MultiDeviceNotificationPresenter::RemoveMultiDeviceSetupNotification() {
notification_status_ = Status::kNoNotificationVisible;
message_center_->RemoveNotification(kNotificationId,
message_center_->RemoveNotification(kSetupNotificationId,
/* by_user */ false);
}

Expand All @@ -148,7 +161,7 @@ void MultiDeviceNotificationPresenter::OnSessionStateChanged(
void MultiDeviceNotificationPresenter::OnNotificationRemoved(
const std::string& notification_id,
bool by_user) {
if (by_user && notification_id == kNotificationId) {
if (by_user && notification_id == kSetupNotificationId) {
UMA_HISTOGRAM_ENUMERATION(
"MultiDeviceSetup_NotificationDismissed",
GetMetricValueForNotification(notification_status_),
Expand All @@ -160,7 +173,14 @@ void MultiDeviceNotificationPresenter::OnNotificationClicked(
const std::string& notification_id,
const base::Optional<int>& button_index,
const base::Optional<base::string16>& reply) {
if (notification_id != kNotificationId)
if (notification_id == kWifiSyncNotificationId) {
Shell::Get()->system_tray_model()->client()->ShowConnectedDevicesSettings();
message_center_->RemoveNotification(kWifiSyncNotificationId,
/* by_user */ false);
return;
}

if (notification_id != kSetupNotificationId)
return;

DCHECK(notification_status_ != Status::kNoNotificationVisible);
Expand Down Expand Up @@ -219,7 +239,7 @@ void MultiDeviceNotificationPresenter::ObserveMultiDeviceSetupIfPossible() {
message_center_->AddObserver(this);
}

void MultiDeviceNotificationPresenter::ShowNotification(
void MultiDeviceNotificationPresenter::ShowSetupNotification(
const Status notification_status,
const base::string16& title,
const base::string16& message) {
Expand All @@ -229,28 +249,33 @@ void MultiDeviceNotificationPresenter::ShowNotification(
UMA_HISTOGRAM_ENUMERATION("MultiDeviceSetup_NotificationShown",
GetMetricValueForNotification(notification_status),
kNotificationTypeMax);
if (message_center_->FindVisibleNotificationById(kNotificationId)) {
message_center_->UpdateNotification(kNotificationId,
CreateNotification(title, message));
} else {
message_center_->AddNotification(CreateNotification(title, message));
}

ShowNotification(kSetupNotificationId, title, message);
notification_status_ = notification_status;
}

std::unique_ptr<message_center::Notification>
MultiDeviceNotificationPresenter::CreateNotification(
void MultiDeviceNotificationPresenter::ShowNotification(
const std::string& id,
const base::string16& title,
const base::string16& message) {
return CreateSystemNotification(
message_center::NotificationType::NOTIFICATION_TYPE_SIMPLE,
kNotificationId, title, message, base::string16() /* display_source */,
GURL() /* origin_url */,
message_center::NotifierId(message_center::NotifierType::SYSTEM_COMPONENT,
kNotifierMultiDevice),
message_center::RichNotificationData(), nullptr /* delegate */,
kNotificationMultiDeviceSetupIcon,
message_center::SystemNotificationWarningLevel::NORMAL);
std::unique_ptr<message_center::Notification> notification =
CreateSystemNotification(
message_center::NotificationType::NOTIFICATION_TYPE_SIMPLE, id, title,
message, base::string16() /* display_source */,
GURL() /* origin_url */,
message_center::NotifierId(
message_center::NotifierType::SYSTEM_COMPONENT,
kNotifierMultiDevice),
message_center::RichNotificationData(), nullptr /* delegate */,
kNotificationMultiDeviceSetupIcon,
message_center::SystemNotificationWarningLevel::NORMAL);

if (message_center_->FindVisibleNotificationById(kSetupNotificationId)) {
message_center_->UpdateNotification(id, std::move(notification));
return;
}

message_center_->AddNotification(std::move(notification));
}

void MultiDeviceNotificationPresenter::FlushForTesting() {
Expand Down
12 changes: 7 additions & 5 deletions ash/multi_device_setup/multi_device_notification_presenter.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class ASH_EXPORT MultiDeviceNotificationPresenter
const std::string& new_host_device_name) override;
void OnNewChromebookAddedForExistingUser(
const std::string& new_host_device_name) override;
void OnBecameEligibleForWifiSync() override;

// SessionObserver:
void OnUserSessionAdded(const AccountId& account_id) override;
Expand All @@ -79,7 +80,8 @@ class ASH_EXPORT MultiDeviceNotificationPresenter
friend class MultiDeviceNotificationPresenterTest;

// MultiDevice setup notification ID.
static const char kNotificationId[];
static const char kSetupNotificationId[];
static const char kWifiSyncNotificationId[];

// Represents each possible MultiDevice setup notification that the setup flow
// can show with a "none" option for the general state with no notification
Expand Down Expand Up @@ -107,12 +109,12 @@ class ASH_EXPORT MultiDeviceNotificationPresenter
Status notification_status);

void ObserveMultiDeviceSetupIfPossible();
void ShowNotification(const Status notification_status,
void ShowSetupNotification(const Status notification_status,
const base::string16& title,
const base::string16& message);
void ShowNotification(const std::string& id,
const base::string16& title,
const base::string16& message);
std::unique_ptr<message_center::Notification> CreateNotification(
const base::string16& title,
const base::string16& message);

void FlushForTesting();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,36 +156,65 @@ class MultiDeviceNotificationPresenterTest : public NoSessionAshTestBase {
InvokePendingMojoCalls();
}

void ShowWifiSyncNotification() {
EXPECT_TRUE(fake_multidevice_setup_->delegate().is_bound());
fake_multidevice_setup_->delegate()->OnBecameEligibleForWifiSync();
InvokePendingMojoCalls();
}

void ClickNotification() {
test_message_center_.ClickOnNotification(
MultiDeviceNotificationPresenter::kNotificationId);
MultiDeviceNotificationPresenter::kSetupNotificationId);
}

void ClickWifiSyncNotification() {
test_message_center_.ClickOnNotification(
MultiDeviceNotificationPresenter::kWifiSyncNotificationId);
}

void DismissNotification(bool by_user) {
test_message_center_.RemoveNotification(
MultiDeviceNotificationPresenter::kNotificationId, by_user);
MultiDeviceNotificationPresenter::kSetupNotificationId, by_user);
}

void VerifyNewUserPotentialHostExistsNotificationIsVisible() {
VerifyNotificationIsVisible(
VerifySetupNotificationIsVisible(
MultiDeviceNotificationPresenter::Status::kNewUserNotificationVisible);
}

void VerifyExistingUserHostSwitchedNotificationIsVisible() {
VerifyNotificationIsVisible(
VerifySetupNotificationIsVisible(
MultiDeviceNotificationPresenter::Status::
kExistingUserHostSwitchedNotificationVisible);
}

void VerifyExistingUserNewChromebookAddedNotificationIsVisible() {
VerifyNotificationIsVisible(
VerifySetupNotificationIsVisible(
MultiDeviceNotificationPresenter::Status::
kExistingUserNewChromebookNotificationVisible);
}

void VerifyWifiSyncNotificationIsVisible() {
const message_center::Notification* kVisibleNotification =
test_message_center_.FindVisibleNotificationById(
MultiDeviceNotificationPresenter::kWifiSyncNotificationId);
base::string16 title = l10n_util::GetStringUTF16(
IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_TITLE);
base::string16 message = l10n_util::GetStringFUTF16(
IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_MESSAGE,
base::ASCIIToUTF16(kTestDeviceType));
EXPECT_EQ(title, kVisibleNotification->title());
EXPECT_EQ(message, kVisibleNotification->message());
}

void VerifyNoNotificationIsVisible() {
EXPECT_FALSE(test_message_center_.FindVisibleNotificationById(
MultiDeviceNotificationPresenter::kNotificationId));
MultiDeviceNotificationPresenter::kSetupNotificationId));
}

void VerifyNoWifiSyncNotificationIsVisible() {
EXPECT_FALSE(test_message_center_.FindVisibleNotificationById(
MultiDeviceNotificationPresenter::kWifiSyncNotificationId));
}

void AssertPotentialHostBucketCount(std::string histogram, int count) {
Expand Down Expand Up @@ -232,11 +261,11 @@ class MultiDeviceNotificationPresenterTest : public NoSessionAshTestBase {
std::unique_ptr<MultiDeviceNotificationPresenter> notification_presenter_;

private:
void VerifyNotificationIsVisible(
void VerifySetupNotificationIsVisible(
MultiDeviceNotificationPresenter::Status notification_status) {
const message_center::Notification* kVisibleNotification =
test_message_center_.FindVisibleNotificationById(
MultiDeviceNotificationPresenter::kNotificationId);
MultiDeviceNotificationPresenter::kSetupNotificationId);
base::string16 title;
base::string16 message;
switch (notification_status) {
Expand Down Expand Up @@ -436,6 +465,21 @@ TEST_F(
AssertNewChromebookBucketCount("MultiDeviceSetup_NotificationShown", 1);
}

TEST_F(MultiDeviceNotificationPresenterTest,
TestWifiSyncNotification_TapNotification) {
SignIntoAccount();

ShowWifiSyncNotification();
VerifyWifiSyncNotificationIsVisible();

ClickWifiSyncNotification();

VerifyNoWifiSyncNotificationIsVisible();

EXPECT_EQ(test_system_tray_client_->show_connected_devices_settings_count(),
1);
}

TEST_F(MultiDeviceNotificationPresenterTest,
TestHostExistingUserNewChromebookAddedNotification_TapNotification) {
SignIntoAccount();
Expand Down
2 changes: 2 additions & 0 deletions chromeos/services/multidevice_setup/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ static_library("multidevice_setup") {
"//chromeos/services/secure_channel/public/mojom",
"//components/pref_registry",
"//components/prefs:prefs",
"//components/session_manager/core",
"//url",
]

Expand Down Expand Up @@ -162,6 +163,7 @@ source_set("unit_tests") {
"//chromeos/services/multidevice_setup/public/cpp:unit_tests",
"//chromeos/services/multidevice_setup/public/mojom",
"//chromeos/services/secure_channel/public/cpp/client:test_support",
"//components/session_manager/core",
"//components/sync_preferences:test_support",
"//testing/gmock",
"//testing/gtest",
Expand Down
1 change: 1 addition & 0 deletions chromeos/services/multidevice_setup/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ include_rules = [
"+chrome/common/url_constants.h",
"+components/proximity_auth/logging/logging.h",
'+components/keyed_service/core',
"+components/session_manager/core",
"+components/sync_preferences/testing_pref_service_syncable.h",
"+components/variations",
"+mojo/public/cpp/bindings",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ class AccountStatusChangeDelegateNotifier {
friend class MultiDeviceSetupImpl;
friend class MultiDeviceSetupAccountStatusChangeDelegateNotifierTest;
friend class MultiDeviceSetupImplTest;
friend class WifiSyncFeatureManagerImpl;
friend class MultiDeviceSetupWifiSyncFeatureManagerImplTest;

void FlushForTesting();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ void FakeAccountStatusChangeDelegate::OnNewChromebookAddedForExistingUser(
++num_existing_user_chromebook_added_events_handled_;
}

void FakeAccountStatusChangeDelegate::OnBecameEligibleForWifiSync() {
++num_eligible_for_wifi_sync_events_handled_;
}

} // namespace multidevice_setup

} // namespace chromeos
Original file line number Diff line number Diff line change
Expand Up @@ -23,35 +23,41 @@ class FakeAccountStatusChangeDelegate

mojo::PendingRemote<mojom::AccountStatusChangeDelegate> GenerateRemote();

size_t num_new_user_potential_host_events_handled() {
size_t num_new_user_potential_host_events_handled() const {
return num_new_user_potential_host_events_handled_;
}

size_t num_no_longer_new_user_events_handled() {
size_t num_no_longer_new_user_events_handled() const {
return num_no_longer_new_user_events_handled_;
}

size_t num_existing_user_host_switched_events_handled() {
size_t num_existing_user_host_switched_events_handled() const {
return num_existing_user_host_switched_events_handled_;
}

size_t num_existing_user_chromebook_added_events_handled() {
size_t num_existing_user_chromebook_added_events_handled() const {
return num_existing_user_chromebook_added_events_handled_;
}

size_t num_eligible_for_wifi_sync_events_handled() const {
return num_eligible_for_wifi_sync_events_handled_;
}

// mojom::AccountStatusChangeDelegate:
void OnPotentialHostExistsForNewUser() override;
void OnNoLongerNewUser() override;
void OnConnectedHostSwitchedForExistingUser(
const std::string& new_host_device_name) override;
void OnNewChromebookAddedForExistingUser(
const std::string& new_host_device_name) override;
void OnBecameEligibleForWifiSync() override;

private:
size_t num_new_user_potential_host_events_handled_ = 0u;
size_t num_no_longer_new_user_events_handled_ = 0u;
size_t num_existing_user_host_switched_events_handled_ = 0u;
size_t num_existing_user_chromebook_added_events_handled_ = 0u;
size_t num_eligible_for_wifi_sync_events_handled_ = 0u;

mojo::ReceiverSet<mojom::AccountStatusChangeDelegate> receivers_;

Expand Down
Loading

0 comments on commit 695ff57

Please sign in to comment.