diff --git a/browser/ui/sidebar/sidebar_browsertest.cc b/browser/ui/sidebar/sidebar_browsertest.cc index 8d18f00eeed7..7ffb5ec61e05 100644 --- a/browser/ui/sidebar/sidebar_browsertest.cc +++ b/browser/ui/sidebar/sidebar_browsertest.cc @@ -286,6 +286,18 @@ class SidebarBrowserTest : public InProcessBrowserTest { return std::distance(items.cbegin(), iter); } + bool SidebarContainerObserving(SidePanelEntryId id) { + auto* sidebar_container_view = + static_cast(controller()->sidebar()); + for (const auto& entry : + sidebar_container_view->panel_entry_observations_.sources()) { + if (entry->key().id() == id) { + return true; + } + } + return false; + } + raw_ptr item_added_bubble_anchor_ = nullptr; std::unique_ptr run_loop_; base::WeakPtrFactory weak_factory_{this}; @@ -316,14 +328,14 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, BasicTest) { EXPECT_EQ(expected_count, model()->GetAllSidebarItems().size()); // Activate item that opens in panel. const size_t first_panel_item_index = GetFirstPanelItemIndex(); - controller()->ActivateItemAt(first_panel_item_index); + const auto& first_panel_item = + controller()->model()->GetAllSidebarItems()[first_panel_item_index]; + controller()->ActivatePanelItem(first_panel_item.built_in_item_type); + WaitUntil( + base::BindLambdaForTesting([&]() { return !!model()->active_index(); })); EXPECT_THAT(model()->active_index(), Optional(first_panel_item_index)); EXPECT_TRUE(controller()->IsActiveIndex(first_panel_item_index)); - // Try to activate item at index 1. - // Default item at index 1 opens in new tab. So, sidebar active index is not - // changed. Still active index is 2. - // Get first index of item that opens in panel. const size_t first_web_item_index = GetFirstWebItemIndex(); const auto item = model()->GetAllSidebarItems()[first_web_item_index]; @@ -333,10 +345,15 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, BasicTest) { EXPECT_THAT(model()->active_index(), Optional(active_item_index)); // Setting std::nullopt means deactivate current active tab. - controller()->ActivateItemAt(std::nullopt); + controller()->DeactivateCurrentPanel(); + WaitUntil( + base::BindLambdaForTesting([&]() { return !model()->active_index(); })); EXPECT_THAT(model()->active_index(), Eq(std::nullopt)); - controller()->ActivateItemAt(active_item_index); + controller()->ActivatePanelItem(first_panel_item.built_in_item_type); + WaitUntil( + base::BindLambdaForTesting([&]() { return !!model()->active_index(); })); + EXPECT_THAT(model()->active_index(), Optional(active_item_index)); auto* sidebar_service = SidebarServiceFactory::GetForProfile(browser()->profile()); @@ -902,6 +919,22 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, TabSpecificAndGlobalPanelsTest) { WaitUntil(base::BindLambdaForTesting([&]() { return panel_ui->GetCurrentEntryId() == SidePanelEntryId::kBookmarks; })); + + // Check per-url contextual panel close. + // If current tab load another url, customize panel should be hidden. + tab_model()->ActivateTabAt(0); + WaitUntil(base::BindLambdaForTesting([&]() { + return panel_ui->GetCurrentEntryId() == SidePanelEntryId::kCustomizeChrome; + })); + + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), GURL("brave://newtab/"))); + WaitUntil(base::BindLambdaForTesting([&]() { + return panel_ui->GetCurrentEntryId() == SidePanelEntryId::kBookmarks; + })); + // As previous customize panel per-url panel, it's closed by deregistering + // after loading another url. Then, sidebar container should stop observing + // its entry. + EXPECT_FALSE(SidebarContainerObserving(SidePanelEntryId::kCustomizeChrome)); } IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, DisabledItemsTest) { @@ -1121,7 +1154,24 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTestWithAIChat, TabSpecificPanel) { ASSERT_TRUE(global_item_index.has_value()); auto tab_specific_item_index = model()->GetIndexOf(kTabSpecificItemType); ASSERT_TRUE(tab_specific_item_index.has_value()); - // Open 2 more tabs + + ASSERT_TRUE(ui_test_utils::NavigateToURLWithDisposition( + browser(), GURL("brave://newtab/"), + WindowOpenDisposition::NEW_FOREGROUND_TAB, + ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP)); + ASSERT_EQ(tab_model()->GetTabCount(), 2); + + // Open contextual panel from Tab 0. + tab_model()->ActivateTabAt(0); + SimulateSidebarItemClickAt(tab_specific_item_index.value()); + EXPECT_EQ(model()->active_index(), tab_specific_item_index); + + // Delete Tab 0 and check model doesn't have active index. + tab_model()->DetachAndDeleteWebContentsAt(0); + EXPECT_FALSE(!!model()->active_index()); + ASSERT_EQ(tab_model()->GetTabCount(), 1); + + // Create two more tab for test below. ASSERT_TRUE(ui_test_utils::NavigateToURLWithDisposition( browser(), GURL("brave://newtab/"), WindowOpenDisposition::NEW_FOREGROUND_TAB, @@ -1131,6 +1181,7 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTestWithAIChat, TabSpecificPanel) { WindowOpenDisposition::NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP)); ASSERT_EQ(tab_model()->GetTabCount(), 3); + // Open a "global" panel from Tab 0 tab_model()->ActivateTabAt(0); // Tab changed flag should be cleared after ActivateTabAt() executed. @@ -1207,6 +1258,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(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/sidebar/sidebar_controller.cc b/browser/ui/sidebar/sidebar_controller.cc index feda646b53bc..e841d7365278 100644 --- a/browser/ui/sidebar/sidebar_controller.cc +++ b/browser/ui/sidebar/sidebar_controller.cc @@ -224,6 +224,18 @@ void SidebarController::AddItemWithCurrentTab() { SidebarItem::BuiltInItemType::kNone, false)); } +void SidebarController::UpdateActiveItemState( + std::optional active_panel_item) { + if (!active_panel_item) { + ActivateItemAt(std::nullopt); + return; + } + + if (auto index = sidebar_model_->GetIndexOf(*active_panel_item)) { + ActivateItemAt(*index); + } +} + void SidebarController::SetSidebar(Sidebar* sidebar) { DCHECK(!sidebar_); // |sidebar| can be null in unit test. diff --git a/browser/ui/sidebar/sidebar_controller.h b/browser/ui/sidebar/sidebar_controller.h index 64a6cc9a0329..77dcc13ac44f 100644 --- a/browser/ui/sidebar/sidebar_controller.h +++ b/browser/ui/sidebar/sidebar_controller.h @@ -52,6 +52,8 @@ class SidebarController : public SidebarService::Observer { std::optional index, WindowOpenDisposition disposition = WindowOpenDisposition::CURRENT_TAB); void AddItemWithCurrentTab(); + void UpdateActiveItemState(std::optional + active_panel_item = std::nullopt); // Ask panel item activation state change to SidePanelUI. void ActivatePanelItem(SidebarItem::BuiltInItemType panel_item); diff --git a/browser/ui/sidebar/sidebar_utils.cc b/browser/ui/sidebar/sidebar_utils.cc index 3efb59542fdd..58af87379838 100644 --- a/browser/ui/sidebar/sidebar_utils.cc +++ b/browser/ui/sidebar/sidebar_utils.cc @@ -149,6 +149,24 @@ SidePanelEntryId SidePanelIdFromSideBarItemType(BuiltInItemType type) { "not have a panel Id."; } +std::optional BuiltInItemTypeFromSidePanelId( + SidePanelEntryId id) { + switch (id) { + case SidePanelEntryId::kReadingList: + return BuiltInItemType::kReadingList; + case SidePanelEntryId::kBookmarks: + return BuiltInItemType::kBookmarks; + case SidePanelEntryId::kPlaylist: + return BuiltInItemType::kPlaylist; + case SidePanelEntryId::kChatUI: + return BuiltInItemType::kChatUI; + default: + break; + } + + return std::nullopt; +} + SidePanelEntryId SidePanelIdFromSideBarItem(const SidebarItem& item) { CHECK(item.open_in_panel) << static_cast(item.built_in_item_type); return SidePanelIdFromSideBarItemType(item.built_in_item_type); diff --git a/browser/ui/sidebar/sidebar_utils.h b/browser/ui/sidebar/sidebar_utils.h index cf25944c35ed..a58fce6aa498 100644 --- a/browser/ui/sidebar/sidebar_utils.h +++ b/browser/ui/sidebar/sidebar_utils.h @@ -29,6 +29,8 @@ GURL ConvertURLToBuiltInItemURL(const GURL& url); SidePanelEntryId SidePanelIdFromSideBarItemType( SidebarItem::BuiltInItemType type); SidePanelEntryId SidePanelIdFromSideBarItem(const SidebarItem& item); +std::optional BuiltInItemTypeFromSidePanelId( + SidePanelEntryId id); void SetLastUsedSidePanel(PrefService* prefs, std::optional id); std::optional GetLastUsedSidePanel(Browser* browser); diff --git a/browser/ui/views/frame/brave_browser_view.cc b/browser/ui/views/frame/brave_browser_view.cc index 67bdb1036959..e252199152d6 100644 --- a/browser/ui/views/frame/brave_browser_view.cc +++ b/browser/ui/views/frame/brave_browser_view.cc @@ -779,6 +779,10 @@ WalletButton* BraveBrowserView::GetWalletButton() { return static_cast(toolbar())->wallet_button(); } +void BraveBrowserView::WillShowSidePanel(bool show_on_deregistered) { + sidebar_container_view_->WillShowSidePanel(show_on_deregistered); +} + void BraveBrowserView::NotifyDialogPositionRequiresUpdate() { GetBrowserViewLayout()->NotifyDialogPositionRequiresUpdate(); } diff --git a/browser/ui/views/frame/brave_browser_view.h b/browser/ui/views/frame/brave_browser_view.h index 59f5e042df31..ec79310a5db5 100644 --- a/browser/ui/views/frame/brave_browser_view.h +++ b/browser/ui/views/frame/brave_browser_view.h @@ -57,6 +57,7 @@ class BraveBrowser; class BraveHelpBubbleHostView; class ContentsLayoutManager; class SidebarContainerView; +class SidePanelEntry; class SplitViewLocationBar; class SplitViewSeparator; class VerticalTabStripWidgetDelegateView; @@ -80,6 +81,7 @@ class BraveBrowserView : public BrowserView, void CloseWalletBubble(); WalletButton* GetWalletButton(); views::View* GetWalletButtonAnchorView(); + void WillShowSidePanel(bool show_on_deregistered); // Triggers layout of web modal dialogs void NotifyDialogPositionRequiresUpdate(); 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 dc3c5e23852f..4750019371db 100644 --- a/browser/ui/views/side_panel/brave_side_panel_coordinator.cc +++ b/browser/ui/views/side_panel/brave_side_panel_coordinator.cc @@ -148,6 +148,26 @@ 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(browser_view_); + CHECK(browser_view_->unified_side_panel()->children().size() == 1); + const auto content_wrapper = + browser_view_->unified_side_panel()->children()[0]; + bool show_on_deregistered = false; + // If current entry is not null and its content is already removed from + // wrapper when new panel is about to be shown, new panel is shown by + // deregistering current panel. See the comment of + // SidebarContainerView::WillShowSidePanel() about why |show_on_deregistered| + // is needed. + // Instead, we can override base class' Show(const UniqueKey& entry, ...) + // method as we can know whether it's shown by deregistering with + // |open_trigger| arg. However, need patching to make it virtual as base class + // already have overridden same name methods. + if (current_entry_.get() && content_wrapper->children().empty()) { + show_on_deregistered = true; + } + brave_browser_view->WillShowSidePanel(show_on_deregistered); SidePanelCoordinator::PopulateSidePanel(supress_animations, unique_key, entry, std::move(content_view)); } 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 642903b805b8..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 #include -#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: @@ -35,7 +35,6 @@ class BraveSidePanelCoordinator : public SidePanelCoordinator { const UniqueKey& unique_key, SidePanelEntry* entry, std::optional> content_view) override; - void NotifyPinnedContainerOfActiveStateChange(SidePanelEntryKey key, bool is_active) override; diff --git a/browser/ui/views/sidebar/sidebar_container_view.cc b/browser/ui/views/sidebar/sidebar_container_view.cc index 47e050963485..d38f54f2887f 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.cc +++ b/browser/ui/views/sidebar/sidebar_container_view.cc @@ -38,6 +38,7 @@ #include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h" #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h" #include "chrome/browser/ui/exclusive_access/fullscreen_within_tab_helper.h" +#include "chrome/browser/ui/tabs/public/tab_features.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/side_panel/side_panel_entry.h" @@ -127,6 +128,7 @@ SidebarContainerView::SidebarContainerView( SetNotifyEnterExitOnChild(true); side_panel_ = AddChildView(std::move(side_panel)); + side_panel_view_state_observation_.Observe(side_panel_coordinator_); } SidebarContainerView::~SidebarContainerView() = default; @@ -138,18 +140,6 @@ void SidebarContainerView::Init() { sidebar_model_observation_.Observe(sidebar_model_); browser_->tab_strip_model()->AddObserver(this); - auto* browser_view = BrowserView::GetBrowserViewForBrowser(browser_); - DCHECK(browser_view); - - auto* global_registry = side_panel_coordinator_->GetWindowRegistry(); - panel_registry_observations_.AddObservation(global_registry); - - for (const auto& entry : global_registry->entries()) { - DVLOG(1) << "Observing panel entry in ctor: " - << SidePanelEntryIdToString(entry->key().id()); - panel_entry_observations_.AddObservation(entry.get()); - } - show_side_panel_button_.Init( kShowSidePanelButton, browser_->profile()->GetPrefs(), base::BindRepeating(&SidebarContainerView::UpdateToolbarButtonVisibility, @@ -184,6 +174,24 @@ bool SidebarContainerView::IsSidebarVisible() const { return sidebar_control_view_ && sidebar_control_view_->GetVisible(); } +void SidebarContainerView::WillShowSidePanel(bool show_on_deregistered) { + deregister_when_hidden_ = show_on_deregistered; + + // It's good timing to start observing any panel entries + // from global and contextual if not yet observed. + auto* tab_model = browser_->tab_strip_model(); + auto* active_web_contents = tab_model->GetActiveWebContents(); + if (!active_web_contents) { + return; + } + StartObservingContextualSidePanelEntry(active_web_contents); + + auto* global_registry = side_panel_coordinator_->GetWindowRegistry(); + for (const auto& entry : global_registry->entries()) { + StartObservingForEntry(entry.get()); + } +} + bool SidebarContainerView::IsFullscreenForCurrentEntry() const { // For now, we only supports fullscreen from playlist. if (side_panel_coordinator_->GetCurrentEntryId() != @@ -758,8 +766,13 @@ void SidebarContainerView::OnEntryShown(SidePanelEntry* entry) { } void SidebarContainerView::OnEntryHidden(SidePanelEntry* entry) { - // Make sure item is deselected DVLOG(1) << "Panel hidden: " << SidePanelEntryIdToString(entry->key().id()); + + if (deregister_when_hidden_) { + StopObservingForEntry(entry); + deregister_when_hidden_ = false; + } + auto* controller = browser_->sidebar_controller(); // Handling if |entry| is managed one. @@ -791,26 +804,46 @@ void SidebarContainerView::OnEntryHidden(SidePanelEntry* entry) { } } -void SidebarContainerView::OnEntryRegistered(SidePanelRegistry* registry, - SidePanelEntry* entry) { - // Observe when it's shown or hidden - DVLOG(1) << "Observing panel entry in registry observer: " +void SidebarContainerView::OnEntryWillHide(SidePanelEntry* entry, + SidePanelEntryHideReason reason) { + DVLOG(1) << "Panel will hide: " << SidePanelEntryIdToString(entry->key().id()); - panel_entry_observations_.AddObservation(entry); + + // If |reason| is panel closing, we could deregister. And it'll be + // re-registered when panel is shown if that entry is still live in tab's + // registry. + // We only stop observing when |entry|'s panel is hidden by closing. + // If it's hidden by replacing with other panel, we should not stop + // to know the timing that it's shown again. + if (reason == SidePanelEntryHideReason::kSidePanelClosed) { + deregister_when_hidden_ = true; + } } -void SidebarContainerView::OnEntryWillDeregister(SidePanelRegistry* registry, - SidePanelEntry* entry) { - // Stop observing - DVLOG(1) << "Unobserving panel entry in registry observer: " - << SidePanelEntryIdToString(entry->key().id()); - panel_entry_observations_.RemoveObservation(entry); +void SidebarContainerView::OnTabWillBeRemoved(content::WebContents* contents, + int index) { + // At this time, we can stop observing as TabFeatures is available. + StopObservingContextualSidePanelEntry(contents); } -void SidebarContainerView::OnRegistryDestroying(SidePanelRegistry* registry) { - if (panel_registry_observations_.IsObservingSource(registry)) { - StopObservingContextualSidePanelRegistry(registry); +void SidebarContainerView::UpdateActiveItemState() { + DVLOG(1) << "Update active item state"; + + auto* controller = browser_->sidebar_controller(); + std::optional current_type; + if (auto entry_id = side_panel_coordinator_->GetCurrentEntryId()) { + current_type = sidebar::BuiltInItemTypeFromSidePanelId(*entry_id); } + 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( @@ -845,70 +878,39 @@ void SidebarContainerView::OnTabStripModelChanged( } return; } - - if (change.type() == TabStripModelChange::kInserted) { - for (const auto& contents : change.GetInsert()->contents) { - StartObservingContextualSidePanelRegistry(contents.contents); - } - return; - } - - if (change.type() == TabStripModelChange::kRemoved) { - bool removed_for_deletion = - (change.GetRemove()->contents[0].remove_reason == - TabStripModelChange::RemoveReason::kDeleted); - // If the tab is removed for deletion the side panel registry has already - // been destroyed. We stop observing that registry in - // SidePanelRegistryObserver::OnRegistryDestroying override above. - if (!removed_for_deletion) { - for (const auto& contents : change.GetRemove()->contents) { - StopObservingContextualSidePanelRegistry(contents.contents); - } - return; - } - } } -void SidebarContainerView::StopObservingContextualSidePanelRegistry( +void SidebarContainerView::StopObservingContextualSidePanelEntry( content::WebContents* contents) { - auto* registry = SidePanelRegistry::GetDeprecated(contents); - StopObservingContextualSidePanelRegistry(registry); -} + auto* tab = tabs::TabInterface::GetFromContents(contents); + if (!tab->GetTabFeatures()) { + return; + } -void SidebarContainerView::StopObservingContextualSidePanelRegistry( - SidePanelRegistry* registry) { + auto* registry = tab->GetTabFeatures()->side_panel_registry(); if (!registry) { return; } - panel_registry_observations_.RemoveObservation(registry); - for (const auto& entry : registry->entries()) { - if (panel_entry_observations_.IsObservingSource(entry.get())) { - DVLOG(1) << "Removing panel entry observation from removed contextual " - "registry : " - << SidePanelEntryIdToString(entry->key().id()); - panel_entry_observations_.RemoveObservation(entry.get()); - } + StopObservingForEntry(entry.get()); } } -void SidebarContainerView::StartObservingContextualSidePanelRegistry( +void SidebarContainerView::StartObservingContextualSidePanelEntry( content::WebContents* contents) { - auto* registry = SidePanelRegistry::GetDeprecated(contents); - if (!registry) { + auto* tab = tabs::TabInterface::GetFromContents(contents); + if (!tab->GetTabFeatures()) { return; } - panel_registry_observations_.AddObservation(registry); + auto* registry = tab->GetTabFeatures()->side_panel_registry(); + if (!registry) { + return; + } for (const auto& entry : registry->entries()) { - if (!panel_entry_observations_.IsObservingSource(entry.get())) { - DVLOG(1) << "Observing existing panel entry from newly added contextual " - "registry : " - << SidePanelEntryIdToString(entry->key().id()); - panel_entry_observations_.AddObservation(entry.get()); - } + StartObservingForEntry(entry.get()); } SharedPinnedTabService* shared_pinned_tab_service = @@ -930,5 +932,21 @@ void SidebarContainerView::StartObservingContextualSidePanelRegistry( } } +void SidebarContainerView::StartObservingForEntry(SidePanelEntry* entry) { + if (!panel_entry_observations_.IsObservingSource(entry)) { + DVLOG(1) << "Observing panel entry: " + << SidePanelEntryIdToString(entry->key().id()); + panel_entry_observations_.AddObservation(entry); + } +} + +void SidebarContainerView::StopObservingForEntry(SidePanelEntry* entry) { + if (panel_entry_observations_.IsObservingSource(entry)) { + DVLOG(1) << "Removing panel entry observation: " + << SidePanelEntryIdToString(entry->key().id()); + panel_entry_observations_.RemoveObservation(entry); + } +} + BEGIN_METADATA(SidebarContainerView) END_METADATA diff --git a/browser/ui/views/sidebar/sidebar_container_view.h b/browser/ui/views/sidebar/sidebar_container_view.h index aa090fafccd9..21ef93847b29 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.h +++ b/browser/ui/views/sidebar/sidebar_container_view.h @@ -22,7 +22,7 @@ #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_registry_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" @@ -57,7 +57,7 @@ class SidebarContainerView public SidebarShowOptionsEventDetectWidget::Delegate, public sidebar::SidebarModel::Observer, public SidePanelEntryObserver, - public SidePanelRegistryObserver, + public SidePanelViewStateObserver, public TabStripModelObserver { METADATA_HEADER(SidebarContainerView, views::View) public: @@ -78,6 +78,16 @@ class SidebarContainerView BraveSidePanel* side_panel() { return side_panel_; } + // Need to know this showing comes from deregistering current entry + // or not. We're observing contextual/global panel entries when panel is + // shown. And stop observing when it's hidden by closing. Unfortunately, + // OnEntryWillHide() is not called when current entry is closed by + // deregistering. Only OnEntryHidden() is called. + // By using OnEntryWillHide()'s arg, we can know this hidden is from + // closing or not. Fortunately, we can know when WillShowSidePanel() + // is called whether it's closing from deregistration. + // With |show_on_deregistered|, we can stop observing OnEntryHidden(). + void WillShowSidePanel(bool show_on_deregistered); bool IsFullscreenForCurrentEntry() const; void set_operation_from_active_tab_change(bool tab_change) { @@ -120,19 +130,18 @@ class SidebarContainerView // SidePanelEntryObserver: void OnEntryShown(SidePanelEntry* entry) override; void OnEntryHidden(SidePanelEntry* entry) override; - - // SidePanelRegistryObserver: - void OnEntryRegistered(SidePanelRegistry* registry, - SidePanelEntry* entry) override; - void OnEntryWillDeregister(SidePanelRegistry* registry, - SidePanelEntry* entry) override; - void OnRegistryDestroying(SidePanelRegistry* registry) override; + void OnEntryWillHide(SidePanelEntry* entry, + SidePanelEntryHideReason reason) override; // TabStripModelObserver: void OnTabStripModelChanged( TabStripModel* tab_strip_model, const TabStripModelChange& change, const TabStripSelectionChange& selection) override; + void OnTabWillBeRemoved(content::WebContents* contents, int index) override; + + // SidePanelViewStateObserver: + void OnSidePanelDidClose() override; private: friend class sidebar::SidebarBrowserTest; @@ -182,10 +191,11 @@ class SidebarContainerView void StartBrowserWindowEventMonitoring(); void StopBrowserWindowEventMonitoring(); - void StartObservingContextualSidePanelRegistry( - content::WebContents* contents); - void StopObservingContextualSidePanelRegistry(content::WebContents* contents); - void StopObservingContextualSidePanelRegistry(SidePanelRegistry* registry); + void StartObservingContextualSidePanelEntry(content::WebContents* contents); + void StopObservingContextualSidePanelEntry(content::WebContents* contents); + void StartObservingForEntry(SidePanelEntry* entry); + void StopObservingForEntry(SidePanelEntry* entry); + void UpdateActiveItemState(); raw_ptr browser_ = nullptr; raw_ptr side_panel_coordinator_ = nullptr; @@ -195,6 +205,7 @@ class SidebarContainerView bool initialized_ = false; bool sidebar_on_left_ = true; bool operation_from_active_tab_change_ = false; + bool deregister_when_hidden_ = false; base::OneShotTimer sidebar_hide_timer_; sidebar::SidebarService::ShowSidebarOption show_sidebar_option_ = sidebar::SidebarService::ShowSidebarOption::kShowAlways; @@ -205,14 +216,13 @@ class SidebarContainerView std::unique_ptr browser_window_event_monitor_; std::unique_ptr show_options_widget_; BooleanPrefMember show_side_panel_button_; + base::ScopedObservation + side_panel_view_state_observation_{this}; base::ScopedObservation sidebar_model_observation_{this}; base::ScopedMultiSourceObservation panel_entry_observations_{this}; - base::ScopedMultiSourceObservation - panel_registry_observations_{this}; }; #endif // BRAVE_BROWSER_UI_VIEWS_SIDEBAR_SIDEBAR_CONTAINER_VIEW_H_ 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 index 4c102901ca2d..09cb0bdb9730 100644 --- 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 @@ -18,7 +18,6 @@ #include "chrome/browser/ui/tabs/tab_strip_model_observer.h" #include "chrome/browser/ui/views/side_panel/side_panel_entry.h" #include "chrome/browser/ui/views/side_panel/side_panel_registry.h" -#include "chrome/browser/ui/views/side_panel/side_panel_registry_observer.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"