Re-order brave menus at app menu

Insert & reorder brave menus based on corresponding commands enable status.
If we want to add/remove from app menu, adjust enable commands status
instead of adjusting it in app menu model directly.
App menu model will add/delete based on commands status.

Fix brave/brave-browser#5552

browser/ui/brave_browser_command_controller.cc

@@ -141,10 +141,11 @@ void BraveBrowserCommandController::UpdateCommandForWebcompatReporter() {
 
 #if BUILDFLAG(ENABLE_TOR)
 void BraveBrowserCommandController::UpdateCommandForTor() {
- const bool is_tor_enabled =
- !brave::IsTorDisabledForProfile(browser_->profile());
- UpdateCommandEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE, is_tor_enabled);
- UpdateCommandEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR, is_tor_enabled);
+ // Enable new tor connection only for tor profile.
+ UpdateCommandEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE,
+ brave::IsTorProfile(browser_->profile()));
+ UpdateCommandEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR,
+ !brave::IsTorDisabledForProfile(browser_->profile()));
 }
 #endif
 
@@ -153,7 +154,9 @@ void BraveBrowserCommandController::UpdateCommandForBraveSync() {
 }
 
 void BraveBrowserCommandController::UpdateCommandForBraveWallet() {
- UpdateCommandEnabled(IDC_SHOW_BRAVE_WALLET, true);
+ UpdateCommandEnabled(
+ IDC_SHOW_BRAVE_WALLET,
+ browser_->profile()->GetPrefs()->GetBoolean(kBraveWalletEnabled));
 }
 
 bool BraveBrowserCommandController::ExecuteBraveCommandWithDisposition(

browser/ui/toolbar/brave_app_menu_model.cc

@@ -5,24 +5,10 @@
 
 #include "brave/browser/ui/toolbar/brave_app_menu_model.h"
 
-#include "brave/app/brave_command_ids.h"
-#include "brave/browser/profiles/profile_util.h"
-#include "brave/common/pref_names.h"
-#include "brave/components/brave_wallet/browser/buildflags/buildflags.h"
-#include "brave/grit/brave_generated_resources.h"
 #include "chrome/app/chrome_command_ids.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/browser.h"
-#include "components/prefs/pref_service.h"
+#include "chrome/grit/generated_resources.h"
 
-BraveAppMenuModel::BraveAppMenuModel(
- ui::AcceleratorProvider* provider,
- Browser* browser,
- AppMenuIconController* app_menu_icon_controller)
- : AppMenuModel(provider, browser, app_menu_icon_controller),
- browser_(browser) {}
-
-BraveAppMenuModel::~BraveAppMenuModel() {}
+BraveAppMenuModel::~BraveAppMenuModel() = default;
 
 void BraveAppMenuModel::Build() {
  // Insert brave items after build chromium items.
@@ -31,62 +17,144 @@ void BraveAppMenuModel::Build() {
 }
 
 void BraveAppMenuModel::InsertBraveMenuItems() {
- // Sync & Rewards pages are redirected to normal window when it is loaded in
- // private window. So, only hide them in guest(tor) window.
- if (!browser_->profile()->IsGuestSession()) {
- bool walletEnabled = browser_->profile()->GetPrefs()->
- GetBoolean(kBraveWalletEnabled);
- InsertItemWithStringIdAt(
- GetIndexOfCommandId(IDC_SHOW_DOWNLOADS),
- IDC_SHOW_BRAVE_REWARDS,
- IDS_SHOW_BRAVE_REWARDS);
- if (walletEnabled) {
-#if BUILDFLAG(BRAVE_WALLET_ENABLED)
- InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_SHOW_BRAVE_REWARDS),
- IDC_SHOW_BRAVE_WALLET, IDS_SHOW_BRAVE_WALLET);
-#endif
- }
- InsertItemWithStringIdAt(
-#if BUILDFLAG(BRAVE_WALLET_ENABLED)
- GetIndexOfCommandId(walletEnabled ? IDC_SHOW_BRAVE_WALLET :
- IDC_SHOW_BRAVE_REWARDS),
-#else
- GetIndexOfCommandId(IDC_SHOW_BRAVE_REWARDS),
-#endif
- IDC_SHOW_BRAVE_SYNC, IDS_SHOW_BRAVE_SYNC);
- }
+ // Insert & reorder brave menus based on corresponding commands enable status.
+ // If we you want to add/remove from app menu, adjust commands enable status
+ // at BraveBrowserCommandController.
 
- // Insert Create New Profile item
- auto profile_items_insert_index = GetIndexOfCommandId(IDC_ZOOM_MENU) - 1;
- InsertItemWithStringIdAt(profile_items_insert_index,
- IDC_OPEN_GUEST_PROFILE,
- IDS_OPEN_GUEST_PROFILE);
- InsertItemWithStringIdAt(
- GetIndexOfCommandId(IDC_OPEN_GUEST_PROFILE),
- IDC_ADD_NEW_PROFILE,
- IDS_ADD_NEW_PROFILE);
- InsertSeparatorAt(
- GetIndexOfCommandId(IDC_ADD_NEW_PROFILE), ui::NORMAL_SEPARATOR);
-
- InsertItemWithStringIdAt(
- GetIndexOfCommandId(IDC_SHOW_DOWNLOADS),
- IDC_SHOW_BRAVE_ADBLOCK,
- IDS_SHOW_BRAVE_ADBLOCK);
- if (brave::IsTorProfile(browser_->profile())) {
+ // Step 1. Configure tab & windows section.
+ if (IsCommandIdEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE)) {
  InsertItemWithStringIdAt(
  GetIndexOfCommandId(IDC_NEW_WINDOW),
  IDC_NEW_TOR_CONNECTION_FOR_SITE,
  IDS_NEW_TOR_CONNECTION_FOR_SITE);
  }
-
  if (IsCommandIdEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR)) {
  InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_NEW_INCOGNITO_WINDOW) + 1,
  IDC_NEW_OFFTHERECORD_WINDOW_TOR,
  IDS_NEW_OFFTHERECORD_WINDOW_TOR);
  }
 
