Commit f3e0fc09 authored by Evan Stade's avatar Evan Stade Committed by Commit Bot

Revert "Chrome OS - Update captive portal notification to use"

This reverts commit 6e56b952.

Reason for revert: I realized this crashes if a captive portal is
detected during login (before there's an active profile).

Original change's description:
> Chrome OS - Update captive portal notification to use
> NotificationDisplayService.
> 
> Test by commenting out most of OnPortalDetectionCompleted() (in addition
> to unit tests).
> 
> Bug: 783018
> Change-Id: I403967f28883c588d9c2e5fd43effe05e41f7eda
> Reviewed-on: https://chromium-review.googlesource.com/807445
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#522214}

TBR=stevenjb@chromium.org,sky@chromium.org,estade@chromium.org

Change-Id: I951a56b8bb5a9c44e6aeed157cb6cbda88e2be93
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 783018
Reviewed-on: https://chromium-review.googlesource.com/812470Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522251}
parent 18dc6fc9
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "chrome/browser/chromeos/net/network_portal_detector_impl.h" #include "chrome/browser/chromeos/net/network_portal_detector_impl.h"
#include "chrome/browser/chromeos/net/network_portal_detector_test_utils.h" #include "chrome/browser/chromeos/net/network_portal_detector_test_utils.h"
#include "chrome/browser/chromeos/net/network_portal_notification_controller.h" #include "chrome/browser/chromeos/net/network_portal_notification_controller.h"
#include "chrome/browser/notifications/notification_display_service_tester.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
...@@ -33,6 +32,11 @@ ...@@ -33,6 +32,11 @@
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/cros_system_api/dbus/service_constants.h" #include "third_party/cros_system_api/dbus/service_constants.h"
#include "ui/message_center/message_center.h"
#include "ui/message_center/message_center_observer.h"
using message_center::MessageCenter;
using message_center::MessageCenterObserver;
namespace chromeos { namespace chromeos {
...@@ -65,6 +69,42 @@ void SetConnected(const std::string& service_path) { ...@@ -65,6 +69,42 @@ void SetConnected(const std::string& service_path) {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
class TestObserver : public MessageCenterObserver {
public:
TestObserver() : run_loop_(new base::RunLoop()) {
MessageCenter::Get()->AddObserver(this);
}
~TestObserver() override { MessageCenter::Get()->RemoveObserver(this); }
void WaitAndReset() {
run_loop_->Run();
run_loop_.reset(new base::RunLoop());
}
void OnNotificationDisplayed(
const std::string& notification_id,
const message_center::DisplaySource source) override {
if (notification_id == kNotificationId) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
run_loop_->QuitClosure());
}
}
void OnNotificationRemoved(const std::string& notification_id,
bool by_user) override {
if (notification_id == kNotificationId && by_user) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
run_loop_->QuitClosure());
}
}
private:
std::unique_ptr<base::RunLoop> run_loop_;
DISALLOW_COPY_AND_ASSIGN(TestObserver);
};
} // namespace } // namespace
class NetworkPortalDetectorImplBrowserTest class NetworkPortalDetectorImplBrowserTest
...@@ -117,6 +157,8 @@ class NetworkPortalDetectorImplBrowserTest ...@@ -117,6 +157,8 @@ class NetworkPortalDetectorImplBrowserTest
return network_portal_detector_->strategy_.get(); return network_portal_detector_->strategy_.get();
} }
MessageCenter* message_center() { return MessageCenter::Get(); }
void SetIgnoreNoNetworkForTesting() { void SetIgnoreNoNetworkForTesting() {
network_portal_detector_->notification_controller_ network_portal_detector_->notification_controller_
->SetIgnoreNoNetworkForTesting(); ->SetIgnoreNoNetworkForTesting();
...@@ -130,9 +172,6 @@ class NetworkPortalDetectorImplBrowserTest ...@@ -130,9 +172,6 @@ class NetworkPortalDetectorImplBrowserTest
protected: protected:
AccountId test_account_id_; AccountId test_account_id_;
// This lives here because it has to outlast other profile-keyed services.
std::unique_ptr<NotificationDisplayServiceTester> display_service_;
private: private:
NetworkPortalDetectorImpl* network_portal_detector_; NetworkPortalDetectorImpl* network_portal_detector_;
...@@ -150,6 +189,8 @@ IN_PROC_BROWSER_TEST_F(NetworkPortalDetectorImplBrowserTest, ...@@ -150,6 +189,8 @@ IN_PROC_BROWSER_TEST_F(NetworkPortalDetectorImplBrowserTest,
InSessionDetection) { InSessionDetection) {
typedef NetworkPortalNotificationController Controller; typedef NetworkPortalNotificationController Controller;
TestObserver observer;
EnumHistogramChecker ui_checker( EnumHistogramChecker ui_checker(
kNotificationMetric, Controller::NOTIFICATION_METRIC_COUNT, NULL); kNotificationMetric, Controller::NOTIFICATION_METRIC_COUNT, NULL);
EnumHistogramChecker action_checker( EnumHistogramChecker action_checker(
...@@ -157,8 +198,6 @@ IN_PROC_BROWSER_TEST_F(NetworkPortalDetectorImplBrowserTest, ...@@ -157,8 +198,6 @@ IN_PROC_BROWSER_TEST_F(NetworkPortalDetectorImplBrowserTest,
LoginUser(test_account_id_); LoginUser(test_account_id_);
content::RunAllPendingInMessageLoop(); content::RunAllPendingInMessageLoop();
display_service_ = std::make_unique<NotificationDisplayServiceTester>(
ProfileManager::GetActiveUserProfile());
// User connects to wifi. // User connects to wifi.
SetConnected(kWifiServicePath); SetConnected(kWifiServicePath);
...@@ -166,25 +205,30 @@ IN_PROC_BROWSER_TEST_F(NetworkPortalDetectorImplBrowserTest, ...@@ -166,25 +205,30 @@ IN_PROC_BROWSER_TEST_F(NetworkPortalDetectorImplBrowserTest,
ASSERT_EQ(PortalDetectorStrategy::STRATEGY_ID_SESSION, strategy()->Id()); ASSERT_EQ(PortalDetectorStrategy::STRATEGY_ID_SESSION, strategy()->Id());
// No notification until portal detection is completed. // No notification until portal detection is completed.
EXPECT_FALSE(display_service_->GetNotification(kNotificationId)); ASSERT_FALSE(message_center()->FindVisibleNotificationById(kNotificationId));
RestartDetection(); RestartDetection();
CompleteURLFetch(net::OK, 200, NULL); CompleteURLFetch(net::OK, 200, NULL);
// Check that wifi is marked as behind the portal and that notification // Check that wifi is marked as behind the portal and that notification
// is displayed. // is displayed.
EXPECT_TRUE(display_service_->GetNotification(kNotificationId)); ASSERT_TRUE(message_center()->FindVisibleNotificationById(kNotificationId));
ASSERT_EQ(NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL, ASSERT_EQ(NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL,
network_portal_detector::GetInstance() network_portal_detector::GetInstance()
->GetCaptivePortalState(kWifiGuid) ->GetCaptivePortalState(kWifiGuid)
.status); .status);
// Wait until notification is displayed.
observer.WaitAndReset();
ASSERT_TRUE( ASSERT_TRUE(
ui_checker.Expect(Controller::NOTIFICATION_METRIC_DISPLAYED, 1)->Check()); ui_checker.Expect(Controller::NOTIFICATION_METRIC_DISPLAYED, 1)->Check());
ASSERT_TRUE(action_checker.Check()); ASSERT_TRUE(action_checker.Check());
// User explicitly closes the notification. // User explicitly closes the notification.
display_service_->RemoveNotification(NotificationHandler::Type::TRANSIENT, message_center()->RemoveNotification(kNotificationId, true);
kNotificationId, true);
// Wait until notification is closed.
observer.WaitAndReset();
ASSERT_TRUE(ui_checker.Check()); ASSERT_TRUE(ui_checker.Check());
ASSERT_TRUE( ASSERT_TRUE(
...@@ -208,6 +252,8 @@ void NetworkPortalDetectorImplBrowserTestIgnoreProxy::TestImpl( ...@@ -208,6 +252,8 @@ void NetworkPortalDetectorImplBrowserTestIgnoreProxy::TestImpl(
const bool preference_value) { const bool preference_value) {
using Controller = NetworkPortalNotificationController; using Controller = NetworkPortalNotificationController;
TestObserver observer;
EnumHistogramChecker ui_checker( EnumHistogramChecker ui_checker(
kNotificationMetric, Controller::NOTIFICATION_METRIC_COUNT, nullptr); kNotificationMetric, Controller::NOTIFICATION_METRIC_COUNT, nullptr);
EnumHistogramChecker action_checker( EnumHistogramChecker action_checker(
...@@ -215,8 +261,6 @@ void NetworkPortalDetectorImplBrowserTestIgnoreProxy::TestImpl( ...@@ -215,8 +261,6 @@ void NetworkPortalDetectorImplBrowserTestIgnoreProxy::TestImpl(
LoginUser(test_account_id_); LoginUser(test_account_id_);
content::RunAllPendingInMessageLoop(); content::RunAllPendingInMessageLoop();
display_service_ = std::make_unique<NotificationDisplayServiceTester>(
ProfileManager::GetActiveUserProfile());
SetIgnoreNoNetworkForTesting(); SetIgnoreNoNetworkForTesting();
...@@ -229,23 +273,26 @@ void NetworkPortalDetectorImplBrowserTestIgnoreProxy::TestImpl( ...@@ -229,23 +273,26 @@ void NetworkPortalDetectorImplBrowserTestIgnoreProxy::TestImpl(
EXPECT_EQ(PortalDetectorStrategy::STRATEGY_ID_SESSION, strategy()->Id()); EXPECT_EQ(PortalDetectorStrategy::STRATEGY_ID_SESSION, strategy()->Id());
// No notification until portal detection is completed. // No notification until portal detection is completed.
EXPECT_FALSE(display_service_->GetNotification(kNotificationId)); EXPECT_FALSE(message_center()->FindVisibleNotificationById(kNotificationId));
RestartDetection(); RestartDetection();
CompleteURLFetch(net::OK, 200, nullptr); CompleteURLFetch(net::OK, 200, nullptr);
// Check that WiFi is marked as behind a portal and that a notification // Check that WiFi is marked as behind a portal and that a notification
// is displayed. // is displayed.
ASSERT_TRUE(display_service_->GetNotification(kNotificationId)); EXPECT_TRUE(message_center()->FindVisibleNotificationById(kNotificationId));
EXPECT_EQ(NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL, EXPECT_EQ(NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL,
network_portal_detector::GetInstance() network_portal_detector::GetInstance()
->GetCaptivePortalState(kWifiGuid) ->GetCaptivePortalState(kWifiGuid)
.status); .status);
// Wait until notification is displayed.
observer.WaitAndReset();
EXPECT_TRUE( EXPECT_TRUE(
ui_checker.Expect(Controller::NOTIFICATION_METRIC_DISPLAYED, 1)->Check()); ui_checker.Expect(Controller::NOTIFICATION_METRIC_DISPLAYED, 1)->Check());
EXPECT_TRUE(action_checker.Check()); EXPECT_TRUE(action_checker.Check());
display_service_->GetNotification(kNotificationId)->delegate()->Click(); message_center()->ClickOnNotification(kNotificationId);
content::RunAllPendingInMessageLoop(); content::RunAllPendingInMessageLoop();
......
...@@ -28,8 +28,6 @@ ...@@ -28,8 +28,6 @@
#include "chrome/browser/chromeos/net/network_portal_web_dialog.h" #include "chrome/browser/chromeos/net/network_portal_web_dialog.h"
#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h" #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/notifications/notification_display_service.h"
#include "chrome/browser/notifications/notification_handler.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/browser_dialogs.h" #include "chrome/browser/ui/browser_dialogs.h"
#include "chrome/browser/ui/scoped_tabbed_browser_displayer.h" #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
...@@ -49,11 +47,14 @@ ...@@ -49,11 +47,14 @@
#include "third_party/cros_system_api/dbus/service_constants.h" #include "third_party/cros_system_api/dbus/service_constants.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h" #include "ui/base/resource/resource_bundle.h"
#include "ui/message_center/message_center.h"
#include "ui/message_center/notification.h" #include "ui/message_center/notification.h"
#include "ui/message_center/notification_types.h" #include "ui/message_center/notification_types.h"
#include "ui/message_center/notifier_id.h" #include "ui/message_center/notifier_id.h"
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
using message_center::Notification;
namespace chromeos { namespace chromeos {
namespace { namespace {
...@@ -63,6 +64,11 @@ bool IsPortalNotificationEnabled() { ...@@ -63,6 +64,11 @@ bool IsPortalNotificationEnabled() {
switches::kDisableNetworkPortalNotification); switches::kDisableNetworkPortalNotification);
} }
void CloseNotification() {
message_center::MessageCenter::Get()->RemoveNotification(
NetworkPortalNotificationController::kNotificationId, false);
}
Profile* GetProfileForPrimaryUser() { Profile* GetProfileForPrimaryUser() {
const user_manager::User* primary_user = const user_manager::User* primary_user =
user_manager::UserManager::Get()->GetPrimaryUser(); user_manager::UserManager::Get()->GetPrimaryUser();
...@@ -71,16 +77,6 @@ Profile* GetProfileForPrimaryUser() { ...@@ -71,16 +77,6 @@ Profile* GetProfileForPrimaryUser() {
return ProfileHelper::Get()->GetProfileByUser(primary_user); return ProfileHelper::Get()->GetProfileByUser(primary_user);
} }
void CloseNotification() {
Profile* profile = GetProfileForPrimaryUser();
if (!profile)
return;
NotificationDisplayService::GetForProfile(profile)->Close(
NotificationHandler::Type::TRANSIENT,
NetworkPortalNotificationController::kNotificationId);
}
// Note that NetworkingConfigService may change after login as the profile // Note that NetworkingConfigService may change after login as the profile
// switches from the login to the user's profile. // switches from the login to the user's profile.
extensions::NetworkingConfigService* GetNetworkingConfigService( extensions::NetworkingConfigService* GetNetworkingConfigService(
...@@ -121,6 +117,7 @@ class NetworkPortalNotificationControllerDelegate ...@@ -121,6 +117,7 @@ class NetworkPortalNotificationControllerDelegate
controller_(controller) {} controller_(controller) {}
// Overridden from message_center::NotificationDelegate: // Overridden from message_center::NotificationDelegate:
void Display() override;
void Close(bool by_user) override; void Close(bool by_user) override;
void Click() override; void Click() override;
void ButtonClick(int button_click) override; void ButtonClick(int button_click) override;
...@@ -142,6 +139,13 @@ class NetworkPortalNotificationControllerDelegate ...@@ -142,6 +139,13 @@ class NetworkPortalNotificationControllerDelegate
DISALLOW_COPY_AND_ASSIGN(NetworkPortalNotificationControllerDelegate); DISALLOW_COPY_AND_ASSIGN(NetworkPortalNotificationControllerDelegate);
}; };
void NetworkPortalNotificationControllerDelegate::Display() {
UMA_HISTOGRAM_ENUMERATION(
NetworkPortalNotificationController::kNotificationMetric,
NetworkPortalNotificationController::NOTIFICATION_METRIC_DISPLAYED,
NetworkPortalNotificationController::NOTIFICATION_METRIC_COUNT);
}
void NetworkPortalNotificationControllerDelegate::Close(bool by_user) { void NetworkPortalNotificationControllerDelegate::Close(bool by_user) {
if (clicked_) if (clicked_)
return; return;
...@@ -302,13 +306,8 @@ void NetworkPortalNotificationController::OnPortalDetectionCompleted( ...@@ -302,13 +306,8 @@ void NetworkPortalNotificationController::OnPortalDetectionCompleted(
network->guid()); network->guid());
} }
NotificationDisplayService::GetForProfile(GetProfileForPrimaryUser()) message_center::MessageCenter::Get()->AddNotification(
->Display(NotificationHandler::Type::TRANSIENT, GetNotification(network, state));
*GetNotification(network, state));
UMA_HISTOGRAM_ENUMERATION(
NetworkPortalNotificationController::kNotificationMetric,
NetworkPortalNotificationController::NOTIFICATION_METRIC_DISPLAYED,
NetworkPortalNotificationController::NOTIFICATION_METRIC_COUNT);
} }
void NetworkPortalNotificationController::ShowDialog() { void NetworkPortalNotificationController::ShowDialog() {
......
...@@ -6,13 +6,14 @@ ...@@ -6,13 +6,14 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/macros.h" #include "base/macros.h"
#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h" #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
#include "chrome/browser/notifications/notification_display_service_tester.h"
#include "chrome/test/base/browser_with_test_window_test.h"
#include "chrome/test/base/testing_profile_manager.h"
#include "chromeos/chromeos_switches.h" #include "chromeos/chromeos_switches.h"
#include "chromeos/network/network_state.h" #include "chromeos/network/network_state.h"
#include "components/user_manager/scoped_user_manager.h" #include "components/user_manager/scoped_user_manager.h"
#include "components/user_manager/user_manager.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 chromeos {
...@@ -21,32 +22,64 @@ namespace { ...@@ -21,32 +22,64 @@ namespace {
const char* const kNotificationId = const char* const kNotificationId =
NetworkPortalNotificationController::kNotificationId; NetworkPortalNotificationController::kNotificationId;
bool HasNotification() {
MessageCenter* message_center = MessageCenter::Get();
return message_center->FindVisibleNotificationById(kNotificationId);
}
class NotificationObserver : public message_center::MessageCenterObserver {
public:
NotificationObserver() : add_count_(0), remove_count_(0), update_count_(0) {}
// Overridden from message_center::MessageCenterObserver:
void OnNotificationAdded(const std::string& notification_id) override {
if (notification_id == kNotificationId)
++add_count_;
}
void OnNotificationRemoved(const std::string& notification_id,
bool /* by_user */) override {
if (notification_id == kNotificationId)
++remove_count_;
}
void OnNotificationUpdated(const std::string& notification_id) override {
if (notification_id == kNotificationId)
++update_count_;
}
unsigned add_count() const { return add_count_; }
unsigned remove_count() const { return remove_count_; }
unsigned update_count() const { return update_count_; }
private:
unsigned add_count_;
unsigned remove_count_;
unsigned update_count_;
DISALLOW_COPY_AND_ASSIGN(NotificationObserver);
};
} // namespace } // namespace
class NetworkPortalNotificationControllerTest class NetworkPortalNotificationControllerTest : public testing::Test {
: public BrowserWithTestWindowTest {
public: public:
NetworkPortalNotificationControllerTest() NetworkPortalNotificationControllerTest()
: user_manager_(new chromeos::FakeChromeUserManager()), : user_manager_enabler_(
user_manager_enabler_(base::WrapUnique(user_manager_)), std::make_unique<chromeos::FakeChromeUserManager>()),
controller_(nullptr) {} controller_(nullptr) {}
~NetworkPortalNotificationControllerTest() override {} ~NetworkPortalNotificationControllerTest() override {}
void SetUp() override { void SetUp() override {
BrowserWithTestWindowTest::SetUp();
base::CommandLine* cl = base::CommandLine::ForCurrentProcess(); base::CommandLine* cl = base::CommandLine::ForCurrentProcess();
cl->AppendSwitch(switches::kEnableNetworkPortalNotification); cl->AppendSwitch(switches::kEnableNetworkPortalNotification);
MessageCenter::Initialize();
AccountId account_id = AccountId::FromUserEmail("user@example.com"); MessageCenter::Get()->AddObserver(&observer_);
user_manager_->AddUser(account_id);
user_manager_->LoginUser(account_id);
display_service_ =
std::make_unique<NotificationDisplayServiceTester>(profile());
} }
TestingProfile* CreateProfile() override { void TearDown() override {
return profile_manager()->CreateTestingProfile("user@example.com"); MessageCenter::Get()->RemoveObserver(&observer_);
MessageCenter::Shutdown();
} }
protected: protected:
...@@ -56,18 +89,13 @@ class NetworkPortalNotificationControllerTest ...@@ -56,18 +89,13 @@ class NetworkPortalNotificationControllerTest
controller_.OnPortalDetectionCompleted(network, state); controller_.OnPortalDetectionCompleted(network, state);
} }
bool HasNotification() { NotificationObserver& observer() { return observer_; }
return !!display_service_->GetNotification(kNotificationId);
}
std::unique_ptr<NotificationDisplayServiceTester> display_service_; private:
chromeos::FakeChromeUserManager* user_manager_;
user_manager::ScopedUserManager user_manager_enabler_; user_manager::ScopedUserManager user_manager_enabler_;
NetworkPortalNotificationController controller_; NetworkPortalNotificationController controller_;
// NotificationObserver observer_; NotificationObserver observer_;
AccountId account_id_;
private:
DISALLOW_COPY_AND_ASSIGN(NetworkPortalNotificationControllerTest); DISALLOW_COPY_AND_ASSIGN(NetworkPortalNotificationControllerTest);
}; };
...@@ -104,8 +132,7 @@ TEST_F(NetworkPortalNotificationControllerTest, NetworkChanged) { ...@@ -104,8 +132,7 @@ TEST_F(NetworkPortalNotificationControllerTest, NetworkChanged) {
OnPortalDetectionCompleted(&wifi1, wifi1_state); OnPortalDetectionCompleted(&wifi1, wifi1_state);
ASSERT_TRUE(HasNotification()); ASSERT_TRUE(HasNotification());
display_service_->RemoveNotification(NotificationHandler::Type::TRANSIENT, MessageCenter::Get()->RemoveNotification(kNotificationId, true /* by_user */);
kNotificationId, true /* by_user */);
ASSERT_FALSE(HasNotification()); ASSERT_FALSE(HasNotification());
// User already closed notification about portal state for this network, // User already closed notification about portal state for this network,
...@@ -139,45 +166,45 @@ TEST_F(NetworkPortalNotificationControllerTest, NotificationUpdated) { ...@@ -139,45 +166,45 @@ TEST_F(NetworkPortalNotificationControllerTest, NotificationUpdated) {
// be displayed. // be displayed.
NetworkState wifi1("wifi1"); NetworkState wifi1("wifi1");
wifi1.SetGuid("wifi1"); wifi1.SetGuid("wifi1");
wifi1.PropertyChanged("Name", base::Value("wifi1"));
OnPortalDetectionCompleted(&wifi1, portal_state); OnPortalDetectionCompleted(&wifi1, portal_state);
ASSERT_TRUE(HasNotification()); ASSERT_TRUE(HasNotification());
EXPECT_EQ(1u, display_service_ EXPECT_EQ(1u, observer().add_count());
->GetDisplayedNotificationsForType( EXPECT_EQ(0u, observer().remove_count());
NotificationHandler::Type::TRANSIENT) EXPECT_EQ(0u, observer().update_count());
.size());
const base::string16 initial_message =
display_service_->GetNotification(kNotificationId)->message();
// Second network is also behind a captive portal, so notification // Second network is also behind a captive portal, so notification
// should be updated. // should be updated.
NetworkState wifi2("wifi2"); NetworkState wifi2("wifi2");
wifi2.SetGuid("wifi2"); wifi2.SetGuid("wifi2");
wifi2.PropertyChanged("Name", base::Value("wifi2"));
OnPortalDetectionCompleted(&wifi2, portal_state); OnPortalDetectionCompleted(&wifi2, portal_state);
ASSERT_TRUE(HasNotification()); ASSERT_TRUE(HasNotification());
EXPECT_EQ(1u, display_service_ EXPECT_EQ(1u, observer().add_count());
->GetDisplayedNotificationsForType( EXPECT_EQ(0u, observer().remove_count());
NotificationHandler::Type::TRANSIENT) EXPECT_EQ(1u, observer().update_count());
.size());
EXPECT_NE(initial_message,
display_service_->GetNotification(kNotificationId)->message());
// User closes the notification. // User closes the notification.
display_service_->RemoveNotification(NotificationHandler::Type::TRANSIENT, MessageCenter::Get()->RemoveNotification(kNotificationId, true /* by_user */);
kNotificationId, true /* by_user */);
ASSERT_FALSE(HasNotification()); 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 detector notified that second network is still behind captive
// portal, but user already closed the notification, so there should // portal, but user already closed the notification, so there should
// not be any notifications. // not be any notifications.
OnPortalDetectionCompleted(&wifi2, portal_state); OnPortalDetectionCompleted(&wifi2, portal_state);
ASSERT_FALSE(HasNotification()); 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 // Network was switched (by shill or by user) to wifi1. Notification
// should be displayed. // should be displayed.
OnPortalDetectionCompleted(&wifi1, portal_state); OnPortalDetectionCompleted(&wifi1, portal_state);
ASSERT_TRUE(HasNotification()); ASSERT_TRUE(HasNotification());
EXPECT_EQ(2u, observer().add_count());
EXPECT_EQ(1u, observer().remove_count());
EXPECT_EQ(1u, observer().update_count());
} }
} // namespace chromeos } // namespace chromeos
...@@ -14,7 +14,6 @@ ...@@ -14,7 +14,6 @@
#include "chrome/browser/chromeos/net/network_portal_detector_impl.h" #include "chrome/browser/chromeos/net/network_portal_detector_impl.h"
#include "chrome/browser/chromeos/net/network_portal_notification_controller.h" #include "chrome/browser/chromeos/net/network_portal_notification_controller.h"
#include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/notifications/notification_display_service_tester.h"
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/shill_device_client.h" #include "chromeos/dbus/shill_device_client.h"
#include "chromeos/dbus/shill_profile_client.h" #include "chromeos/dbus/shill_profile_client.h"
...@@ -24,8 +23,8 @@ ...@@ -24,8 +23,8 @@
#include "extensions/test/result_catcher.h" #include "extensions/test/result_catcher.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "third_party/cros_system_api/dbus/service_constants.h" #include "third_party/cros_system_api/dbus/service_constants.h"
#include "ui/message_center/notification.h" #include "ui/message_center/message_center.h"
#include "ui/message_center/notification_delegate.h" #include "ui/message_center/message_center_observer.h"
using chromeos::DBusThreadManager; using chromeos::DBusThreadManager;
using chromeos::NetworkPortalDetector; using chromeos::NetworkPortalDetector;
...@@ -34,6 +33,8 @@ using chromeos::NetworkPortalNotificationController; ...@@ -34,6 +33,8 @@ using chromeos::NetworkPortalNotificationController;
using chromeos::ShillDeviceClient; using chromeos::ShillDeviceClient;
using chromeos::ShillProfileClient; using chromeos::ShillProfileClient;
using chromeos::ShillServiceClient; using chromeos::ShillServiceClient;
using message_center::MessageCenter;
using message_center::MessageCenterObserver;
namespace { namespace {
...@@ -41,6 +42,36 @@ const char kWifiDevicePath[] = "/device/stub_wifi_device1"; ...@@ -41,6 +42,36 @@ const char kWifiDevicePath[] = "/device/stub_wifi_device1";
const char kWifi1ServicePath[] = "stub_wifi1"; const char kWifi1ServicePath[] = "stub_wifi1";
const char kWifi1ServiceGUID[] = "wifi1_guid"; const char kWifi1ServiceGUID[] = "wifi1_guid";
class TestNotificationObserver : public MessageCenterObserver {
public:
TestNotificationObserver() {
MessageCenter::Get()->AddObserver(this);
}
~TestNotificationObserver() override {
MessageCenter::Get()->RemoveObserver(this);
}
void WaitForNotificationToDisplay() {
run_loop_.Run();
}
void OnNotificationDisplayed(
const std::string& notification_id,
const message_center::DisplaySource source) override {
if (notification_id ==
NetworkPortalNotificationController::kNotificationId) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
run_loop_.QuitClosure());
}
}
private:
base::RunLoop run_loop_;
DISALLOW_COPY_AND_ASSIGN(TestNotificationObserver);
};
} // namespace } // namespace
class NetworkingConfigTest class NetworkingConfigTest
...@@ -51,9 +82,6 @@ class NetworkingConfigTest ...@@ -51,9 +82,6 @@ class NetworkingConfigTest
ExtensionApiTest::SetUpOnMainThread(); ExtensionApiTest::SetUpOnMainThread();
content::RunAllPendingInMessageLoop(); content::RunAllPendingInMessageLoop();
display_service_ =
std::make_unique<NotificationDisplayServiceTester>(profile());
DBusThreadManager* const dbus_manager = DBusThreadManager::Get(); DBusThreadManager* const dbus_manager = DBusThreadManager::Get();
ShillServiceClient::TestInterface* const service_test = ShillServiceClient::TestInterface* const service_test =
dbus_manager->GetShillServiceClient()->GetTestInterface(); dbus_manager->GetShillServiceClient()->GetTestInterface();
...@@ -117,10 +145,10 @@ class NetworkingConfigTest ...@@ -117,10 +145,10 @@ class NetworkingConfigTest
.status; .status;
} }
protected:
NetworkPortalDetectorImpl* network_portal_detector_ = nullptr; NetworkPortalDetectorImpl* network_portal_detector_ = nullptr;
private:
const extensions::Extension* extension_ = nullptr; const extensions::Extension* extension_ = nullptr;
std::unique_ptr<NotificationDisplayServiceTester> display_service_;
}; };
IN_PROC_BROWSER_TEST_F(NetworkingConfigTest, ApiAvailability) { IN_PROC_BROWSER_TEST_F(NetworkingConfigTest, ApiAvailability) {
...@@ -141,16 +169,19 @@ IN_PROC_BROWSER_TEST_F(NetworkingConfigTest, FullTest) { ...@@ -141,16 +169,19 @@ IN_PROC_BROWSER_TEST_F(NetworkingConfigTest, FullTest) {
// This will cause the extension to register for wifi1. // This will cause the extension to register for wifi1.
ASSERT_TRUE(RunExtensionTest("full_test.html")) << message_; ASSERT_TRUE(RunExtensionTest("full_test.html")) << message_;
TestNotificationObserver observer;
SimulateCaptivePortal(); SimulateCaptivePortal();
// Wait until a captive portal notification is displayed and verify that it is // Wait until a captive portal notification is displayed and verify that it is
// the expected captive portal notification. // the expected captive portal notification.
auto notification = display_service_->GetNotification( observer.WaitForNotificationToDisplay();
NetworkPortalNotificationController::kNotificationId); EXPECT_TRUE(MessageCenter::Get()->FindVisibleNotificationById(
ASSERT_TRUE(notification); NetworkPortalNotificationController::kNotificationId));
// Simulate the user click which leads to the extension being notified. // Simulate the user click which leads to the extension being notified.
notification->delegate()->ButtonClick( MessageCenter::Get()->ClickOnNotificationButton(
NetworkPortalNotificationController::kNotificationId,
NetworkPortalNotificationController::kUseExtensionButtonIndex); NetworkPortalNotificationController::kUseExtensionButtonIndex);
extensions::ResultCatcher catcher; extensions::ResultCatcher catcher;
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#include "chrome/browser/signin/signin_error_notifier_factory_ash.h" #include "chrome/browser/signin/signin_error_notifier_factory_ash.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/notifications/notification_display_service_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/signin_error_controller_factory.h" #include "chrome/browser/signin/signin_error_controller_factory.h"
#include "chrome/browser/signin/signin_error_notifier_ash.h" #include "chrome/browser/signin/signin_error_notifier_ash.h"
...@@ -17,7 +16,6 @@ SigninErrorNotifierFactory::SigninErrorNotifierFactory() ...@@ -17,7 +16,6 @@ SigninErrorNotifierFactory::SigninErrorNotifierFactory()
"SigninErrorNotifier", "SigninErrorNotifier",
BrowserContextDependencyManager::GetInstance()) { BrowserContextDependencyManager::GetInstance()) {
DependsOn(SigninErrorControllerFactory::GetInstance()); DependsOn(SigninErrorControllerFactory::GetInstance());
DependsOn(NotificationDisplayServiceFactory::GetInstance());
} }
SigninErrorNotifierFactory::~SigninErrorNotifierFactory() {} SigninErrorNotifierFactory::~SigninErrorNotifierFactory() {}
......
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