Added in-session captive portal notification.

chrome/browser/chromeos/net/network_portal_detector_notification_unittest.cc

+// Copyright 2014 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 "base/command_line.h"
+#include "chrome/browser/chromeos/net/network_portal_detector_notification.h"
+#include "chromeos/chromeos_switches.h"
+#include "chromeos/network/network_state.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/message_center/message_center.h"
+#include "ui/message_center/message_center_observer.h"
+
+using message_center::MessageCenter;
+
+namespace chromeos {
+
+namespace {
+
+const char* kNotificationId =
+    NetworkPortalDetectorNotification::kNotificationId;
+
+bool HasNotification() {
+  return MessageCenter::Get()->HasNotification(kNotificationId);
+}
+
+class NotificationObserver : public message_center::MessageCenterObserver {
+ public:
+  NotificationObserver() : add_count_(0), remove_count_(0), update_count_(0) {}
+
+  // Overridden from message_center::MessageCenterObserver:
+  virtual void OnNotificationAdded(
+      const std::string& notification_id) OVERRIDE {
+    if (notification_id == kNotificationId)
+      ++add_count_;
+  }
+
+  virtual void OnNotificationRemoved(const std::string& notification_id,
+                                     bool /* by_user */) OVERRIDE {
+    if (notification_id == kNotificationId)
+      ++remove_count_;
+  }
+
+  virtual void OnNotificationUpdated(
+      const std::string& notification_id) OVERRIDE {
+    if (notification_id == kNotificationId)
+      ++update_count_;
+  }
+
+  unsigned add_count() const { return add_count_; }

[oshima, 2014/02/19 18:59:41]

size_t

[ygorshenin1, 2014/02/20 16:31:39]

size_t values are usually arch-dependent sizes of memory objects or indexes for
memory objects. As these fields are just counters for a very limited number of
events, not sure that size_t is better than unsigned for them.

On 2014/02/19 18:59:41, oshima wrote:

+  unsigned remove_count() const { return remove_count_; }
+  unsigned update_count() const { return update_count_; }
+
+ private:
+  unsigned add_count_;
+  unsigned remove_count_;
+  unsigned update_count_;

[oshima, 2014/02/19 18:59:41]

DISALLOW_COPY_AND_ASSIGN

[ygorshenin1, 2014/02/20 16:31:39]

Done.

+};
+
+}  // namespace
+
+class NetworkPortalDetectorNotificationTest : public testing::Test {
+ public:
+  NetworkPortalDetectorNotificationTest() {}
+  virtual ~NetworkPortalDetectorNotificationTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    CommandLine* cl = CommandLine::ForCurrentProcess();
+    cl->AppendSwitch(switches::kEnableNetworkPortalNotification);
+    MessageCenter::Initialize();
+  }
+
+  virtual void TearDown() OVERRIDE { MessageCenter::Shutdown(); }
+
+  void OnPortalDetectionCompleted(
+      const NetworkState* network,
+      const NetworkPortalDetector::CaptivePortalState& state) {
+    notification_.OnPortalDetectionCompleted(network, state);
+  }
+
+ private:
+  NetworkPortalDetectorNotification notification_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkPortalDetectorNotificationTest);
+};
+
+TEST_F(NetworkPortalDetectorNotificationTest, NetworkStateChanged) {
+  NetworkState wifi("wifi");
+  NetworkPortalDetector::CaptivePortalState wifi_state;
+
+  // Notification is not displayed for online state.
+  wifi_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE;
+  wifi_state.response_code = 204;
+  OnPortalDetectionCompleted(&wifi, wifi_state);
+  ASSERT_FALSE(HasNotification());
+
+  // Notification is displayed for portal state
+  wifi_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL;
+  wifi_state.response_code = 200;
+  OnPortalDetectionCompleted(&wifi, wifi_state);
+  ASSERT_TRUE(HasNotification());
+
+  // Notification is closed for online state.
+  wifi_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE;
+  wifi_state.response_code = 204;
+  OnPortalDetectionCompleted(&wifi, wifi_state);
+  ASSERT_FALSE(HasNotification());
+}
+
+TEST_F(NetworkPortalDetectorNotificationTest, NetworkChanged) {
+  NetworkState wifi1("wifi1");
+  NetworkPortalDetector::CaptivePortalState wifi1_state;
+  wifi1_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL;
+  wifi1_state.response_code = 200;
+  OnPortalDetectionCompleted(&wifi1, wifi1_state);
+  ASSERT_TRUE(HasNotification());
+
+  MessageCenter::Get()->RemoveNotification(kNotificationId, true /* by_user */);
+  ASSERT_FALSE(HasNotification());
+
+  // User already closed notification about portal state for this network,
+  // so notification shouldn't be displayed second time.
+  OnPortalDetectionCompleted(&wifi1, wifi1_state);
+  ASSERT_FALSE(HasNotification());
+
+  NetworkState wifi2("wifi2");
+  NetworkPortalDetector::CaptivePortalState wifi2_state;
+  wifi2_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE;
+  wifi2_state.response_code = 204;
+
+  // Second network is in online state, so there shouldn't be any
+  // notifications.
+  OnPortalDetectionCompleted(&wifi2, wifi2_state);
+  ASSERT_FALSE(HasNotification());
+
+  // User switches back to the first network, so notification should
+  // be displayed.
+  OnPortalDetectionCompleted(&wifi1, wifi1_state);
+  ASSERT_TRUE(HasNotification());
+}
+
+TEST_F(NetworkPortalDetectorNotificationTest, NotificationUpdated) {
+  NotificationObserver observer;
+  MessageCenter::Get()->AddObserver(&observer);

[Jun Mukai, 2014/02/19 19:17:25]

I personally like to put this 'AddObserver/RemoveObserver' things into
constructor/destructor of this class, so the correspondence of add/remove
becomes much clearer.

[ygorshenin1, 2014/02/20 16:31:39]

Done.

+
+  NetworkPortalDetector::CaptivePortalState portal_state;
+  portal_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL;
+  portal_state.response_code = 200;
+
+  // First network is behind a captive portal, so notification should
+  // be displayed.
+  NetworkState wifi1("wifi1");
+  OnPortalDetectionCompleted(&wifi1, portal_state);
+  ASSERT_TRUE(HasNotification());
+  EXPECT_EQ(1u, observer.add_count());
+  EXPECT_EQ(0u, observer.remove_count());
+  EXPECT_EQ(0u, observer.update_count());
+
+  // Second network is also behind a captive portal, so notification
+  // should be updated.
+  NetworkState wifi2("wifi2");
+  OnPortalDetectionCompleted(&wifi2, portal_state);
+  ASSERT_TRUE(HasNotification());
+  EXPECT_EQ(1u, observer.add_count());
+  EXPECT_EQ(0u, observer.remove_count());
+  EXPECT_EQ(1u, observer.update_count());
+
+  // User closes the notification.
+  MessageCenter::Get()->RemoveNotification(kNotificationId, true /* by_user */);
+  ASSERT_FALSE(HasNotification());
+  EXPECT_EQ(1u, observer.add_count());
+  EXPECT_EQ(1u, observer.remove_count());
+  EXPECT_EQ(1u, observer.update_count());
+
+  // Portal detector notified that second network is still behind captive
+  // portal, but user already closed the notification, so there should
+  // not be any notifications.
+  OnPortalDetectionCompleted(&wifi2, portal_state);
+  ASSERT_FALSE(HasNotification());
+  EXPECT_EQ(1u, observer.add_count());
+  EXPECT_EQ(1u, observer.remove_count());
+  EXPECT_EQ(1u, observer.update_count());
+
+  // Network was switched (by shill or by user) to wifi1. Notification
+  // should be displayed.
+  OnPortalDetectionCompleted(&wifi1, portal_state);
+  ASSERT_TRUE(HasNotification());
+  EXPECT_EQ(2u, observer.add_count());
+  EXPECT_EQ(1u, observer.remove_count());
+  EXPECT_EQ(1u, observer.update_count());
+
+  MessageCenter::Get()->RemoveObserver(&observer);
+}
+
+}  // namespace chromeos