- InsertItemWithStringIdAt(
- GetIndexOfCommandId(IDC_ABOUT),
- IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER,
- IDS_SHOW_BRAVE_WEBCOMPAT_REPORTER);
+ // Step 2. Configure second section that includes history, downloads and
+ // bookmark. Then, insert brave items.
+
+ // First, reorder original menus We want to move them in order of bookmark,
+ // download and extensions.
+ const int bookmark_item_index = GetIndexOfCommandId(IDC_BOOKMARKS_MENU);
+ // If bookmark is not used, we don't need to adjust download item.
+ if (bookmark_item_index != -1) {
+ // Place download menu under bookmark.
+ DCHECK(IsCommandIdEnabled(IDC_SHOW_DOWNLOADS));
+ RemoveItemAt(GetIndexOfCommandId(IDC_SHOW_DOWNLOADS));
+ InsertItemWithStringIdAt(bookmark_item_index,
+ IDC_SHOW_DOWNLOADS,
+ IDS_SHOW_DOWNLOADS);
+ }
+ // Move extensions menu under download.
+ ui::SimpleMenuModel* model = static_cast<ui::SimpleMenuModel*>(
+ GetSubmenuModelAt(GetIndexOfCommandId(IDC_MORE_TOOLS_MENU)));
+ DCHECK(model);
+ model->RemoveItemAt(model->GetIndexOfCommandId(IDC_MANAGE_EXTENSIONS));
+
+ if (IsCommandIdEnabled(IDC_MANAGE_EXTENSIONS)) {
+ InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_SHOW_DOWNLOADS) + 1,
+ IDC_MANAGE_EXTENSIONS,
+ IDS_SHOW_EXTENSIONS);
+ }
+
+ if (IsCommandIdEnabled(IDC_SHOW_BRAVE_REWARDS)) {
+ InsertItemWithStringIdAt(GetIndexOfBraveRewardsItem(),
+ IDC_SHOW_BRAVE_REWARDS,
+ IDS_SHOW_BRAVE_REWARDS);
+ }
+
+ // Insert wallet menu after download menu.
+ if (IsCommandIdEnabled(IDC_SHOW_BRAVE_WALLET)) {
+ InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_SHOW_DOWNLOADS) + 1,
+ IDC_SHOW_BRAVE_WALLET,
+ IDS_SHOW_BRAVE_WALLET);
+ }
+
+ // Insert sync menu
+ if (IsCommandIdEnabled(IDC_SHOW_BRAVE_SYNC)) {
+ InsertItemWithStringIdAt(GetIndexOfBraveSyncItem(),
+ IDC_SHOW_BRAVE_SYNC,
+ IDS_SHOW_BRAVE_SYNC);
+ }
+
+ // Insert adblock menu at last. Assumed this is always enabled.
+ DCHECK(IsCommandIdEnabled(IDC_SHOW_BRAVE_ADBLOCK));
+ InsertItemWithStringIdAt(GetIndexOfBraveAdBlockItem(),
+ IDC_SHOW_BRAVE_ADBLOCK,
+ IDS_SHOW_BRAVE_ADBLOCK);
+
+ // Insert Create New Profile item
+ if (IsCommandIdEnabled(IDC_OPEN_GUEST_PROFILE)) {
+ InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_ZOOM_MENU) - 1,
+ IDC_OPEN_GUEST_PROFILE,
+ IDS_OPEN_GUEST_PROFILE);
+ InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_OPEN_GUEST_PROFILE),
+ IDC_ADD_NEW_PROFILE,
+ IDS_ADD_NEW_PROFILE);
+ InsertSeparatorAt(GetIndexOfCommandId(IDC_ADD_NEW_PROFILE),
+ ui::NORMAL_SEPARATOR);
+ }
+
+ // Insert webcompat reporter item.
+ InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_ABOUT),
+ IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER,
+ IDS_SHOW_BRAVE_WEBCOMPAT_REPORTER);
+}
+
+int BraveAppMenuModel::GetIndexOfBraveAdBlockItem() const {
+ // Insert as a last item in second section.
+ int adblock_item_index = -1;
+ adblock_item_index = GetIndexOfCommandId(IDC_SHOW_BRAVE_SYNC);
+ if (adblock_item_index != -1)
+ return adblock_item_index + 1;
+
+ adblock_item_index = GetIndexOfCommandId(IDC_MANAGE_EXTENSIONS);
+ if (adblock_item_index != -1)
+ return adblock_item_index + 1;
+
+ adblock_item_index = GetIndexOfCommandId(IDC_SHOW_BRAVE_WALLET);
+ if (adblock_item_index != -1)
+ return adblock_item_index + 1;
+
+ adblock_item_index = GetIndexOfCommandId(IDC_SHOW_DOWNLOADS);
+ DCHECK_NE(-1, adblock_item_index) << "No download item";
+ return adblock_item_index + 1;
+}
+
+int BraveAppMenuModel::GetIndexOfBraveRewardsItem() const {
+ // Insert rewards menu at first of this section. If history menu is not
+ // available, check below items.
+ int rewards_index = -1;
+ rewards_index = GetIndexOfCommandId(IDC_RECENT_TABS_MENU);
+ if (rewards_index != -1)
+ return rewards_index;
+
+ rewards_index = GetIndexOfCommandId(IDC_BOOKMARKS_MENU);
+ if (rewards_index != -1)
+ return rewards_index;
+
+ rewards_index = GetIndexOfCommandId(IDC_SHOW_DOWNLOADS);
+ DCHECK_NE(-1, rewards_index) << "No download item";
+ return rewards_index;
+}
+
+int BraveAppMenuModel::GetIndexOfBraveSyncItem() const {
+ // Insert sync menu under extensions menu. If extensions menu is not
+ // available, check above items.
+ int sync_index = -1;
+ sync_index = GetIndexOfCommandId(IDC_MANAGE_EXTENSIONS);
+ if (sync_index != -1)
+ return sync_index + 1;
+
+ sync_index = GetIndexOfCommandId(IDC_SHOW_BRAVE_WALLET);
+ if (sync_index != -1)
+ return sync_index + 1;
+
+ sync_index = GetIndexOfCommandId(IDC_SHOW_DOWNLOADS);
+ DCHECK_NE(-1, sync_index) << "No download item";
+ return sync_index + 1;
 }

