From 3eb965f15425786b78f4895a20aa0de4703e6955 Mon Sep 17 00:00:00 2001
From: Simon Hong <shong@brave.com>
Date: Tue, 22 Oct 2024 16:06:16 +0900
Subject: [PATCH] Fixed contextual panel item is not activated after attaching
 to window

fix https://github.com/brave/brave-browser/issues/41782

We called SidebarContainerView::WillShowSidePanel() to observer
tab's contextual entry/global entry when panel starts to show
from some places. But there was corner-case that WillShowSidePanel()
is not called when panel is shown. It happened when tab is attached
to another window. And realized that PopulateSidePanel() is always
called whenever panel is shown in whatever situation.
Fixed by calling WillShowSidePanel() from PopulateSidePanel().
---
 browser/ui/sidebar/sidebar_browsertest.cc     | 15 +++++++++++++
 .../brave_side_panel_coordinator.cc           | 22 +++++--------------
 .../side_panel/brave_side_panel_coordinator.h |  2 +-
 3 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/browser/ui/sidebar/sidebar_browsertest.cc b/browser/ui/sidebar/sidebar_browsertest.cc
index df8c5741864c..57642f8a456d 100644
--- a/browser/ui/sidebar/sidebar_browsertest.cc
+++ b/browser/ui/sidebar/sidebar_browsertest.cc
@@ -1242,6 +1242,21 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTestWithAIChat,
   EXPECT_TRUE(model()->active_index().has_value());
   EXPECT_EQ(model()->active_index(),
             model()->GetIndexOf(SidebarItem::BuiltInItemType::kBookmarks));
+
+  // Check tab specific panel item is still activated after moving to another
+  // browser.
+  tab_model()->ActivateTabAt(1);
+  EXPECT_EQ(model()->active_index(), tab_specific_item_index);
+
+  auto* browser2 = CreateBrowser(browser()->profile());
+  auto* browser2_model =
+      static_cast<BraveBrowser*>(browser2)->sidebar_controller()->model();
+  auto* browser2_tab_model = browser2->tab_strip_model();
+
+  auto detached_tab = tab_model()->DetachTabAtForInsertion(1);
+  browser2_tab_model->AppendTab(std::move(detached_tab), /* foreground */ true);
+  EXPECT_EQ(browser2_model->active_index(),
+            browser2_model->GetIndexOf(SidebarItem::BuiltInItemType::kChatUI));
 }
 
 IN_PROC_BROWSER_TEST_F(SidebarBrowserTestWithAIChat,
diff --git a/browser/ui/views/side_panel/brave_side_panel_coordinator.cc b/browser/ui/views/side_panel/brave_side_panel_coordinator.cc
index 16295c36c53f..a4fc9cade194 100644
--- a/browser/ui/views/side_panel/brave_side_panel_coordinator.cc
+++ b/browser/ui/views/side_panel/brave_side_panel_coordinator.cc
@@ -42,11 +42,6 @@ void BraveSidePanelCoordinator::Show(
   sidebar::SetLastUsedSidePanel(browser_view_->GetProfile()->GetPrefs(),
                                 entry_key.id());
 
-  // Notify to give opportunity to observe another panel entries from
-  // global or active tab's contextual registry.
-  auto* brave_browser_view = static_cast<BraveBrowserView*>(browser_view_);
-  brave_browser_view->WillShowSidePanel();
-
   SidePanelCoordinator::Show(entry_key, open_trigger);
 }
 
@@ -91,13 +86,6 @@ void BraveSidePanelCoordinator::Toggle() {
 void BraveSidePanelCoordinator::Toggle(
     SidePanelEntryKey key,
     SidePanelUtil::SidePanelOpenTrigger open_trigger) {
-  // Notify to give opportunity to observe another panel entries from
-  // global or active tab's contextual registry.
-  if (!IsSidePanelShowing()) {
-    auto* brave_browser_view = static_cast<BraveBrowserView*>(browser_view_);
-    brave_browser_view->WillShowSidePanel();
-  }
-
   SidePanelCoordinator::Toggle(key, open_trigger);
 }
 
@@ -160,6 +148,11 @@ void BraveSidePanelCoordinator::PopulateSidePanel(
     return;
   }
 
+  // Notify to give opportunity to observe another panel entries from
+  // global or active tab's contextual registry.
+  auto* brave_browser_view = static_cast<BraveBrowserView*>(browser_view_);
+  brave_browser_view->WillShowSidePanel();
+
   SidePanelCoordinator::PopulateSidePanel(supress_animations, unique_key, entry,
                                           std::move(content_view));
 }
@@ -167,11 +160,6 @@ void BraveSidePanelCoordinator::PopulateSidePanel(
 void BraveSidePanelCoordinator::NotifyPinnedContainerOfActiveStateChange(
     SidePanelEntryKey key,
     bool is_active) {
-  // // Notify to give opportunity to observe another panel entries from
-  // // global or active tab's contextual registry.
-  // auto* brave_browser_view = static_cast<BraveBrowserView*>(browser_view_);
-  // brave_browser_view->WillShowSidePanel();
-
   if (!browser_view_->toolbar()->pinned_toolbar_actions_container()) {
     return;
   }
diff --git a/browser/ui/views/side_panel/brave_side_panel_coordinator.h b/browser/ui/views/side_panel/brave_side_panel_coordinator.h
index 98fc1134fd4f..a0f108b39d87 100644
--- a/browser/ui/views/side_panel/brave_side_panel_coordinator.h
+++ b/browser/ui/views/side_panel/brave_side_panel_coordinator.h
@@ -9,7 +9,7 @@
 #include <memory>
 #include <optional>
 
-#include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h"  // IWYU pragma: export
+#include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h"
 
 class BraveSidePanelCoordinator : public SidePanelCoordinator {
  public: