Merge pull request #26056 from brave/sidebar_deprecate_OnEntryWillDer…

…egister Removed using SidePanelCoordinator::OnEntryWillDeregister
brave · Oct 22, 2024 · 74374a2 · 74374a2
2 parents 7b7391e + ed498e3
commit 74374a2
Show file tree

Hide file tree

Showing 6 changed files with 18 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,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
@@ -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(