From 80b33dcfe6f28bf7d3c7e74be009c60805359398 Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Wed, 19 Nov 2025 11:41:05 -0500 Subject: [PATCH] [Shields UI] Fix race condition when responding to repeated reloads (#32422) --- .../brave_shields_page_info_controller.cc | 11 ++++++++++- .../location_bar/brave_shields_page_info_controller.h | 2 ++ .../brave_page_info_bubble_view_browsertest.cc | 4 +++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/browser/ui/views/location_bar/brave_shields_page_info_controller.cc b/browser/ui/views/location_bar/brave_shields_page_info_controller.cc index bd51a71033d..0ac8b133911 100644 --- a/browser/ui/views/location_bar/brave_shields_page_info_controller.cc +++ b/browser/ui/views/location_bar/brave_shields_page_info_controller.cc @@ -6,6 +6,7 @@ #include "brave/browser/ui/views/location_bar/brave_shields_page_info_controller.h" #include "base/check_deref.h" +#include "base/task/sequenced_task_runner.h" #include "brave/browser/ui/views/page_info/brave_page_info_bubble_view.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/views/location_bar/location_icon_view.h" @@ -64,7 +65,15 @@ void BraveShieldsPageInfoController::OnTabStripModelChanged( void BraveShieldsPageInfoController::OnResourcesChanged() {} void BraveShieldsPageInfoController::OnRepeatedReloadsDetected() { - ShowBubbleForRepeatedReloads(); + // Post a task to show the page info bubble. Since this event occurs in + // response to a navigation finished event, and the page info bubble may close + // itself when a navigation finished event occurs, posting a task eliminates + // the possibility that the bubble will immediately close. + base::SequencedTaskRunner::GetCurrentDefault()->PostTask( + FROM_HERE, + base::BindOnce( + &BraveShieldsPageInfoController::ShowBubbleForRepeatedReloads, + weak_factory_.GetWeakPtr())); } void BraveShieldsPageInfoController::ShowBubbleForRepeatedReloads() { diff --git a/browser/ui/views/location_bar/brave_shields_page_info_controller.h b/browser/ui/views/location_bar/brave_shields_page_info_controller.h index db1724f7b74..0035662764b 100644 --- a/browser/ui/views/location_bar/brave_shields_page_info_controller.h +++ b/browser/ui/views/location_bar/brave_shields_page_info_controller.h @@ -7,6 +7,7 @@ #define BRAVE_BROWSER_UI_VIEWS_LOCATION_BAR_BRAVE_SHIELDS_PAGE_INFO_CONTROLLER_H_ #include "base/memory/raw_ref.h" +#include "base/memory/weak_ptr.h" #include "brave/browser/brave_shields/brave_shields_tab_helper.h" #include "chrome/browser/ui/tabs/tab_strip_model_observer.h" @@ -44,6 +45,7 @@ class BraveShieldsPageInfoController raw_ref tab_strip_model_; raw_ref location_icon_view_; + base::WeakPtrFactory weak_factory_{this}; }; #endif // BRAVE_BROWSER_UI_VIEWS_LOCATION_BAR_BRAVE_SHIELDS_PAGE_INFO_CONTROLLER_H_ diff --git a/browser/ui/views/page_info/brave_page_info_bubble_view_browsertest.cc b/browser/ui/views/page_info/brave_page_info_bubble_view_browsertest.cc index acf7360ae2e..48e0dec18a8 100644 --- a/browser/ui/views/page_info/brave_page_info_bubble_view_browsertest.cc +++ b/browser/ui/views/page_info/brave_page_info_bubble_view_browsertest.cc @@ -5,6 +5,7 @@ #include "brave/browser/ui/views/page_info/brave_page_info_bubble_view.h" +#include "base/test/run_until.h" #include "base/test/scoped_feature_list.h" #include "brave/browser/ui/page_info/features.h" #include "brave/browser/ui/views/page_info/brave_page_info_view_ids.h" @@ -336,8 +337,9 @@ IN_PROC_BROWSER_TEST_F(BravePageInfoBubbleViewRepeatedReloadsBrowserTest, reload(); EXPECT_FALSE(GetBubbleView()); - // The second reload should open the bubble with the Shield tab visible. + // The second reload should open the bubble with the Shields tab visible. reload(); + ASSERT_TRUE(base::test::RunUntil([] { return GetBubbleView(); })); auto* bubble_view = GetBubbleView(); ASSERT_TRUE(bubble_view); EXPECT_TRUE(IsShieldsViewDrawn(bubble_view));