Wi-Fi Sync: Make improvements to announcement notification.

* Adds a "Turn on" button that actually enables the feature.
* Adds a "Cancel" button that dismisses.
* Deep-links to focus on the feature toggle in settings.
* Shows on resume (for users that don't use the lockscreen).

(cherry picked from commit b45fc03)

Bug: 1182329
Change-Id: I99515a323a68a4b2942374cdbcbc4119fd7bea18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2728409
Commit-Queue: Jon Mann <jonmann@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#860391}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2740422
Auto-Submit: Jon Mann <jonmann@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4430@{#188}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}

ash/ash_strings.grd

@@ -2717,6 +2717,12 @@ This file contains the strings for ash.
       <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>
+      <message name="IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_TURN_ON_BUTTON" desc="Button shown on the bottom of notification to enable Wi-Fi Sync for newly eligible users.">
+        Turn on
+      </message>
+      <message name="IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_CANCEL_BUTTON" desc="Cancel button shown on the bottom of the Wi-Fi Sync announcement notification.">
+        Cancel
+      </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.">

ash/ash_strings_grd/IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_CANCEL_BUTTON.png.sha1

@@ -0,0 +1 @@
+7c2983c6a03e11330d9dc49a7b096aed74bbbb5f

ash/ash_strings_grd/IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_TURN_ON_BUTTON.png.sha1

@@ -0,0 +1 @@
+7c2983c6a03e11330d9dc49a7b096aed74bbbb5f

ash/multi_device_setup/multi_device_notification_presenter.cc

@@ -23,7 +23,6 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/chromeos/devicetype_utils.h"
 #include "ui/message_center/message_center.h"
-#include "ui/message_center/public/cpp/notification_types.h"
 #include "ui/message_center/public/cpp/notifier_id.h"
 
 namespace ash {
@@ -139,7 +138,15 @@ void MultiDeviceNotificationPresenter::OnBecameEligibleForWifiSync() {
   base::string16 message = l10n_util::GetStringFUTF16(
       IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_MESSAGE,
       ui::GetChromeOSDeviceName());
-  ShowNotification(kWifiSyncNotificationId, title, message);
+  message_center::RichNotificationData optional_fields;
+  optional_fields.buttons.push_back(
+      message_center::ButtonInfo(l10n_util::GetStringUTF16(
+          IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_TURN_ON_BUTTON)));
+  optional_fields.buttons.push_back(
+      message_center::ButtonInfo(l10n_util::GetStringUTF16(
+          IDS_ASH_MULTI_DEVICE_WIFI_SYNC_AVAILABLE_CANCEL_BUTTON)));
+
+  ShowNotification(kWifiSyncNotificationId, title, message, optional_fields);
   base::UmaHistogramEnumeration("MultiDeviceSetup_NotificationShown",
                                 NotificationType::kWifiSyncAnnouncement);
 }
@@ -184,9 +191,27 @@ void MultiDeviceNotificationPresenter::OnNotificationClicked(
     const base::Optional<int>& button_index,
     const base::Optional<base::string16>& reply) {
   if (notification_id == kWifiSyncNotificationId) {
-    Shell::Get()->system_tray_model()->client()->ShowConnectedDevicesSettings();
     message_center_->RemoveNotification(kWifiSyncNotificationId,
                                         /* by_user */ false);
+
+    if (button_index) {
+      switch (*button_index) {
+        case 0:  // "Turn on" button
+          PA_LOG(INFO) << "Enabling Wi-Fi Sync.";
+          multidevice_setup_remote_->SetFeatureEnabledState(
+              chromeos::multidevice_setup::mojom::Feature::kWifiSync,
+              /*enabled=*/true, /*auth_token=*/base::nullopt,
+              /*callback=*/base::DoNothing());
+          break;
+        case 1:  // "Cancel" button
+          base::UmaHistogramEnumeration(
+              "MultiDeviceSetup_NotificationDismissed",
+              NotificationType::kWifiSyncAnnouncement);
+          return;
+      }
+    }
+
+    Shell::Get()->system_tray_model()->client()->ShowWifiSyncSettings();
     base::UmaHistogramEnumeration("MultiDeviceSetup_NotificationClicked",
                                   NotificationType::kWifiSyncAnnouncement);
     return;
@@ -262,14 +287,16 @@ void MultiDeviceNotificationPresenter::ShowSetupNotification(
       "MultiDeviceSetup_NotificationShown",
       GetMetricValueForNotification(notification_status));
 
-  ShowNotification(kSetupNotificationId, title, message);
+  ShowNotification(kSetupNotificationId, title, message,
+                   message_center::RichNotificationData());
   notification_status_ = notification_status;
 }
 
 void MultiDeviceNotificationPresenter::ShowNotification(
     const std::string& id,
     const base::string16& title,
-    const base::string16& message) {
+    const base::string16& message,
+    message_center::RichNotificationData optional_fields) {
   std::unique_ptr<message_center::Notification> notification =
       CreateSystemNotification(
           message_center::NotificationType::NOTIFICATION_TYPE_SIMPLE, id, title,
@@ -278,7 +305,7 @@ void MultiDeviceNotificationPresenter::ShowNotification(
           message_center::NotifierId(
               message_center::NotifierType::SYSTEM_COMPONENT,
               kNotifierMultiDevice),
-          message_center::RichNotificationData(), nullptr /* delegate */,
+          optional_fields, nullptr /* delegate */,
           kNotificationMultiDeviceSetupIcon,
           message_center::SystemNotificationWarningLevel::NORMAL);
 

ash/multi_device_setup/multi_device_notification_presenter.h

@@ -22,6 +22,7 @@
 namespace message_center {
 class MessageCenter;
 class Notification;
+class RichNotificationData;
 }  // namespace message_center
 
 namespace ash {
@@ -117,7 +118,8 @@ class ASH_EXPORT MultiDeviceNotificationPresenter
                              const base::string16& message);
   void ShowNotification(const std::string& id,
                         const base::string16& title,
-                        const base::string16& message);
+                        const base::string16& message,
+                        message_center::RichNotificationData optional_fields);
 
   void FlushForTesting();
 

ash/multi_device_setup/multi_device_notification_presenter_unittest.cc

@@ -81,6 +81,14 @@ class TestMessageCenter : public message_center::FakeMessageCenter {
       observer.OnNotificationClicked(id, base::nullopt, base::nullopt);
   }
 
+  void ClickOnNotificationButton(const std::string& id,
+                                 int button_index) override {
+    EXPECT_TRUE(notification_);
+    EXPECT_EQ(id, notification_->id());
+    for (auto& observer : observer_list())
+      observer.OnNotificationClicked(id, button_index, base::nullopt);
+  }
+
  private:
   std::unique_ptr<message_center::Notification> notification_;
 
@@ -172,6 +180,12 @@ class MultiDeviceNotificationPresenterTest : public NoSessionAshTestBase {
         MultiDeviceNotificationPresenter::kWifiSyncNotificationId);
   }
 
+  void ClickWifiSyncNotificationButton(int button_index) {
+    test_message_center_.ClickOnNotificationButton(
+        MultiDeviceNotificationPresenter::kWifiSyncNotificationId,
+        button_index);
+  }
+
   void DismissWifiSyncNotification(bool by_user) {
     test_message_center_.RemoveNotification(
         MultiDeviceNotificationPresenter::kWifiSyncNotificationId, by_user);
@@ -493,14 +507,48 @@ TEST_F(MultiDeviceNotificationPresenterTest,
 
   VerifyNoWifiSyncNotificationIsVisible();
 
-  EXPECT_EQ(test_system_tray_client_->show_connected_devices_settings_count(),
-            1);
+  EXPECT_EQ(test_system_tray_client_->show_wifi_sync_settings_count(), 1);
+
+  AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationClicked", 1);
+  AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationDismissed", 0);
+  AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationShown", 1);
+}
+
+TEST_F(MultiDeviceNotificationPresenterTest,
+       TestWifiSyncNotification_TapTurnOnButton) {
+  SignIntoAccount();
+
+  ShowWifiSyncNotification();
+  VerifyWifiSyncNotificationIsVisible();
+
+  ClickWifiSyncNotificationButton(0);
+
+  VerifyNoWifiSyncNotificationIsVisible();
+
+  EXPECT_EQ(test_system_tray_client_->show_wifi_sync_settings_count(), 1);
 
   AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationClicked", 1);
   AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationDismissed", 0);
   AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationShown", 1);
 }
 
+TEST_F(MultiDeviceNotificationPresenterTest,
+       TestWifiSyncNotification_TapCancelButton) {
+  SignIntoAccount();
+
+  ShowWifiSyncNotification();
+  VerifyWifiSyncNotificationIsVisible();
+
+  ClickWifiSyncNotificationButton(1);
+  VerifyNoWifiSyncNotificationIsVisible();
+
+  EXPECT_EQ(test_system_tray_client_->show_wifi_sync_settings_count(), 0);
+
+  AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationClicked", 0);
+  AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationDismissed", 1);
+  AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationShown", 1);
+}
+
 TEST_F(MultiDeviceNotificationPresenterTest,
        TestWifiSyncNotification_DismissedNotification) {
   SignIntoAccount();
@@ -511,8 +559,7 @@ TEST_F(MultiDeviceNotificationPresenterTest,
   DismissWifiSyncNotification(/*by_user=*/true);
   VerifyNoWifiSyncNotificationIsVisible();
 
-  EXPECT_EQ(test_system_tray_client_->show_connected_devices_settings_count(),
-            0);
+  EXPECT_EQ(test_system_tray_client_->show_wifi_sync_settings_count(), 0);
 
   AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationClicked", 0);
   AssertWifiSyncBucketCount("MultiDeviceSetup_NotificationDismissed", 1);

ash/public/cpp/system_tray_client.h

@@ -60,6 +60,9 @@ class ASH_PUBLIC_EXPORT SystemTrayClient {
   // Shows settings related to tether network.
   virtual void ShowTetherNetworkSettings() = 0;
 
+  // Shows settings related to Wi-Fi Sync v2.
+  virtual void ShowWifiSyncSettings() = 0;
+
   // Shows the about chrome OS page and checks for updates after the page is
   // loaded.
   virtual void ShowAboutChromeOS() = 0;

ash/public/cpp/test/test_system_tray_client.cc

@@ -40,6 +40,10 @@ void TestSystemTrayClient::ShowConnectedDevicesSettings() {
 
 void TestSystemTrayClient::ShowTetherNetworkSettings() {}
 
+void TestSystemTrayClient::ShowWifiSyncSettings() {
+  show_wifi_sync_settings_count_++;
+}
+
 void TestSystemTrayClient::ShowAboutChromeOS() {}
 
 void TestSystemTrayClient::ShowHelp() {}

ash/public/cpp/test/test_system_tray_client.h

@@ -32,6 +32,7 @@ class ASH_PUBLIC_EXPORT TestSystemTrayClient : public SystemTrayClient {
   void ShowIMESettings() override;
   void ShowConnectedDevicesSettings() override;
   void ShowTetherNetworkSettings() override;
+  void ShowWifiSyncSettings() override;
   void ShowAboutChromeOS() override;
   void ShowHelp() override;
   void ShowAccessibilityHelp() override;
@@ -66,11 +67,16 @@ class ASH_PUBLIC_EXPORT TestSystemTrayClient : public SystemTrayClient {
     return show_os_settings_privacy_and_security_count_;
   }
 
+  int show_wifi_sync_settings_count() const {
+    return show_wifi_sync_settings_count_;
+  }
+
  private:
   int show_bluetooth_settings_count_ = 0;
   int show_multi_device_setup_count_ = 0;
   int show_connected_devices_settings_count_ = 0;
   int show_os_settings_privacy_and_security_count_ = 0;
+  int show_wifi_sync_settings_count_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(TestSystemTrayClient);
 };

chrome/browser/ui/ash/system_tray_client.cc

@@ -369,6 +369,14 @@ void SystemTrayClient::ShowTetherNetworkSettings() {
       chromeos::settings::mojom::kMobileDataNetworksSubpagePath);
 }
 
+void SystemTrayClient::ShowWifiSyncSettings() {
+  ShowSettingsSubPageForActiveUser(
+      std::string(chromeos::settings::mojom::kMultiDeviceFeaturesSubpagePath) +
+      "?settingId=" +
+      base::NumberToString(static_cast<int32_t>(
+          chromeos::settings::mojom::Setting::kWifiSyncOnOff)));
+}
+
 void SystemTrayClient::ShowAboutChromeOS() {
   // We always want to check for updates when showing the about page from the
   // Ash UI.

chrome/browser/ui/ash/system_tray_client.h

@@ -67,6 +67,7 @@ class SystemTrayClient : public ash::SystemTrayClient,
   void ShowIMESettings() override;
   void ShowConnectedDevicesSettings() override;
   void ShowTetherNetworkSettings() override;
+  void ShowWifiSyncSettings() override;
   void ShowAboutChromeOS() override;
   void ShowHelp() override;
   void ShowAccessibilityHelp() override;

chromeos/services/multidevice_setup/wifi_sync_feature_manager_impl.cc

@@ -9,6 +9,7 @@
 #include "ash/constants/ash_features.h"
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
+#include "base/power_monitor/power_monitor.h"
 #include "base/stl_util.h"
 #include "chromeos/components/multidevice/logging/logging.h"
 #include "chromeos/components/multidevice/software_feature.h"
@@ -91,7 +92,12 @@ WifiSyncFeatureManagerImpl::WifiSyncFeatureManagerImpl(
       timer_(std::move(timer)) {
   host_status_provider_->AddObserver(this);
   device_sync_client_->AddObserver(this);
-  session_manager::SessionManager::Get()->AddObserver(this);
+
+  if (pref_service_->GetBoolean(kCanShowAnnouncementPrefName)) {
+    session_manager::SessionManager::Get()->AddObserver(this);
+    base::PowerMonitor::AddObserver(this);
+    did_register_session_observers_ = true;
+  }
 
   if (GetCurrentState() == CurrentState::kValidPendingRequest) {
     AttemptSetWifiSyncHostStateNetworkRequest(false /* is_retry */);
@@ -105,7 +111,10 @@ WifiSyncFeatureManagerImpl::WifiSyncFeatureManagerImpl(
 WifiSyncFeatureManagerImpl::~WifiSyncFeatureManagerImpl() {
   host_status_provider_->RemoveObserver(this);
   device_sync_client_->RemoveObserver(this);
-  session_manager::SessionManager::Get()->RemoveObserver(this);
+  if (did_register_session_observers_) {
+    session_manager::SessionManager::Get()->RemoveObserver(this);
+    base::PowerMonitor::RemoveObserver(this);
+  }
 }
 
 void WifiSyncFeatureManagerImpl::OnHostStatusChange(
@@ -134,18 +143,23 @@ void WifiSyncFeatureManagerImpl::OnNewDevicesSynced() {
 }
 
 void WifiSyncFeatureManagerImpl::OnSessionStateChanged() {
-  if (session_manager::SessionManager::Get()->IsUserSessionBlocked()) {
-    return;
-  }
+  ShowAnnouncementNotificationIfEligible();
+}
 
-  // Show the announcement notification when the device is unlocked and
-  // eligible for wi-fi sync.  This is done on unlock to avoid showing the
-  // notification on the first sign-in when it would distract from showoff
-  // and other announcements.
+void WifiSyncFeatureManagerImpl::OnResume() {
   ShowAnnouncementNotificationIfEligible();
 }
 
 void WifiSyncFeatureManagerImpl::ShowAnnouncementNotificationIfEligible() {
+  // Show the announcement notification when the device is unlocked and
+  // eligible for wi-fi sync.  This is done on unlock/resume to avoid showing
+  // it on the first sign-in when it would distract from showoff and other
+  // announcements.
+
+  if (session_manager::SessionManager::Get()->IsUserSessionBlocked()) {
+    return;
+  }
+
   if (!IsFeatureAllowed(mojom::Feature::kWifiSync, pref_service_)) {
     return;
   }

chromeos/services/multidevice_setup/wifi_sync_feature_manager_impl.h

@@ -9,6 +9,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "base/power_monitor/power_observer.h"
 #include "base/timer/timer.h"
 #include "chromeos/components/multidevice/remote_device_ref.h"
 #include "chromeos/services/device_sync/public/cpp/device_sync_client.h"
@@ -35,6 +36,7 @@ class WifiSyncFeatureManagerImpl
     : public WifiSyncFeatureManager,
       public HostStatusProvider::Observer,
       public device_sync::DeviceSyncClient::Observer,
+      public base::PowerObserver,
       public session_manager::SessionManagerObserver {
  public:
   class Factory {
@@ -86,6 +88,9 @@ class WifiSyncFeatureManagerImpl
   // SessionManagerObserver:
   void OnSessionStateChanged() override;
 
+  // PowerObserver:
+  void OnResume() override;
+
   // WifiSyncFeatureManager:
 
   // Attempts to enable/disable WIFI_SYNC_HOST on the backend for the host
@@ -143,6 +148,7 @@ class WifiSyncFeatureManagerImpl
   AccountStatusChangeDelegateNotifier* delegate_notifier_;
   std::unique_ptr<base::OneShotTimer> timer_;
 
+  bool did_register_session_observers_ = false;
   bool network_request_in_flight_ = false;
 
   base::WeakPtrFactory<WifiSyncFeatureManagerImpl> weak_ptr_factory_{this};