Don't create ghost windows when re-attaching a window to a browser

Ilie-Lesan · Oct 21, 2024 · eaad7ee · eaad7ee
1 parent a622f9d
commit eaad7ee
Show file tree

Hide file tree

Showing 9 changed files with 27 additions and 59 deletions.
diff --git a/browser/ui/brave_browser.cc b/browser/ui/brave_browser.cc
@@ -125,18 +125,6 @@ void BraveBrowser::OnTabClosing(content::WebContents* contents) {
   }
 }
 
-void BraveBrowser::TabStripEmpty() {
-  if (unload_controller_.is_attempting_to_close_browser() ||
-      !is_type_normal() || ignore_enable_closing_last_tab_pref_) {
-    Browser::TabStripEmpty();
-    return;
-  }
-
-  base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
-      FROM_HERE, base::BindOnce(&chrome::AddTabAt, this, GetNewTabURL(), -1,
-                                true, std::nullopt));
-}
-
 void BraveBrowser::RunFileChooser(
     content::RenderFrameHost* render_frame_host,
     scoped_refptr<content::FileSelectListener> listener,

diff --git a/browser/ui/brave_browser.h b/browser/ui/brave_browser.h
@@ -52,7 +52,6 @@ class BraveBrowser : public Browser {
   void ResetTryToCloseWindow() override;
 
   void OnTabClosing(content::WebContents* contents) override;
-  void TabStripEmpty() override;
 
   void RunFileChooser(content::RenderFrameHost* render_frame_host,
                       scoped_refptr<content::FileSelectListener> listener,
@@ -72,10 +71,6 @@ class BraveBrowser : public Browser {
 
   void set_confirmed_to_close(bool close) { confirmed_to_close_ = close; }
 
-  void set_ignore_enable_closing_last_tab_pref() {
-    ignore_enable_closing_last_tab_pref_ = true;
-  }
-
  private:
   friend class BraveTestLauncherDelegate;
   friend class WindowClosingConfirmBrowserTest;
@@ -95,7 +90,6 @@ class BraveBrowser : public Browser {
   // When "kEnableClosingLastTab" is false, browser will try to add new tab in
   // TabStripEmpty() if there is no tab. But, in some cases, we should not add
   // new tab, like when user tries to "Bring all tabs" to other window.
-  bool ignore_enable_closing_last_tab_pref_ = false;
 
   base::WeakPtrFactory<BraveBrowser> weak_ptr_factory_{this};
 };

diff --git a/browser/ui/browser_commands.cc b/browser/ui/browser_commands.cc
@@ -699,8 +699,6 @@ void BringAllTabs(Browser* browser) {
   base::ranges::for_each(browsers, [&detached_pinned_tabs,
                                     &detached_unpinned_tabs, &browsers_to_close,
                                     shared_pinned_tab_enabled](auto* other) {
-    static_cast<BraveBrowser*>(other)
-        ->set_ignore_enable_closing_last_tab_pref();
 
     auto* tab_strip_model = other->tab_strip_model();
     const int pinned_tab_count = tab_strip_model->IndexOfFirstNonPinnedTab();

diff --git a/browser/ui/tabs/brave_tab_strip_model.cc b/browser/ui/tabs/brave_tab_strip_model.cc
@@ -30,6 +30,22 @@ BraveTabStripModel::BraveTabStripModel(
     : TabStripModel(delegate, profile, group_model_factory) {}
 BraveTabStripModel::~BraveTabStripModel() = default;
 
+void BraveTabStripModel::CloseTabs(
+    base::span<content::WebContents* const> items,
+    uint32_t close_types) {
+  if (IsLastTab() && !profile_->GetPrefs()->GetBoolean(kEnableClosingLastTab)) {
+    content::WebContents* active_web_contents = GetActiveWebContents();
+    Browser* browser = chrome::FindBrowserWithTab(active_web_contents);
+    DCHECK(browser);
+    if (!browser->IsAttemptingToCloseBrowser() &&
+        delegate()->IsNormalWindow()) {
+      GURL url(chrome::kChromeUINewTabURL);
+      delegate()->AddTabAt(url, -1, true);
+    }
+  }
+  TabStripModel::CloseTabs(items, close_types);
+}
+
 void BraveTabStripModel::SelectRelativeTab(TabRelativeDirection direction,
                                            TabStripUserGestureDetails detail) {
   if (GetTabCount() == 0) {
@@ -97,3 +113,8 @@ void BraveTabStripModel::CloseTabs(base::span<int> indices,
   }
   TabStripModel::CloseTabs(contentses, close_types);
 }
+
+bool BraveTabStripModel::IsLastTab() const {
+  return (GetTabCount() == 1);
+}
+
diff --git a/browser/ui/tabs/brave_tab_strip_model.h b/browser/ui/tabs/brave_tab_strip_model.h
@@ -43,10 +43,14 @@ class BraveTabStripModel : public TabStripModel {
       uint32_t close_flags = TabCloseTypes::CLOSE_CREATE_HISTORICAL_TAB);
 
   // TabStripModel:
+  void CloseTabs(base::span<content::WebContents* const> items,
+                 uint32_t close_types) override;
+
   void SelectRelativeTab(TabRelativeDirection direction,
                          TabStripUserGestureDetails detail) override;
 
  private:
+  bool IsLastTab() const;
   // List of tab indexes sorted by most recently used
   std::vector<int> mru_cycle_list_;
 };

diff --git a/chromium_src/chrome/browser/ui/browser.h b/chromium_src/chrome/browser/ui/browser.h
@@ -14,7 +14,6 @@
 #define ShouldDisplayFavicon virtual ShouldDisplayFavicon
 #define TryToCloseWindow virtual TryToCloseWindow
 #define OnTabClosing virtual OnTabClosing
-#define TabStripEmpty virtual TabStripEmpty
 #define ResetTryToCloseWindow virtual ResetTryToCloseWindow
 #define FullscreenControllerInteractiveTest \
   FullscreenControllerInteractiveTest;      \
@@ -26,7 +25,6 @@
 #undef FullscreenControllerInteractiveTest
 #undef ResetTryToCloseWindow
 #undef TryToCloseWindow
-#undef TabStripEmpty
 #undef OnTabClosing
 #undef ShouldDisplayFavicon
 #undef ScheduleUIUpdate

diff --git a/chromium_src/chrome/browser/ui/tabs/tab_strip_model.h b/chromium_src/chrome/browser/ui/tabs/tab_strip_model.h
@@ -6,6 +6,7 @@
 #ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_TABS_TAB_STRIP_MODEL_H_
 #define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_TABS_TAB_STRIP_MODEL_H_
 
+#define CloseTabs virtual CloseTabs
 #define SelectRelativeTab virtual SelectRelativeTab
 #define TAB_STRIP_MODEL_H_ friend class BraveTabStripModel;
 #define IsReadLaterSupportedForAny virtual IsReadLaterSupportedForAny
@@ -14,6 +15,7 @@
 #include "src/chrome/browser/ui/tabs/tab_strip_model.h"  // IWYU pragma: export
 #undef IsReadLaterSupportedForAny
 #undef SelectRelativeTab
+#undef CloseTabs
 #undef TAB_STRIP_MODEL_H_
 #undef TabDragController
 

diff --git a/chromium_src/chrome/browser/ui/unload_controller.cc b/chromium_src/chrome/browser/ui/unload_controller.cc
diff --git a/chromium_src/chrome/browser/ui/unload_controller.h b/chromium_src/chrome/browser/ui/unload_controller.h