[side search] Implements clobbering+ logic for RHS side panels

This CL introduces an improved clobbering experience for RHS browser
side panels.

The current clobbering experience is whenever a RHS side panel is
shown all other open side panels for that window are immediately
closed. This naive clobbering logic results in UX issues where a user
may not expect their global panel to be dismissed when they open a
tab specific side search instance.

The improved logic covered by this CL implements the following
policy:
  1. Only one RHS panel is present in a window at a time.
  2. If a contextual panel (side search) is opened it will open over
     the top of any open global panels.
  3. If the user switches away from the tab with the contextual panel
     the previously open global panel will be restored (if it was
     open preciously).

Bug: 1309679
Change-Id: Ide39a8e562b3d2879b0f31712c11cd6979ab1c62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3547558
Reviewed-by: Dana Fried <dfried@chromium.org>
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#985128}

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);

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);

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};

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;

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<views::View*> 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<views::View> panel_view;
+    absl::optional<bool> captured_visibility_state;
+  };
+  using Panels = std::vector<PanelStateEntry>;
+
+  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<views::View> 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<views::View, views::ViewObserver>
+      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<SidePanelButtonHighlighter>(
             toolbar_->side_panel_button(), panels);
+
+    side_panel_visibility_controller_ =
+        std::make_unique<SidePanelVisibilityController>(
+            side_search_side_panel_, lens_side_panel_,
+            right_aligned_side_panel_);
   }
 
 #if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)

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<SidePanelButtonHighlighter> 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<SidePanelVisibilityController>
+      side_panel_visibility_controller_;
+
 #if BUILDFLAG(GOOGLE_CHROME_BRANDING)
   // A controller that handles content hosted in the Lens side panel.
   std::unique_ptr<lens::LensSidePanelController> lens_side_panel_controller_;

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()) {

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());

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);