Split wifi toggle notification from TrayNetwork.

The handling of RequestToggleWifi and wifi toggle notification was done
in TrayNetwork, which is a SystemTrayItem. We're going to get rid of
SystemTrayItems as UnifiedSystemTray will replace them, so this should
be a NotificationController.

TEST=WifiToggleNotificationController
BUG=837507

Change-Id: I6693db6df2c8ab2fcf8f7954adcd61a48e00f24d
Reviewed-on: https://chromium-review.googlesource.com/1046289
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556726}

ash/BUILD.gn

@@ -647,6 +647,8 @@ component("ash") {
     "system/network/vpn_list.h",
     "system/network/vpn_list_view.cc",
     "system/network/vpn_list_view.h",
+    "system/network/wifi_toggle_notification_controller.cc",
+    "system/network/wifi_toggle_notification_controller.h",
     "system/networking_config_delegate.cc",
     "system/networking_config_delegate.h",
     "system/night_light/night_light_controller.cc",
@@ -1681,6 +1683,7 @@ test("ash_unittests") {
     "system/network/sms_observer_unittest.cc",
     "system/network/tray_network_unittest.cc",
     "system/network/vpn_list_unittest.cc",
+    "system/network/wifi_toggle_notification_controller_unittest.cc",
     "system/night_light/night_light_controller_unittest.cc",
     "system/night_light/time_of_day_unittest.cc",
     "system/night_light/tray_night_light_unittest.cc",

ash/system/network/tray_network.cc

@@ -27,8 +27,6 @@
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/image/image_skia_operations.h"
-#include "ui/message_center/message_center.h"
-#include "ui/message_center/public/cpp/notification.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/controls/link_listener.h"
@@ -38,37 +36,18 @@ using chromeos::NetworkHandler;
 using chromeos::NetworkState;
 using chromeos::NetworkStateHandler;
 using chromeos::NetworkTypePattern;
-using message_center::Notification;
 
 namespace ash {
 namespace tray {
 
 namespace {
 
-constexpr char kWifiToggleNotificationId[] = "wifi-toggle";
-constexpr char kNotifierWifiToggle[] = "ash.wifi-toggle";
-
 // Returns the connected, non-virtual (aka VPN), network.
 const NetworkState* GetConnectedNetwork() {
   NetworkStateHandler* handler = NetworkHandler::Get()->network_state_handler();
   return handler->ConnectedNetworkByType(NetworkTypePattern::NonVirtual());
 }
 
-std::unique_ptr<Notification> CreateNotification(bool wifi_enabled) {
-  const int string_id = wifi_enabled
-                            ? IDS_ASH_STATUS_TRAY_NETWORK_WIFI_ENABLED
-                            : IDS_ASH_STATUS_TRAY_NETWORK_WIFI_DISABLED;
-  std::unique_ptr<Notification> notification(new Notification(
-      message_center::NOTIFICATION_TYPE_SIMPLE, kWifiToggleNotificationId,
-      base::string16(), l10n_util::GetStringUTF16(string_id),
-      gfx::Image(network_icon::GetImageForWiFiEnabledState(wifi_enabled)),
-      base::string16() /* display_source */, GURL(),
-      message_center::NotifierId(message_center::NotifierId::SYSTEM_COMPONENT,
-                                 kNotifierWifiToggle),
-      message_center::RichNotificationData(), nullptr));
-  return notification;
-}
-
 }  // namespace
 
 class NetworkTrayView : public TrayItemView,
@@ -237,14 +216,9 @@ TrayNetwork::TrayNetwork(SystemTray* system_tray)
       default_(nullptr),
       detailed_(nullptr) {
   network_state_observer_.reset(new TrayNetworkStateObserver(this));
-  SystemTrayNotifier* notifier = Shell::Get()->system_tray_notifier();
-  notifier->AddNetworkObserver(this);
 }
 
-TrayNetwork::~TrayNetwork() {
-  SystemTrayNotifier* notifier = Shell::Get()->system_tray_notifier();
-  notifier->RemoveNetworkObserver(this);
-}
+TrayNetwork::~TrayNetwork() = default;
 
 views::View* TrayNetwork::CreateTrayView(LoginStatus status) {
   CHECK(tray_ == nullptr);
@@ -287,23 +261,6 @@ void TrayNetwork::OnDetailedViewDestroyed() {
   detailed_ = nullptr;
 }
 
-void TrayNetwork::RequestToggleWifi() {
-  // This will always be triggered by a user action (e.g. keyboard shortcut)
-  NetworkStateHandler* handler = NetworkHandler::Get()->network_state_handler();
-  bool enabled = handler->IsTechnologyEnabled(NetworkTypePattern::WiFi());
-  Shell::Get()->metrics()->RecordUserMetricsAction(
-      enabled ? UMA_STATUS_AREA_DISABLE_WIFI : UMA_STATUS_AREA_ENABLE_WIFI);
-  handler->SetTechnologyEnabled(NetworkTypePattern::WiFi(), !enabled,
-                                chromeos::network_handler::ErrorCallback());
-  message_center::MessageCenter* message_center =
-      message_center::MessageCenter::Get();
-  if (message_center->FindVisibleNotificationById(
-          tray::kWifiToggleNotificationId)) {
-    message_center->RemoveNotification(tray::kWifiToggleNotificationId, false);
-  }
-  message_center->AddNotification(tray::CreateNotification(!enabled));
-}
-
 void TrayNetwork::NetworkStateChanged(bool notify_a11y) {
   if (tray_) {
     tray_->UpdateNetworkStateHandlerIcon();

ash/system/network/tray_network.h

@@ -8,7 +8,6 @@
 #include <memory>
 #include <set>
 
-#include "ash/system/network/network_observer.h"
 #include "ash/system/network/tray_network_state_observer.h"
 #include "ash/system/tray/system_tray_item.h"
 #include "base/macros.h"
@@ -22,7 +21,6 @@ class NetworkTrayView;
 }
 
 class TrayNetwork : public SystemTrayItem,
-                    public NetworkObserver,
                     public TrayNetworkStateObserver::Delegate {
  public:
   explicit TrayNetwork(SystemTray* system_tray);
@@ -38,9 +36,6 @@ class TrayNetwork : public SystemTrayItem,
   void OnDefaultViewDestroyed() override;
   void OnDetailedViewDestroyed() override;
 
-  // NetworkObserver
-  void RequestToggleWifi() override;
-
   // TrayNetworkStateObserver::Delegate
   void NetworkStateChanged(bool notify_a11y) override;
 

ash/system/network/tray_network_unittest.cc

@@ -15,9 +15,6 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/network/network_handler.h"
 #include "components/prefs/testing_pref_service.h"
-#include "ui/message_center/message_center.h"
-
-using message_center::MessageCenter;
 
 namespace ash {
 namespace {
@@ -79,23 +76,6 @@ TEST_F(TrayNetworkTest, Basics) {
   EXPECT_TRUE(tray_network->detailed()->visible());
 }
 
-// Verifies that toggling Wi-Fi (usually via keyboard) shows a notification.
-TEST_F(TrayNetworkTest, ToggleWifi) {
-  TrayNetwork* tray_network =
-      SystemTrayTestApi(GetPrimarySystemTray()).tray_network();
-
-  // No notifications at startup.
-  ASSERT_EQ(0u, MessageCenter::Get()->NotificationCount());
-
-  // Simulate a user action to toggle Wi-Fi.
-  tray_network->RequestToggleWifi();
-
-  // Notification was shown.
-  EXPECT_EQ(1u, MessageCenter::Get()->NotificationCount());
-  EXPECT_TRUE(MessageCenter::Get()->HasPopupNotifications());
-  EXPECT_TRUE(MessageCenter::Get()->FindVisibleNotificationById("wifi-toggle"));
-}
-
 // Open network info bubble and close network detailed view. Confirm that it
 // doesn't crash.
 TEST_F(TrayNetworkTest, NetworkInfoBubble) {

ash/system/network/wifi_toggle_notification_controller.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/system/network/wifi_toggle_notification_controller.h"
+
+#include "ash/shell.h"
+#include "ash/strings/grit/ash_strings.h"
+#include "ash/system/network/network_icon.h"
+#include "ash/system/tray/system_tray_notifier.h"
+#include "chromeos/network/network_state_handler.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/message_center/message_center.h"
+#include "ui/message_center/public/cpp/notification.h"
+
+using chromeos::NetworkHandler;
+using chromeos::NetworkStateHandler;
+using chromeos::NetworkTypePattern;
+using message_center::Notification;
+
+namespace ash {
+
+namespace {
+
+constexpr char kWifiToggleNotificationId[] = "wifi-toggle";
+constexpr char kNotifierWifiToggle[] = "ash.wifi-toggle";
+
+std::unique_ptr<Notification> CreateNotification(bool wifi_enabled) {
+  const int string_id = wifi_enabled
+                            ? IDS_ASH_STATUS_TRAY_NETWORK_WIFI_ENABLED
+                            : IDS_ASH_STATUS_TRAY_NETWORK_WIFI_DISABLED;
+  std::unique_ptr<Notification> notification = std::make_unique<Notification>(
+      message_center::NOTIFICATION_TYPE_SIMPLE, kWifiToggleNotificationId,
+      base::string16(), l10n_util::GetStringUTF16(string_id),
+      gfx::Image(network_icon::GetImageForWiFiEnabledState(wifi_enabled)),
+      base::string16() /* display_source */, GURL(),
+      message_center::NotifierId(message_center::NotifierId::SYSTEM_COMPONENT,
+                                 kNotifierWifiToggle),
+      message_center::RichNotificationData(), nullptr);
+  return notification;
+}
+
+}  // namespace
+
+WifiToggleNotificationController::WifiToggleNotificationController() {
+  Shell::Get()->system_tray_notifier()->AddNetworkObserver(this);
+}
+
+WifiToggleNotificationController::~WifiToggleNotificationController() {
+  Shell::Get()->system_tray_notifier()->RemoveNetworkObserver(this);
+}
+
+void WifiToggleNotificationController::RequestToggleWifi() {
+  // This will always be triggered by a user action (e.g. keyboard shortcut)
+  NetworkStateHandler* handler = NetworkHandler::Get()->network_state_handler();
+  bool enabled = handler->IsTechnologyEnabled(NetworkTypePattern::WiFi());
+  Shell::Get()->metrics()->RecordUserMetricsAction(
+      enabled ? UMA_STATUS_AREA_DISABLE_WIFI : UMA_STATUS_AREA_ENABLE_WIFI);
+  handler->SetTechnologyEnabled(NetworkTypePattern::WiFi(), !enabled,
+                                chromeos::network_handler::ErrorCallback());
+  message_center::MessageCenter* message_center =
+      message_center::MessageCenter::Get();
+  if (message_center->FindVisibleNotificationById(kWifiToggleNotificationId))
+    message_center->RemoveNotification(kWifiToggleNotificationId, false);
+  message_center->AddNotification(CreateNotification(!enabled));
+}
+
+}  // namespace ash

ash/system/network/wifi_toggle_notification_controller.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_SYSTEM_NETWORK_WIFI_TOGGLE_NOTIFICATION_CONTROLLER_H_
+#define ASH_SYSTEM_NETWORK_WIFI_TOGGLE_NOTIFICATION_CONTROLLER_H_
+
+#include "ash/system/network/network_observer.h"
+#include "base/macros.h"
+
+namespace ash {
+
+class WifiToggleNotificationController : public NetworkObserver {
+ public:
+  WifiToggleNotificationController();
+  ~WifiToggleNotificationController() override;
+
+  // NetworkObserver:
+  void RequestToggleWifi() override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(WifiToggleNotificationController);
+};
+
+}  // namespace ash
+
+#endif  // ASH_SYSTEM_NETWORK_WIFI_TOGGLE_NOTIFICATION_CONTROLLER_H_

ash/system/network/wifi_toggle_notification_controller_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/system/network/wifi_toggle_notification_controller.h"
+
+#include "ash/public/cpp/config.h"
+#include "ash/shell.h"
+#include "ash/system/tray/system_tray_notifier.h"
+#include "ash/test/ash_test_base.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/network/network_handler.h"
+#include "components/prefs/testing_pref_service.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/message_center/message_center.h"
+
+using message_center::MessageCenter;
+
+namespace ash {
+
+class WifiToggleNotificationControllerTest : public AshTestBase {
+ public:
+  WifiToggleNotificationControllerTest() = default;
+  ~WifiToggleNotificationControllerTest() override = default;
+
+  // testing::Test:
+  void SetUp() override {
+    chromeos::DBusThreadManager::Initialize();
+    // Initializing NetworkHandler before ash is more like production.
+    chromeos::NetworkHandler::Initialize();
+    AshTestBase::SetUp();
+    // Mash doesn't do this yet, so don't do it in tests either.
+    // http://crbug.com/718072
+    if (Shell::GetAshConfig() != Config::MASH) {
+      chromeos::NetworkHandler::Get()->InitializePrefServices(&profile_prefs_,
+                                                              &local_state_);
+    }
+    // Networking stubs may have asynchronous initialization.
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void TearDown() override {
+    // This roughly matches production shutdown order.
+    if (Shell::GetAshConfig() != Config::MASH) {
+      chromeos::NetworkHandler::Get()->ShutdownPrefServices();
+    }
+    AshTestBase::TearDown();
+    chromeos::NetworkHandler::Shutdown();
+    chromeos::DBusThreadManager::Shutdown();
+  }
+
+ private:
+  TestingPrefServiceSimple profile_prefs_;
+  TestingPrefServiceSimple local_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(WifiToggleNotificationControllerTest);
+};
+
+// Verifies that toggling Wi-Fi (usually via keyboard) shows a notification.
+TEST_F(WifiToggleNotificationControllerTest, ToggleWifi) {
+  // No notifications at startup.
+  ASSERT_EQ(0u, MessageCenter::Get()->NotificationCount());
+
+  // Simulate a user action to toggle Wi-Fi.
+  Shell::Get()->system_tray_notifier()->NotifyRequestToggleWifi();
+
+  // Notification was shown.
+  EXPECT_EQ(1u, MessageCenter::Get()->NotificationCount());
+  EXPECT_TRUE(MessageCenter::Get()->HasPopupNotifications());
+  EXPECT_TRUE(MessageCenter::Get()->FindVisibleNotificationById("wifi-toggle"));
+}
+
+}  // namespace ash

ash/system/system_notification_controller.cc

@@ -5,6 +5,7 @@
 #include "ash/system/system_notification_controller.h"
 
 #include "ash/system/caps_lock_notification_controller.h"
+#include "ash/system/network/wifi_toggle_notification_controller.h"
 #include "ash/system/power/power_notification_controller.h"
 #include "ash/system/screen_security/screen_security_notification_controller.h"
 #include "ash/system/session/session_limit_notification_controller.h"
@@ -20,7 +21,8 @@ SystemNotificationController::SystemNotificationController()
       screen_security_(
           std::make_unique<ScreenSecurityNotificationController>()),
       session_limit_(std::make_unique<SessionLimitNotificationController>()),
-      supervised_(std::make_unique<SupervisedNotificationController>()) {}
+      supervised_(std::make_unique<SupervisedNotificationController>()),
+      wifi_toggle_(std::make_unique<WifiToggleNotificationController>()) {}
 
 SystemNotificationController::~SystemNotificationController() = default;
 

ash/system/system_notification_controller.h

@@ -16,6 +16,7 @@ class PowerNotificationController;
 class ScreenSecurityNotificationController;
 class SessionLimitNotificationController;
 class SupervisedNotificationController;
+class WifiToggleNotificationController;
 
 // Class that owns individual notification controllers.
 class SystemNotificationController {
@@ -29,6 +30,7 @@ class SystemNotificationController {
   const std::unique_ptr<ScreenSecurityNotificationController> screen_security_;
   const std::unique_ptr<SessionLimitNotificationController> session_limit_;
   const std::unique_ptr<SupervisedNotificationController> supervised_;
+  const std::unique_ptr<WifiToggleNotificationController> wifi_toggle_;
 
   DISALLOW_COPY_AND_ASSIGN(SystemNotificationController);
 };