Discharge7443/brave-core
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove minimize/restore from split view modal dialog test (#34347)

Browse Source 
* Fix split view tab_is_active_ correction for permission manager

The BRAVE_PERMISSION_REQUEST_MANAGER_ON_VISIBILITY_CHANGED macro that
calls UpdateTabIsHiddenWithTabActivationState() is unreachable on
desktop because OnVisibilityChanged() returns early when
tab_subscriptions_ is not empty. This means OnTabActiveStateChanged()
was calling OnVisibilityChanged() expecting the correction to run, but
it never did on desktop.

Additionally, UpdateTabIsHiddenWithTabActivationState() only corrected
tab_is_active_ in one direction (true->false for inactive split tabs)
but not the reverse (false->true for the active split tab).

Fix by calling UpdateTabIsHiddenWithTabActivationState() directly from
OnTabActiveStateChanged() instead of through the unreachable macro, and
make the correction bidirectional so tab_is_active_ always matches the
split view activation state.

Resolves https://github.com/brave/brave-browser/issues/53276

* Fix IsWebContentsVisible for split view dialog visibility

Address review feedback: the test failure is about web modal dialog
visibility, not permission bubbles. The root cause is that after
Minimize()+Restore(), platform_util::IsVisible() can temporarily
return false for the active split tab. This caused the Brave macro
in WebContentsModalDialogManager::OnVisibilityChanged() to swallow
the HIDDEN->VISIBLE transition, preventing ShowNextDialog() from
being called.

Fix: Use tab->IsActivated() as the source of truth for split view
tabs in BraveBrowser::IsWebContentsVisible(), which is always
accurate regardless of platform visibility timing. Revert the
PermissionRequestManager changes from the previous attempt.

Resolves brave/brave-browser#53276

* Updated test code

Removed window minimize/restored state during the test.
This window change is not important factor for split tab's
modal dialog test.
Claude code suspects that minimize/retored state change could
make platform_util::IsVisible(). Let's see this intermittent test
failure happens again w/o window state change.

---------

Co-authored-by: Simon Hong <shong@brave.com>
This commit is contained in:
Netzenbot
2026-03-04 17:43:26 -05:00
committed by GitHub
co-authored by Simon Hong
parent f6e265e97b
commit cf867366ac
1 changed files with 0 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
browser/ui/views/split_view/split_view_browsertest.cc
-13
View File
@@ -752,19 +752,6 @@ IN_PROC_BROWSER_TEST_F(SplitViewCommonBrowserTest, InactiveSplitTabTest) {
[&]() { return !GetIsTabHiddenFromPermissionManagerFromTabAt(1); }));
EXPECT_TRUE(GetIsTabHiddenFromPermissionManagerFromTabAt(2));
// Check that the proper state is set after the browser window is restored
// from minimized state.
browser()->window()->Minimize();
browser()->window()->Restore();
EXPECT_TRUE(GetIsTabHiddenFromPermissionManagerFromTabAt(0));
ASSERT_TRUE(base::test::RunUntil(
[&]() { return !GetIsTabHiddenFromPermissionManagerFromTabAt(1); }));
EXPECT_TRUE(GetIsTabHiddenFromPermissionManagerFromTabAt(2));
EXPECT_TRUE(tab_strip_model->GetTabAtIndex(1)->IsActivated());
EXPECT_FALSE(GetIsWebContentsBlockedFromTabAt(0));
EXPECT_FALSE(GetIsWebContentsBlockedFromTabAt(1));
// Launch dialog from inactive split tab (at 0).
bool did_suppress = false;
GetTabModalDialogManagerAt(0)->RunJavaScriptDialog(