From 1932bad13a0c80187d4157415b79b08563c9a0c1 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 8 Jan 2025 14:10:01 +0900 Subject: [PATCH] Fixed blank tab is shown after attaching split view fix https://github.com/brave/brave-browser/issues/41852 When split view transfers to another browser, its web contents should be cleared before attached to target browser. It's done by SplitView::UpdateContentsWebViewVisual(). So far, we posted it to next turn when we break tile. If we break a tile by unsplit, it doesn't cause any issue because any other secondary web view doesn't host that secondary web contents. However, if it's attached to another browser window, it's another story. When we detach a split view from browser A and attach to browser B, browser A breaks a tile after detached. And then, they are attached to browser B and we make them as a split view. If we don't clear secondary web contents after break the tile synchronously, browser A and browser B's secondary web view has same web contents till posed `UpdateContentsWebViewVisual(). This could cause unexpected behavior like above issue. So, secondary web view should be cleared synchronously after breaking a tile. --- browser/ui/views/frame/brave_browser_view.cc | 6 ++---- browser/ui/views/frame/brave_browser_view.h | 2 +- browser/ui/views/frame/split_view_browsertest.cc | 1 - 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/browser/ui/views/frame/brave_browser_view.cc b/browser/ui/views/frame/brave_browser_view.cc index f999c22b6e4c..c732d4c2d2bf 100644 --- a/browser/ui/views/frame/brave_browser_view.cc +++ b/browser/ui/views/frame/brave_browser_view.cc @@ -856,14 +856,12 @@ void BraveBrowserView::OnTileTabs(const TabTile& tile) { UpdateContentsWebViewVisual(); } -void BraveBrowserView::OnWillBreakTile(const TabTile& tile) { +void BraveBrowserView::OnDidBreakTile(const TabTile& tile) { if (!IsActiveWebContentsTiled(tile)) { return; } - base::SequencedTaskRunner::GetCurrentDefault()->PostTask( - FROM_HERE, base::BindOnce(&BraveBrowserView::UpdateContentsWebViewVisual, - weak_ptr_.GetWeakPtr())); + UpdateContentsWebViewVisual(); } void BraveBrowserView::OnSwapTabsInTile(const TabTile& tile) { diff --git a/browser/ui/views/frame/brave_browser_view.h b/browser/ui/views/frame/brave_browser_view.h index 40fbd122f9a2..dea453bd356f 100644 --- a/browser/ui/views/frame/brave_browser_view.h +++ b/browser/ui/views/frame/brave_browser_view.h @@ -131,7 +131,7 @@ class BraveBrowserView : public BrowserView, // SplitViewBrowserDataObserver: void OnTileTabs(const TabTile& tile) override; - void OnWillBreakTile(const TabTile& tile) override; + void OnDidBreakTile(const TabTile& tile) override; void OnSwapTabsInTile(const TabTile& tile) override; views::WebView* secondary_contents_web_view() { diff --git a/browser/ui/views/frame/split_view_browsertest.cc b/browser/ui/views/frame/split_view_browsertest.cc index b1aa7e46b9c3..36a57e5e7231 100644 --- a/browser/ui/views/frame/split_view_browsertest.cc +++ b/browser/ui/views/frame/split_view_browsertest.cc @@ -82,7 +82,6 @@ IN_PROC_BROWSER_TEST_F(SplitViewBrowserTest, split_view_data->IsTabTiled(tab_strip_model().GetTabHandleAt(0))); // Then, the secondary web view should become hidden - base::RunLoop().RunUntilIdle(); EXPECT_FALSE(secondary_contents_view().GetVisible()); }