mac: add Tab menu

This change adds a new top-level menu to the system menu, used to
manipulate and select tabs in the current window. This menu contains
some static items, moved from the Window menu, and then a list of
entries corresponding to open tabs in the current window. Specifically,
this change:

1) Adds a new class, TabMenuBridge, which binds a TabStripModel to a
   NSMenu;
2) Adds a new menu item to the main menu named "Tab";
3) Creates a TabMenuBridge for any tabbed browser window in the
   app controller.

The motivation for this change was twofold: first, to allow easier
access to commands that are currently only present in the tab's
context menu, and second, to allow easier access to tabs in crowded
tabstrips. The first purpose is well-defined and understood but the
second is very much experimental.

Testing:
Build, run, and enjoy the new Tab menu :). There are some unit tests in
TabMenuBridgeTest.* as well.

Future work:
* Renaming the static items to better fit their new context. This was
  not done here to avoid breaking existing user key equivalents, which
  identify items by their full name.
* Exploring other orderings of tabs in this menu.
* Adding other tab actions to this menu, like "Bookmark" or possibly
  page actions.
* Adding a search bar to search within tab titles/origins?
* Adding key equivalents for the static actions (which will resolve
  the actual accessibility bug this commit is for)

Bug: 818261
Change-Id: I790cc2c5a4468719e42b782a318864ecd81bd1f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1728718
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Auto-Submit: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683682}

chrome/app/chrome_command_ids.h

@@ -220,6 +220,7 @@
 #define IDC_CHROME_MENU                 44002     // OSX only
 #define IDC_HIDE_APP                    44003     // OSX only
 #define IDC_HISTORY_MENU                46000     // OSX only
+#define IDC_TAB_MENU                    46001     // OSX only
 #define IDC_PROFILE_MAIN_MENU           46100     // OSX only
 #define IDC_INPUT_METHODS_MENU          46300     // Linux only
 

chrome/app/generated_resources.grd

@@ -7704,6 +7704,9 @@ Please help our engineers fix this problem. Tell us what happened right before y
         <message name="IDS_WINDOW_MENU_MAC" desc="The menu title of the Mac window menu.">
           Window
         </message>
+        <message name="IDS_TAB_MENU_MAC" desc="The menu title of the Mac tab menu.">
+          Tab
+        </message>
         <message name="IDS_HELP_MENU_MAC" desc="The menu title of the Mac help menu.">
           Help
         </message>

chrome/browser/app_controller_mac.h

@@ -30,6 +30,7 @@ class Profile;
 class QuitWithAppsController;
 class ScopedKeepAlive;
 @class ShareMenuController;
+class TabMenuBridge;
 
 // The application controller object, created by loading the MainMenu nib.
 // This handles things like responding to menus when there are no windows
@@ -71,6 +72,8 @@ class ScopedKeepAlive;
   // Controller for the macOS system share menu.
   base::scoped_nsobject<ShareMenuController> shareMenuController_;
 
+  std::unique_ptr<TabMenuBridge> tabMenuBridge_;
+
   // If we're told to open URLs (in particular, via |-application:openFiles:| by
   // Launch Services) before we've launched the browser, we queue them up in
   // |startupUrls_| so that they can go in the first browser window/tab.
@@ -162,6 +165,7 @@ class ScopedKeepAlive;
 
 - (BookmarkMenuBridge*)bookmarkMenuBridge;
 - (HistoryMenuBridge*)historyMenuBridge;
+- (TabMenuBridge*)tabMenuBridge;
 
 // Initializes the AppShimMenuController. This enables changing the menu bar for
 // apps.

chrome/browser/app_controller_mac.mm

@@ -75,6 +75,7 @@
 #include "chrome/browser/ui/cocoa/last_active_browser_cocoa.h"
 #import "chrome/browser/ui/cocoa/profiles/profile_menu_controller.h"
 #import "chrome/browser/ui/cocoa/share_menu_controller.h"
+#import "chrome/browser/ui/cocoa/tab_menu_bridge.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
 #include "chrome/browser/ui/startup/startup_browser_creator_impl.h"
