diff --git a/chrome/browser/ui/browser_element_identifiers.cc b/chrome/browser/ui/browser_element_identifiers.cc index 58fba1905ec8ad..171977508582e6 100644 --- a/chrome/browser/ui/browser_element_identifiers.cc +++ b/chrome/browser/ui/browser_element_identifiers.cc @@ -17,6 +17,7 @@ DEFINE_ELEMENT_IDENTIFIER_VALUE(kLocationIconElementId); DEFINE_ELEMENT_IDENTIFIER_VALUE(kMediaButtonElementId); DEFINE_ELEMENT_IDENTIFIER_VALUE(kOmniboxElementId); DEFINE_ELEMENT_IDENTIFIER_VALUE(kReadLaterButtonElementId); +DEFINE_ELEMENT_IDENTIFIER_VALUE(kReadLaterSidePanelWebViewElementId); DEFINE_ELEMENT_IDENTIFIER_VALUE(kSavePasswordComboboxElementId); DEFINE_ELEMENT_IDENTIFIER_VALUE(kSideSearchButtonElementId); DEFINE_ELEMENT_IDENTIFIER_VALUE(kTabAlertIndicatorButtonElementId); diff --git a/chrome/browser/ui/browser_element_identifiers.h b/chrome/browser/ui/browser_element_identifiers.h index 4b669639749c77..6cb881e63915b5 100644 --- a/chrome/browser/ui/browser_element_identifiers.h +++ b/chrome/browser/ui/browser_element_identifiers.h @@ -26,6 +26,7 @@ DECLARE_ELEMENT_IDENTIFIER_VALUE(kLocationIconElementId); DECLARE_ELEMENT_IDENTIFIER_VALUE(kMediaButtonElementId); DECLARE_ELEMENT_IDENTIFIER_VALUE(kOmniboxElementId); DECLARE_ELEMENT_IDENTIFIER_VALUE(kReadLaterButtonElementId); +DECLARE_ELEMENT_IDENTIFIER_VALUE(kReadLaterSidePanelWebViewElementId); DECLARE_ELEMENT_IDENTIFIER_VALUE(kSavePasswordComboboxElementId); DECLARE_ELEMENT_IDENTIFIER_VALUE(kSideSearchButtonElementId); DECLARE_ELEMENT_IDENTIFIER_VALUE(kTabAlertIndicatorButtonElementId); diff --git a/chrome/browser/ui/ui_features.cc b/chrome/browser/ui/ui_features.cc index 1db28ccbb0c82d..10ed7d6fece76c 100644 --- a/chrome/browser/ui/ui_features.cc +++ b/chrome/browser/ui/ui_features.cc @@ -124,6 +124,12 @@ const base::Feature kClobberAllSideSearchSidePanels{ const base::Feature kSidePanelDragAndDrop{"SidePanelDragAndDrop", base::FEATURE_DISABLED_BY_DEFAULT}; +// Adds improved support for handling multiple contextual and global RHS browser +// side panels. Designed specifically to handle the interim state before the v2 +// side panel project launches. +const base::Feature kSidePanelImprovedClobbering{ + "SidePanelImprovedClobbering", base::FEATURE_DISABLED_BY_DEFAULT}; + // Enables tabs to scroll in the tabstrip. https://crbug.com/951078 const base::Feature kScrollableTabStrip{"ScrollableTabStrip", base::FEATURE_DISABLED_BY_DEFAULT}; diff --git a/chrome/browser/ui/ui_features.h b/chrome/browser/ui/ui_features.h index d7ac1511c4249f..d5f4a81087648c 100644 --- a/chrome/browser/ui/ui_features.h +++ b/chrome/browser/ui/ui_features.h @@ -68,6 +68,7 @@ extern const base::Feature kScrollableTabStripButtons; // ui_features.cc. This is currently temporarily in reading_list_switches.h. extern const base::Feature kSidePanel; extern const base::Feature kSidePanelDragAndDrop; +extern const base::Feature kSidePanelImprovedClobbering; #if BUILDFLAG(ENABLE_SIDE_SEARCH) extern const base::Feature kSideSearch; diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc index e19f59b99eec62..bbf4a79b6a6de6 100644 --- a/chrome/browser/ui/views/frame/browser_view.cc +++ b/chrome/browser/ui/views/frame/browser_view.cc @@ -23,6 +23,7 @@ #include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_macros.h" #include "base/metrics/user_metrics.h" +#include "base/ranges/algorithm.h" #include "base/strings/string_number_conversions.h" #include "base/task/single_thread_task_runner.h" #include "base/threading/thread_task_runner_handle.h" @@ -688,6 +689,110 @@ class BrowserView::SidePanelButtonHighlighter : public views::ViewObserver { const std::vector side_panels_; }; +/////////////////////////////////////////////////////////////////////////////// +// BrowserView::SidePanelVisibilityController: +// +// Coordinating class that manages side panel visibility such that there is a +// single RHS side panel open at a given time. It enforces the following policy: +// - Only one RHS panel is visible at a time. +// - When the contextual panel is shown, the state of the global panels is +// captured and global panels are hidden. +// - When the contextual panel is hidden, the state of the global panels is +// restored. +// +// TODO(tluk): This is intended to manage the visibility of the read later +// (global), google lens (global) and side search (contextual) panels for the +// interim period before side panel v2 rolls out. +class BrowserView::SidePanelVisibilityController : public views::ViewObserver { + public: + // Structures that hold the global panel views and their captured visibility + // state. + struct PanelStateEntry { + const raw_ptr panel_view; + absl::optional captured_visibility_state; + }; + using Panels = std::vector; + + SidePanelVisibilityController(views::View* side_search_panel, + views::View* lens_panel, + views::View* rhs_panel) + : side_search_panel_(side_search_panel) { + if (lens_panel) + global_panels_.push_back({lens_panel, absl::nullopt}); + if (rhs_panel) + global_panels_.push_back({rhs_panel, absl::nullopt}); + + // Observing the side search panel is only necessary when enabling the + // improved clobbering functionality. + if (side_search_panel_ && + base::FeatureList::IsEnabled(features::kSidePanelImprovedClobbering)) { + side_search_panel_observation_.Observe(side_search_panel_); + } + } + ~SidePanelVisibilityController() override = default; + + // views::ViewObserver: + void OnViewVisibilityChanged(views::View* observed_view, + View* starting_from) override { + DCHECK_EQ(side_search_panel_, observed_view); + if (side_search_panel_->GetVisible()) { + CaptureGlobalPanelVisibilityStateAndHide(); + } else { + RestoreGlobalPanelVisibilityState(); + } + } + + // Called when the contextual panel is shown. Captures the current visibility + // state of the global panel before hiding the panel. The captured state of + // the global panels remains valid while the contextual panel is open. + void CaptureGlobalPanelVisibilityStateAndHide() { + for (PanelStateEntry& entry : global_panels_) { + auto panel_view = entry.panel_view; + entry.captured_visibility_state = panel_view->GetVisible(); + panel_view->SetVisible(false); + } + } + + // Called when the contextual panel is hidden. Restores the visibility state + // of the global panels. + void RestoreGlobalPanelVisibilityState() { + for (PanelStateEntry& entry : global_panels_) { + if (entry.captured_visibility_state.has_value()) { + entry.panel_view->SetVisible(entry.captured_visibility_state.value()); + + // After restoring global panel state reset the stored visibility bits. + // These will not remain valid while the contextual panel is closed. + entry.captured_visibility_state.reset(); + } + } + } + + // Returns true if one of its managed panels is currently visible in the + // browser window. + bool IsManagedSidePanelVisible() const { + if (side_search_panel_ && side_search_panel_->GetVisible()) + return true; + return base::ranges::any_of(global_panels_, + [](const PanelStateEntry& entry) { + return entry.panel_view->GetVisible(); + }); + } + + private: + // We observe the side search panel when improved clobbering is enabled to + // implement the correct view visibility transitions. + const raw_ptr side_search_panel_; + + // The set of global panels that this maintains visibility for. + Panels global_panels_; + + // Keep track of the side search panel's visibility so that we can hide / + // restore global panels as the side search panel is shown / hidden + // respectively. + base::ScopedObservation + side_search_panel_observation_{this}; +}; + /////////////////////////////////////////////////////////////////////////////// // BrowserView, public: @@ -874,10 +979,11 @@ BrowserView::~BrowserView() { if (tabstrip_) tabstrip_->parent()->RemoveChildViewT(tabstrip_.get()); - // This highlighter refers to side-panel objects (children of this) and to - // children inside ToolbarView and of this, remove this observer before those - // children are removed. + // This highlighter and visibility controller refer to side-panel objects + // (children of this) and to children inside ToolbarView and of this, remove + // this observer before those children are removed. side_panel_button_highlighter_.reset(); + side_panel_visibility_controller_.reset(); // Child views maintain PrefMember attributes that point to // OffTheRecordProfile's PrefService which gets deleted by ~Browser. @@ -3201,33 +3307,30 @@ void BrowserView::CloseTabSearchBubble() { bool BrowserView::CloseOpenRightAlignedSidePanel(bool exclude_lens, bool exclude_side_search) { - // Hide Chrome side panel (Reading List/Bookmarks) if enabled and showing. - if (toolbar()->side_panel_button() && - right_aligned_side_panel()->GetVisible()) { - toolbar()->side_panel_button()->HideSidePanel(); - return true; + // Check if any side panels are open before closing side panels. + if (!side_panel_visibility_controller_ || + !side_panel_visibility_controller_->IsManagedSidePanelVisible()) { + return false; } -#if BUILDFLAG(GOOGLE_CHROME_BRANDING) - // Hide the Lens side panel if it's showing instead. - if (!exclude_lens && lens_side_panel_controller_ && - lens_side_panel_controller_->IsShowing()) { - lens_side_panel_controller_->Close(); - return true; - } -#endif // BUILDFLAG(GOOGLE_CHROME_BRANDING) + // Ensure all side panels are closed. Close contextual panels first. #if BUILDFLAG(ENABLE_SIDE_SEARCH) // Hide side search panel if it's right aligned. - if (!exclude_side_search && - base::FeatureList::IsEnabled(features::kSideSearchDSESupport) && - side_search_side_panel_ && side_search_side_panel_->GetVisible()) { + if (!exclude_side_search && side_search_controller_ && + base::FeatureList::IsEnabled(features::kSideSearchDSESupport)) { side_search_controller_->CloseSidePanel(); - return true; } #endif // BUILDFLAG(ENABLE_SIDE_SEARCH) - return false; + toolbar()->side_panel_button()->HideSidePanel(); + +#if BUILDFLAG(GOOGLE_CHROME_BRANDING) + if (!exclude_lens && lens_side_panel_controller_) + lens_side_panel_controller_->Close(); +#endif // BUILDFLAG(GOOGLE_CHROME_BRANDING) + + return true; } void BrowserView::MaybeClobberAllSideSearchSidePanels() { @@ -3244,6 +3347,16 @@ void BrowserView::MaybeClobberAllSideSearchSidePanels() { #endif // BUILDFLAG(ENABLE_SIDE_SEARCH) } +void BrowserView::RightAlignedSidePanelWasClosed() { + // For the improved side panel clobbering experience we must close all side + // panels for the window when the user explicitly closes a participating side + // panel. + if (base::FeatureList::IsEnabled(features::kSidePanelImprovedClobbering)) { + CloseOpenRightAlignedSidePanel(); + MaybeClobberAllSideSearchSidePanels(); + } +} + #if BUILDFLAG(ENABLE_SIDE_SEARCH) bool BrowserView::IsSideSearchPanelVisible() const { if (side_search_controller_) @@ -3491,6 +3604,11 @@ void BrowserView::AddedToWidget() { side_panel_button_highlighter_ = std::make_unique( toolbar_->side_panel_button(), panels); + + side_panel_visibility_controller_ = + std::make_unique( + side_search_side_panel_, lens_side_panel_, + right_aligned_side_panel_); } #if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) diff --git a/chrome/browser/ui/views/frame/browser_view.h b/chrome/browser/ui/views/frame/browser_view.h index 786a096ed4cc4a..a9a8e1e6220d9a 100644 --- a/chrome/browser/ui/views/frame/browser_view.h +++ b/chrome/browser/ui/views/frame/browser_view.h @@ -732,6 +732,11 @@ class BrowserView : public BrowserWindow, // kClobberAllSideSearchSidePanels is enabled. void MaybeClobberAllSideSearchSidePanels(); + // Called by right aligned side panels when they are explicitly closed by + // users. This is used to implement improved clobbering logic for the right + // aligned side panels. + void RightAlignedSidePanelWasClosed(); + #if BUILDFLAG(ENABLE_SIDE_SEARCH) bool IsSideSearchPanelVisible() const override; void MaybeRestoreSideSearchStatePerWindow( @@ -747,6 +752,7 @@ class BrowserView : public BrowserWindow, FRIEND_TEST_ALL_PREFIXES(BrowserViewTest, AccessibleWindowTitle); class AccessibilityModeObserver; class SidePanelButtonHighlighter; + class SidePanelVisibilityController; // If the browser is in immersive full screen mode, it will reveal the // tabstrip for a short duration. This is useful for shortcuts that perform @@ -1012,6 +1018,14 @@ class BrowserView : public BrowserWindow, // as well as the side panels it's observing. std::unique_ptr side_panel_button_highlighter_; + // TODO(tluk): Move this functionality into SidePanelCoordinator when the side + // panel v2 project rolls out. + // This controller manages the visibility of the read later, side search and + // lens side panels. It ensures only one panel is visible at a given time and + // the contextual panel interacts as expected with the global panels. + std::unique_ptr + side_panel_visibility_controller_; + #if BUILDFLAG(GOOGLE_CHROME_BRANDING) // A controller that handles content hosted in the Lens side panel. std::unique_ptr lens_side_panel_controller_; diff --git a/chrome/browser/ui/views/lens/lens_side_panel_controller.cc b/chrome/browser/ui/views/lens/lens_side_panel_controller.cc index 0ec960a51ec13a..c0ac3673719465 100644 --- a/chrome/browser/ui/views/lens/lens_side_panel_controller.cc +++ b/chrome/browser/ui/views/lens/lens_side_panel_controller.cc @@ -105,6 +105,7 @@ void LensSidePanelController::Close() { GURL(), content::Referrer(), ui::PAGE_TRANSITION_FROM_API, std::string()); side_panel_->SetVisible(false); + browser_view_->RightAlignedSidePanelWasClosed(); base::RecordAction(base::UserMetricsAction("LensSidePanel.Hide")); } if (browser_view_->toolbar()->side_panel_button()) { diff --git a/chrome/browser/ui/views/side_panel/read_later_side_panel_web_view.cc b/chrome/browser/ui/views/side_panel/read_later_side_panel_web_view.cc index a6fe959a5eee18..f0b8ede05fa2b6 100644 --- a/chrome/browser/ui/views/side_panel/read_later_side_panel_web_view.cc +++ b/chrome/browser/ui/views/side_panel/read_later_side_panel_web_view.cc @@ -10,10 +10,12 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/bookmarks/bookmark_utils.h" #include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_element_identifiers.h" #include "chrome/browser/ui/ui_features.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/common/webui_url_constants.h" #include "chrome/grit/generated_resources.h" +#include "ui/views/view_class_properties.h" ReadLaterSidePanelWebView::ReadLaterSidePanelWebView( Browser* browser, @@ -31,6 +33,8 @@ ReadLaterSidePanelWebView::ReadLaterSidePanelWebView( /*webui_resizes_host=*/false, /*esc_closes_ui=*/false)), browser_(browser) { + SetProperty(views::kElementIdentifierKey, + kReadLaterSidePanelWebViewElementId); if (base::FeatureList::IsEnabled(features::kSidePanelDragAndDrop)) { extensions::BookmarkManagerPrivateDragEventRouter::CreateForWebContents( contents_wrapper()->web_contents()); diff --git a/chrome/browser/ui/views/side_search/side_search_browser_controller.cc b/chrome/browser/ui/views/side_search/side_search_browser_controller.cc index d3763c5572db5e..de469f0c47487c 100644 --- a/chrome/browser/ui/views/side_search/side_search_browser_controller.cc +++ b/chrome/browser/ui/views/side_search/side_search_browser_controller.cc @@ -448,6 +448,7 @@ bool SideSearchBrowserController::GetSidePanelToggledOpen() const { void SideSearchBrowserController::SidePanelCloseButtonPressed() { CloseSidePanel(SideSearchCloseActionType::kTapOnSideSearchCloseButton); + browser_view_->RightAlignedSidePanelWasClosed(); } void SideSearchBrowserController::OpenSidePanel() { @@ -541,7 +542,11 @@ void SideSearchBrowserController::UpdateSidePanel() { const bool will_show_side_panel = can_show_side_panel_for_page && GetSidePanelToggledOpen(); + // When side search is shown we only need to close other side panels for the + // basic clobbering experience. The improved experience leverages a + // SidePanelVisibilityController on the browser view. if (base::FeatureList::IsEnabled(features::kSideSearchDSESupport) && + !base::FeatureList::IsEnabled(features::kSidePanelImprovedClobbering) && will_show_side_panel) { browser_view_->CloseOpenRightAlignedSidePanel(/*exclude_lens=*/false, /*exclude_side_search=*/true); diff --git a/chrome/browser/ui/views/side_search/side_search_browser_controller_interactive_uitest.cc b/chrome/browser/ui/views/side_search/side_search_browser_controller_interactive_uitest.cc index cbbcbf2618ef26..d7e264ebbdee80 100644 --- a/chrome/browser/ui/views/side_search/side_search_browser_controller_interactive_uitest.cc +++ b/chrome/browser/ui/views/side_search/side_search_browser_controller_interactive_uitest.cc @@ -22,6 +22,8 @@ #include "chrome/browser/ui/ui_features.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/side_panel/side_panel.h" +#include "chrome/browser/ui/views/side_panel/side_panel_web_ui_view.h" +#include "chrome/browser/ui/views/toolbar/side_panel_toolbar_button.h" #include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "chrome/common/webui_url_constants.h" #include "chrome/test/base/in_process_browser_test.h" @@ -80,6 +82,10 @@ class SideSearchBrowserControllerTest ASSERT_TRUE(embedded_test_server()->InitializeAndListen()); InProcessBrowserTest::SetUp(); } + void TearDown() override { + InProcessBrowserTest::TearDown(); + scoped_feature_list_.Reset(); + } void SetUpOnMainThread() override { host_resolver()->AddRule("*", "127.0.0.1"); @@ -740,6 +746,246 @@ IN_PROC_BROWSER_TEST_P(SideSearchBrowserControllerTest, EXPECT_NE(nullptr, GetSidePanelContentsFor(browser(), 0)); } +// Fixture for testing side panel clobbering behavior with global panels. +class SideSearchDSEClobberingTest : public SideSearchBrowserControllerTest { + public: + // SideSearchBrowserControllerTest: + void SetUp() override { + scoped_feature_list_.InitWithFeatures( + {features::kSidePanel, features::kSidePanelImprovedClobbering}, {}); + SideSearchBrowserControllerTest::SetUp(); + } + void TearDown() override { + SideSearchBrowserControllerTest::TearDown(); + scoped_feature_list_.Reset(); + } + + // Immediately open and make visible the global side panel. + void ShowGlobalSidePanel(Browser* browser) { + ASSERT_FALSE(GetGlobalSidePanelFor(browser)->GetVisible()); + auto* side_panel_button = GetToolbarSidePanelButtonFor(browser); + views::test::ButtonTestApi(side_panel_button).NotifyClick(GetDummyEvent()); + + // The WebUI typically loads and is shown asynchronously. Synchronously show + // the view here for testing. + views::View* web_view = + views::ElementTrackerViews::GetInstance()->GetFirstMatchingView( + kReadLaterSidePanelWebViewElementId, + browser->window()->GetElementContext()); + DCHECK(web_view); + views::AsViewClass(web_view)->ShowUI(); + + BrowserViewFor(browser)->GetWidget()->LayoutRootViewIfNecessary(); + } + + // Uses the toolbar side panel button to close whichever side panel is + // currently open. + void CloseActiveSidePanel(Browser* browser) { + ASSERT_TRUE(GetGlobalSidePanelFor(browser)->GetVisible() || + GetSidePanelFor(browser)); + auto* side_panel_button = GetToolbarSidePanelButtonFor(browser); + views::test::ButtonTestApi(side_panel_button).NotifyClick(GetDummyEvent()); + BrowserViewFor(browser)->GetWidget()->LayoutRootViewIfNecessary(); + } + + // Sets up a browser with three tabs, an open global panel and an open side + // search panel for the last tab. + void SetupBrowserForClobberingTests(Browser* browser) { + auto* global_panel = GetGlobalSidePanelFor(browser); + EXPECT_FALSE(global_panel->GetVisible()); + ShowGlobalSidePanel(browser); + EXPECT_TRUE(global_panel->GetVisible()); + + // Add another two tabs, the global panel should remain open for each. + AppendTab(browser, GetNonMatchingUrl()); + ActivateTabAt(browser, 1); + EXPECT_TRUE(global_panel->GetVisible()); + + AppendTab(browser, GetNonMatchingUrl()); + ActivateTabAt(browser, 2); + EXPECT_TRUE(global_panel->GetVisible()); + + // Open the side search contextual panel for the current active tab. + auto* side_search_panel = GetSidePanelFor(browser); + NavigateToMatchingSearchPageAndOpenSidePanel(browser); + EXPECT_TRUE(side_search_panel->GetVisible()); + EXPECT_FALSE(global_panel->GetVisible()); + } + + SidePanelToolbarButton* GetToolbarSidePanelButtonFor(Browser* browser) { + views::View* button_view = + views::ElementTrackerViews::GetInstance()->GetFirstMatchingView( + kReadLaterButtonElementId, browser->window()->GetElementContext()); + return button_view ? views::AsViewClass(button_view) + : nullptr; + } + + SidePanel* GetGlobalSidePanelFor(Browser* browser) { + return BrowserViewFor(browser)->right_aligned_side_panel(); + } + + private: + base::test::ScopedFeatureList scoped_feature_list_; +}; + +// Only instantiate tests for the DSE configuration. +INSTANTIATE_TEST_SUITE_P(All, + SideSearchDSEClobberingTest, + ::testing::Values(true)); + +IN_PROC_BROWSER_TEST_P(SideSearchDSEClobberingTest, + GlobalBrowserSidePanelIsToggleable) { + auto* global_panel = GetGlobalSidePanelFor(browser()); + EXPECT_FALSE(global_panel->GetVisible()); + ShowGlobalSidePanel(browser()); + EXPECT_TRUE(global_panel->GetVisible()); +} + +IN_PROC_BROWSER_TEST_P(SideSearchDSEClobberingTest, + ContextualPanelsDoNotClobberGlobalPanels) { + SetupBrowserForClobberingTests(browser()); + auto* global_panel = GetGlobalSidePanelFor(browser()); + auto* side_search_panel = GetSidePanelFor(browser()); + + // Switching to tabs with no open contextual panels should instead show the + // global panel. + ActivateTabAt(browser(), 1); + EXPECT_TRUE(global_panel->GetVisible()); + EXPECT_FALSE(side_search_panel->GetVisible()); + + ActivateTabAt(browser(), 0); + EXPECT_TRUE(global_panel->GetVisible()); + EXPECT_FALSE(side_search_panel->GetVisible()); + + // Switching back to the tab with the contextual panel should show the + // contextual panel and not the global panel. + ActivateTabAt(browser(), 2); + EXPECT_FALSE(global_panel->GetVisible()); + EXPECT_TRUE(side_search_panel->GetVisible()); +} + +IN_PROC_BROWSER_TEST_P(SideSearchDSEClobberingTest, + OpeningGlobalPanelsClosesAllContextualPanels) { + auto* global_panel = GetGlobalSidePanelFor(browser()); + auto* side_search_panel = GetSidePanelFor(browser()); + AppendTab(browser(), GetNonMatchingUrl()); + AppendTab(browser(), GetNonMatchingUrl()); + + // There should be three tabs and no panels open. + for (int i = 0; i < 3; ++i) { + ActivateTabAt(browser(), i); + EXPECT_FALSE(global_panel->GetVisible()); + EXPECT_FALSE(side_search_panel->GetVisible()); + } + + // Open a contextual panel on the last tab. + ActivateTabAt(browser(), 2); + NavigateToMatchingSearchPageAndOpenSidePanel(browser()); + EXPECT_FALSE(global_panel->GetVisible()); + EXPECT_TRUE(side_search_panel->GetVisible()); + + // Switch to the first tab and open a global panel. + ActivateTabAt(browser(), 0); + ShowGlobalSidePanel(browser()); + EXPECT_TRUE(global_panel->GetVisible()); + EXPECT_FALSE(side_search_panel->GetVisible()); + + // The global panel should now be open for all browser tabs. + for (int i = 0; i < 3; ++i) { + ActivateTabAt(browser(), i); + EXPECT_TRUE(global_panel->GetVisible()); + EXPECT_FALSE(side_search_panel->GetVisible()); + } +} + +IN_PROC_BROWSER_TEST_P( + SideSearchDSEClobberingTest, + ContextualAndGlobalPanelsBehaveAsExpectedWhenDraggingBetweenWindows) { + // Open two browsers with three tabs each. Both have open global side panel + // and an open side search panel for their last tab. + Browser* browser2 = CreateBrowser(browser()->profile()); + SetupBrowserForClobberingTests(browser()); + SetupBrowserForClobberingTests(browser2); + + // Move the currently active tab with side search from browser2 to browser1. + std::unique_ptr web_contents = + browser2->tab_strip_model()->DetachWebContentsAtForInsertion(2); + browser()->tab_strip_model()->InsertWebContentsAt(3, std::move(web_contents), + TabStripModel::ADD_ACTIVE); + + // The global panel should now be visibe in browser2 and the contextual panel + // should be visible in browser1. + auto* global_panel1 = GetGlobalSidePanelFor(browser()); + auto* global_panel2 = GetGlobalSidePanelFor(browser2); + auto* side_search_panel1 = GetSidePanelFor(browser()); + auto* side_search_panel2 = GetSidePanelFor(browser2); + + EXPECT_TRUE(global_panel2->GetVisible()); + EXPECT_FALSE(side_search_panel2->GetVisible()); + + EXPECT_FALSE(global_panel1->GetVisible()); + EXPECT_TRUE(side_search_panel1->GetVisible()); + + // In browser1 switch to the tab that originally had the side search panel + // open. The global panels should remain closed. + ActivateTabAt(browser(), 2); + EXPECT_FALSE(global_panel1->GetVisible()); + EXPECT_TRUE(side_search_panel1->GetVisible()); + + // In browser1 switch to tabs that did not have a side search panel open. The + // side search panel should be hidden and the global panel should be visible. + ActivateTabAt(browser(), 1); + EXPECT_TRUE(global_panel1->GetVisible()); + EXPECT_FALSE(side_search_panel1->GetVisible()); + + ActivateTabAt(browser(), 0); + EXPECT_TRUE(global_panel1->GetVisible()); + EXPECT_FALSE(side_search_panel1->GetVisible()); +} + +IN_PROC_BROWSER_TEST_P(SideSearchDSEClobberingTest, + ClosingTheContextualPanelClosesAllBrowserPanels) { + SetupBrowserForClobberingTests(browser()); + auto* global_panel = GetGlobalSidePanelFor(browser()); + auto* side_search_panel = GetSidePanelFor(browser()); + + // Append an additional browser tab with an open side search panel. + AppendTab(browser(), GetNonMatchingUrl()); + ActivateTabAt(browser(), 3); + NavigateToMatchingSearchPageAndOpenSidePanel(browser()); + + // Close the contextual panel. The global and contextual panels in the current + // and other tabs should all be closed. + CloseActiveSidePanel(browser()); + for (int i = 0; i < 3; ++i) { + ActivateTabAt(browser(), i); + EXPECT_FALSE(global_panel->GetVisible()); + EXPECT_FALSE(side_search_panel->GetVisible()); + } +} + +IN_PROC_BROWSER_TEST_P(SideSearchDSEClobberingTest, + ClosingTheGlobalPanelClosesAllBrowserPanels) { + SetupBrowserForClobberingTests(browser()); + auto* global_panel = GetGlobalSidePanelFor(browser()); + auto* side_search_panel = GetSidePanelFor(browser()); + + // Append an additional browser tab with an open side search panel. + AppendTab(browser(), GetNonMatchingUrl()); + ActivateTabAt(browser(), 3); + NavigateToMatchingSearchPageAndOpenSidePanel(browser()); + + // Close the global panel. The global and contextual panels in the current + // and other tabs should all be closed. + ActivateTabAt(browser(), 0); + CloseActiveSidePanel(browser()); + for (int i = 0; i < 3; ++i) { + ActivateTabAt(browser(), i); + EXPECT_FALSE(global_panel->GetVisible()); + EXPECT_FALSE(side_search_panel->GetVisible()); + } +} + // Base class for Extensions API tests for the side panel WebContents. class SideSearchExtensionsTest : public SideSearchBrowserControllerTest { public: diff --git a/chrome/browser/ui/views/toolbar/side_panel_toolbar_button.cc b/chrome/browser/ui/views/toolbar/side_panel_toolbar_button.cc index 11616419d5ea18..31d965189320f0 100644 --- a/chrome/browser/ui/views/toolbar/side_panel_toolbar_button.cc +++ b/chrome/browser/ui/views/toolbar/side_panel_toolbar_button.cc @@ -13,6 +13,7 @@ #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_element_identifiers.h" #include "chrome/browser/ui/read_later/reading_list_model_factory.h" +#include "chrome/browser/ui/ui_features.h" #include "chrome/browser/ui/views/chrome_view_class_properties.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/side_panel/read_later_side_panel_web_view.h" @@ -135,6 +136,7 @@ void SidePanelToolbarButton::HideSidePanel() { side_panel_webview_.get()); side_panel_webview_ = nullptr; SetTooltipText(l10n_util::GetStringUTF16(IDS_TOOLTIP_SIDE_PANEL_SHOW)); + browser_view->RightAlignedSidePanelWasClosed(); } }