Side Panel: Use SidePanelUI::GetSidePanelUI across the board

This is part of the refactor to move the side panel API from c/b/ui/views to c/b/ui so that c/b/ui code can invoke it without violating dependency rules. (cherry picked from commit 78633f8) Doc: https://docs.google.com/document/d/1wsGLTuNbjkX9ZLbDxAI6iBPWXrgIURSGFqdotQkIROk/edit?usp=sharing&resourcekey=0-fksLkDg3e3Ste-r6SL-aOQ Bug: 1341399 Change-Id: I231e971a14f87910168e20617849038cacd1d886 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4549717 Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com> Reviewed-by: Thomas Lukaszewicz <tluk@chromium.org> Reviewed-by: Caroline Rising <corising@chromium.org> Commit-Queue: Yuheng Huang <yuhengh@chromium.org> Reviewed-by: Peter Boström <pbos@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1149188} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4582103 Owners-Override: Prudhvikumar Bommana <pbommana@google.com> Commit-Queue: Shakti Sahu <shaktisahu@chromium.org> Reviewed-by: Prudhvikumar Bommana <pbommana@google.com> Cr-Commit-Position: refs/branch-heads/5790@{#291} Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
chromium · Jun 3, 2023 · b9e66d7 · b9e66d7
1 parent 0ea2a8f
commit b9e66d7
Show file tree

Hide file tree

Showing 46 changed files with 224 additions and 233 deletions.
diff --git a/chrome/browser/ui/browser_command_controller.cc b/chrome/browser/ui/browser_command_controller.cc
@@ -43,6 +43,7 @@
 #include "chrome/browser/ui/passwords/ui_utils.h"
 #include "chrome/browser/ui/side_panel/side_panel_entry_id.h"
 #include "chrome/browser/ui/side_panel/side_panel_enums.h"
