Indicate in the Window menu which Chrome window has an active sound playing

chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm

Index: chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
diff --git a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
index 9fe0804d70d90ee72dc004027bf3f31a1ff86b0e..aabe6036a8910ed6a6c43eeb9e921a21a21ae359 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
+++ b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
@@ -251,6 +251,11 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
 - (void)setNewTabButtonHoverState:(BOOL)showHover;
 - (void)themeDidChangeNotification:(NSNotification*)notification;
 - (void)setNewTabImages;
+- (void)updateWindowMediaState:(TabMediaState)mediaState
+                            on:(content::WebContents*)changed;
+- (BOOL)isAnyOtherTab:(content::WebContents*)selected
+            withState:(TabMediaState)state;
+
 @end
 
 // A simple view class that contains the traffic light buttons. This class
@@ -1626,7 +1631,10 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
     }
   }
 
-  [tabController setMediaState:chrome::GetTabMediaStateForContents(contents)];
+  TabMediaState mediaState = chrome::GetTabMediaStateForContents(contents);
+
+  [self updateWindowMediaState:mediaState on:contents];
+  [tabController setMediaState:mediaState];
 
   [tabController updateVisibility];
 }
@@ -2334,4 +2342,41 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
   }
 }
 
+- (void)updateWindowMediaState:(TabMediaState)mediaState

[miu, 2015/10/21 20:06:52]

Please document this method and indicate what the window-level media state will
be in each of the possible cases.  It'd be a lot easier for future maintainers
of this code (so they don't have to spend several minutes eval'ing this logic).

+                               on:(content::WebContents*)selected {

[Robert Sesek, 2015/10/22 00:01:52]

nit: align the colons here, too.

+  NSWindow* window = [tabStripView_ window];
+  BrowserWindowController* windowController =
+        [BrowserWindowController browserWindowControllerForWindow:window];

[Robert Sesek, 2015/10/22 00:01:52]

nit: this is over-indented by two spaces

+  if (mediaState == TAB_MEDIA_STATE_NONE) {
+    if (![self isAnyOtherTab:selected
+          withState:TAB_MEDIA_STATE_AUDIO_PLAYING] &&
+        ![self isAnyOtherTab:selected withState:TAB_MEDIA_STATE_AUDIO_MUTING])

[Robert Sesek, 2015/10/22 00:01:52]

nit: both the if and else bodies need {}

+      [windowController setMediaState:TAB_MEDIA_STATE_NONE];
+    else if ([self isAnyOtherTab:selected
+              withState:TAB_MEDIA_STATE_AUDIO_MUTING])
+      [windowController setMediaState:TAB_MEDIA_STATE_AUDIO_MUTING];
+  }
+  else if (mediaState == TAB_MEDIA_STATE_AUDIO_MUTING) {
+    if (![self isAnyOtherTab:selected withState:TAB_MEDIA_STATE_AUDIO_PLAYING])
+      [windowController setMediaState:TAB_MEDIA_STATE_AUDIO_MUTING];
+  }  else {
+    [windowController setMediaState:mediaState];
+  }
+}
+
+- (BOOL)isAnyOtherTab:(content::WebContents*)selected

[miu, 2015/10/21 20:06:52]

ditto: Please add a short method-level comment summarizing what this does.

+                              withState:(TabMediaState)state{

[Robert Sesek, 2015/10/22 00:01:52]

nit: align the colons

+  const int existingTabCount = tabStripModel_->count();
+  for (int i = 0; i < existingTabCount; ++i) {
+      content::WebContents* currentContents =

[miu, 2015/10/21 20:06:52]

Just after this statement, add the following so the rest of the block doesn't
need to be needlessly executed:

  if (selected == currentContents)
    continue;

+          tabStripModel_->GetWebContentsAt(i);
+      TabMediaState currentMediaStateForContents =
+                    chrome::GetTabMediaStateForContents(currentContents);
+    if (selected != currentContents &&

[miu, 2015/10/21 20:06:52]

(see above comment)...then you don't need the first half of this expression.

+        currentMediaStateForContents == state)
+      return YES;
+  }
+  return NO;
+}
+
 @end