[CrOS PhoneHub] Clean up SilencePhoneQuickActionController

This CL removes an Observer interface and related functions which are no
longer used. They previously were used to facilitate the "Locate phone"
feature, whose implementation was changed to no longer need this
functionality.

Fixed: 1156382
Bug: 1106937
Change-Id: Ic3c7b4208388c6ac96e307a3a7364c4171edd378
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2605844
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Regan Hsu <hsuregan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839623}

ash/system/phonehub/silence_phone_quick_action_controller.cc

@@ -36,14 +36,6 @@ SilencePhoneQuickActionController::~SilencePhoneQuickActionController() {
   dnd_controller_->RemoveObserver(this);
 }
 
-void SilencePhoneQuickActionController::AddObserver(Observer* observer) {
-  observer_list_.AddObserver(observer);
-}
-
-void SilencePhoneQuickActionController::RemoveObserver(Observer* observer) {
-  observer_list_.RemoveObserver(observer);
-}
-
 bool SilencePhoneQuickActionController::IsItemEnabled() {
   return item_->IsToggled();
 }
@@ -133,8 +125,6 @@ void SilencePhoneQuickActionController::SetItemState(ActionState state) {
         IDS_ASH_PHONE_HUB_QUICK_ACTIONS_TOGGLE_TOOLTIP, item_->GetItemLabel(),
         tooltip_state));
   }
