[CrOS PhoneHub] Adjust hotspot states and add notification when failure.

- Change the states in enable hotspot to fit new design.
- Add a notification when enable hotspot fails.

BUG=1106937,1126208

Change-Id: Id9092649d5cfcee9f6153c74a258b869a81f89dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2495621
Commit-Queue: Andre Le <leandre@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Tim Song <tengs@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821374}

ash/ash_strings.grd

@@ -1121,6 +1121,12 @@ This file contains the strings for ash.
       <message name="IDS_ASH_PHONE_HUB_NOTIFICATION_OPT_IN_DISMISS_BUTTON" desc="Label for the dismiss button to opt out showing notifications from the phone.">
         Dismiss
       </message>
+      <message name="IDS_ASH_PHONE_HUB_NOTIFICATION_HOTSPOT_FAILED_TITLE" desc="Title inside a PhoneHub notification when enable hotspot failed to find a connection.">
+        Hotspot connection failed
+      </message>
+      <message name="IDS_ASH_PHONE_HUB_NOTIFICATION_HOTSPOT_FAILED_MESSAGE" desc="Message inside a PhoneHub notification when enable hotspot failed to find a connection.">
+        Tap to configure
+      </message>
 
       <message name="IDS_ASH_STYLUS_TOOLS_CAPTURE_REGION_ACTION" desc="Title of the capture region action in the stylus tools (a pop-up panel next to the status tray). This causes a partial screenshot to be taken (the user selects an area of the screen to take a screenshot of).">
         Capture region

ash/ash_strings_grd/IDS_ASH_PHONE_HUB_NOTIFICATION_HOTSPOT_FAILED_MESSAGE.png.sha1

+c5de29f1206bf408d71a64fe5e18efbe426c4427
\ No newline at end of file

ash/ash_strings_grd/IDS_ASH_PHONE_HUB_NOTIFICATION_HOTSPOT_FAILED_TITLE.png.sha1

+c5de29f1206bf408d71a64fe5e18efbe426c4427
\ No newline at end of file

ash/public/cpp/system_tray_client.h

