From 68624bc8beddc1de631db91ae7ebca787dfe76a2 Mon Sep 17 00:00:00 2001 From: Claudio DeSouza Date: Thu, 16 Apr 2026 11:01:24 +0100 Subject: [PATCH] [cr149] `GetAppMenuControl()` removed from `ToolbarView` We are now accessing this value through a private accessor with the parent class. Chromium changes: https://chromium.googlesource.com/chromium/src/+/0800caaa61eb1b551c622e41025def941ea6f0c9 commit 0800caaa61eb1b551c622e41025def941ea6f0c9 Author: Youssef Bourouphael Date: Tue Apr 14 06:30:17 2026 -0700 Refactor: Extract AppMenuControl interface This change extracts the core functionalities of the app menu button into a new `AppMenuControl` interface. - Introduced `AppMenuControl` interface defining methods to interact with the app menu button. - Updated `AppMenuButton` to implement the new `AppMenuControl` interface. - Modified `ToolbarButtonProvider` to expose the `AppMenuControl` interface instead of directly returning `AppMenuButton`. - Migrated all call sites that previously accessed `AppMenuButton` directly to use the `AppMenuControl` interface. - Updated bubble anchoring logic to utilize `views::BubbleAnchor`, allowing for more flexible anchoring options. - Many existing calls to `GetAppMenuButton()` were migrated to use `views::ElementTrackerViews` to get the button view. This refactoring is a prerequisite for future work on a Web UI version of the app menu. Bug: 470045312 Change-Id: I01d722ad76687e0f1e409f04a4feef4d0386540e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7722207 Reviewed-by: Paul Jensen Reviewed-by: Eshwar Stalin Commit-Queue: Youseff Bourouphel Cr-Commit-Position: refs/heads/main@{#1614401} --- browser/ui/views/toolbar/brave_app_menu.cc | 5 +++-- browser/ui/views/toolbar/brave_app_menu.h | 5 ++++- browser/ui/views/toolbar/brave_toolbar_view.cc | 17 +++++++++-------- .../toolbar/brave_toolbar_view_browsertest.cc | 6 +++--- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/browser/ui/views/toolbar/brave_app_menu.cc b/browser/ui/views/toolbar/brave_app_menu.cc index b19c13dd1fb..2b31a2ab5d5 100644 --- a/browser/ui/views/toolbar/brave_app_menu.cc +++ b/browser/ui/views/toolbar/brave_app_menu.cc @@ -193,8 +193,9 @@ SidebarShowOptionMenu::SidebarShowOptionMenu(BraveAppMenu* app_menu, BraveAppMenu::BraveAppMenu(Browser* browser, ui::MenuModel* model, - int run_types) - : AppMenu(browser, model, run_types), + int run_types, + base::RepeatingClosure on_menu_closed_callback) + : AppMenu(browser, model, run_types, std::move(on_menu_closed_callback)), menu_metrics_( g_brave_browser_process->process_misc_metrics()->menu_metrics()) { DCHECK(menu_metrics_); diff --git a/browser/ui/views/toolbar/brave_app_menu.h b/browser/ui/views/toolbar/brave_app_menu.h index 4599751c1b6..ef76480cf5d 100644 --- a/browser/ui/views/toolbar/brave_app_menu.h +++ b/browser/ui/views/toolbar/brave_app_menu.h @@ -12,7 +12,10 @@ class BraveAppMenu : public AppMenu { public: - BraveAppMenu(Browser* browser, ui::MenuModel* model, int run_types); + BraveAppMenu(Browser* browser, + ui::MenuModel* model, + int run_types, + base::RepeatingClosure on_menu_closed_callback); ~BraveAppMenu() override; BraveAppMenu(const BraveAppMenu&) = delete; diff --git a/browser/ui/views/toolbar/brave_toolbar_view.cc b/browser/ui/views/toolbar/brave_toolbar_view.cc index c7a4940f202..a2864fc9936 100644 --- a/browser/ui/views/toolbar/brave_toolbar_view.cc +++ b/browser/ui/views/toolbar/brave_toolbar_view.cc @@ -39,6 +39,7 @@ #include "chrome/browser/ui/ui_features.h" #include "chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h" #include "chrome/browser/ui/views/frame/browser_view.h" +#include "chrome/browser/ui/views/toolbar/browser_app_menu_button.h" #include "chrome/browser/ui/views/toolbar/toolbar_button.h" #include "chrome/browser/ui/views/toolbar/toolbar_divider.h" #include "chrome/browser/ui/views/toolbar/toolbar_view.h" @@ -324,13 +325,13 @@ void BraveToolbarView::Init() { SetBraveButtonFlexBehavior(bookmark_); side_panel_ = AddChildViewAt(std::make_unique(browser()), - *GetIndexOf(GetAppMenuButton()) - 1); + *GetIndexOf(app_menu_button()) - 1); SetBraveButtonFlexBehavior(side_panel_); #if BUILDFLAG(ENABLE_BRAVE_WALLET) - wallet_ = AddChildViewAt( - std::make_unique(GetAppMenuButton(), profile), - *GetIndexOf(GetAppMenuButton()) - 1); + wallet_ = + AddChildViewAt(std::make_unique(app_menu_button(), profile), + *GetIndexOf(app_menu_button()) - 1); wallet_->SetTriggerableEventFlags(ui::EF_LEFT_MOUSE_BUTTON | ui::EF_MIDDLE_MOUSE_BUTTON); wallet_->UpdateImageAndText(); @@ -344,7 +345,7 @@ void BraveToolbarView::Init() { // setup a watcher for policy pref. if (ai_chat::IsAllowedForContext(browser_->profile(), false)) { ai_chat_button_ = AddChildViewAt(std::make_unique(browser()), - *GetIndexOf(GetAppMenuButton()) - 1); + *GetIndexOf(app_menu_button()) - 1); SetBraveButtonFlexBehavior(ai_chat_button_); show_ai_chat_button_.Init( ai_chat::prefs::kBraveAIChatShowToolbarButton, @@ -362,7 +363,7 @@ void BraveToolbarView::Init() { #if BUILDFLAG(ENABLE_BRAVE_VPN) if (brave_vpn::BraveVpnServiceFactory::GetForProfile(profile)) { brave_vpn_ = AddChildViewAt(std::make_unique(browser()), - *GetIndexOf(GetAppMenuButton()) - 1); + *GetIndexOf(app_menu_button()) - 1); SetBraveButtonFlexBehavior(brave_vpn_); show_brave_vpn_button_.Init( brave_vpn::prefs::kBraveVPNShowButton, profile->GetPrefs(), @@ -378,7 +379,7 @@ void BraveToolbarView::Init() { // Make sure that avatar button should be located right before the app menu. if (auto* avatar = GetAvatarToolbarButton()) { - ReorderChildView(avatar, *GetIndexOf(GetAppMenuButton()) - 1); + ReorderChildView(avatar, *GetIndexOf(app_menu_button()) - 1); } if (tabs::utils::SupportsBraveVerticalTabs(browser_)) { @@ -670,7 +671,7 @@ void BraveToolbarView::UpdateVerticalTabTogglePlacement() { size_t target_idx = 0; if (place_near_app_menu) { - const auto menu_idx = GetIndexOf(GetAppMenuButton()); + const auto menu_idx = GetIndexOf(app_menu_button()); CHECK(menu_idx.has_value()); CHECK_GT(*menu_idx, 0u); target_idx = *menu_idx - 1; diff --git a/browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc b/browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc index a9d790ae38b..8fd4387eef2 100644 --- a/browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc +++ b/browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc @@ -511,7 +511,7 @@ IN_PROC_BROWSER_TEST_F(BraveToolbarViewTest, // Check avatar is positioned at the right before app menu button. views::View* avatar = toolbar_button_provider_->GetAvatarToolbarButton(); ASSERT_TRUE(!!avatar); - views::View* app_menu = toolbar_button_provider_->GetAppMenuButton(); + views::View* app_menu = toolbar_view_->app_menu_button(); ASSERT_TRUE(!!app_menu); EXPECT_EQ(toolbar_view_->GetIndexOf(avatar).value(), toolbar_view_->GetIndexOf(app_menu).value() - 1ul); @@ -631,7 +631,7 @@ IN_PROC_BROWSER_TEST_F(BraveToolbarViewTest, ASSERT_TRUE(ix_on_right.has_value()) << "toggle missing from toolbar (kVerticalTabsOnRight=true)"; - views::View* menu = toolbar_button_provider_->GetAppMenuButton(); + views::View* menu = toolbar_view_->app_menu_button(); ASSERT_TRUE(menu); const auto menu_ix = toolbar_view_->GetIndexOf(menu); ASSERT_TRUE(menu_ix.has_value() && *menu_ix > 0) @@ -677,7 +677,7 @@ IN_PROC_BROWSER_TEST_F(BraveToolbarViewRTLTest, auto* toggle = toolbar_view_->vertical_tab_toggle_button(); ASSERT_TRUE(toggle) << "vertical tabs enabled in RTL with on-left pref"; - views::View* menu = toolbar_button_provider_->GetAppMenuButton(); + views::View* menu = toolbar_view_->app_menu_button(); ASSERT_TRUE(menu); const auto menu_ix_left = toolbar_view_->GetIndexOf(menu); ASSERT_TRUE(menu_ix_left.has_value() && *menu_ix_left > 0)