From d627ce96b58bb2b28b1a360da32d9d22b33b9fa9 Mon Sep 17 00:00:00 2001
From: Sangwoo Ko <sko@brave.com>
Date: Fri, 28 Apr 2023 13:39:11 +0900
Subject: [PATCH] Hide Vertical Tab strip widget when disabled (#18241)

On Mac, the vertical tab strip widget shows up when restoring the
browser window. This is because upstream implementation makes all
child widgets regardless of their previous visibility.

In order to fix that, observes the widget's visibility and double-check
it should be visible.
---
 ...vertical_tab_strip_widget_delegate_view.cc | 29 +++++++++++++++++--
 .../vertical_tab_strip_widget_delegate_view.h |  5 +++-
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/browser/ui/views/frame/vertical_tab_strip_widget_delegate_view.cc b/browser/ui/views/frame/vertical_tab_strip_widget_delegate_view.cc
index 2a9d78414d4e..55dff27119a3 100644
--- a/browser/ui/views/frame/vertical_tab_strip_widget_delegate_view.cc
+++ b/browser/ui/views/frame/vertical_tab_strip_widget_delegate_view.cc
@@ -10,6 +10,7 @@
 #include "base/check.h"
 #include "brave/browser/ui/views/frame/brave_browser_view.h"
 #include "brave/browser/ui/views/frame/vertical_tab_strip_region_view.h"
+#include "brave/browser/ui/views/tabs/vertical_tab_utils.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/theme_copying_widget.h"
@@ -134,11 +135,15 @@ VerticalTabStripWidgetDelegateView::VerticalTabStripWidgetDelegateView(
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
   host_view_observation_.Observe(host_);
-  widget_observation_.Observe(host_->GetWidget());
+  widget_observation_.AddObservation(host_->GetWidget());
 
   ChildPreferredSizeChanged(region_view_);
 }
 
+void VerticalTabStripWidgetDelegateView::AddedToWidget() {
+  widget_observation_.AddObservation(GetWidget());
+}
+
 void VerticalTabStripWidgetDelegateView::ChildPreferredSizeChanged(
     views::View* child) {
   if (!host_)
@@ -180,9 +185,27 @@ void VerticalTabStripWidgetDelegateView::OnViewIsDeleting(
   host_ = nullptr;
 }
 
+void VerticalTabStripWidgetDelegateView::OnWidgetVisibilityChanged(
+    views::Widget* widget,
+    bool visible) {
+  if (widget == GetWidget()) {
+    if (!tabs::utils::ShouldShowVerticalTabs(browser_view_->browser()) &&
+        visible) {
+      // This happens when restoring browser window. The upstream implementation
+      // make child widgets visible regardless of their previous visibility.
+      // https://github.com/brave/brave-browser/issues/29917
+      widget->Hide();
+    }
+  }
+}
+
 void VerticalTabStripWidgetDelegateView::OnWidgetBoundsChanged(
     views::Widget* widget,
     const gfx::Rect& new_bounds) {
+  if (widget == GetWidget()) {
+    return;
+  }
+
   // The parent widget could be resized because fullscreen status changed.
   // Try resetting preferred size.
   ChildPreferredSizeChanged(region_view_);
@@ -204,6 +227,8 @@ void VerticalTabStripWidgetDelegateView::UpdateWidgetBounds() {
     return;
   }
 
+  DCHECK(tabs::utils::ShouldShowVerticalTabs(browser_view_->browser()));
+
   auto insets = host_->GetInsets();
   widget_bounds.set_width(widget_bounds.width() + insets.width());
   if (GetInsets() != insets) {
@@ -228,7 +253,7 @@ void VerticalTabStripWidgetDelegateView::UpdateWidgetBounds() {
 
 void VerticalTabStripWidgetDelegateView::OnWidgetDestroying(
     views::Widget* widget) {
-  widget_observation_.Reset();
+  widget_observation_.RemoveObservation(widget);
 }
 
 #if BUILDFLAG(IS_MAC)
diff --git a/browser/ui/views/frame/vertical_tab_strip_widget_delegate_view.h b/browser/ui/views/frame/vertical_tab_strip_widget_delegate_view.h
index fbf7bad5687c..7784c04fd03e 100644
--- a/browser/ui/views/frame/vertical_tab_strip_widget_delegate_view.h
+++ b/browser/ui/views/frame/vertical_tab_strip_widget_delegate_view.h
@@ -8,6 +8,7 @@
 
 #include <memory>
 
+#include "base/scoped_multi_source_observation.h"
 #include "ui/views/view_observer.h"
 #include "ui/views/widget/widget_delegate.h"
 #include "ui/views/widget/widget_observer.h"
@@ -47,6 +48,7 @@ class VerticalTabStripWidgetDelegateView : public views::WidgetDelegateView,
   }
 
   // views::WidgetDelegateView:
+  void AddedToWidget() override;
   void ChildPreferredSizeChanged(views::View* child) override;
 
   // views::ViewObserver:
@@ -56,6 +58,7 @@ class VerticalTabStripWidgetDelegateView : public views::WidgetDelegateView,
   void OnViewIsDeleting(views::View* observed_view) override;
 
   // views::WidgetObserver:
+  void OnWidgetVisibilityChanged(views::Widget* widget, bool visible) override;
   void OnWidgetBoundsChanged(views::Widget* widget,
                              const gfx::Rect& new_bounds) override;
   void OnWidgetDestroying(views::Widget* widget) override;
@@ -76,7 +79,7 @@ class VerticalTabStripWidgetDelegateView : public views::WidgetDelegateView,
   base::ScopedObservation<views::View, views::ViewObserver>
       host_view_observation_{this};
 
-  base::ScopedObservation<views::Widget, views::WidgetObserver>
+  base::ScopedMultiSourceObservation<views::Widget, views::WidgetObserver>
       widget_observation_{this};
 };