-  for (auto& observer : observer_list_)
-    observer.OnSilencePhoneItemStateChanged();
 }
 
 void SilencePhoneQuickActionController::CheckRequestedState() {

ash/system/phonehub/silence_phone_quick_action_controller.h

@@ -6,8 +6,6 @@
 #define ASH_SYSTEM_PHONEHUB_SILENCE_PHONE_QUICK_ACTION_CONTROLLER_H_
 
 #include "ash/system/phonehub/quick_action_controller_base.h"
-#include "base/observer_list.h"
-#include "base/observer_list_types.h"
 #include "chromeos/components/phonehub/do_not_disturb_controller.h"
 
 namespace base {
@@ -21,14 +19,6 @@ class ASH_EXPORT SilencePhoneQuickActionController
     : public QuickActionControllerBase,
       public chromeos::phonehub::DoNotDisturbController::Observer {
  public:
-  class Observer : public base::CheckedObserver {
-   public:
-    ~Observer() override = default;
-
-    // Called when the state of the item has changed.
-    virtual void OnSilencePhoneItemStateChanged() = 0;
-  };
-
   explicit SilencePhoneQuickActionController(
       chromeos::phonehub::DoNotDisturbController* dnd_controller);
   ~SilencePhoneQuickActionController() override;
@@ -37,9 +27,6 @@ class ASH_EXPORT SilencePhoneQuickActionController
   SilencePhoneQuickActionController operator=(
       SilencePhoneQuickActionController&) = delete;
 
-  void AddObserver(Observer* observer);
-  void RemoveObserver(Observer* observer);
-
   // Return true if the item is enabled/toggled.
   bool IsItemEnabled();
 
@@ -80,9 +67,6 @@ class ASH_EXPORT SilencePhoneQuickActionController
   // 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_;
-
-  // Registered observers.
-  base::ObserverList<Observer> observer_list_;
 };
 
 }  // namespace ash

ash/system/phonehub/silence_phone_quick_action_controller_unittest.cc

@@ -9,9 +9,7 @@
 
 namespace ash {
 
-class SilencePhoneQuickActionControllerTest
-    : public AshTestBase,
-      public SilencePhoneQuickActionController::Observer {
+class SilencePhoneQuickActionControllerTest : public AshTestBase {
  public:
   SilencePhoneQuickActionControllerTest() = default;
 
@@ -26,21 +24,16 @@ class SilencePhoneQuickActionControllerTest
     controller_ = std::make_unique<SilencePhoneQuickActionController>(
         dnd_controller_.get());
 
-    controller_->AddObserver(this);
     controller_->CreateItem();
   }
 
   void TearDown() override {
-    controller_->RemoveObserver(this);
     controller_.reset();
     dnd_controller_.reset();
     AshTestBase::TearDown();
   }
 
  protected:
-  // SilencePhoneQuickActionController::Observer:
-  void OnSilencePhoneItemStateChanged() override { ++num_calls_; }
-
   SilencePhoneQuickActionController* controller() { return controller_.get(); }
 
   chromeos::phonehub::FakeDoNotDisturbController* dnd_controller() {
@@ -52,37 +45,28 @@ class SilencePhoneQuickActionControllerTest
            controller_->GetItemState();
   }
 
-  size_t GetNumObserverCalls() { return num_calls_; }
-
  private:
   std::unique_ptr<SilencePhoneQuickActionController> controller_;
   std::unique_ptr<chromeos::phonehub::FakeDoNotDisturbController>
       dnd_controller_;
-  size_t num_calls_ = 0;
 };
 
 TEST_F(SilencePhoneQuickActionControllerTest, ItemStateChanged) {
   // Set request to fail to avoid triggering state's changes by the model.
   dnd_controller()->SetShouldRequestFail(true);
 
-  // Initially, there's one observer call during initiation.
-  EXPECT_EQ(1u, GetNumObserverCalls());
-
   // Allow the button to be enabled.
   dnd_controller()->SetDoNotDisturbStateInternal(
       /*is_dnd_enabled=*/false, /*can_request_new_dnd_state=*/true);
-  EXPECT_EQ(2u, GetNumObserverCalls());
 
   // Press the button to enabled state will trigger observer.
   controller()->OnButtonPressed(false /* is_now_enabled */);
-  EXPECT_EQ(3u, GetNumObserverCalls());
 
   // Item state changed to enabled.
   EXPECT_TRUE(controller()->IsItemEnabled());
 
   // Press the button to disabled state will trigger observer.
   controller()->OnButtonPressed(true /* is_now_enabled */);
-  EXPECT_EQ(4u, GetNumObserverCalls());
 
   // Item state changed to disabled.
   EXPECT_FALSE(controller()->IsItemEnabled());
@@ -94,7 +78,7 @@ TEST_F(SilencePhoneQuickActionControllerTest, CanRequestNewDndState) {
   // Set DoNotDisturbState to not allow any new request.
   dnd_controller()->SetDoNotDisturbStateInternal(
       /*is_dnd_enabled=*/false, /*can_request_new_dnd_state=*/false);
-  EXPECT_EQ(1u, GetNumObserverCalls());
+
   // Since no new state requests are allowed, expect the button to be disabled.
   EXPECT_FALSE(controller()->IsItemEnabled());
   EXPECT_TRUE(IsButtonDisabled());
@@ -103,7 +87,7 @@ TEST_F(SilencePhoneQuickActionControllerTest, CanRequestNewDndState) {
   // work profile.
   dnd_controller()->SetDoNotDisturbStateInternal(
       /*is_dnd_enabled=*/true, /*can_request_new_dnd_state=*/false);
-  EXPECT_EQ(2u, GetNumObserverCalls());
+
   // The button should still be disabled despite the phone has DoNotDisturb mode
   // enabled. However, the underlying toggle has been flipped to enabled.
   EXPECT_TRUE(controller()->IsItemEnabled());
@@ -112,7 +96,6 @@ TEST_F(SilencePhoneQuickActionControllerTest, CanRequestNewDndState) {
   // Flip toggle state back to enabled, but still in a work profile.
   dnd_controller()->SetDoNotDisturbStateInternal(
       /*is_dnd_enabled=*/false, /*can_request_new_dnd_state=*/false);
-  EXPECT_EQ(3u, GetNumObserverCalls());
   EXPECT_FALSE(controller()->IsItemEnabled());
   EXPECT_TRUE(IsButtonDisabled());
 }