cocoa: mark active tab in Tab menu

The active tab now gets a check mark, by analogy with the active window
in the Window menu.

Bug: 990838
Change-Id: I2589913a271eaffe0b03ff200d06cc14d7a958b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2071045
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744293}

chrome/browser/ui/cocoa/tab_menu_bridge.mm

@@ -109,6 +109,8 @@ void TabMenuBridge::AddDynamicItemsFromModel() {
                action:@selector(activateTab:)
         keyEquivalent:@""]);
     [item setTarget:menu_listener_.get()];
+    if (model_->active_index() == i)
+      [item setState:NSOnState];
     UpdateItemForWebContents(item, model_->GetWebContentsAt(i));
     [menu_item_.submenu addItem:item.get()];
   }
@@ -131,10 +133,6 @@ void TabMenuBridge::OnTabStripModelChanged(
   DCHECK(tab_strip_model);
   DCHECK_EQ(tab_strip_model, model_);
 
-  // The menu doesn't represent selection in any way, so ignore it.
-  if (change.type() == TabStripModelChange::kSelectionOnly)
-    return;
-
   // If a single WebContents is being replaced, just regenerate that one menu
   // item.
   if (change.type() == TabStripModelChange::kReplaced) {

chrome/browser/ui/cocoa/tab_menu_bridge_unittest.mm

@@ -117,6 +117,12 @@ class TabMenuBridgeTest : public ::testing::Test {
     }
   }
 
+  void ActivateModelTabNamed(const std::string& name) {
+    int index = ModelIndexForTabNamed(name);
+    DCHECK(index >= 0);
+    model()->ActivateTabAt(index);
+  }
+
   NSMenuItem* MenuItemForTabNamed(const std::string& name) {
     return [menu() itemWithTitle:base::SysUTF8ToNSString(name)];
   }
@@ -137,6 +143,19 @@ class TabMenuBridgeTest : public ::testing::Test {
     return base::UTF16ToUTF8(model()->GetActiveWebContents()->GetTitle());
   }
 
+  void ExpectActiveMenuItemNameIs(const std::string& name) {
+    std::vector<std::string> active_items;
+    // Check the static items too, to make sure none of them are checked.
+    for (int i = 0; i < menu().numberOfItems; ++i) {
+      NSMenuItem* item = [menu() itemAtIndex:i];
+      if (item.state == NSOnState)
+        active_items.push_back(base::SysNSStringToUTF8(item.title));
+    }
+
+    ASSERT_EQ(active_items.size(), 1u);
+    EXPECT_EQ(name, active_items[0]);
+  }
+
  private:
   NSMenuItem* ItemWithTitle(NSString* title) {
     return [[NSMenuItem alloc] initWithTitle:title
@@ -243,3 +262,22 @@ TEST_F(TabMenuBridgeTest, SwappingBridgeRecreatesMenu) {
   // does not correctly forget about the TabStripModel, this test will crash
   // here in ASAN builds.
 }
+
+TEST_F(TabMenuBridgeTest, ActiveItemTracksChanges) {
+  TabMenuBridge bridge(model(), menu_root());
+  bridge.BuildMenu();
+
+  AddModelTabNamed("Tab 1");
+  AddModelTabNamed("Tab 2");
+  AddModelTabNamed("Tab 3");
+  ExpectActiveMenuItemNameIs("Tab 3");
+
+  ActivateModelTabNamed("Tab 2");
+  ExpectActiveMenuItemNameIs("Tab 2");
+
+  ActivateModelTabNamed("Tab 3");
+  ExpectActiveMenuItemNameIs("Tab 3");
+
+  RemoveModelTabNamed("Tab 1");
+  ExpectActiveMenuItemNameIs("Tab 3");
+}