[CrOS PhoneHub] Add timeout to silence phone controller.

- Added a timer to the controller when the button is pressed. If after a long time and the current state is different from the requested state, it means something when wrong and we should switch back to the original state.
- Remove the connecting state in the controller.

BUG=1106937,1126208

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

ash/system/phonehub/quick_action_item.h

@@ -16,7 +16,8 @@ class Label;
 namespace ash {
 
 // A toggle button with labels used in the quick action view.
-class QuickActionItem : public views::View, public views::ButtonListener {
+class ASH_EXPORT QuickActionItem : public views::View,
+                                   public views::ButtonListener {
  public:
   class Delegate {
    public:

ash/system/phonehub/quick_actions_view_unittest.cc

@@ -22,11 +22,15 @@ class DummyEvent : public ui::Event {
   DummyEvent() : Event(ui::ET_UNKNOWN, base::TimeTicks(), 0) {}
 };
 
+constexpr base::TimeDelta kWaitForRequestTimeout =
+    base::TimeDelta::FromSeconds(10);
+
 }  // namespace
 
 class QuickActionsViewTest : public AshTestBase {
  public:
-  QuickActionsViewTest() = default;
+  QuickActionsViewTest()
+      : AshTestBase(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
   ~QuickActionsViewTest() override = default;
 
   // AshTestBase:
@@ -105,6 +109,22 @@ TEST_F(QuickActionsViewTest, SilencePhoneToggle) {
   actions_view()->silence_phone_for_testing()->ButtonPressed(nullptr,
                                                              DummyEvent());
   EXPECT_FALSE(dnd_controller()->IsDndEnabled());
+
+  // Test the error state.
+  dnd_controller()->SetShouldRequestFail(true);
+  actions_view()->silence_phone_for_testing()->ButtonPressed(nullptr,
+                                                             DummyEvent());
+
+  // In error state, do not disturb is disabled but the button should still be
+  // on after being pressed.
+  EXPECT_FALSE(dnd_controller()->IsDndEnabled());
+  EXPECT_TRUE(actions_view()->silence_phone_for_testing()->IsToggled());
+
+  // After a certain time, the button should be corrected to be off.
+  task_environment()->FastForwardBy(kWaitForRequestTimeout);
+  EXPECT_FALSE(actions_view()->silence_phone_for_testing()->IsToggled());
+
+  dnd_controller()->SetShouldRequestFail(false);
 }
 
 TEST_F(QuickActionsViewTest, LocatePhoneToggle) {

ash/system/phonehub/silence_phone_quick_action_controller.cc

@@ -7,10 +7,20 @@
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/phonehub/quick_action_item.h"
+#include "base/timer/timer.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace ash {
 
+namespace {
+
+// Time to wait until we check the state of the phone to prevent showing wrong
+// state
+constexpr base::TimeDelta kWaitForRequestTimeout =
+    base::TimeDelta::FromSeconds(10);
+
+}  // namespace
+
 SilencePhoneQuickActionController::SilencePhoneQuickActionController(
     chromeos::phonehub::DoNotDisturbController* dnd_controller)
     : dnd_controller_(dnd_controller) {
@@ -31,18 +41,32 @@ QuickActionItem* SilencePhoneQuickActionController::CreateItem() {
 }
 
 void SilencePhoneQuickActionController::OnButtonPressed(bool is_now_enabled) {
-  SetState(ActionState::kConnecting);
+  requested_state_ = is_now_enabled ? ActionState::kOff : ActionState::kOn;
+  SetItemState(requested_state_.value());
+
+  check_requested_state_timer_ = std::make_unique<base::OneShotTimer>();
+  check_requested_state_timer_->Start(
+      FROM_HERE, kWaitForRequestTimeout,
+      base::BindOnce(&SilencePhoneQuickActionController::CheckRequestedState,
+                     base::Unretained(this)));
+
   dnd_controller_->RequestNewDoNotDisturbState(!is_now_enabled);
-  // TODO(leandre): Add a timer to switch back to off state after connecting
-  // failed.
 }
 
 void SilencePhoneQuickActionController::OnDndStateChanged() {
-  dnd_controller_->IsDndEnabled() ? SetState(ActionState::kOn)
-                                  : SetState(ActionState::kOff);
+  state_ =
+      dnd_controller_->IsDndEnabled() ? ActionState::kOn : ActionState::kOff;
+  SetItemState(state_);
+
+  // If |requested_state_| correctly resembles the current state, reset it and
+  // the timer.
+  if (state_ == requested_state_) {
+    check_requested_state_timer_.reset();
+    requested_state_.reset();
+  }
 }
 
-void SilencePhoneQuickActionController::SetState(ActionState state) {
+void SilencePhoneQuickActionController::SetItemState(ActionState state) {
   bool icon_enabled;
   int state_text_id;
   int sub_label_text;
@@ -52,11 +76,6 @@ void SilencePhoneQuickActionController::SetState(ActionState 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;
-      state_text_id = IDS_ASH_PHONE_HUB_QUICK_ACTIONS_CONNECTING_STATE_TOOLTIP;
-      sub_label_text = IDS_ASH_PHONE_HUB_QUICK_ACTIONS_CONNECTING_STATE;
-      break;
     case ActionState::kOn:
       icon_enabled = true;
       state_text_id = IDS_ASH_PHONE_HUB_QUICK_ACTIONS_ENABLED_STATE_TOOLTIP;
@@ -73,4 +92,14 @@ void SilencePhoneQuickActionController::SetState(ActionState state) {
                                  item_->GetItemLabel(), tooltip_state));
 }
 
+void SilencePhoneQuickActionController::CheckRequestedState() {
+  // If the current state is different from the requested state, it means that
+  // we fail to change the state, so switch back to the original one.
+  if (state_ != requested_state_)
+    SetItemState(state_);
+
+  check_requested_state_timer_.reset();
+  requested_state_.reset();
+}
+
 }  // namespace ash

ash/system/phonehub/silence_phone_quick_action_controller.h

@@ -8,6 +8,10 @@
 #include "ash/system/phonehub/quick_action_controller_base.h"
 #include "chromeos/components/phonehub/do_not_disturb_controller.h"
 
+namespace base {
+class OneShotTimer;
+}  // namespace base
+
 namespace ash {
 
 // Controller of a quick action item that toggles silence phone mode.
@@ -33,13 +37,28 @@ class SilencePhoneQuickActionController
  private:
   // All the possible states that the silence phone button can be viewed. Each
   // state has a corresponding icon, labels and tooltip view.
-  enum class ActionState { kOff, kConnecting, kOn };
+  enum class ActionState { kOff, kOn };
 
   // Set the item (including icon, label and tooltips) to a certain state.
-  void SetState(ActionState state);
+  void SetItemState(ActionState state);
+
+  // Check to see if the requested state is similar to current state of the
+  // phone. Make changes to item's state if necessary.
+  void CheckRequestedState();
 
   chromeos::phonehub::DoNotDisturbController* dnd_controller_ = nullptr;
   QuickActionItem* item_ = nullptr;
+
+  // Keep track the current state of the item.
+  ActionState state_;
+
+  // State that user requests when clicking the button.
+  base::Optional<ActionState> requested_state_;
+
+  // Timer that fires to prevent showing wrong state in the item. It will check
+  // if the requested state is similar to the current state after the button is
+  // pressed for a certain time.
+  std::unique_ptr<base::OneShotTimer> check_requested_state_timer_;
 };
 
 }  // namespace ash

chromeos/components/phonehub/fake_do_not_disturb_controller.cc

@@ -25,8 +25,14 @@ void FakeDoNotDisturbController::SetDoNotDisturbStateInternal(
 }
 
 void FakeDoNotDisturbController::RequestNewDoNotDisturbState(bool enabled) {
+  if (!should_request_fail_)
     SetDoNotDisturbStateInternal(enabled);
 }
 
+void FakeDoNotDisturbController::SetShouldRequestFail(
+    bool should_request_fail) {
+  should_request_fail_ = should_request_fail;
+}
+
 }  // namespace phonehub
 }  // namespace chromeos

chromeos/components/phonehub/fake_do_not_disturb_controller.h

@@ -20,8 +20,14 @@ class FakeDoNotDisturbController : public DoNotDisturbController {
   void SetDoNotDisturbStateInternal(bool is_dnd_enabled) override;
   void RequestNewDoNotDisturbState(bool enabled) override;
 
+  void SetShouldRequestFail(bool should_request_fail);
+
  private:
   bool is_dnd_enabled_ = false;
+
+  // Indicates if the connection to the phone is working correctly. If it is
+  // true, there is a problem and the phone cannot change its state.
+  bool should_request_fail_ = false;
 };
 
 }  // namespace phonehub