[CrOS PhoneHub] Wire up buttons for disconnected view.

This CL adds handlers for "Learn More" and "Refresh" buttons on the
disconnected view.

Misc:
1. Minor refactoring on |PhoneHubTrayTest|.
2. Adds initial value for |num_schedule_connection_now_calls_|.

BUG=1106937,1126208
TEST=unittested.

Change-Id: I3e6c74b331710c1f6607865061f12267f8d27af6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2439639
Commit-Queue: Meilin Wang <meilinw@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Tim Song <tengs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813311}

ash/system/phonehub/connection_error_view.cc

@@ -6,11 +6,14 @@
 
 #include <memory>
 
+#include "ash/public/cpp/new_window_delegate.h"
 #include "ash/public/cpp/resources/grit/ash_public_unscaled_resources.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/style/ash_color_provider.h"
 #include "ash/system/phonehub/phone_hub_interstitial_view.h"
+#include "ash/system/phonehub/phone_hub_view_ids.h"
 #include "ash/system/unified/rounded_label_button.h"
+#include "chromeos/components/phonehub/connection_scheduler.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/views/layout/fill_layout.h"
@@ -19,13 +22,19 @@ namespace ash {
 
 namespace {
 
-// Tag value used to uniquely identify the "Learn More" and "Refresh" buttons.
-constexpr int kLearnMoreButtonTag = 1;
-constexpr int kRefreshButtonTag = 2;
+// TODO(meilinw): replace it with the real one.
+constexpr char kLearnMoreUrl[] =
+    "https://support.google.com/chromebook/?p=multi_device";
 
 }  // namespace
 
-ConnectionErrorView::ConnectionErrorView(ErrorStatus error) {
+ConnectionErrorView::ConnectionErrorView(
+    ErrorStatus error,
+    chromeos::phonehub::ConnectionScheduler* connection_scheduler)
+    : connection_scheduler_(connection_scheduler) {
+  SetID(error == ErrorStatus::kDisconnected
+            ? PhoneHubViewID::kDisconnectedView
+            : PhoneHubViewID::kReconnectingView);
   SetLayoutManager(std::make_unique<views::FillLayout>());
   content_view_ = AddChildView(std::make_unique<PhoneHubInterstitialView>(
       /*show_progress=*/error == ErrorStatus::kReconnecting));
@@ -55,13 +64,13 @@ ConnectionErrorView::ConnectionErrorView(ErrorStatus error) {
   learn_more->SetEnabledTextColors(
       AshColorProvider::Get()->GetContentLayerColor(
           AshColorProvider::ContentLayerType::kTextColorPrimary));
-  learn_more->set_tag(kLearnMoreButtonTag);
+  learn_more->SetID(PhoneHubViewID::kDisconnectedLearnMoreButton);
   content_view_->AddButton(std::move(learn_more));
 
   auto refresh = std::make_unique<RoundedLabelButton>(
       this, l10n_util::GetStringUTF16(
                 IDS_ASH_PHONE_HUB_CONNECTION_ERROR_DIALOG_REFRESH_BUTTON));
-  refresh->set_tag(kRefreshButtonTag);
+  refresh->SetID(PhoneHubViewID::kDisconnectedRefreshButton);
   content_view_->AddButton(std::move(refresh));
 }
 
@@ -69,7 +78,16 @@ ConnectionErrorView::~ConnectionErrorView() = default;
 
 void ConnectionErrorView::ButtonPressed(views::Button* sender,
                                         const ui::Event& event) {
-  // TODO(meilinw): implement button pressed actions.
+  switch (sender->GetID()) {
+    case PhoneHubViewID::kDisconnectedRefreshButton:
+      // Retry the connection attempt.
+      connection_scheduler_->ScheduleConnectionNow();
+      return;
+    case PhoneHubViewID::kDisconnectedLearnMoreButton:
+      NewWindowDelegate::GetInstance()->NewTabWithUrl(
+          GURL(kLearnMoreUrl), /*from_user_interaction=*/true);
+      return;
+  }
 }
 
 BEGIN_METADATA(ConnectionErrorView, views::View)

ash/system/phonehub/connection_error_view.h

@@ -8,6 +8,12 @@
 #include "ash/ash_export.h"
 #include "ui/views/controls/button/button.h"
 
+namespace chromeos {
+namespace phonehub {
+class ConnectionScheduler;
+}  // namespace phonehub
+}  // namespace chromeos
+
 namespace ash {
 
 class PhoneHubInterstitialView;
@@ -25,7 +31,9 @@ class ASH_EXPORT ConnectionErrorView : public views::View,
     kReconnecting,  // Attempts to resume the connection to the phone.
   };
 
-  explicit ConnectionErrorView(ErrorStatus error);
+  ConnectionErrorView(
+      ErrorStatus error,
+      chromeos::phonehub::ConnectionScheduler* connection_scheduler);
   ConnectionErrorView(const ConnectionErrorView&) = delete;
   ConnectionErrorView& operator=(const ConnectionErrorView&) = delete;
   ~ConnectionErrorView() override;
@@ -34,6 +42,8 @@ class ASH_EXPORT ConnectionErrorView : public views::View,
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
  private:
+  chromeos::phonehub::ConnectionScheduler* connection_scheduler_ = nullptr;
+
   PhoneHubInterstitialView* content_view_ = nullptr;
 };
 

ash/system/phonehub/phone_hub_tray_unittest.cc

@@ -11,6 +11,7 @@
 #include "ash/system/status_area_widget_test_helper.h"
 #include "ash/test/ash_test_base.h"
 #include "base/test/scoped_feature_list.h"
+#include "chromeos/components/phonehub/fake_connection_scheduler.h"
 #include "chromeos/components/phonehub/fake_notification_access_manager.h"
 #include "chromeos/components/phonehub/fake_phone_hub_manager.h"
 #include "chromeos/constants/chromeos_features.h"
@@ -60,10 +61,8 @@ class PhoneHubTrayTest : public AshTestBase {
     return phone_hub_manager_.fake_notification_access_manager();
   }
 
-  void ClickTrayButton() {
-    ui::GestureEvent tap(0, 0, 0, base::TimeTicks(),
-                         ui::GestureEventDetails(ui::ET_GESTURE_TAP));
-    phone_hub_tray_->PerformAction(tap);
+  chromeos::phonehub::FakeConnectionScheduler* GetConnectionScheduler() {
+    return phone_hub_manager_.fake_connection_scheduler();
   }
 
   // Simulate a mouse click on the given view.
@@ -76,14 +75,32 @@ class PhoneHubTrayTest : public AshTestBase {
     base::RunLoop().RunUntilIdle();
   }
 
+  void ClickTrayButton() { ClickOnAndWait(phone_hub_tray_); }
+
   MockNewWindowDelegate& new_window_delegate() { return new_window_delegate_; }
 
+  views::View* content_view() {
+    return phone_hub_tray_->content_view_for_testing();
+  }
+
   NotificationOptInView* notification_opt_in_view() {
     return static_cast<NotificationOptInView*>(
         phone_hub_tray_->GetBubbleView()->GetViewByID(
             PhoneHubViewID::kNotificationOptInView));
   }
 
+  views::Button* disconnected_refresh_button() {
+    return static_cast<views::Button*>(
+        phone_hub_tray_->GetBubbleView()->GetViewByID(
+            PhoneHubViewID::kDisconnectedRefreshButton));
+  }
+
+  views::Button* disconnected_learn_more_button() {
+    return static_cast<views::Button*>(
+        phone_hub_tray_->GetBubbleView()->GetViewByID(
+            PhoneHubViewID::kDisconnectedLearnMoreButton));
+  }
+
  protected:
   PhoneHubTray* phone_hub_tray_ = nullptr;
   chromeos::phonehub::FakePhoneHubManager phone_hub_manager_;
@@ -182,18 +199,44 @@ TEST_F(PhoneHubTrayTest, TransitionContentView) {
   ClickTrayButton();
   EXPECT_TRUE(phone_hub_tray_->is_active());
 
-  auto* content_view = phone_hub_tray_->content_view_for_testing();
-  EXPECT_TRUE(content_view);
+  EXPECT_TRUE(content_view());
   // TODO(tengs) Test the actual view id.
-  EXPECT_EQ(0, content_view->GetID());
+  EXPECT_EQ(0, content_view()->GetID());
 
   GetFeatureStatusProvider()->SetStatus(
       chromeos::phonehub::FeatureStatus::kEnabledButDisconnected);
 
-  content_view = phone_hub_tray_->content_view_for_testing();
-  EXPECT_TRUE(content_view);
-  // TODO(tengs) Test the actual view id.
-  EXPECT_EQ(0, content_view->GetID());
+  EXPECT_TRUE(content_view());
+  EXPECT_EQ(PhoneHubViewID::kDisconnectedView, content_view()->GetID());
+}
+
+TEST_F(PhoneHubTrayTest, ClickButtonsOnDisconnectedView) {
+  // Simulates a phone disconnected error state to show the disconnected view.
+  GetFeatureStatusProvider()->SetStatus(
+      chromeos::phonehub::FeatureStatus::kEnabledButDisconnected);
+
+  ClickTrayButton();
+  EXPECT_TRUE(phone_hub_tray_->is_active());
+  EXPECT_EQ(PhoneHubViewID::kDisconnectedView, content_view()->GetID());
+
+  // Simulates a click on the "Refresh" button.
+  EXPECT_EQ(0u, GetConnectionScheduler()->num_schedule_connection_now_calls());
+  ClickOnAndWait(disconnected_refresh_button());
+
+  // Clicking "Refresh" button should schedule a connection attempt.
+  EXPECT_EQ(1u, GetConnectionScheduler()->num_schedule_connection_now_calls());
+
+  // Clicking "Learn More" button should open the corresponding help center
+  // article in a browser tab.
+  EXPECT_CALL(new_window_delegate(), NewTabWithUrl)
+      .WillOnce([](const GURL& url, bool from_user_interaction) {
+        EXPECT_EQ(GURL("https://support.google.com/chromebook/?p=multi_device"),
+                  url);
+        EXPECT_TRUE(from_user_interaction);
+      });
+
+  // Simulates a click on the "Learn more" button.
+  ClickOnAndWait(disconnected_learn_more_button());
 }
 
 }  // namespace ash

ash/system/phonehub/phone_hub_ui_controller.cc

@@ -62,10 +62,12 @@ std::unique_ptr<views::View> PhoneHubUiController::CreateContentView(
       return std::make_unique<InitialConnectingView>();
     case UiState::kPhoneConnecting:
       return std::make_unique<ConnectionErrorView>(
-          ConnectionErrorView::ErrorStatus::kReconnecting);
+          ConnectionErrorView::ErrorStatus::kReconnecting,
+          phone_hub_manager_->GetConnectionScheduler());
     case UiState::kConnectionError:
       return std::make_unique<ConnectionErrorView>(
-          ConnectionErrorView::ErrorStatus::kDisconnected);
+          ConnectionErrorView::ErrorStatus::kDisconnected,
+          phone_hub_manager_->GetConnectionScheduler());
     case UiState::kPhoneConnected:
       return std::make_unique<PhoneConnectedView>(bubble_view,
                                                   phone_hub_manager_);

ash/system/phonehub/phone_hub_ui_controller_unittest.cc

@@ -125,8 +125,7 @@ TEST_F(PhoneHubUiControllerTest, PhoneDisconnected) {
             controller_.ui_state());
 
   auto content_view = controller_.CreateContentView(/*bubble_view=*/nullptr);
-  // TODO(tengs): Test the actual view id.
-  EXPECT_EQ(0, content_view->GetID());
+  EXPECT_EQ(PhoneHubViewID::kDisconnectedView, content_view->GetID());
 }
 
 TEST_F(PhoneHubUiControllerTest, PhoneConnecting) {
@@ -135,8 +134,7 @@ TEST_F(PhoneHubUiControllerTest, PhoneConnecting) {
             controller_.ui_state());
 
   auto content_view = controller_.CreateContentView(/*bubble_view=*/nullptr);
-  // TODO(tengs): Test the actual view id.
-  EXPECT_EQ(0, content_view->GetID());
+  EXPECT_EQ(PhoneHubViewID::kReconnectingView, content_view->GetID());
 }
 
 TEST_F(PhoneHubUiControllerTest, PhoneConnected) {

ash/system/phonehub/phone_hub_view_ids.h

@@ -22,6 +22,12 @@ enum PhoneHubViewID {
   kOnboardingView,
   kOnboardingGetStartedButton,
   kOnboardingDismissButton,
+
+  // Connection error view and its components.
+  kDisconnectedView,
+  kReconnectingView,
+  kDisconnectedLearnMoreButton,
+  kDisconnectedRefreshButton,
 };
 
 }  // namespace ash

chromeos/components/phonehub/fake_connection_scheduler.h

@@ -25,7 +25,7 @@ class FakeConnectionScheduler : public ConnectionScheduler {
   // ConnectionScheduler:
   void ScheduleConnectionNow() override;
 
-  size_t num_schedule_connection_now_calls_;
+  size_t num_schedule_connection_now_calls_ = 0u;
 };
 
 }  // namespace phonehub