Merge pull request #26808 from brave/fix_tab_count_in_detached_window

Fixed detached window has unnecessary tabs
2024-12-04 13:44:30 +09:00
parent eb4eab4471 b6e180056b
commit 209f0bcc49
3 changed files with 82 additions and 0 deletions
@@ -10,7 +10,9 @@
 #include "brave/components/constants/pref_names.h"
 #include "chrome/browser/devtools/devtools_window_testing.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/startup/launch_mode_recorder.h"
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
@@ -161,3 +163,52 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserBrowserTest,
  EXPECT_TRUE(
      base::test::RunUntil([] { return chrome::GetTotalBrowserCount() == 1; }));
 }
+
+IN_PROC_BROWSER_TEST_F(BraveBrowserBrowserTest,
+                       CreateAnotherWindowWithExistingTab) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  browser()->profile()->GetPrefs()->SetBoolean(kEnableClosingLastTab, false);
+  TabStripModel* tab_strip = browser()->tab_strip_model();
+
+  auto page_url = embedded_test_server()->GetURL("/empty.html");
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), page_url));
+
+  ASSERT_EQ(1, tab_strip->count());
+  EXPECT_EQ(page_url,
+            tab_strip->GetWebContentsAt(0)->GetURL().possibly_invalid_spec());
+
+  // Close the last tab.
+  tab_strip->GetActiveWebContents()->Close();
+  ASSERT_EQ(0, tab_strip->count());
+
+  // Wait till another new tab is opened.
+  EXPECT_TRUE(
+      base::test::RunUntil([tab_strip] { return tab_strip->count() == 1; }));
+  EXPECT_EQ(browser()->GetNewTabURL(),
+            tab_strip->GetWebContentsAt(0)->GetURL().possibly_invalid_spec());
+
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GURL("https://www.brave.com/"),
+      WindowOpenDisposition::NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP);
+  ASSERT_EQ(2, tab_strip->count());
+
+  // Create another browser with existing tab.
+  chrome::MoveTabsToNewWindow(browser(), {1});
+  ASSERT_EQ(1, tab_strip->count());
+
+  // Get new browser.
+  Browser* new_browser = nullptr;
+  for (Browser* b : *BrowserList::GetInstance()) {
+    if (b != browser()) {
+      new_browser = b;
+      break;
+    }
+  }
+  ASSERT_TRUE(new_browser);
+  base::RunLoop().RunUntilIdle();
+
+  // Check new browser by detaching a tab from another window has
+  // one tab.
+  EXPECT_EQ(1, new_browser->tab_strip_model()->count());
+}
@@ -0,0 +1,19 @@
+/* Copyright (c) 2024 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+#include "brave/components/constants/pref_names.h"
+
+// Prevent detached tab has unnecessary tab restore steps.
+// When last window's last tab is closed, |has_open_trackable_browsers_|
+// becomes false. If we turn off "Close window when closing last tab" option,
+// another new tab is created after last tab is closed. So, it's not the last
+// actually. It could make ShouldRestore() give true when creating
+// new browser by detaching a tab.
+#define BRAVE_SESSION_SERVICE_TAB_CLOSED \
+  if (profile()->GetPrefs()->GetBoolean(kEnableClosingLastTab))
+
+#include "src/chrome/browser/sessions/session_service.cc"
+
+#undef BRAVE_SESSION_SERVICE_TAB_CLOSED
@@ -0,0 +1,12 @@
+diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc
+index 69e538baf515892624b9505a00f19268d6d6e5c7..4b120111dd92f971809e1ded4ce51a46f8ea722a 100644
+--- a/chrome/browser/sessions/session_service.cc
+++ b/chrome/browser/sessions/session_service.cc
+@@ -371,6 +371,7 @@ void SessionService::TabClosed(SessionID window_id, SessionID tab_id) {
+               window_id) == window_closing_ids_.end()) &&
+         IsOnlyOneTabLeft()) {
+       // This is the last tab in the last tabbed browser.
+      BRAVE_SESSION_SERVICE_TAB_CLOSED
+       has_open_trackable_browsers_ = false;
+     }
+   }