[CrOS Tether] Open the Tether settings page when a tether notification body is clicked.

BUG=672263

Review-Url: https://codereview.chromium.org/2861583002
Cr-Commit-Position: refs/heads/master@{#469214}

chrome/browser/chromeos/net/tether_notification_presenter.cc

@@ -8,6 +8,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/proximity_auth/logging/logging.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -20,6 +21,24 @@ namespace chromeos {
 
 namespace tether {
 
+namespace {
+
+const char kTetherSettingsSubpage[] = "networks?type=Tether";
+
+class SettingsUiDelegateImpl
+    : public TetherNotificationPresenter::SettingsUiDelegate {
+ public:
+  SettingsUiDelegateImpl() {}
+  ~SettingsUiDelegateImpl() override {}
+
+  void ShowSettingsSubPageForProfile(Profile* profile,
+                                     const std::string& sub_page) override {
+    chrome::ShowSettingsSubPageForProfile(profile, sub_page);
+  }
+};
+
+}  // namespace
+
 // static
 constexpr const char TetherNotificationPresenter::kTetherNotifierId[] =
     "cros_tether_notification_ids.notifier_id";
@@ -62,10 +81,13 @@ TetherNotificationPresenter::CreateNotification(
 }
 
 TetherNotificationPresenter::TetherNotificationPresenter(
+    Profile* profile,
     message_center::MessageCenter* message_center,
     NetworkConnect* network_connect)
-    : message_center_(message_center),
+    : profile_(profile),
+      message_center_(message_center),
       network_connect_(network_connect),
+      settings_ui_delegate_(base::WrapUnique(new SettingsUiDelegateImpl())),
       weak_ptr_factory_(this) {
   message_center_->AddObserver(this);
 }
@@ -141,8 +163,8 @@ void TetherNotificationPresenter::RemoveConnectionToHostFailedNotification() {
 void TetherNotificationPresenter::OnNotificationClicked(
     const std::string& notification_id) {
   PA_LOG(INFO) << "Notification with ID " << notification_id << " was clicked.";
-
-  // TODO(khorimoto): Open the settings page.
+  settings_ui_delegate_->ShowSettingsSubPageForProfile(profile_,
+                                                       kTetherSettingsSubpage);
 }
 
 void TetherNotificationPresenter::OnNotificationButtonClicked(
@@ -153,12 +175,15 @@ void TetherNotificationPresenter::OnNotificationButtonClicked(
                << " was clicked.";
 
   if (notification_id == kPotentialHotspotNotificationId && button_index == 0) {
-    // TODO (hansberry): Only directly start a connection if this is not the
-    // first time the user has connected to a host.
     network_connect_->ConnectToNetworkId(hotspot_nearby_device_.GetDeviceId());
   }
 }
 
+void TetherNotificationPresenter::SetSettingsUiDelegateForTesting(
+    std::unique_ptr<SettingsUiDelegate> settings_ui_delegate) {
+  settings_ui_delegate_ = std::move(settings_ui_delegate);
+}
+
 void TetherNotificationPresenter::ShowNotification(
     std::unique_ptr<message_center::Notification> notification) {
   std::string notification_id = notification->id();

chrome/browser/chromeos/net/tether_notification_presenter.h

@@ -18,6 +18,8 @@
 #include "ui/message_center/message_center_observer.h"
 #include "ui/message_center/notification.h"
 
+class Profile;
+
 namespace message_center {
 class MessageCenter;
 class Notification;
@@ -33,9 +35,10 @@ class TetherNotificationPresenter
     : public NotificationPresenter,
       public message_center::MessageCenterObserver {
  public:
-  // Caller must ensure that |message_center| amd |network_connect| outlive
-  // this instance.
-  TetherNotificationPresenter(message_center::MessageCenter* message_center,
+  // Caller must ensure that |profile|, |message_center|, and |network_connect|
+  // outlive this instance.
+  TetherNotificationPresenter(Profile* profile,
+                              message_center::MessageCenter* message_center,
                               NetworkConnect* network_connect);
   ~TetherNotificationPresenter() override;
 
@@ -52,9 +55,17 @@ class TetherNotificationPresenter
   void OnNotificationButtonClicked(const std::string& notification_id,
                                    int button_index) override;
 
- private:
-  friend class TetherNotificationPresenterTest;
+  class SettingsUiDelegate {
+   public:
+    virtual ~SettingsUiDelegate() {}
 
+    // Displays the settings page (opening a new window if necessary) at the
+    // provided subpage for the user with the Profile |profile|.
+    virtual void ShowSettingsSubPageForProfile(Profile* profile,
+                                               const std::string& sub_page) = 0;
+  };
+
+ private:
   static const char kTetherNotifierId[];
   static const char kPotentialHotspotNotificationId[];
   static const char kActiveHostNotificationId[];
@@ -69,12 +80,20 @@ class TetherNotificationPresenter
       const base::string16& message,
       const message_center::RichNotificationData rich_notification_data);
 
+  friend class TetherNotificationPresenterTest;
+
+  void SetSettingsUiDelegateForTesting(
+      std::unique_ptr<SettingsUiDelegate> settings_ui_delegate);
+
   void ShowNotification(
       std::unique_ptr<message_center::Notification> notification);
 
+  Profile* profile_;
   message_center::MessageCenter* message_center_;
   NetworkConnect* network_connect_;
 
+  std::unique_ptr<SettingsUiDelegate> settings_ui_delegate_;
+
   cryptauth::RemoteDevice hotspot_nearby_device_;
   base::WeakPtrFactory<TetherNotificationPresenter> weak_ptr_factory_;
 

chrome/browser/chromeos/net/tether_notification_presenter_unittest.cc

@@ -8,7 +8,9 @@
 
 #include "base/memory/ptr_util.h"
 #include "base/observer_list.h"
+#include "chrome/test/base/testing_profile.h"
 #include "components/cryptauth/remote_device_test_util.h"
+#include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/message_center/fake_message_center.h"
@@ -136,16 +138,46 @@ class TetherNotificationPresenterTest : public testing::Test {
     std::string network_id_to_connect_;
   };
 
+  class TestSettingsUiDelegate
+      : public TetherNotificationPresenter::SettingsUiDelegate {
+   public:
+    TestSettingsUiDelegate() {}
+    ~TestSettingsUiDelegate() override {}
+
+    Profile* last_profile() { return last_profile_; }
+    std::string last_settings_subpage() { return last_settings_subpage_; }
+
+    // TetherNotificationPresenter::SettingsUiDelegate:
+    void ShowSettingsSubPageForProfile(Profile* profile,
+                                       const std::string& sub_page) override {
+      last_profile_ = profile;
+      last_settings_subpage_ = sub_page;
+    }
+
+   private:
+    Profile* last_profile_ = nullptr;
+    std::string last_settings_subpage_;
+  };
+
  protected:
   TetherNotificationPresenterTest() : test_device_(CreateTestRemoteDevice()) {}
 
   void SetUp() override {
+    TestingProfile::Builder builder;
+    profile_ = builder.Build();
     test_message_center_ = base::WrapUnique(new TestMessageCenter());
     test_network_connect_ = base::WrapUnique(new TestNetworkConnect());
     notification_presenter_ = base::WrapUnique(new TetherNotificationPresenter(
-        test_message_center_.get(), test_network_connect_.get()));
+        profile_.get(), test_message_center_.get(),
+        test_network_connect_.get()));
+
+    test_settings_ui_delegate_ = new TestSettingsUiDelegate();
+    notification_presenter_->SetSettingsUiDelegateForTesting(
+        base::WrapUnique(test_settings_ui_delegate_));
   }
 
+  void TearDown() override { profile_.reset(); }
+
   std::string GetActiveHostNotificationId() {
     return std::string(TetherNotificationPresenter::kActiveHostNotificationId);
   }
@@ -155,10 +187,24 @@ class TetherNotificationPresenterTest : public testing::Test {
         TetherNotificationPresenter::kPotentialHotspotNotificationId);
   }
 
+  void VerifySettingsOpened() {
+    EXPECT_EQ(profile_.get(), test_settings_ui_delegate_->last_profile());
+    EXPECT_EQ("networks?type=Tether",
+              test_settings_ui_delegate_->last_settings_subpage());
+  }
+
+  void VerifySettingsNotOpened() {
+    EXPECT_FALSE(test_settings_ui_delegate_->last_profile());
+    EXPECT_TRUE(test_settings_ui_delegate_->last_settings_subpage().empty());
+  }
+
+  const content::TestBrowserThreadBundle thread_bundle_;
   const cryptauth::RemoteDevice test_device_;
 
+  std::unique_ptr<TestingProfile> profile_;
   std::unique_ptr<TestMessageCenter> test_message_center_;
   std::unique_ptr<TestNetworkConnect> test_network_connect_;
+  TestSettingsUiDelegate* test_settings_ui_delegate_;
 
   std::unique_ptr<TetherNotificationPresenter> notification_presenter_;
 
@@ -183,6 +229,8 @@ TEST_F(TetherNotificationPresenterTest,
   EXPECT_FALSE(test_message_center_->FindVisibleNotificationById(
       GetActiveHostNotificationId()));
   EXPECT_EQ(0u, test_message_center_->GetNumNotifications());
+
+  VerifySettingsNotOpened();
 }
 
 TEST_F(TetherNotificationPresenterTest,
@@ -199,7 +247,7 @@ TEST_F(TetherNotificationPresenterTest,
 
   // Tap the notification.
   test_message_center_->NotifyNotificationTapped(GetActiveHostNotificationId());
-  // TODO(khorimoto): Test that the tethering settings page is opened.
+  VerifySettingsOpened();
 }
 
 TEST_F(TetherNotificationPresenterTest,
@@ -217,6 +265,8 @@ TEST_F(TetherNotificationPresenterTest,
   notification_presenter_->RemovePotentialHotspotNotification();
   EXPECT_FALSE(test_message_center_->FindVisibleNotificationById(
       GetPotentialHotspotNotificationId()));
+
+  VerifySettingsNotOpened();
 }
 
 TEST_F(TetherNotificationPresenterTest,
@@ -234,7 +284,7 @@ TEST_F(TetherNotificationPresenterTest,
   // Tap the notification.
   test_message_center_->NotifyNotificationTapped(
       GetPotentialHotspotNotificationId());
-  // TODO(khorimoto): Test that the tethering settings page is opened.
+  VerifySettingsOpened();
 }
 
 TEST_F(TetherNotificationPresenterTest,
@@ -255,9 +305,6 @@ TEST_F(TetherNotificationPresenterTest,
 
   EXPECT_EQ(test_device_.GetDeviceId(),
             test_network_connect_->network_id_to_connect());
-
-  // TODO(hansberry): Test for the case of the user not yet going through
-  // the connection dialog.
 }
 
 TEST_F(TetherNotificationPresenterTest,
@@ -275,6 +322,8 @@ TEST_F(TetherNotificationPresenterTest,
   notification_presenter_->RemovePotentialHotspotNotification();
   EXPECT_FALSE(test_message_center_->FindVisibleNotificationById(
       GetPotentialHotspotNotificationId()));
+
+  VerifySettingsNotOpened();
 }
 
 TEST_F(TetherNotificationPresenterTest,
@@ -292,7 +341,7 @@ TEST_F(TetherNotificationPresenterTest,
   // Tap the notification.
   test_message_center_->NotifyNotificationTapped(
       GetPotentialHotspotNotificationId());
-  // TODO(khorimoto): Test that the tethering settings page is opened.
+  VerifySettingsOpened();
 }
 
 TEST_F(TetherNotificationPresenterTest,
@@ -323,6 +372,8 @@ TEST_F(TetherNotificationPresenterTest,
   notification_presenter_->RemovePotentialHotspotNotification();
   EXPECT_FALSE(test_message_center_->FindVisibleNotificationById(
       GetPotentialHotspotNotificationId()));
+
+  VerifySettingsNotOpened();
 }
 
 }  // namespace tether

chrome/browser/chromeos/tether/tether_service.cc

@@ -68,7 +68,7 @@ void TetherService::StartTetherIfEnabled() {
 
   auto notification_presenter =
       base::MakeUnique<chromeos::tether::TetherNotificationPresenter>(
-          message_center::MessageCenter::Get(),
+          profile_, message_center::MessageCenter::Get(),
           chromeos::NetworkConnect::Get());
   chromeos::tether::Initializer::Init(
       cryptauth_service_, std::move(notification_presenter),