From 0013246553b565e218f813001d058cb8e88dd7f1 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Tue, 22 Oct 2024 10:39:53 +0900 Subject: [PATCH] Merge pull request #26056 from brave/sidebar_deprecate_OnEntryWillDeregister Removed using SidePanelCoordinator::OnEntryWillDeregister --- browser/ui/views/frame/brave_browser_view.cc | 4 ---- browser/ui/views/frame/brave_browser_view.h | 1 - .../brave_side_panel_coordinator.cc | 11 --------- .../side_panel/brave_side_panel_coordinator.h | 3 --- .../views/sidebar/sidebar_container_view.cc | 23 +++++++++++++------ .../ui/views/sidebar/sidebar_container_view.h | 3 ++- 6 files changed, 18 insertions(+), 27 deletions(-) diff --git a/browser/ui/views/frame/brave_browser_view.cc b/browser/ui/views/frame/brave_browser_view.cc index 2931a2dc1ce3..28cd17d985a5 100644 --- a/browser/ui/views/frame/brave_browser_view.cc +++ b/browser/ui/views/frame/brave_browser_view.cc @@ -783,10 +783,6 @@ void BraveBrowserView::WillShowSidePanel() { sidebar_container_view_->WillShowSidePanel(); } -void BraveBrowserView::WillDeregisterSidePanelEntry(SidePanelEntry* entry) { - sidebar_container_view_->WillDeregisterSidePanelEntry(entry); -} - 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 ed60dccbb57a..d85ffbc2f233 100644 --- a/browser/ui/views/frame/brave_browser_view.h +++ b/browser/ui/views/frame/brave_browser_view.h @@ -82,7 +82,6 @@ class BraveBrowserView : public BrowserView, WalletButton* GetWalletButton(); views::View* GetWalletButtonAnchorView(); void WillShowSidePanel(); - void WillDeregisterSidePanelEntry(SidePanelEntry* entry); // 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 38d0c31cfefb..16295c36c53f 100644 --- a/browser/ui/views/side_panel/brave_side_panel_coordinator.cc +++ b/browser/ui/views/side_panel/brave_side_panel_coordinator.cc @@ -179,14 +179,3 @@ void BraveSidePanelCoordinator::NotifyPinnedContainerOfActiveStateChange( SidePanelCoordinator::NotifyPinnedContainerOfActiveStateChange(key, is_active); } - -void BraveSidePanelCoordinator::OnEntryWillDeregister( - SidePanelRegistry* registry, - SidePanelEntry* entry) { - SidePanelCoordinator::OnEntryWillDeregister(registry, entry); - - // This could give the opportunity to stop observing from |entry| if - // this deregister happens while tab is still live. - auto* brave_browser_view = static_cast(browser_view_); - brave_browser_view->WillDeregisterSidePanelEntry(entry); -} 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 24347b804715..98fc1134fd4f 100644 --- a/browser/ui/views/side_panel/brave_side_panel_coordinator.h +++ b/browser/ui/views/side_panel/brave_side_panel_coordinator.h @@ -35,9 +35,6 @@ class BraveSidePanelCoordinator : public SidePanelCoordinator { const UniqueKey& unique_key, SidePanelEntry* entry, std::optional> content_view) override; - void OnEntryWillDeregister(SidePanelRegistry* registry, - SidePanelEntry* entry) 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 a40af89ef92f..d34983f93b91 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.cc +++ b/browser/ui/views/sidebar/sidebar_container_view.cc @@ -190,13 +190,6 @@ void SidebarContainerView::WillShowSidePanel() { } } -void SidebarContainerView::WillDeregisterSidePanelEntry(SidePanelEntry* entry) { - // If entry's life cycle is tied with tab, we stop observing from - // OnTabWillBeRemoved(). However, some entry could be deregistered while tab - // is live. In that case, we need to stop observing here explicitely. - StopObservingForEntry(entry); -} - bool SidebarContainerView::IsFullscreenForCurrentEntry() const { // For now, we only supports fullscreen from playlist. if (side_panel_coordinator_->GetCurrentEntryId() != @@ -804,6 +797,22 @@ void SidebarContainerView::OnEntryHidden(SidePanelEntry* entry) { } } +void SidebarContainerView::OnEntryWillHide(SidePanelEntry* entry, + SidePanelEntryHideReason reason) { + DVLOG(1) << "Panel will hide: " + << SidePanelEntryIdToString(entry->key().id()); + + // 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 shoudl not stop + // to know the timing that it's shown again. + if (reason == SidePanelEntryHideReason::kSidePanelClosed) { + StopObservingForEntry(entry); + } +} + void SidebarContainerView::OnTabWillBeRemoved(content::WebContents* contents, int index) { // At this time, we can stop observing as TabFeatures is available. diff --git a/browser/ui/views/sidebar/sidebar_container_view.h b/browser/ui/views/sidebar/sidebar_container_view.h index 7a418bbce8b9..bde960eafbb2 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.h +++ b/browser/ui/views/sidebar/sidebar_container_view.h @@ -79,7 +79,6 @@ class SidebarContainerView BraveSidePanel* side_panel() { return side_panel_; } void WillShowSidePanel(); - void WillDeregisterSidePanelEntry(SidePanelEntry* entry); bool IsFullscreenForCurrentEntry() const; void set_operation_from_active_tab_change(bool tab_change) { @@ -122,6 +121,8 @@ class SidebarContainerView // SidePanelEntryObserver: void OnEntryShown(SidePanelEntry* entry) override; void OnEntryHidden(SidePanelEntry* entry) override; + void OnEntryWillHide(SidePanelEntry* entry, + SidePanelEntryHideReason reason) override; // TabStripModelObserver: void OnTabStripModelChanged(