+#include "chrome/browser/ui/side_panel/side_panel_ui.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/tabs/tab_strip_user_gesture_details.h"
@@ -785,8 +786,8 @@ bool BrowserCommandController::ExecuteCommandWithDisposition(
       OpenFeedbackDialog(browser_, kFeedbackSourceBrowserCommand);
       break;
     case IDC_SHOW_SEARCH_COMPANION:
-      browser_->window()->ShowSidePanel(SidePanelEntryId::kSearchCompanion,
-                                        SidePanelOpenTrigger::kAppMenu);
+      SidePanelUI::GetSidePanelUIForBrowser(browser_)->Show(
+          SidePanelEntryId::kSearchCompanion, SidePanelOpenTrigger::kAppMenu);
       break;
 #endif
     case IDC_SHOW_CHROME_LABS:
@@ -808,8 +809,8 @@ bool BrowserCommandController::ExecuteCommandWithDisposition(
       ShowBookmarkManager(browser_->GetBrowserForOpeningWebUi());
       break;
     case IDC_SHOW_BOOKMARK_SIDE_PANEL:
-      browser_->window()->ShowSidePanel(SidePanelEntryId::kBookmarks,
-                                        SidePanelOpenTrigger::kAppMenu);
+      SidePanelUI::GetSidePanelUIForBrowser(browser_)->Show(
+          SidePanelEntryId::kBookmarks, SidePanelOpenTrigger::kAppMenu);
       break;
     case IDC_SHOW_APP_MENU:
       base::RecordAction(base::UserMetricsAction("Accel_Show_App_Menu"));
@@ -1018,8 +1019,8 @@ bool BrowserCommandController::ExecuteCommandWithDisposition(
       break;
 
     case IDC_READING_LIST_MENU_SHOW_UI:
-      browser_->window()->ShowSidePanel(SidePanelEntryId::kReadingList,
-                                        SidePanelOpenTrigger::kAppMenu);
+      SidePanelUI::GetSidePanelUIForBrowser(browser_)->Show(
+          SidePanelEntryId::kReadingList, SidePanelOpenTrigger::kAppMenu);
       break;
 
     default:

diff --git a/chrome/browser/ui/browser_window.h b/chrome/browser/ui/browser_window.h
@@ -640,12 +640,6 @@ class BrowserWindow : public ui::BaseWindow {
   // of a full titlebar. This is only supported for desktop web apps.
   virtual bool IsBorderlessModeEnabled() const = 0;
 
-  // Shows the side panel. If `entry_id` is not provided, shows the last active
-  // entry.
-  virtual void ShowSidePanel(
-      absl::optional<SidePanelEntryId> entry_id = absl::nullopt,
-      absl::optional<SidePanelOpenTrigger> open_trigger = absl::nullopt) = 0;
-
   // Shows the Chrome Labs bubble if enabled.
   virtual void ShowChromeLabs() = 0;
 

diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
@@ -55,6 +55,7 @@
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/color/chrome_color_id.h"
 #include "chrome/browser/ui/layout_constants.h"
+#include "chrome/browser/ui/side_panel/side_panel_ui.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/view_ids.h"
@@ -68,7 +69,6 @@
 #include "chrome/browser/ui/views/event_utils.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/top_container_background.h"
-#include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_util.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
@@ -1704,19 +1704,19 @@ std::unique_ptr<MenuButton> BookmarkBarView::CreateOtherBookmarksButton() {
   if (base::FeatureList::IsEnabled(features::kPowerBookmarksSidePanel)) {
     // Title is set in Loaded.
     button = std::make_unique<BookmarkFolderButton>(base::BindRepeating(
-        [](BookmarkBarView* bar, const ui::Event& event) {
-          SidePanelCoordinator* side_panel_coordinator =
-              bar->browser_view_->side_panel_coordinator();
-          if (side_panel_coordinator->GetCurrentEntryId() ==
+        [](Browser* browser, const ui::Event& event) {
+          SidePanelUI* side_panel_ui =
+              SidePanelUI::GetSidePanelUIForBrowser(browser);
+          if (side_panel_ui->GetCurrentEntryId() ==
               SidePanelEntry::Id::kBookmarks) {
-            side_panel_coordinator->Close();
+            side_panel_ui->Close();
           } else {
-            side_panel_coordinator->Show(
+            side_panel_ui->Show(
                 SidePanelEntry::Id::kBookmarks,
                 SidePanelUtil::SidePanelOpenTrigger::kBookmarkBar);
           }
         },
-        base::Unretained(this)));
+        browser_));
   } else {
     // Title is set in Loaded.
     button = std::make_unique<BookmarkFolderButton>(base::BindRepeating(

diff --git a/chrome/browser/ui/views/commerce/price_tracking_icon_view.cc b/chrome/browser/ui/views/commerce/price_tracking_icon_view.cc
@@ -236,10 +236,9 @@ void PriceTrackingIconView::EnablePriceTracking(bool enable) {
     bool should_show_iph = browser_->window()->MaybeShowFeaturePromo(
         feature_engagement::kIPHPriceTrackingInSidePanelFeature);
     if (should_show_iph) {
-      SidePanelCoordinator* coordinator =
-          BrowserView::GetBrowserViewForBrowser(browser_)
-              ->side_panel_coordinator();
-      if (coordinator) {
+      SidePanelUI* side_panel_ui =
+          SidePanelUI::GetSidePanelUIForBrowser(browser_);
+      if (side_panel_ui) {
         SidePanelRegistry* registry =
             SidePanelCoordinator::GetGlobalSidePanelRegistry(browser_);
         registry->SetActiveEntry(registry->GetEntryForKey(

diff --git a/chrome/browser/ui/views/commerce/price_tracking_icon_view_interactive_uitest.cc b/chrome/browser/ui/views/commerce/price_tracking_icon_view_interactive_uitest.cc
@@ -816,8 +816,7 @@ class PriceTrackingIconViewUnifiedSidePanelInteractiveTest
 IN_PROC_BROWSER_TEST_F(PriceTrackingIconViewUnifiedSidePanelInteractiveTest,
                        TriggerSidePanelIPH) {
   SidePanelCoordinator* coordinator =
-      BrowserView::GetBrowserViewForBrowser(browser())
-          ->side_panel_coordinator();
+      SidePanelUtil::GetSidePanelCoordinatorForBrowser(browser());
   DCHECK(coordinator);
   PrefService* prefs = browser()->profile()->GetPrefs();
   prefs->SetBoolean(prefs::kShouldShowPriceTrackFUEBubble, false);
@@ -857,8 +856,7 @@ IN_PROC_BROWSER_TEST_F(PriceTrackingIconViewUnifiedSidePanelInteractiveTest,
 IN_PROC_BROWSER_TEST_F(PriceTrackingIconViewUnifiedSidePanelInteractiveTest,
                        NotTriggerSidePanelIPH) {
   SidePanelCoordinator* coordinator =
-      BrowserView::GetBrowserViewForBrowser(browser())
-          ->side_panel_coordinator();
+      SidePanelUtil::GetSidePanelCoordinatorForBrowser(browser());
   DCHECK(coordinator);
   PrefService* prefs = browser()->profile()->GetPrefs();
   prefs->SetBoolean(prefs::kShouldShowPriceTrackFUEBubble, false);

diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
@@ -989,11 +989,6 @@ BrowserView::~BrowserView() {
   SidePanelUI::RemoveSidePanelUIForBrowser(browser_.get());
 }
 
-SidePanelCoordinator* BrowserView::side_panel_coordinator() {
-  return static_cast<SidePanelCoordinator*>(
-      SidePanelUI::GetSidePanelUIForBrowser(browser_.get()));
-}
-
 // static
 BrowserWindow* BrowserWindow::FindBrowserWindowWithWebContents(
     content::WebContents* web_contents) {
@@ -2213,12 +2208,6 @@ bool BrowserView::IsBorderlessModeEnabled() const {
   return borderless_mode_enabled_ && window_management_permission_granted_;
 }
 
-void BrowserView::ShowSidePanel(
-    absl::optional<SidePanelEntry::Id> entry_id,
-    absl::optional<SidePanelUtil::SidePanelOpenTrigger> open_trigger) {
-  side_panel_coordinator()->Show(entry_id, open_trigger);
-}
-
 void BrowserView::ShowChromeLabs() {
   if (toolbar()->chrome_labs_button() &&
       toolbar()->chrome_labs_button()->GetVisible()) {
@@ -3853,7 +3842,8 @@ void BrowserView::AddedToWidget() {
       toolbar()->side_panel_container()->ObserveSidePanelView(
           unified_side_panel_);
     } else {
-      unified_side_panel_->AddObserver(side_panel_coordinator());
+      unified_side_panel_->AddObserver(
+          SidePanelUtil::GetSidePanelCoordinatorForBrowser((browser_.get())));
     }
   }
 

diff --git a/chrome/browser/ui/views/frame/browser_view.h b/chrome/browser/ui/views/frame/browser_view.h
@@ -77,7 +77,6 @@ class FullscreenControlHost;
 class InfoBarContainerView;
 class LocationBarView;
 class SidePanel;
-class SidePanelCoordinator;
 class StatusBubbleViews;
 class TabSearchBubbleHost;
 class TabStrip;
@@ -215,8 +214,6 @@ class BrowserView : public BrowserWindow,
 
   SidePanel* unified_side_panel() { return unified_side_panel_; }
 
-  SidePanelCoordinator* side_panel_coordinator();
-
   void set_contents_border_widget(views::Widget* contents_border_widget) {
     GetBrowserViewLayout()->set_contents_border_widget(contents_border_widget);
   }
@@ -518,9 +515,6 @@ class BrowserView : public BrowserWindow,
   bool IsToolbarShowing() const override;
   bool IsLocationBarVisible() const override;
   bool IsBorderlessModeEnabled() const override;
-  void ShowSidePanel(
-      absl::optional<SidePanelEntryId> entry_id,
-      absl::optional<SidePanelOpenTrigger> open_trigger) override;
   void ShowChromeLabs() override;
 
   SharingDialog* ShowSharingDialog(content::WebContents* contents,

diff --git a/chrome/browser/ui/views/lens/lens_side_panel_helper.cc b/chrome/browser/ui/views/lens/lens_side_panel_helper.cc
@@ -75,8 +75,8 @@ views::Widget* OpenLensRegionSearchInstructions(
 }
 
 void CreateLensUnifiedSidePanelEntryForTesting(Browser* browser) {
-  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
-  SidePanelCoordinator* coordinator = browser_view->side_panel_coordinator();
+  SidePanelCoordinator* coordinator =
+      SidePanelUtil::GetSidePanelCoordinatorForBrowser(browser);
   DCHECK(coordinator);
   coordinator->SetNoDelaysForTesting(true);  // IN-TEST
 

diff --git a/chrome/browser/ui/views/page_info/about_this_site_side_panel_coordinator.cc b/chrome/browser/ui/views/page_info/about_this_site_side_panel_coordinator.cc
@@ -8,11 +8,11 @@
 #include "chrome/browser/page_info/page_info_features.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/page_info/about_this_site_side_panel.h"
+#include "chrome/browser/ui/side_panel/side_panel_ui.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/page_info/about_this_site_side_panel_view.h"
 #include "chrome/browser/ui/views/page_info/page_info_view_factory.h"
-#include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_entry.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_registry.h"
 #include "components/page_info/core/about_this_site_service.h"
@@ -63,9 +63,10 @@ AboutThisSideSidePanelCoordinator::~AboutThisSideSidePanelCoordinator() =
 
 void AboutThisSideSidePanelCoordinator::RegisterEntry(
     const GURL& more_about_url) {
-  auto* browser_view = GetBrowserView();
-  if (!browser_view)
+  SidePanelUI* side_panel_ui = GetSidePanelUI();
+  if (!side_panel_ui) {
     return;
+  }
 
   auto* registry = SidePanelRegistry::Get(web_contents());
   last_url_info_ = {web_contents()->GetLastCommittedURL(), more_about_url,
@@ -95,9 +96,10 @@ void AboutThisSideSidePanelCoordinator::RegisterEntry(
 
 void AboutThisSideSidePanelCoordinator::RegisterEntryAndShow(
     const GURL& more_about_url) {
-  auto* browser_view = GetBrowserView();
-  if (!browser_view)
+  SidePanelUI* side_panel_ui = GetSidePanelUI();
+  if (!side_panel_ui) {
     return;
+  }
 
   RegisterEntry(more_about_url);
   registered_but_not_shown_ = false;
@@ -107,10 +109,9 @@ void AboutThisSideSidePanelCoordinator::RegisterEntryAndShow(
     about_this_site_side_panel_view_->OpenUrl(last_url_info_->url_params);
   }
 
-  auto* side_panel_coordinator = browser_view->side_panel_coordinator();
-  if (side_panel_coordinator->GetCurrentEntryId() !=
+  if (side_panel_ui->GetCurrentEntryId() !=
       SidePanelEntry::Id::kAboutThisSite) {
-    side_panel_coordinator->Show(SidePanelEntry::Id::kAboutThisSite);
+    side_panel_ui->Show(SidePanelEntry::Id::kAboutThisSite);
   }
 }
 
@@ -133,17 +134,17 @@ void AboutThisSideSidePanelCoordinator::DidFinishNavigation(
     return;
   }
 
-  auto* browser_view = GetBrowserView();
-  if (!browser_view)
+  SidePanelUI* side_panel_ui = GetSidePanelUI();
+  if (!side_panel_ui) {
     return;
+  }
 
   // If the side panel is open and shows the AboutThisSide panel, close it.
-  auto* side_panel_coordinator = browser_view->side_panel_coordinator();
   if (!page_info::IsKeepSidePanelOnSameTabNavsFeatureEnabled() &&
       about_this_site_side_panel_view_ &&
-      side_panel_coordinator->GetCurrentEntryId() ==
+      side_panel_ui->GetCurrentEntryId() ==
           SidePanelEntry::Id::kAboutThisSite) {
-    side_panel_coordinator->Close();
+    side_panel_ui->Close();
   }
 
   auto* registry = SidePanelRegistry::Get(web_contents());
@@ -162,7 +163,7 @@ void AboutThisSideSidePanelCoordinator::DidFinishNavigation(
   // correct Diner URL.
   if (page_info::IsKeepSidePanelOnSameTabNavsFeatureEnabled() &&
       about_this_site_side_panel_view_ &&
-      side_panel_coordinator->GetCurrentEntryId() ==
+      side_panel_ui->GetCurrentEntryId() ==
           SidePanelEntry::Id::kAboutThisSite) {
     page_info::AboutThisSiteService::OnSameTabNavigation();
     RegisterEntryAndShow(
@@ -172,7 +173,7 @@ void AboutThisSideSidePanelCoordinator::DidFinishNavigation(
 
   // If the about this site side panel is no longer being shown and the view is
   // cached, then we will remove the cached view since it shows the wrong page.
-  if (side_panel_coordinator->GetCurrentEntryId() !=
+  if (side_panel_ui->GetCurrentEntryId() !=
           SidePanelEntry::Id::kAboutThisSite &&
       about_this_site_side_panel_view_) {
     auto* entry = registry->GetEntryForKey(
@@ -203,6 +204,11 @@ BrowserView* AboutThisSideSidePanelCoordinator::GetBrowserView() const {
   return browser ? BrowserView::GetBrowserViewForBrowser(browser) : nullptr;
 }
 
+SidePanelUI* AboutThisSideSidePanelCoordinator::GetSidePanelUI() {
+  auto* browser = chrome::FindBrowserWithWebContents(web_contents());
+  return browser ? SidePanelUI::GetSidePanelUIForBrowser(browser) : nullptr;
+}
+
 GURL AboutThisSideSidePanelCoordinator::GetOpenInNewTabUrl() {
   DCHECK(last_url_info_.has_value());
   DCHECK(!base::Contains(last_url_info_.value().new_tab_url.query_piece(),

diff --git a/chrome/browser/ui/views/page_info/about_this_site_side_panel_coordinator.h b/chrome/browser/ui/views/page_info/about_this_site_side_panel_coordinator.h
@@ -14,6 +14,7 @@
 
 class BrowserView;
 class AboutThisSiteSidePanelView;
+class SidePanelUI;
 
 namespace views {
 class View;
@@ -45,6 +46,8 @@ class AboutThisSideSidePanelCoordinator
 
   BrowserView* GetBrowserView() const;
 
+  SidePanelUI* GetSidePanelUI();
+
   // Called when SidePanel is opened.
   std::unique_ptr<views::View> CreateAboutThisSiteWebView();
 

diff --git a/chrome/browser/ui/views/page_info/about_this_site_side_panel_coordinator_browsertest.cc b/chrome/browser/ui/views/page_info/about_this_site_side_panel_coordinator_browsertest.cc
@@ -70,8 +70,7 @@ class AboutThisSiteSidePanelCoordinatorBrowserTest
   }
 
   SidePanelCoordinator* side_panel_coordinator() {
-    return BrowserView::GetBrowserViewForBrowser(browser())
-        ->side_panel_coordinator();
+    return SidePanelUtil::GetSidePanelCoordinatorForBrowser(browser());
   }
 
   base::test::ScopedFeatureList feature_list_;

diff --git a/...me/browser/ui/views/side_panel/customize_chrome/customize_chrome_side_panel_controller.cc b/...me/browser/ui/views/side_panel/customize_chrome/customize_chrome_side_panel_controller.cc
@@ -11,7 +11,6 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/side_panel/customize_chrome/customize_chrome_tab_helper.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_registry_observer.h"
 #include "chrome/browser/ui/views/side_panel/side_panel_web_ui_view.h"
@@ -67,31 +66,28 @@ void CustomizeChromeSidePanelController::DeregisterEntry() {
 void CustomizeChromeSidePanelController::SetCustomizeChromeSidePanelVisible(
     bool visible,
     CustomizeChromeSection section) {
-  auto* browser_view = GetBrowserView();
-  if (!browser_view)
+  auto* side_panel_ui = GetSidePanelUI();
+  if (!side_panel_ui) {
     return;
+  }
   DCHECK(IsCustomizeChromeEntryAvailable());
   if (visible) {
-    browser_view->side_panel_coordinator()->Show(
-        SidePanelEntry::Id::kCustomizeChrome);
+    side_panel_ui->Show(SidePanelEntry::Id::kCustomizeChrome);
     if (customize_chrome_ui_) {
       customize_chrome_ui_->ScrollToSection(section);
       section_.reset();
     } else {
       section_ = section;
     }
   } else {
-    browser_view->side_panel_coordinator()->Close();
+    side_panel_ui->Close();
   }
 }
 
 bool CustomizeChromeSidePanelController::IsCustomizeChromeEntryShowing() const {
-  auto* browser_view = GetBrowserView();
-  if (!browser_view)
-    return false;
-  auto* side_panel_coordinator = browser_view->side_panel_coordinator();
-  return side_panel_coordinator->IsSidePanelShowing() &&
-         (side_panel_coordinator->GetCurrentEntryId() ==
+  auto* side_panel_ui = GetSidePanelUI();
+  return side_panel_ui && side_panel_ui->IsSidePanelShowing() &&
+         (side_panel_ui->GetCurrentEntryId() ==
           SidePanelEntry::Id::kCustomizeChrome);
 }
 
@@ -137,7 +133,7 @@ CustomizeChromeSidePanelController::CreateCustomizeChromeWebView() {
   return customize_chrome_web_view;
 }
 
-BrowserView* CustomizeChromeSidePanelController::GetBrowserView() const {
+SidePanelUI* CustomizeChromeSidePanelController::GetSidePanelUI() const {
   auto* browser = chrome::FindBrowserWithWebContents(web_contents_);
-  return browser ? BrowserView::GetBrowserViewForBrowser(browser) : nullptr;
+  return browser ? SidePanelUI::GetSidePanelUIForBrowser(browser) : nullptr;
 }