browser/ui/toolbar/brave_app_menu_model.h

@@ -10,20 +10,20 @@
 
 class BraveAppMenuModel : public AppMenuModel {
  public:
- BraveAppMenuModel(ui::AcceleratorProvider* provider,
- Browser* browser,
- AppMenuIconController* app_menu_icon_controller = nullptr);
+ using AppMenuModel::AppMenuModel;
  ~BraveAppMenuModel() override;
 
+ BraveAppMenuModel(const BraveAppMenuModel&) = delete;
+ BraveAppMenuModel& operator=(const BraveAppMenuModel&) = delete;
+
  private:
  // AppMenuModel overrides:
  void Build() override;
 
  void InsertBraveMenuItems();
-
- Browser* const browser_; // weak
-
- DISALLOW_COPY_AND_ASSIGN(BraveAppMenuModel);
+ int GetIndexOfBraveRewardsItem() const;
+ int GetIndexOfBraveAdBlockItem() const;
+ int GetIndexOfBraveSyncItem() const;
 };
 
 #endif // BRAVE_BROWSER_UI_TOOLBAR_BRAVE_APP_MENU_MODEL_H_

browser/ui/toolbar/brave_app_menu_model_browsertest.cc

@@ -52,8 +52,8 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, BasicTest) {
 #if BUILDFLAG(ENABLE_TOR)
  EXPECT_NE(
  -1, normal_model.GetIndexOfCommandId(IDC_NEW_OFFTHERECORD_WINDOW_TOR));
- // Check tor browser commands are disabled.
- EXPECT_TRUE(
+ // Check new tor browser command is only enabled for normal window.
+ EXPECT_FALSE(
  command_controller->IsCommandEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE));
  EXPECT_TRUE(
  command_controller->IsCommandEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR));