Fixed crash when running extension item from app menu in private wind…

…ow (uplift to 1.62.x) (#21339)

Uplift of #21331 (squashed) to beta

app/brave_main_delegate_browsertest.cc

@@ -192,7 +192,6 @@ IN_PROC_BROWSER_TEST_F(BraveMainDelegateBrowserTest, DisabledFeatures) {
 #endif
     &features::kDigitalGoodsApi,
     &features::kDIPS,
-    &features::kExtensionsMenuInAppMenu,
     &features::kFedCm,
     &features::kFedCmWithoutThirdPartyCookies,
     &features::kFirstPartySets,

browser/ui/toolbar/app_menu_icons.cc

@@ -37,6 +37,7 @@ const std::map<int, const gfx::VectorIcon&>& GetCommandIcons() {
           {IDC_VIEW_PASSWORDS, kLeoKeyIcon},
           {IDC_SHOW_DOWNLOADS, kLeoDownloadIcon},
           {IDC_MANAGE_EXTENSIONS, kLeoBrowserExtensionsIcon},
+          {IDC_EXTENSIONS_SUBMENU_MANAGE_EXTENSIONS, kLeoBrowserExtensionsIcon},
           {IDC_ZOOM_MENU, kLeoSearchZoomInIcon},
           {IDC_PRINT, kLeoPrintIcon},
           {IDC_MORE_TOOLS_MENU, kLeoWindowScrewdriverIcon},

browser/ui/toolbar/brave_app_menu_model.cc

@@ -296,10 +296,22 @@ void BraveAppMenuModel::BuildBrowserSection() {
                              IDS_SHOW_DOWNLOADS);
   }
 
+  // Use this command's enabled state to not having it in guest window.
+  // It's disabled in guest window. Upstream's guest window has extensions
+  // menu in app menu, but we hide it.
   if (IsCommandIdEnabled(IDC_MANAGE_EXTENSIONS)) {
+    // Upstream enabled extensions submenu by default.
+    CHECK(features::IsExtensionMenuInRootAppMenu());
+
+    // Use IDC_EXTENSIONS_SUBMENU_MANAGE_EXTENSIONS instead of
+    // IDC_MANAGE_EXTENSIONS because executing it from private(tor) window
+    // causes crash as LogSafetyHubInteractionMetrics() tries to refer
+    // SafetyHubMenuNotificationService. But it's not instantiated in private
+    // window. Upstream also has this crash if ExtensionsMenuInAppMenu feature
+    // is disabled.
     InsertItemWithStringIdAt(
         GetIndexOfCommandId(IDC_SHOW_DOWNLOADS).value() + 1,
-        IDC_MANAGE_EXTENSIONS, IDS_SHOW_EXTENSIONS);
+        IDC_EXTENSIONS_SUBMENU_MANAGE_EXTENSIONS, IDS_SHOW_EXTENSIONS);
   }
 }
 
