Revert "[CrOS Hotspot] Add hotspot icon animation in quick settings"

This reverts commit 8238871.

Reason for revert: HotspotDetailedViewTest failing on linux-chromeos-dbg bots

Bug:1449561

Original change's description:
> [CrOS Hotspot] Add hotspot icon animation in quick settings
>
> This CL adds the hotspot icon animation when hotspot is enabling
>
> Video: https://drive.google.com/file/d/1ZHHg47-ymOGjw61P7YLQuSYCpSy-7pBB/view?usp=drive_link&resourcekey=0-cTP2CVMuH6lbKhbeDm1IfQ
>
> Bug: b/280693408
> Change-Id: I67fed0eb67556abddbc35a0ed6db894817c99e8f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4546862
> Commit-Queue: Jason Zhang <jiajunz@google.com>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1149869}

Bug: b/280693408
Change-Id: Id26428ebd66f5381fe112af0d749aa7ecd12c973
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4568050
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Owners-Override: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1150158}

ash/BUILD.gn

@@ -1414,11 +1414,6 @@ component("ash") {
     "system/hotspot/hotspot_detailed_view_controller.h",
     "system/hotspot/hotspot_feature_pod_controller.cc",
     "system/hotspot/hotspot_feature_pod_controller.h",
-    "system/hotspot/hotspot_icon.cc",
-    "system/hotspot/hotspot_icon.h",
-    "system/hotspot/hotspot_icon_animation.cc",
-    "system/hotspot/hotspot_icon_animation.h",
-    "system/hotspot/hotspot_icon_animation_observer.h",
     "system/hotspot/hotspot_info_cache.cc",
     "system/hotspot/hotspot_info_cache.h",
     "system/hotspot/hotspot_notifier.cc",
@@ -3284,7 +3279,6 @@ test("ash_unittests") {
     "system/hotspot/hotspot_detailed_view_controller_unittest.cc",
     "system/hotspot/hotspot_detailed_view_unittest.cc",
     "system/hotspot/hotspot_feature_pod_controller_unittest.cc",
-    "system/hotspot/hotspot_icon_unittest.cc",
     "system/hotspot/hotspot_info_cache_unittest.cc",
     "system/hotspot/hotspot_notifier_unittest.cc",
     "system/hotspot/hotspot_tray_view_unittest.cc",

ash/resources/vector_icons/BUILD.gn

@@ -115,10 +115,8 @@ aggregate_vector_icons("ash_vector_icons") {
     "holding_space_download.icon",
     "holding_space_refresh.icon",
     "hollow_check_circle.icon",
-    "hotspot_dot.icon",
     "hotspot_off.icon",
     "hotspot_on.icon",
-    "hotspot_one_arc.icon",
     "ime_menu_emoticon.icon",
     "ime_menu_microphone.icon",
     "ime_menu_on_screen_keyboard.icon",

ash/shell.cc

@@ -135,7 +135,6 @@
 #include "ash/system/federated/federated_service_controller_impl.h"
 #include "ash/system/firmware_update/firmware_update_notification_controller.h"
 #include "ash/system/geolocation/geolocation_controller.h"
-#include "ash/system/hotspot/hotspot_icon_animation.h"
 #include "ash/system/hotspot/hotspot_info_cache.h"
 #include "ash/system/human_presence/human_presence_orientation_controller.h"
 #include "ash/system/human_presence/snooping_protection_controller.h"
@@ -1572,7 +1571,6 @@ void Shell::Init(
   }
 
   if (features::IsHotspotEnabled()) {
-    hotspot_icon_animation_ = std::make_unique<HotspotIconAnimation>();
     hotspot_info_cache_ = std::make_unique<HotspotInfoCache>();
   }
 

ash/shell.h

@@ -156,7 +156,6 @@ class GlanceablesController;
 class GlanceablesV2Controller;
 class ColorEnhancementController;
 class HoldingSpaceController;
-class HotspotIconAnimation;
 class HotspotInfoCache;
 class HumanPresenceOrientationController;
 class ImeControllerImpl;
@@ -559,9 +558,6 @@ class ASH_EXPORT Shell : public SessionObserver,
   ColorEnhancementController* color_enhancement_controller() {
     return color_enhancement_controller_.get();
   }
-  HotspotIconAnimation* hotspot_icon_animation() {
-    return hotspot_icon_animation_.get();
-  }
   HotspotInfoCache* hotspot_info_cache() { return hotspot_info_cache_.get(); }
   HumanPresenceOrientationController* human_presence_orientation_controller() {
     return human_presence_orientation_controller_.get();
@@ -1111,7 +1107,6 @@ class ASH_EXPORT Shell : public SessionObserver,
   std::unique_ptr<DisplayColorManager> display_color_manager_;
   std::unique_ptr<DisplayErrorObserver> display_error_observer_;
   std::unique_ptr<ProjectingObserver> projecting_observer_;
-  std::unique_ptr<HotspotIconAnimation> hotspot_icon_animation_;
   std::unique_ptr<HotspotInfoCache> hotspot_info_cache_;
   std::unique_ptr<display::DisplayPortObserver> display_port_observer_;
 

ash/system/hotspot/hotspot_detailed_view.cc

@@ -12,8 +12,6 @@
 #include "ash/style/rounded_container.h"
 #include "ash/style/switch.h"
 #include "ash/style/typography.h"
-#include "ash/system/hotspot/hotspot_icon.h"
-#include "ash/system/hotspot/hotspot_icon_animation.h"
 #include "ash/system/model/system_tray_model.h"
 #include "ash/system/tray/detailed_view_delegate.h"
 #include "ash/system/tray/hover_highlight_view.h"
@@ -61,21 +59,14 @@ HotspotDetailedView::HotspotDetailedView(
   CreateContainer();
 }
 
-HotspotDetailedView::~HotspotDetailedView() {
-  Shell::Get()->hotspot_icon_animation()->RemoveObserver(this);
-}
+HotspotDetailedView::~HotspotDetailedView() = default;
 
 void HotspotDetailedView::UpdateViewForHotspot(HotspotInfoPtr hotspot_info) {
-  if (hotspot_info->state == HotspotState::kEnabling) {
-    Shell::Get()->hotspot_icon_animation()->AddObserver(this);
-  } else if (state_ == HotspotState::kEnabling) {
-    Shell::Get()->hotspot_icon_animation()->RemoveObserver(this);
-  }
-
-  if (state_ != hotspot_info->state) {
-    state_ = hotspot_info->state;
-    UpdateIcon();
-  }
+  // Update the Hotspot icon.
+  hotspot_icon_->SetImage(ui::ImageModel::FromVectorIcon(
+      IsEnabledOrEnabling(hotspot_info->state) ? kHotspotOnIcon
+                                               : kHotspotOffIcon,
+      cros_tokens::kCrosSysOnSurface));
 
   UpdateSubText(hotspot_info);
   UpdateToggleState(hotspot_info->state, hotspot_info->allow_status);
@@ -126,8 +117,6 @@ void HotspotDetailedView::CreateContainer() {
   auto hotspot_icon = std::make_unique<views::ImageView>();
   hotspot_icon->SetID(
       static_cast<int>(HotspotDetailedViewChildId::kHotspotIcon));
-  hotspot_icon->SetImage(ui::ImageModel::FromVectorIcon(
-      kHotspotOffIcon, cros_tokens::kCrosSysOnSurface));
   hotspot_icon_ = hotspot_icon.get();
   entry_row_->AddViewAndLabel(std::move(hotspot_icon), u"");
   entry_row_->text_label()->SetText(l10n_util::GetStringFUTF16(
@@ -169,15 +158,6 @@ void HotspotDetailedView::ToggleHotspot(bool new_state) {
   delegate_->OnToggleClicked(new_state);
 }
 
-void HotspotDetailedView::HotspotIconChanged() {
-  UpdateIcon();
-}
-
-void HotspotDetailedView::UpdateIcon() {
-  hotspot_icon_->SetImage(ui::ImageModel::FromVectorIcon(
-      hotspot_icon::GetIconForHotspot(state_), cros_tokens::kCrosSysOnSurface));
-}
-
 void HotspotDetailedView::UpdateToggleState(
     const HotspotState& state,
     const HotspotAllowStatus& allow_status) {

ash/system/hotspot/hotspot_detailed_view.h

@@ -6,7 +6,6 @@
 #define ASH_SYSTEM_HOTSPOT_HOTSPOT_DETAILED_VIEW_H_
 
 #include "ash/ash_export.h"
-#include "ash/system/hotspot/hotspot_icon_animation_observer.h"
 #include "ash/system/tray/tray_detailed_view.h"
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -28,8 +27,7 @@ class Switch;
 // This class defines both the interface used to interact with the detailed
 // Hotspot page within the quick settings. This class includes the declaration
 // for the delegate interface it uses to propagate user interactions.
-class ASH_EXPORT HotspotDetailedView : public TrayDetailedView,
-                                       public HotspotIconAnimationObserver {
+class ASH_EXPORT HotspotDetailedView : public TrayDetailedView {
  public:
   METADATA_HEADER(HotspotDetailedView);
 
@@ -56,9 +54,6 @@ class ASH_EXPORT HotspotDetailedView : public TrayDetailedView,
   void HandleViewClicked(views::View* view) override;
   void CreateExtraTitleRowButtons() override;
 
-  // HotspotIconAnimationObserver:
-  void HotspotIconChanged() override;
-
  private:
   friend class HotspotDetailedViewControllerTest;
   friend class HotspotDetailedViewTest;
@@ -85,17 +80,13 @@ class ASH_EXPORT HotspotDetailedView : public TrayDetailedView,
   // Handles toggling Hotspot via the UI to `new_state`.
   void ToggleHotspot(bool new_state);
 
-  void UpdateIcon();
   void UpdateToggleState(
       const hotspot_config::mojom::HotspotState& state,
       const hotspot_config::mojom::HotspotAllowStatus& allow_status);
   void UpdateSubText(const hotspot_config::mojom::HotspotInfoPtr& hotspot_info);
   void UpdateExtraIcon(
       const hotspot_config::mojom::HotspotAllowStatus& allow_status);
 
-  hotspot_config::mojom::HotspotState state_ =
-      hotspot_config::mojom::HotspotState::kDisabled;
-
   const raw_ptr<Delegate, ExperimentalAsh> delegate_;
 
   // Owned by views hierarchy.

ash/system/hotspot/hotspot_detailed_view_unittest.cc

@@ -4,21 +4,14 @@
 
 #include "ash/system/hotspot/hotspot_detailed_view.h"
 
-#include "ash/constants/ash_features.h"
 #include "ash/public/cpp/test/test_system_tray_client.h"
-#include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/style/rounded_container.h"
 #include "ash/style/switch.h"
 #include "ash/system/tray/detailed_view_delegate.h"
 #include "ash/system/tray/hover_highlight_view.h"
 #include "ash/test/ash_test_base.h"
 #include "base/memory/raw_ptr.h"
-#include "base/test/scoped_feature_list.h"
-#include "base/test/task_environment.h"
-#include "chromeos/ash/services/hotspot_config/public/cpp/cros_hotspot_config_test_helper.h"
-#include "ui/chromeos/styles/cros_tokens_color_mappings.h"
-#include "ui/gfx/image/image_unittest_util.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/test/views_test_utils.h"
@@ -66,18 +59,10 @@ class FakeDetailedViewDelegate : public DetailedViewDelegate {
 
 class HotspotDetailedViewTest : public AshTestBase {
  public:
-  HotspotDetailedViewTest()
-      : AshTestBase(std::make_unique<base::test::TaskEnvironment>(
-            base::test::TaskEnvironment::MainThreadType::UI,
-            base::test::TaskEnvironment::TimeSource::MOCK_TIME)) {}
+  HotspotDetailedViewTest() = default;
   ~HotspotDetailedViewTest() override = default;
 
   void SetUp() override {
-    scoped_feature_list_.InitWithFeatures(
-        {features::kHotspot, features::kQsRevamp}, {});
-    cros_hotspot_config_test_helper_ =
-        std::make_unique<hotspot_config::CrosHotspotConfigTestHelper>(
-            /*use_fake_implementation=*/true);
     AshTestBase::SetUp();
 
     auto hotspot_detailed_view = std::make_unique<HotspotDetailedView>(
@@ -93,7 +78,6 @@ class HotspotDetailedViewTest : public AshTestBase {
     widget_.reset();
 
     AshTestBase::TearDown();
-    cros_hotspot_config_test_helper_.reset();
   }
 
   void UpdateHotspotView(HotspotState state,
@@ -121,11 +105,6 @@ class HotspotDetailedViewTest : public AshTestBase {
         HotspotDetailedView::HotspotDetailedViewChildId::kToggle);
   }
 
-  views::ImageView* GetHotspotIcon() {
-    return FindViewById<views::ImageView*>(
-        HotspotDetailedView::HotspotDetailedViewChildId::kHotspotIcon);
-  }
-
   views::ImageView* GetExtraIcon() {
     return FindViewById<views::ImageView*>(
         HotspotDetailedView::HotspotDetailedViewChildId::kExtraIcon);
@@ -175,9 +154,6 @@ class HotspotDetailedViewTest : public AshTestBase {
         hotspot_detailed_view_->GetViewByID(static_cast<int>(id)));
   }
 
-  base::test::ScopedFeatureList scoped_feature_list_;
-  std::unique_ptr<hotspot_config::CrosHotspotConfigTestHelper>
-      cros_hotspot_config_test_helper_;
   std::unique_ptr<views::Widget> widget_;
   FakeHotspotDetailedViewDelegate hotspot_detailed_view_delegate_;
   FakeDetailedViewDelegate detailed_view_delegate_;
@@ -215,11 +191,6 @@ TEST_F(HotspotDetailedViewTest, HotspotEnabledUI) {
   AssertToggleOn(/*expected_toggle_on=*/true);
   views::ImageView* extra_icon = GetExtraIcon();
   EXPECT_FALSE(extra_icon->GetVisible());
-  views::ImageView* hotspot_icon = GetHotspotIcon();
-  ASSERT_TRUE(hotspot_icon);
-  ui::ImageModel image_model = ui::ImageModel::FromVectorIcon(
-      kHotspotOnIcon, cros_tokens::kCrosSysOnSurface);
-  EXPECT_EQ(image_model, hotspot_icon->GetImageModel());
 
   UpdateHotspotView(HotspotState::kEnabled, HotspotAllowStatus::kAllowed, 1);
   AssertSubtextLabel(u"1 device connected");
@@ -238,20 +209,6 @@ TEST_F(HotspotDetailedViewTest, HotspotEnablingUI) {
   AssertToggleOn(/*expected_toggle_on=*/true);
   views::ImageView* extra_icon = GetExtraIcon();
   EXPECT_FALSE(extra_icon->GetVisible());
-  views::ImageView* hotspot_icon = GetHotspotIcon();
-  ASSERT_TRUE(hotspot_icon);
-  ui::ImageModel image_model = ui::ImageModel::FromVectorIcon(
-      kHotspotDotIcon, cros_tokens::kCrosSysOnSurface);
-  EXPECT_EQ(image_model, hotspot_icon->GetImageModel());
-  // Verifies the hotspot icon is animating when enabling.
-  task_environment()->FastForwardBy(base::Milliseconds(500));
-  image_model = ui::ImageModel::FromVectorIcon(kHotspotOneArcIcon,
-                                               cros_tokens::kCrosSysOnSurface);
-  EXPECT_EQ(image_model, hotspot_icon->GetImageModel());
-  task_environment()->FastForwardBy(base::Milliseconds(500));
-  image_model = ui::ImageModel::FromVectorIcon(kHotspotOnIcon,
-                                               cros_tokens::kCrosSysOnSurface);
-  EXPECT_EQ(image_model, hotspot_icon->GetImageModel());
 }
 
 TEST_F(HotspotDetailedViewTest, HotspotDisablingUI) {
@@ -264,11 +221,6 @@ TEST_F(HotspotDetailedViewTest, HotspotDisablingUI) {
   AssertToggleOn(/*expected_toggle_on=*/false);
   views::ImageView* extra_icon = GetExtraIcon();
   EXPECT_FALSE(extra_icon->GetVisible());
-  views::ImageView* hotspot_icon = GetHotspotIcon();
-  ASSERT_TRUE(hotspot_icon);
-  ui::ImageModel image_model = ui::ImageModel::FromVectorIcon(
-      kHotspotOffIcon, cros_tokens::kCrosSysOnSurface);
-  EXPECT_EQ(image_model, hotspot_icon->GetImageModel());
 }
 
 TEST_F(HotspotDetailedViewTest, HotspotDisabledAndAllowedUI) {
@@ -281,11 +233,6 @@ TEST_F(HotspotDetailedViewTest, HotspotDisabledAndAllowedUI) {
   AssertToggleOn(/*expected_toggle_on=*/false);
   views::ImageView* extra_icon = GetExtraIcon();
   EXPECT_FALSE(extra_icon->GetVisible());
-  views::ImageView* hotspot_icon = GetHotspotIcon();
-  ASSERT_TRUE(hotspot_icon);
-  ui::ImageModel image_model = ui::ImageModel::FromVectorIcon(
-      kHotspotOffIcon, cros_tokens::kCrosSysOnSurface);
-  EXPECT_EQ(image_model, hotspot_icon->GetImageModel());
 }
 
 TEST_F(HotspotDetailedViewTest, HotspotDisabledAndNoMobileNetworkUI) {
@@ -299,11 +246,6 @@ TEST_F(HotspotDetailedViewTest, HotspotDisabledAndNoMobileNetworkUI) {
   AssertToggleOn(/*expected_toggle_on=*/false);
   views::ImageView* extra_icon = GetExtraIcon();
   EXPECT_FALSE(extra_icon->GetVisible());
-  views::ImageView* hotspot_icon = GetHotspotIcon();
-  ASSERT_TRUE(hotspot_icon);
-  ui::ImageModel image_model = ui::ImageModel::FromVectorIcon(
-      kHotspotOffIcon, cros_tokens::kCrosSysOnSurface);
-  EXPECT_EQ(image_model, hotspot_icon->GetImageModel());
 }
 
 TEST_F(HotspotDetailedViewTest,
@@ -320,11 +262,6 @@ TEST_F(HotspotDetailedViewTest,
   EXPECT_TRUE(extra_icon->GetVisible());
   EXPECT_EQ(u"Your mobile network doesn't support hotspot",
             extra_icon->GetTooltipText());
-  views::ImageView* hotspot_icon = GetHotspotIcon();
-  ASSERT_TRUE(hotspot_icon);
-  ui::ImageModel image_model = ui::ImageModel::FromVectorIcon(
-      kHotspotOffIcon, cros_tokens::kCrosSysOnSurface);
-  EXPECT_EQ(image_model, hotspot_icon->GetImageModel());
 }
 
 TEST_F(HotspotDetailedViewTest, HotspotDisabledAndBlockedByPolicyUI) {
@@ -340,11 +277,6 @@ TEST_F(HotspotDetailedViewTest, HotspotDisabledAndBlockedByPolicyUI) {
   EXPECT_TRUE(extra_icon->GetVisible());
   EXPECT_EQ(u"This setting is managed by your administrator",
             extra_icon->GetTooltipText());
-  views::ImageView* hotspot_icon = GetHotspotIcon();
-  ASSERT_TRUE(hotspot_icon);
-  ui::ImageModel image_model = ui::ImageModel::FromVectorIcon(
-      kHotspotOffIcon, cros_tokens::kCrosSysOnSurface);
-  EXPECT_EQ(image_model, hotspot_icon->GetImageModel());
 }
 
 TEST_F(HotspotDetailedViewTest, PressingEntryRowNotifiesDelegate) {