[WIP][cr132] SidePanelViewStateObserver deleted

Chromium change: https://chromium.googlesource.com/chromium/src/+/14d2af218a8a285c75dacce592ba891b224f0342 commit 14d2af218a8a285c75dacce592ba891b224f0342 Author: Erik Chen <[email protected]> Date: Wed Oct 30 23:53:06 2024 +0000 Reduce fragility in SidePanelCoordinator (part 18 / N) This CL is a refactor with no intended behavior change. This CL deletes SidePanelViewStateObserver. The class no longer has any use cases in production code, and the use cases in test code have alternatives. Change-Id: I24d5bbb37a15f33ceb0ab27cc7da14e43d1416ba Bug: 363743081
brave · Oct 31, 2024 · 07af0e3 · 07af0e3
1 parent 0025863
commit 07af0e3
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 22 deletions.
diff --git a/browser/ui/brave_browser_command_controller_browsertest.cc b/browser/ui/brave_browser_command_controller_browsertest.cc
@@ -9,6 +9,7 @@
 #include <optional>
 
 #include "base/functional/callback_helpers.h"
+#include "base/test/run_until.h"
 #include "base/test/scoped_feature_list.h"
 #include "brave/app/brave_command_ids.h"
 #include "brave/browser/ui/browser_commands.h"
@@ -60,7 +61,6 @@
 #include "chrome/browser/ui/views/side_panel/side_panel_entry_id.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_entry_key.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_enums.h"
-#include "chrome/browser/ui/views/side_panel/side_panel_test_utils.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_ui.h"
 #endif
 
@@ -181,6 +181,15 @@ class BraveBrowserCommandControllerTest : public InProcessBrowserTest {
   }
 #endif
 
+#if defined(TOOLKIT_VIEWS)
+  void WaitForSidePanelClose() {
+    ASSERT_TRUE(base::test::RunUntil([&]() {
+      return browser()->GetBrowserView().unified_side_panel()->state() ==
+             SidePanel::State::kClosed;
+    }));
+  }
+#endif  // #if defined(TOOLKIT_VIEWS)
+
  private:
   policy::MockConfigurationPolicyProvider provider_;
   base::test::ScopedFeatureList scoped_feature_list_;
@@ -453,7 +462,10 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
       SidePanelEntry::Key(SidePanelEntryId::kChatUI);
   auto* side_panel_coordinator =
       browser()->GetFeatures().side_panel_coordinator();
-  auto side_panel_waiter = SidePanelWaiter(side_panel_coordinator);
+  ASSERT_TRUE(base::test::RunUntil([&]() {
+    return browser()->GetBrowserView().unified_side_panel()->state() ==
+           SidePanel::State::kClosed;
+  }));
 
   // initially no panel is showing
   EXPECT_FALSE(side_panel_coordinator->IsSidePanelEntryShowing(ai_chat_key));
@@ -464,7 +476,7 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
   EXPECT_TRUE(side_panel_coordinator->IsSidePanelEntryShowing(ai_chat_key));
   // after command again, no panel is showing
   browser()->command_controller()->ExecuteCommand(IDC_TOGGLE_AI_CHAT);
-  side_panel_waiter.WaitForSidePanelClose();
+  WaitForSidePanelClose();
   EXPECT_FALSE(side_panel_coordinator->IsSidePanelEntryShowing(ai_chat_key));
   EXPECT_FALSE(side_panel_coordinator->IsSidePanelShowing());
 
@@ -479,7 +491,7 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
   EXPECT_TRUE(side_panel_coordinator->IsSidePanelEntryShowing(ai_chat_key));
   // after command again, no panel is showing
   browser()->command_controller()->ExecuteCommand(IDC_TOGGLE_AI_CHAT);
-  side_panel_waiter.WaitForSidePanelClose();
+  WaitForSidePanelClose();
   EXPECT_FALSE(side_panel_coordinator->IsSidePanelEntryShowing(ai_chat_key));
   EXPECT_FALSE(side_panel_coordinator->IsSidePanelShowing());
 }

diff --git a/browser/ui/views/sidebar/sidebar_container_view.cc b/browser/ui/views/sidebar/sidebar_container_view.cc
@@ -128,7 +128,6 @@ SidebarContainerView::SidebarContainerView(
 
   SetNotifyEnterExitOnChild(true);
   side_panel_ = AddChildView(std::move(side_panel));
-  side_panel_view_state_observation_.Observe(side_panel_coordinator_);
 }
 
 SidebarContainerView::~SidebarContainerView() = default;
@@ -831,15 +830,6 @@ void SidebarContainerView::UpdateActiveItemState() {
   controller->UpdateActiveItemState(current_type);
 }
 
-void SidebarContainerView::OnSidePanelDidClose() {
-  // As contextual registry is owned by TabFeatures,
-  // that registry is destroyed before coordinator notifies OnEntryHidden() when
-  // tab is closed. In this case, we should update sidebar ui(active item state)
-  // with this notification. If not, sidebar ui's active item state is not
-  // changed.
-  UpdateActiveItemState();
-}
-
 void SidebarContainerView::OnTabStripModelChanged(
     TabStripModel* tab_strip_model,
     const TabStripModelChange& change,

diff --git a/browser/ui/views/sidebar/sidebar_container_view.h b/browser/ui/views/sidebar/sidebar_container_view.h
@@ -20,7 +20,6 @@
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_entry_observer.h"
-#include "chrome/browser/ui/views/side_panel/side_panel_view_state_observer.h"
 #include "components/prefs/pref_member.h"
 #include "ui/events/event_observer.h"
 #include "ui/gfx/animation/slide_animation.h"
@@ -55,7 +54,6 @@ class SidebarContainerView
       public SidebarShowOptionsEventDetectWidget::Delegate,
       public sidebar::SidebarModel::Observer,
       public SidePanelEntryObserver,
-      public SidePanelViewStateObserver,
       public TabStripModelObserver {
   METADATA_HEADER(SidebarContainerView, views::View)
  public:
@@ -135,9 +133,6 @@ class SidebarContainerView
       const TabStripSelectionChange& selection) override;
   void OnTabWillBeRemoved(content::WebContents* contents, int index) override;
 
-  // SidePanelViewStateObserver:
-  void OnSidePanelDidClose() override;
-
  private:
   friend class sidebar::SidebarBrowserTest;
 
@@ -214,8 +209,6 @@ class SidebarContainerView
   std::unique_ptr<views::EventMonitor> browser_window_event_monitor_;
   std::unique_ptr<SidebarShowOptionsEventDetectWidget> show_options_widget_;
   BooleanPrefMember show_side_panel_button_;
-  base::ScopedObservation<SidePanelCoordinator, SidePanelViewStateObserver>
-      side_panel_view_state_observation_{this};
   base::ScopedObservation<sidebar::SidebarModel,
                           sidebar::SidebarModel::Observer>
       sidebar_model_observation_{this};

diff --git a/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.h b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.h
@@ -20,7 +20,6 @@
 #include "chrome/browser/ui/views/side_panel/side_panel_registry.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_ui.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_util.h"
-#include "chrome/browser/ui/views/side_panel/side_panel_view_state_observer.h"
 #include "ui/views/view_observer.h"
 
 #define CreateHeader                      \