@@ -54,6 +54,9 @@ class ASH_PUBLIC_EXPORT SystemTrayClient {
   // Shows settings related to MultiDevice features.
   virtual void ShowConnectedDevicesSettings() = 0;
 
+  // Shows settings related to tether network.
+  virtual void ShowTetherNetworkSettings() = 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

@@ -38,6 +38,8 @@ void TestSystemTrayClient::ShowConnectedDevicesSettings() {
   show_connected_devices_settings_count_++;
 }
 
+void TestSystemTrayClient::ShowTetherNetworkSettings() {}
+
 void TestSystemTrayClient::ShowAboutChromeOS() {}
 
 void TestSystemTrayClient::ShowHelp() {}

ash/public/cpp/test/test_system_tray_client.h

@@ -31,6 +31,7 @@ class ASH_PUBLIC_EXPORT TestSystemTrayClient : public SystemTrayClient {
   void ShowChromeSlow() override;
   void ShowIMESettings() override;
   void ShowConnectedDevicesSettings() override;
+  void ShowTetherNetworkSettings() override;
   void ShowAboutChromeOS() override;
   void ShowHelp() override;
   void ShowAccessibilityHelp() override;

ash/system/model/system_tray_model.cc

@@ -129,7 +129,7 @@ void SystemTrayModel::SetPhoneHubManager(
   Shell::Get()
       ->message_center_controller()
       ->phone_hub_notification_controller()
-      ->SetManager(phone_hub_manager->GetNotificationManager());
+      ->SetManager(phone_hub_manager);
 }
 
 }  // namespace ash

ash/system/phonehub/enable_hotspot_quick_action_controller.cc

@@ -48,10 +48,9 @@ void EnableHotspotQuickActionController::OnTetherStatusChanged() {
       item_->SetVisible(false);
       return;
     case Status::kConnectionUnavailable:
-      SetState(ActionState::kNotAvailable);
-      break;
+      FALLTHROUGH;
     case Status::kConnectionAvailable:
-      SetState(ActionState::kNotConnected);
+      SetState(ActionState::kOff);
       break;
     case Status::kConnecting:
       SetState(ActionState::kConnecting);
@@ -70,17 +69,10 @@ void EnableHotspotQuickActionController::SetState(ActionState state) {
   int state_text_id;
   int sub_label_text;
   switch (state) {
-    case ActionState::kNotAvailable:
-      icon_enabled = false;
-      state_text_id =
-          IDS_ASH_PHONE_HUB_QUICK_ACTIONS_NOT_AVAILABLE_STATE_TOOLTIP;
-      sub_label_text = IDS_ASH_PHONE_HUB_QUICK_ACTIONS_NOT_AVAILABLE_STATE;
-      break;
-    case ActionState::kNotConnected:
+    case ActionState::kOff:
       icon_enabled = false;
-      state_text_id =
-          IDS_ASH_PHONE_HUB_QUICK_ACTIONS_NOT_CONNECTED_STATE_TOOLTIP;
-      sub_label_text = IDS_ASH_PHONE_HUB_QUICK_ACTIONS_NOT_CONNECTED_STATE;
+      state_text_id = IDS_ASH_PHONE_HUB_QUICK_ACTIONS_DISABLED_STATE_TOOLTIP;
+      sub_label_text = IDS_ASH_PHONE_HUB_QUICK_ACTIONS_OFF_STATE;
       break;
     case ActionState::kConnecting:
       icon_enabled = true;

ash/system/phonehub/enable_hotspot_quick_action_controller.h

@@ -33,12 +33,7 @@ class EnableHotspotQuickActionController
  private:
   // All the possible states that the enable hotspot button can be viewed. Each
   // state has a corresponding icon, labels and tooltip view.
-  enum class ActionState {
-    kNotAvailable,
-    kNotConnected,
-    kConnecting,
-    kConnected
-  };
+  enum class ActionState { kOff, kConnecting, kConnected };
 
   // Set the item (including icon, label and tooltips) to a certain state.
   void SetState(ActionState state);

ash/system/phonehub/phone_hub_notification_controller.cc

@@ -4,7 +4,9 @@
 
 #include "ash/system/phonehub/phone_hub_notification_controller.h"
 
+#include "ash/public/cpp/notification_utils.h"
 #include "ash/public/cpp/system_tray_client.h"
+#include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/model/system_tray_model.h"
@@ -16,6 +18,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/timer/timer.h"
 #include "chromeos/components/phonehub/notification.h"
+#include "chromeos/components/phonehub/phone_hub_manager.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/message_center/message_center.h"
@@ -32,6 +35,8 @@ namespace {
 const char kNotifierId[] = "chrome://phonehub";
 const char kNotifierIdSeparator[] = "-";
 const char kNotificationCustomViewType[] = "phonehub";
+const char kPhoneHubInstantTetherNotificationId[] =
+    "chrome://phonehub-instant-tether";
 const int kReplyButtonIndex = 0;
 
 // The amount of time the reply button is disabled after sending an inline
@@ -189,18 +194,33 @@ PhoneHubNotificationController::PhoneHubNotificationController() {
 PhoneHubNotificationController::~PhoneHubNotificationController() {
   if (manager_)
     manager_->RemoveObserver(this);
+  if (tether_controller_)
+    tether_controller_->RemoveObserver(this);
 }
 
 void PhoneHubNotificationController::SetManager(
-    chromeos::phonehub::NotificationManager* manager) {
-  if (manager_ == manager)
+    chromeos::phonehub::PhoneHubManager* phone_hub_manager) {
+  chromeos::phonehub::NotificationManager* notification_manager =
+      phone_hub_manager->GetNotificationManager();
+  chromeos::phonehub::TetherController* tether_controller =
+      phone_hub_manager->GetTetherController();
+
+  if (manager_ == notification_manager &&
+      tether_controller_ == tether_controller) {
     return;
+  }
 
   if (manager_)
     manager_->RemoveObserver(this);
 
-  manager_ = manager;
+  manager_ = notification_manager;
   manager_->AddObserver(this);
+
+  if (tether_controller_)
+    tether_controller_->RemoveObserver(this);
+
+  tether_controller_ = tether_controller;
+  tether_controller_->AddObserver(this);
 }
 
 void PhoneHubNotificationController::OnNotificationsAdded(
@@ -228,6 +248,42 @@ void PhoneHubNotificationController::OnNotificationsRemoved(
   }
 }
 
+void PhoneHubNotificationController::OnAttemptConnectionScanFailed() {
+  // Add a notification if tether failed.
+  scoped_refptr<message_center::NotificationDelegate> delegate =
+      base::MakeRefCounted<message_center::HandleNotificationClickDelegate>(
+          base::BindRepeating([](base::Optional<int> button_index) {
+            // When clicked, open Tether Settings page if we can open WebUI
+            // settings, otherwise do nothing.
+            if (TrayPopupUtils::CanOpenWebUISettings()) {
+              Shell::Get()
+                  ->system_tray_model()
+                  ->client()
+                  ->ShowTetherNetworkSettings();
+            } else {
+              LOG(WARNING) << "Cannot open Tether Settings since it's not "
+                              "possible to opening WebUI settings";
+            }
+          }));
+  std::unique_ptr<message_center::Notification> notification =
+      CreateSystemNotification(
+          message_center::NOTIFICATION_TYPE_SIMPLE,
+          kPhoneHubInstantTetherNotificationId,
+          l10n_util::GetStringUTF16(
+              IDS_ASH_PHONE_HUB_NOTIFICATION_HOTSPOT_FAILED_TITLE),
+          l10n_util::GetStringUTF16(
+              IDS_ASH_PHONE_HUB_NOTIFICATION_HOTSPOT_FAILED_MESSAGE),
+          base::string16() /*display_source */, GURL() /* origin_url */,
+          message_center::NotifierId(
+              message_center::NotifierType::SYSTEM_COMPONENT,
+              kPhoneHubInstantTetherNotificationId),
+          message_center::RichNotificationData(), std::move(delegate),
+          kPhoneHubEnableHotspotOnIcon,
+          message_center::SystemNotificationWarningLevel::NORMAL);
+  message_center::MessageCenter::Get()->AddNotification(
+      std::move(notification));
+}
+
 void PhoneHubNotificationController::OpenSettings() {
   DCHECK(TrayPopupUtils::CanOpenWebUISettings());
   Shell::Get()->system_tray_model()->client()->ShowConnectedDevicesSettings();

ash/system/phonehub/phone_hub_notification_controller.h

@@ -9,10 +9,12 @@
 
 #include "ash/ash_export.h"
 #include "chromeos/components/phonehub/notification_manager.h"
+#include "chromeos/components/phonehub/tether_controller.h"
 
 namespace chromeos {
 namespace phonehub {
 class Notification;
+class PhoneHubManager;
 }  // namespace phonehub
 }  // namespace chromeos
 
@@ -26,7 +28,8 @@ namespace ash {
 // This controller creates and manages a message_center::Notification for each
 // PhoneHub corresponding notification.
 class ASH_EXPORT PhoneHubNotificationController
-    : public chromeos::phonehub::NotificationManager::Observer {
+    : public chromeos::phonehub::NotificationManager::Observer,
+      public chromeos::phonehub::TetherController::Observer {
  public:
   PhoneHubNotificationController();
   ~PhoneHubNotificationController() override;
@@ -37,7 +40,7 @@ class ASH_EXPORT PhoneHubNotificationController
 
   // Sets the NotifictionManager that provides the underlying PhoneHub
   // notifications.
-  void SetManager(chromeos::phonehub::NotificationManager* manager);
+  void SetManager(chromeos::phonehub::PhoneHubManager* phone_hub_manager);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(PhoneHubNotificationControllerTest,
@@ -53,6 +56,10 @@ class ASH_EXPORT PhoneHubNotificationController
   void OnNotificationsRemoved(
       const base::flat_set<int64_t>& notification_ids) override;
 
+  // chromeos::phonehub::TetherController::Observer:
+  void OnAttemptConnectionScanFailed() override;
+  void OnTetherStatusChanged() override {}
+
   // Callbacks for user interactions.
   void OpenSettings();
   void DismissNotification(int64_t notification_id);
@@ -75,6 +82,7 @@ class ASH_EXPORT PhoneHubNotificationController
       const message_center::Notification& notification);
 
   chromeos::phonehub::NotificationManager* manager_ = nullptr;
+  chromeos::phonehub::TetherController* tether_controller_ = nullptr;
   std::unordered_map<int64_t, std::unique_ptr<NotificationDelegate>>
       notification_map_;
 };

ash/system/phonehub/phone_hub_notification_controller_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
 #include "chromeos/components/phonehub/fake_notification_manager.h"
+#include "chromeos/components/phonehub/fake_phone_hub_manager.h"
 #include "chromeos/components/phonehub/notification.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -69,7 +70,8 @@ class PhoneHubNotificationControllerTest : public AshTestBase {
     controller_ = Shell::Get()
                       ->message_center_controller()
                       ->phone_hub_notification_controller();
-    controller_->SetManager(&notification_manager_);
+    controller_->SetManager(&phone_hub_manager_);
+    notification_manager_ = phone_hub_manager_.fake_notification_manager();
 
     fake_notifications_.insert(CreateNotification(kPhoneHubNotificationId0));
     fake_notifications_.insert(CreateNotification(kPhoneHubNotificationId1));
@@ -79,14 +81,15 @@ class PhoneHubNotificationControllerTest : public AshTestBase {
  protected:
   base::test::ScopedFeatureList feature_list_;
   message_center::MessageCenter* message_center_;
-  chromeos::phonehub::FakeNotificationManager notification_manager_;
+  chromeos::phonehub::FakePhoneHubManager phone_hub_manager_;
+  chromeos::phonehub::FakeNotificationManager* notification_manager_;
   PhoneHubNotificationController* controller_;
   base::flat_set<chromeos::phonehub::Notification> fake_notifications_;
 };
 
 TEST_F(PhoneHubNotificationControllerTest, AddNotifications) {
   EXPECT_FALSE(message_center_->NotificationCount());
-  notification_manager_.SetNotificationsInternal(fake_notifications_);
+  notification_manager_->SetNotificationsInternal(fake_notifications_);
   EXPECT_EQ(3u, message_center_->NotificationCount());
 
   ASSERT_TRUE(
@@ -104,7 +107,7 @@ TEST_F(PhoneHubNotificationControllerTest, AddNotifications) {
 
 TEST_F(PhoneHubNotificationControllerTest, UpdateNotifications) {
   EXPECT_FALSE(message_center_->NotificationCount());
-  notification_manager_.SetNotificationsInternal(fake_notifications_);
+  notification_manager_->SetNotificationsInternal(fake_notifications_);
   EXPECT_EQ(3u, message_center_->NotificationCount());
 
   auto* notification =
@@ -123,7 +126,7 @@ TEST_F(PhoneHubNotificationControllerTest, UpdateNotifications) {
       /*inline_reply_id=*/0, base::UTF8ToUTF16(kNewTitle),
       base::UTF8ToUTF16(kNewTextContent));
 
-  notification_manager_.SetNotification(updated_notification);
+  notification_manager_->SetNotification(updated_notification);
 
   notification =
       message_center_->FindVisibleNotificationById(kCrOSNotificationId1);
@@ -133,26 +136,26 @@ TEST_F(PhoneHubNotificationControllerTest, UpdateNotifications) {
 
 TEST_F(PhoneHubNotificationControllerTest, RemoveNotifications) {
   EXPECT_FALSE(message_center_->NotificationCount());
-  notification_manager_.SetNotificationsInternal(fake_notifications_);
+  notification_manager_->SetNotificationsInternal(fake_notifications_);
   EXPECT_EQ(3u, message_center_->NotificationCount());
 
-  notification_manager_.RemoveNotification(kPhoneHubNotificationId0);
+  notification_manager_->RemoveNotification(kPhoneHubNotificationId0);
   EXPECT_EQ(2u, message_center_->NotificationCount());
   EXPECT_FALSE(
       message_center_->FindVisibleNotificationById(kCrOSNotificationId0));
 
-  notification_manager_.RemoveNotificationsInternal(base::flat_set<int64_t>(
+  notification_manager_->RemoveNotificationsInternal(base::flat_set<int64_t>(
       {kPhoneHubNotificationId1, kPhoneHubNotificationId2}));
   EXPECT_FALSE(message_center_->NotificationCount());
 
   // Attempt removing the same notifications again and expect nothing to happen.
-  notification_manager_.RemoveNotificationsInternal(base::flat_set<int64_t>(
+  notification_manager_->RemoveNotificationsInternal(base::flat_set<int64_t>(
       {kPhoneHubNotificationId1, kPhoneHubNotificationId2}));
   EXPECT_FALSE(message_center_->NotificationCount());
 }
 
 TEST_F(PhoneHubNotificationControllerTest, CloseByUser) {
-  notification_manager_.SetNotificationsInternal(fake_notifications_);
+  notification_manager_->SetNotificationsInternal(fake_notifications_);
   EXPECT_EQ(3u, message_center_->NotificationCount());
 
   message_center_->RemoveNotification(kCrOSNotificationId0, /*by_user=*/true);
@@ -162,11 +165,11 @@ TEST_F(PhoneHubNotificationControllerTest, CloseByUser) {
   EXPECT_EQ(
       std::vector<int64_t>({kPhoneHubNotificationId0, kPhoneHubNotificationId1,
                             kPhoneHubNotificationId2}),
-      notification_manager_.dismissed_notification_ids());
+      notification_manager_->dismissed_notification_ids());
 }
 
 TEST_F(PhoneHubNotificationControllerTest, InlineReply) {
-  notification_manager_.SetNotificationsInternal(fake_notifications_);
+  notification_manager_->SetNotificationsInternal(fake_notifications_);
 
   const base::string16 kInlineReply0 = base::UTF8ToUTF16("inline reply 0");
   const base::string16 kInlineReply1 = base::UTF8ToUTF16("inline reply 1");
@@ -175,7 +178,7 @@ TEST_F(PhoneHubNotificationControllerTest, InlineReply) {
   message_center_->ClickOnNotificationButtonWithReply(kCrOSNotificationId1, 0,
                                                       kInlineReply1);
 
-  auto inline_replies = notification_manager_.inline_replies();
+  auto inline_replies = notification_manager_->inline_replies();
   EXPECT_EQ(kPhoneHubNotificationId0, inline_replies[0].notification_id);
   EXPECT_EQ(kInlineReply0, inline_replies[0].inline_reply_text);
   EXPECT_EQ(kPhoneHubNotificationId1, inline_replies[1].notification_id);
@@ -183,7 +186,7 @@ TEST_F(PhoneHubNotificationControllerTest, InlineReply) {
 }
 
 TEST_F(PhoneHubNotificationControllerTest, ClickSettings) {
-  notification_manager_.SetNotificationsInternal(fake_notifications_);
+  notification_manager_->SetNotificationsInternal(fake_notifications_);
   EXPECT_TRUE(
       message_center_->FindVisibleNotificationById(kCrOSNotificationId0));
   EXPECT_EQ(0, GetSystemTrayClient()->show_connected_devices_settings_count());
@@ -217,7 +220,7 @@ TEST_F(PhoneHubNotificationControllerTest, NotificationDataAndImages) {
       /*inline_reply_id=*/0, base::UTF8ToUTF16(kTitle),
       base::UTF8ToUTF16(kTextContent), shared_image, contact_image);
 
-  notification_manager_.SetNotification(fake_notification);
+  notification_manager_->SetNotification(fake_notification);
 
   auto* cros_notification =
       message_center_->FindVisibleNotificationById(kCrOSNotificationId0);
@@ -235,7 +238,7 @@ TEST_F(PhoneHubNotificationControllerTest, NotificationDataAndImages) {
 }
 
 TEST_F(PhoneHubNotificationControllerTest, NotificationHasCustomViewType) {
-  notification_manager_.SetNotificationsInternal(fake_notifications_);
+  notification_manager_->SetNotificationsInternal(fake_notifications_);
   auto* notification =
       message_center_->FindVisibleNotificationById(kCrOSNotificationId0);
 
@@ -244,7 +247,7 @@ TEST_F(PhoneHubNotificationControllerTest, NotificationHasCustomViewType) {
 }
 
 TEST_F(PhoneHubNotificationControllerTest, ReplyBrieflyDisabled) {
-  notification_manager_.SetNotificationsInternal(fake_notifications_);
+  notification_manager_->SetNotificationsInternal(fake_notifications_);
   auto* notification =
       message_center_->FindVisibleNotificationById(kCrOSNotificationId0);
 

chrome/browser/ui/ash/system_tray_client.cc

@@ -266,6 +266,11 @@ void SystemTrayClient::ShowConnectedDevicesSettings() {
       chromeos::settings::mojom::kMultiDeviceFeaturesSubpagePath);
 }
 
+void SystemTrayClient::ShowTetherNetworkSettings() {
+  ShowSettingsSubPageForActiveUser(
+      chromeos::settings::mojom::kMobileDataNetworksSubpagePath);
+}
+
 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

@@ -16,7 +16,7 @@ struct LocaleInfo;
 class SystemTray;
 enum class LoginStatus;
 enum class NotificationStyle;
-}
+}  // namespace ash
 
 // Handles method calls delegated back to chrome from ash. Also notifies ash of
 // relevant state changes in chrome.
@@ -62,6 +62,7 @@ class SystemTrayClient : public ash::SystemTrayClient,
   void ShowChromeSlow() override;
   void ShowIMESettings() override;
   void ShowConnectedDevicesSettings() override;
+  void ShowTetherNetworkSettings() override;
   void ShowAboutChromeOS() override;
   void ShowHelp() override;
   void ShowAccessibilityHelp() override;