@@ -384,19 +396,11 @@ void BraveAppMenuModel::RemoveUpstreamMenus() {
   DCHECK(more_tools_model);
 
   // Remove upstream's extensions item. It'll be added into top level third
-  // section.
-  if (base::FeatureList::IsEnabled(features::kExtensionsMenuInAppMenu) ||
-      features::IsChromeRefresh2023()) {
-    // Hide extensions sub menu.
-    DCHECK(GetIndexOfCommandId(IDC_EXTENSIONS_SUBMENU).has_value());
-    RemoveItemAt(GetIndexOfCommandId(IDC_EXTENSIONS_SUBMENU).value());
-  } else {
-    // Hide extensions item from more tools sub menu.
-    DCHECK(more_tools_model->GetIndexOfCommandId(IDC_MANAGE_EXTENSIONS)
-               .has_value());
-    more_tools_model->RemoveItemAt(
-        more_tools_model->GetIndexOfCommandId(IDC_MANAGE_EXTENSIONS).value());
-  }
+  // section. Upstream enabled extensions submenu by default.
+  CHECK(features::IsExtensionMenuInRootAppMenu());
+  // Hide extensions sub menu.
+  DCHECK(GetIndexOfCommandId(IDC_EXTENSIONS_SUBMENU).has_value());
+  RemoveItemAt(GetIndexOfCommandId(IDC_EXTENSIONS_SUBMENU).value());
 
   // Remove upstream's cast item. It'll be added into more tools sub menu.
   if (media_router::MediaRouterEnabled(browser()->profile())) {
@@ -494,6 +498,11 @@ bool BraveAppMenuModel::IsCommandIdEnabled(int id) const {
              IsIpfsServiceLaunched(browser_context);
   }
 #endif
+  if (id == IDC_EXTENSIONS_SUBMENU_MANAGE_EXTENSIONS) {
+    // Always returns true as this command id is only added when it could be
+    // used.
+    return true;
+  }
   return AppMenuModel::IsCommandIdEnabled(id);
 }
 

browser/ui/toolbar/brave_app_menu_model.h

@@ -37,6 +37,7 @@ class BraveAppMenuModel : public AppMenuModel {
 
  private:
   FRIEND_TEST_ALL_PREFIXES(BraveAppMenuModelBrowserTest, BraveIpfsMenuTest);
+  friend class BraveAppMenuModelBrowserTest;
 
   // AppMenuModel overrides:
   void Build() override;

browser/ui/toolbar/brave_app_menu_model_browsertest.cc

@@ -90,6 +90,13 @@ class BraveAppMenuModelBrowserTest : public InProcessBrowserTest {
     ipfs_service_->GetIpnsKeysManager()->SetKeysForTest(keys);
   }
 #endif
+
+  void RunCommandFromAppMenuModel(Browser* browser, int command_id) {
+    auto* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
+    BraveAppMenuModel model(browser_view->toolbar(), browser);
+    model.Init();
+    model.ExecuteCommand(command_id, /*event_flags=*/0);
+  }
 };
 
 void CheckCommandsAreDisabledInMenuModel(
@@ -190,6 +197,11 @@ void CheckHistoryCommandsAreInOrderInMenuModel(
   CheckCommandsAreInOrderInMenuModel(history_model, history_commands_in_order);
 }
 
+IN_PROC_BROWSER_TEST_F(BraveAppMenuModelBrowserTest, CommandsExecutionTest) {
+  RunCommandFromAppMenuModel(CreateIncognitoBrowser(),
+                             IDC_EXTENSIONS_SUBMENU_MANAGE_EXTENSIONS);
+}
+
 // Test brave menu order test.
 // Brave menu is inserted based on corresponding commands enable status.
 // So, this doesn't test for each profiles(normal, private, tor and guest).
@@ -209,7 +221,7 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuModelBrowserTest, MenuOrderTest) {
     IDC_RECENT_TABS_MENU,
     IDC_BOOKMARKS_MENU,
     IDC_SHOW_DOWNLOADS,
-    IDC_MANAGE_EXTENSIONS,
+    IDC_EXTENSIONS_SUBMENU_MANAGE_EXTENSIONS,
     IDC_ZOOM_MENU,
     IDC_PRINT,
     IDC_FIND,
@@ -263,7 +275,7 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuModelBrowserTest, MenuOrderTest) {
     IDC_SHOW_BRAVE_WALLET,
     IDC_BOOKMARKS_MENU,
     IDC_SHOW_DOWNLOADS,
-    IDC_MANAGE_EXTENSIONS,
+    IDC_EXTENSIONS_SUBMENU_MANAGE_EXTENSIONS,
     IDC_ZOOM_MENU,
     IDC_PRINT,
     IDC_FIND,
@@ -316,7 +328,7 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuModelBrowserTest, MenuOrderTest) {
 #endif
     IDC_RECENT_TABS_MENU,
     IDC_BOOKMARKS_MENU,
-    IDC_MANAGE_EXTENSIONS,
+    IDC_EXTENSIONS_SUBMENU_MANAGE_EXTENSIONS,
   };
 
   CheckCommandsAreDisabledInMenuModel(guest_browser,
@@ -355,7 +367,7 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuModelBrowserTest, MenuOrderTest) {
       IDC_SHOW_BRAVE_WALLET,
       IDC_BOOKMARKS_MENU,
       IDC_SHOW_DOWNLOADS,
-      IDC_MANAGE_EXTENSIONS,
+      IDC_EXTENSIONS_SUBMENU_MANAGE_EXTENSIONS,
       IDC_ZOOM_MENU,
       IDC_PRINT,
       IDC_FIND,

chromium_src/chrome/browser/ui/ui_features.cc

@@ -16,7 +16,6 @@ OVERRIDE_FEATURE_DEFAULT_STATES({{
     {kHaTSWebUI, base::FEATURE_DISABLED_BY_DEFAULT},
 #endif
     {kTabHoverCardImages, base::FEATURE_DISABLED_BY_DEFAULT},
-    {kExtensionsMenuInAppMenu, base::FEATURE_DISABLED_BY_DEFAULT},
 }});
 
 }  // namespace features