Removed using SidePanelCoordinator::OnEntryWillDeregister

In cr132, SidePanelCoordinator::OnEntryWillDeregister() is removed. We relied on it to deregister observed entry from SidebarContainerView. Instead, we'll use OnEntryWillHide() callback. It'll be called when side panel is closed. At that time, we could remove observing it safely, and will start to observe again if that entry is still live in tab's registry.
brave · Oct 17, 2024 · 2a10bb6 · 2a10bb6
1 parent 23033b7
commit 2a10bb6
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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<BraveBrowserView*>(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
@@ -35,9 +35,6 @@ class BraveSidePanelCoordinator : public SidePanelCoordinator {
       const UniqueKey& unique_key,
       SidePanelEntry* entry,
       std::optional<std::unique_ptr<views::View>> 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
@@ -191,13 +191,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() !=
@@ -809,6 +802,19 @@ void SidebarContainerView::OnEntryHidden(SidePanelEntry* entry) {
   }
 }
 
+void SidebarContainerView::OnEntryWillHide(SidePanelEntry* entry,
+                                           SidePanelEntryHideReason reason) {
+  // If |entry| is panel is closed, 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
@@ -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(