Skip to content

Commit

Permalink
Merge pull request #26808 from brave/fix_tab_count_in_detached_window
Browse files Browse the repository at this point in the history
Fixed detached window has unnecessary tabs
  • Loading branch information
simonhong authored Dec 4, 2024
2 parents eb4eab4 + b6e1800 commit 209f0bc
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 0 deletions.
51 changes: 51 additions & 0 deletions browser/ui/brave_browser_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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());
}
19 changes: 19 additions & 0 deletions chromium_src/chrome/browser/sessions/session_service.cc
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions patches/chrome-browser-sessions-session_service.cc.patch
Original file line number Diff line number Diff line change
@@ -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;
}
}

0 comments on commit 209f0bc

Please sign in to comment.