Skip to content

Commit

Permalink
Fixed detached window has unnecessary tabs
Browse files Browse the repository at this point in the history
fix brave/brave-browser#42588

When last tab is closed in the last tabbed browser, SessionService
updates related flag but it's not the actual last tab because another
new tab is created after that tab is closed when
"Close window when closing last tab" is turned off.
  • Loading branch information
simonhong committed Dec 4, 2024
1 parent 89750d5 commit b6e1800
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 b6e1800

Please sign in to comment.