@@ -609,8 +610,19 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session;
 
 - (void)windowDidBecomeMain:(NSNotification*)notify {
   Browser* browser = chrome::FindBrowserWithWindow([notify object]);
-  if (browser)
-    [self windowChangedToProfile:browser->profile()->GetOriginalProfile()];
+  if (!browser)
+    return;
+
+  if (browser->is_type_tabbed()) {
+    tabMenuBridge_ = std::make_unique<TabMenuBridge>(
+        browser->tab_strip_model(),
+        [[NSApp mainMenu] itemWithTag:IDC_TAB_MENU]);
+    tabMenuBridge_->BuildMenu();
+  } else {
+    tabMenuBridge_.reset();
+  }
+
+  [self windowChangedToProfile:browser->profile()->GetOriginalProfile()];
 }
 
 - (void)windowDidResignMain:(NSNotification*)notify {
@@ -1547,6 +1559,10 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session;
   return historyMenuBridge_.get();
 }
 
+- (TabMenuBridge*)tabMenuBridge {
+  return tabMenuBridge_.get();
+}
+
 - (void)initAppShimMenuController {
   if (!appShimMenuController_)
     appShimMenuController_.reset([[AppShimMenuController alloc] init]);

chrome/browser/global_keyboard_shortcuts_mac_browsertest.mm

@@ -230,10 +230,10 @@ IN_PROC_BROWSER_TEST_F(GlobalKeyboardShortcutsTest, MenuCommandPriority) {
   // this code can't modify it.
   NSMenu* main_menu = [NSApp mainMenu];
   ASSERT_NE(nil, main_menu);
-  NSMenuItem* window_menu = [main_menu itemWithTitle:@"Window"];
-  ASSERT_NE(nil, window_menu);
-  ASSERT_TRUE(window_menu.hasSubmenu);
-  NSMenuItem* next_item = [window_menu.submenu itemWithTag:IDC_SELECT_NEXT_TAB];
+  NSMenuItem* tab_menu = [main_menu itemWithTitle:@"Tab"];
+  ASSERT_NE(nil, tab_menu);
+  ASSERT_TRUE(tab_menu.hasSubmenu);
+  NSMenuItem* next_item = [tab_menu.submenu itemWithTag:IDC_SELECT_NEXT_TAB];
   ASSERT_NE(nil, next_item);
   [next_item setKeyEquivalent:@"2"];
   [next_item setKeyEquivalentModifierMask:NSCommandKeyMask];

chrome/browser/ui/BUILD.gn

@@ -2230,6 +2230,8 @@ jumbo_split_static_library("ui") {
       "cocoa/tab_contents/chrome_web_contents_view_delegate_mac.mm",
       "cocoa/tab_contents/web_drag_bookmark_handler_mac.h",
       "cocoa/tab_contents/web_drag_bookmark_handler_mac.mm",
+      "cocoa/tab_menu_bridge.h",
+      "cocoa/tab_menu_bridge.mm",
       "cocoa/task_manager_mac.h",
       "cocoa/task_manager_mac.mm",
       "cocoa/touchbar/browser_window_default_touch_bar.h",

chrome/browser/ui/cocoa/main_menu_builder.mm

@@ -365,24 +365,9 @@ base::scoped_nsobject<NSMenuItem> BuildWindowMenu(
                     .tag(IDC_MAXIMIZE_WINDOW)
                     .action(@selector(performZoom:)),
                 Item().is_separator(),
-                Item(IDS_NEXT_TAB_MAC)
-                    .command_id(IDC_SELECT_NEXT_TAB)
-                    .remove_if(is_pwa),
-                Item(IDS_PREV_TAB_MAC)
-                    .command_id(IDC_SELECT_PREVIOUS_TAB)
-                    .remove_if(is_pwa),
                 Item(IDS_SHOW_AS_TAB)
                     .command_id(IDC_SHOW_AS_TAB)
                     .remove_if(is_pwa),
-                Item(IDS_DUPLICATE_TAB_MAC)
-                    .command_id(IDC_DUPLICATE_TAB)
-                    .remove_if(is_pwa),
-                Item(IDS_MUTE_SITE_MAC)
-                    .command_id(IDC_WINDOW_MUTE_SITE)
-                    .remove_if(is_pwa),
-                Item(IDS_PIN_TAB_MAC)
-                    .command_id(IDC_WINDOW_PIN_TAB)
-                    .remove_if(is_pwa),
                 Item().is_separator().remove_if(is_pwa),
                 Item(IDS_SHOW_DOWNLOADS_MAC)
                     .command_id(IDC_SHOW_DOWNLOADS)
@@ -404,6 +389,29 @@ base::scoped_nsobject<NSMenuItem> BuildWindowMenu(
   return item;
 }
 
+base::scoped_nsobject<NSMenuItem> BuildTabMenu(
+    NSApplication* nsapp,
+    id app_delegate,
+    const base::string16& product_name,
+    bool is_pwa) {
+  if (is_pwa)
+    return base::scoped_nsobject<NSMenuItem>();
+
+  base::scoped_nsobject<NSMenuItem> item =
+      Item(IDS_TAB_MENU_MAC)
+          .tag(IDC_TAB_MENU)
+          .submenu({
+              Item(IDS_NEXT_TAB_MAC).command_id(IDC_SELECT_NEXT_TAB),
+              Item(IDS_PREV_TAB_MAC).command_id(IDC_SELECT_PREVIOUS_TAB),
+              Item(IDS_DUPLICATE_TAB_MAC).command_id(IDC_DUPLICATE_TAB),
+              Item(IDS_MUTE_SITE_MAC).command_id(IDC_WINDOW_MUTE_SITE),
+              Item(IDS_PIN_TAB_MAC).command_id(IDC_WINDOW_PIN_TAB),
+              Item().is_separator(),
+          })
+          .Build();
+  return item;
+}
+
 base::scoped_nsobject<NSMenuItem> BuildHelpMenu(
     NSApplication* nsapp,
     id app_delegate,
@@ -438,9 +446,9 @@ void BuildMainMenu(NSApplication* nsapp,
   using Builder = base::scoped_nsobject<NSMenuItem> (*)(
       NSApplication*, id, const base::string16&, bool);
   static const Builder kBuilderFuncs[] = {
-      &BuildAppMenu,    &BuildFileMenu,    &BuildEditMenu,
-      &BuildViewMenu,   &BuildHistoryMenu, &BuildBookmarksMenu,
-      &BuildPeopleMenu, &BuildWindowMenu,  &BuildHelpMenu,
+      &BuildAppMenu,     &BuildFileMenu,      &BuildEditMenu,   &BuildViewMenu,
+      &BuildHistoryMenu, &BuildBookmarksMenu, &BuildPeopleMenu, &BuildTabMenu,
+      &BuildWindowMenu,  &BuildHelpMenu,
   };
   for (auto* builder : kBuilderFuncs) {
     auto item = builder(nsapp, app_delegate, product_name, is_pwa);

chrome/browser/ui/cocoa/tab_menu_bridge.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_COCOA_TAB_MENU_BRIDGE_H_
+#define CHROME_BROWSER_UI_COCOA_TAB_MENU_BRIDGE_H_
+
+#include "base/gtest_prod_util.h"
+#include "base/mac/scoped_nsobject.h"
+#include "base/macros.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
+
+@class NSMenuItem;
+@class TabMenuListener;
+class TabStripModel;
+
+// A TabMenuBridge bidirectionally connects a list of tabs (represented by a
+// TabStripModel) to an NSMenu, for use in the system menu bar. Specifically,
+// the TabStripBridge appends items to the bottom of the provided NSMenu
+// corresponding to tabs in the TabStripModel, and keeps those items
+// synchronized with changes to the TabStripModel. Clicking one of these items
+// activates the corresponding tab in the TabStripModel. This class assumes
+// that:
+//   1) It owns the bottom "dynamic" part of of the provided menu
+//   2) The number of items not in the "dynamic" part does not change after the
+//      TabMenuBridge is constructed
+//
+// To use this class, construct an instance and call BuildMenu() on it.
+class TabMenuBridge : public TabStripModelObserver {
+ public:
+  // The |menu_item| contains the actual menu this class manages.
+  TabMenuBridge(TabStripModel* model, NSMenuItem* menu_item);
+  ~TabMenuBridge() override;
+
+  // It's legal to call this method more than once - it will clear all the
+  // existing dynamic items added by this instance before adding any new ones,
+  // so multiple calls are idempotent.
+  void BuildMenu();
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(TabMenuBridgeTest, ClickingMenuActivatesTab);
+
+  // These methods are used to make batch changes to the menu.
+  void RemoveAllDynamicItems();
+  void AddDynamicItemsFromModel();
+
+  // This method exists to be called back into from the Cocoa part of this
+  // bridge class (TabMenuListener).
+  void OnDynamicItemChosen(NSMenuItem* item);
+
+  // TabStripModelObserver:
+  void OnTabStripModelChanged(
+      TabStripModel* tab_strip_model,
+      const TabStripModelChange& change,
+      const TabStripSelectionChange& selection) override;
+  void TabChangedAt(content::WebContents* contents,
+                    int index,
+                    TabChangeType change_type) override;
+
+  TabStripModel* model_;
+  NSMenuItem* menu_item_;  // weak
+  base::scoped_nsobject<TabMenuListener> menu_listener_;
+
+  // When created, this class remembers how many items were present in the
+  // non-dynamic section of the menu. This offset is used to map menu items to
+  // their underlying tabs.
+  int dynamic_items_start_;
+
+  DISALLOW_COPY_AND_ASSIGN(TabMenuBridge);
+};
+
+#endif  // CHROME_BROWSER_UI_COCOA_TAB_MENU_BRIDGE_H_

chrome/browser/ui/cocoa/tab_menu_bridge.mm

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/cocoa/tab_menu_bridge.h"
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/callback.h"
+#include "base/mac/scoped_nsobject.h"
+#include "base/strings/sys_string_conversions.h"
+#include "chrome/browser/ui/tab_ui_helper.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+
+using MenuItemCallback = base::RepeatingCallback<void(NSMenuItem*)>;
+
+namespace {
+
+void UpdateItemForWebContents(NSMenuItem* item,
+                              content::WebContents* web_contents) {
+  TabUIHelper* tab_ui_helper = TabUIHelper::FromWebContents(web_contents);
+
+  item.title = base::SysUTF16ToNSString(tab_ui_helper->GetTitle());
+  item.image = tab_ui_helper->GetFavicon().AsNSImage();
+}
+
+}  // namespace
+
+@interface TabMenuListener : NSObject
+- (instancetype)initWithCallback:(MenuItemCallback)callback;
+- (void)activateTab:(id)sender;
+@end
+
+@implementation TabMenuListener {
+  MenuItemCallback callback_;
+}
+
+- (instancetype)initWithCallback:(MenuItemCallback)callback {
+  if ((self = [super init])) {
+    callback_ = callback;
+  }
+  return self;
+}
+
+- (IBAction)activateTab:(id)sender {
+  callback_.Run(sender);
+}
+@end
+
+TabMenuBridge::TabMenuBridge(TabStripModel* model, NSMenuItem* menu_item)
+    : model_(model), menu_item_(menu_item) {
+  menu_listener_.reset([[TabMenuListener alloc]
+      initWithCallback:base::Bind(&TabMenuBridge::OnDynamicItemChosen,
+                                  // Unretained is safe here: this class owns
+                                  // MenuListener, which holds the callback
+                                  // being constructed here, so the callback
+                                  // will be destructed before this class.
+                                  base::Unretained(this))]);
+  model_->AddObserver(this);
+}
+
+TabMenuBridge::~TabMenuBridge() {
+  model_->RemoveObserver(this);
+  RemoveAllDynamicItems();
+}
+
+void TabMenuBridge::BuildMenu() {
+  RemoveAllDynamicItems();
+  AddDynamicItemsFromModel();
+}
+
+void TabMenuBridge::RemoveAllDynamicItems() {
+  for (NSMenuItem* item in menu_item_.submenu.itemArray) {
+    if (item.target == menu_listener_.get())
+      [menu_item_.submenu removeItem:item];
+  }
+}
+
+void TabMenuBridge::AddDynamicItemsFromModel() {
+  dynamic_items_start_ = menu_item_.submenu.numberOfItems;
+  for (int i = 0; i < model_->count(); ++i) {
+    base::scoped_nsobject<NSMenuItem> item([[NSMenuItem alloc]
+        initWithTitle:@""
+               action:@selector(activateTab:)
+        keyEquivalent:@""]);
+    [item setTarget:menu_listener_.get()];
+    UpdateItemForWebContents(item, model_->GetWebContentsAt(i));
+    [menu_item_.submenu addItem:item.get()];
+  }
+}
+
+void TabMenuBridge::OnDynamicItemChosen(NSMenuItem* item) {
+  DCHECK_EQ(item.target, menu_listener_.get());
+  int index = [menu_item_.submenu indexOfItem:item] - dynamic_items_start_;
+  model_->ActivateTabAt(index, TabStripModel::UserGestureDetails(
+                                   TabStripModel::GestureType::kTabMenu));
+}
+
+void TabMenuBridge::OnTabStripModelChanged(
+    TabStripModel* tab_strip_model,
+    const TabStripModelChange& change,
+    const TabStripSelectionChange& selection) {
+  DCHECK_EQ(tab_strip_model, model_);
+
+  // Rather than doing clever updating from |change|, just destroy the dynamic
+  // menu items and re-add them.
+  RemoveAllDynamicItems();
+  AddDynamicItemsFromModel();
+}
+
+void TabMenuBridge::TabChangedAt(content::WebContents* contents,
+                                 int index,
+                                 TabChangeType change_type) {
+  // Ignore loading state changes - they happen very often during page load and
+  // are used to drive the load spinner, which is not interesting to this menu.
+  if (change_type == TabChangeType::kLoadingOnly)
+    return;
+
+  int menu_index = index + dynamic_items_start_;
+
+  // It might seem like this can't happen but actually it can:
+  // 1) Someone calls TabMenuModel::AddWebContents
+  // 2) Some other observer (not this) is notified of the add
+  // 3) That observer responds by doing something that eventually leads into
+  //    UpdateWebContentsStateAt, while this class still hasn't observed the
+  //    OnTabStripModelChanged (but the method that will notify us is on the
+  //    stack)
+  // 4) That UpdateWebContentsStateAt causes this object to observe a
+  //    TabChangedAt for an index it hasn't yet been informed exists
+  // As such, this code early-outs instead of DCHECKing. The newly-added
+  // WebContents will be picked up later anyway when this object does get
+  // notified of the addition.
+  if (menu_index < 0 || menu_index >= menu_item_.submenu.numberOfItems)
+    return;
+
+  NSMenuItem* item = [menu_item_.submenu itemAtIndex:menu_index];
+  UpdateItemForWebContents(item, contents);
+}

chrome/browser/ui/cocoa/tab_menu_bridge_unittest.mm

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/mac/scoped_nsobject.h"
+#include "base/strings/sys_string_conversions.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_task_environment.h"
+#include "chrome/browser/favicon/favicon_utils.h"
+#include "chrome/browser/ui/cocoa/tab_menu_bridge.h"
+#include "chrome/browser/ui/tab_ui_helper.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/test_tab_strip_model_delegate.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/test/test_renderer_host.h"
+#include "content/public/test/web_contents_tester.h"
+#include "testing/gtest_mac.h"
+
+constexpr int kStaticItemCount = 4;
+
+class TabStripModelUiHelperDelegate : public TestTabStripModelDelegate {
+ public:
+  void WillAddWebContents(content::WebContents* contents) override {
+    TestTabStripModelDelegate::WillAddWebContents(contents);
+
+    favicon::CreateContentFaviconDriverForWebContents(contents);
+    TabUIHelper::CreateForWebContents(contents);
+  }
+};
+
+class TabMenuBridgeTest : public ::testing::Test {
+ public:
+  void SetUp() override {
+    profile_ = std::make_unique<TestingProfile>();
+    rvh_test_enabler_ = std::make_unique<content::RenderViewHostTestEnabler>();
+    delegate_ = std::make_unique<TabStripModelUiHelperDelegate>();
+    model_ = std::make_unique<TabStripModel>(delegate_.get(), nullptr);
+    menu_root_.reset(ItemWithTitle(@"Tab"));
+    menu_.reset([[NSMenu alloc] initWithTitle:@"Tab"]);
+    menu_root_.get().submenu = menu_.get();
+
+    AddStaticItems(menu_.get());
+  }
+
+  void TearDown() override { model_->CloseAllTabs(); }
+
+  NSMenuItem* menu_root() { return menu_root_.get(); }
+  NSMenu* menu() { return menu_.get(); }
+  TabStripModel* model() { return model_.get(); }
+  TabStripModelDelegate* delegate() { return delegate_.get(); }
+
+  void AddStaticItems(NSMenu* menu) {
+    [menu_ addItem:ItemWithTitle(@"Static 1")];
+    [menu_ addItem:ItemWithTitle(@"Static 2")];
+    [menu_ addItem:ItemWithTitle(@"Static 3")];
+    [menu_ addItem:SeparatorItem()];
+  }
+
+  std::unique_ptr<content::WebContents> CreateWebContents(
+      const std::string& title) {
+    std::unique_ptr<content::WebContents> contents =
+        content::WebContentsTester::CreateTestWebContents(profile_.get(),
+                                                          nullptr);
+    content::WebContentsTester::For(contents.get())
+        ->SetTitle(base::UTF8ToUTF16(title));
+    return contents;
+  }
+
+  void AddModelTabNamed(const std::string& name) {
+    model()->AppendWebContents(CreateWebContents(name), true);
+  }
+
+  int ModelIndexForTabNamed(const std::string& name) {
+    base::string16 title16 = base::UTF8ToUTF16(name);
+    for (int i = 0; i < model()->count(); ++i) {
+      if (model()->GetWebContentsAt(i)->GetTitle() == title16)
+        return i;
+    }
+    return -1;
+  }
+
+  void RemoveModelTabNamed(const std::string& name) {
+    int index = ModelIndexForTabNamed(name);
+    DCHECK(index >= 0);
+    model()->CloseWebContentsAt(index, TabStripModel::CLOSE_NONE);
+  }
+
+  void RenameModelTabNamed(const std::string& old_name,
+                           const std::string& new_name) {
+    int index = ModelIndexForTabNamed(old_name);
+    if (index >= 0) {
+      content::WebContents* contents = model()->GetWebContentsAt(index);
+      content::WebContentsTester::For(contents)->SetTitle(
+          base::UTF8ToUTF16(new_name));
+      // The way WebContentsTester updates the title avoids the usual
+      // notification mechanism for TabStripModel, so manually synthesize the
+      // update notification here.
+      model()->UpdateWebContentsStateAt(index, TabChangeType::kAll);
+    }
+  }
+
+  NSMenuItem* MenuItemForTabNamed(const std::string& name) {
+    return [menu() itemWithTitle:base::SysUTF8ToNSString(name)];
+  }
+
+  void ExpectDynamicTabsInMenuAre(const std::vector<std::string>& titles) {
+    std::vector<std::string> actual_titles;
+    for (int i = kStaticItemCount; i < menu().numberOfItems; ++i) {
+      actual_titles.push_back(
+          base::SysNSStringToUTF8([menu() itemAtIndex:i].title));
+    }
+
+    ASSERT_EQ(actual_titles.size(), titles.size());
+    for (int i = 0; i < static_cast<int>(titles.size()); ++i)
+      EXPECT_EQ(actual_titles[i], titles[i]);
+  }
+
+  std::string ActiveTabName() {
+    return base::UTF16ToUTF8(model()->GetActiveWebContents()->GetTitle());
+  }
+
+ private:
+  NSMenuItem* ItemWithTitle(NSString* title) {
+    return [[NSMenuItem alloc] initWithTitle:title
+                                      action:nil
+                               keyEquivalent:@""];
+  }
+
+  NSMenuItem* SeparatorItem() { return [NSMenuItem separatorItem]; }
+
+  content::TestBrowserThreadBundle test_browser_thread_bundle_;
+
+  std::unique_ptr<TestingProfile> profile_;
+  std::unique_ptr<content::RenderViewHostTestEnabler> rvh_test_enabler_;
+  std::unique_ptr<TabStripModelUiHelperDelegate> delegate_;
+  std::unique_ptr<TabStripModel> model_;
+  base::scoped_nsobject<NSMenuItem> menu_root_;
+  base::scoped_nsobject<NSMenu> menu_;
+};
+
+TEST_F(TabMenuBridgeTest, CreatesBlankMenu) {
+  TabMenuBridge bridge(model(), menu_root());
+  bridge.BuildMenu();
+  EXPECT_EQ(menu().numberOfItems, kStaticItemCount);
+  ExpectDynamicTabsInMenuAre({});
+}
+
+TEST_F(TabMenuBridgeTest, TracksModelUpdates) {
+  TabMenuBridge bridge(model(), menu_root());
+  bridge.BuildMenu();
+
+  AddModelTabNamed("Tab 1");
+  AddModelTabNamed("Tab 2");
+  AddModelTabNamed("Tab 3");
+  ExpectDynamicTabsInMenuAre({"Tab 1", "Tab 2", "Tab 3"});
+
+  RemoveModelTabNamed("Tab 2");
+  ExpectDynamicTabsInMenuAre({"Tab 1", "Tab 3"});
+
+  AddModelTabNamed("Tab 2");
+  ExpectDynamicTabsInMenuAre({"Tab 1", "Tab 3", "Tab 2"});
+
+  RenameModelTabNamed("Tab 1", "Tab 4");
+  ExpectDynamicTabsInMenuAre({"Tab 4", "Tab 3", "Tab 2"});
+}
+
+TEST_F(TabMenuBridgeTest, ClickingMenuActivatesTab) {
+  TabMenuBridge bridge(model(), menu_root());
+  bridge.BuildMenu();
+
+  AddModelTabNamed("Tab 1");
+  AddModelTabNamed("Tab 2");
+  EXPECT_EQ(ActiveTabName(), "Tab 2");
+
+  NSMenuItem* tab1_item = MenuItemForTabNamed("Tab 1");
+
+  // Don't go through NSMenuItem's normal click-dispatching machinery here - it
+  // would add flake potential to this test without improving coverage. Instead,
+  // call straight through to the C++-side callback:
+  bridge.OnDynamicItemChosen(tab1_item);
+  EXPECT_EQ(ActiveTabName(), "Tab 1");
+
+  // Activation does not re-order the menu.
+  ExpectDynamicTabsInMenuAre({"Tab 1", "Tab 2"});
+}
+
+// This is a regression test for a bug found during development. Previous
+// versions of TabMenuBridge had an RAII-like API where creating a TabMenuBridge
+// would fill in the dynamic menu during construction. Combining this with the
+// common pattern of:
+//    tab_menu_bridge_ = std::make_unique<TabMenuBridge>(...);
+// in the presence of an existing tab_menu_bridge_ led to there temporarily
+// being two TabMenuBridge instances at a time, meaning both of them had their
+// dynamic menu items installed. This, in turn, confused the menu index logic in
+// the new TabMenuBridge - it counted the old TabMenuBridge's dynamic items as
+// static items, and ended up with incorrect indexes. This test exercises that
+// behavior.
+TEST_F(TabMenuBridgeTest, SwappingBridgeRecreatesMenu) {
+  auto bridge = std::make_unique<TabMenuBridge>(model(), menu_root());
+  bridge->BuildMenu();
+
+  AddModelTabNamed("Tab 1");
+
+  auto model2 = std::make_unique<TabStripModel>(delegate(), nullptr);
+  model2->AppendWebContents(CreateWebContents("Tab 2"), true);
+
+  bridge = std::make_unique<TabMenuBridge>(model2.get(), menu_root());
+  bridge->BuildMenu();
+  ExpectDynamicTabsInMenuAre({"Tab 2"});
+
+  // Simulate one of the tabs in the model being updated - if the computed
+  // indexes are wrong, this call will DCHECK.
+  model2->UpdateWebContentsStateAt(0, TabChangeType::kAll);
+
+  model2->CloseAllTabs();
+}

chrome/browser/ui/tabs/tab_strip_model.h

@@ -206,7 +206,15 @@ class TabStripModel {
 
   // User gesture type that triggers ActivateTabAt. kNone indicates that it was
   // not triggered by a user gesture, but by a by-product of some other action.
-  enum class GestureType { kMouse, kTouch, kWheel, kKeyboard, kOther, kNone };
+  enum class GestureType {
+    kMouse,
+    kTouch,
+    kWheel,
+    kKeyboard,
+    kOther,
+    kTabMenu,
+    kNone
+  };
 
   // Encapsulates user gesture information for tab activation
   struct UserGestureDetails {

chrome/test/BUILD.gn

@@ -4725,6 +4725,7 @@ test("unit_tests") {
       "../browser/ui/cocoa/profiles/profile_menu_controller_unittest.mm",
       "../browser/ui/cocoa/scoped_menu_bar_lock_unittest.mm",
       "../browser/ui/cocoa/status_icons/status_icon_mac_unittest.mm",
+      "../browser/ui/cocoa/tab_menu_bridge_unittest.mm",
       "../browser/ui/cocoa/test/cocoa_profile_test.h",
       "../browser/ui/cocoa/test/cocoa_profile_test.mm",
       "../browser/ui/cocoa/test/run_loop_testing_unittest.mm",