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

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

 // Copyright (c) 2012 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.
 
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
 
 #import <QuartzCore/QuartzCore.h>
 
 #include <cmath>
 #include <limits>
@@ skipping 233 matching lines @@
 - (NSInteger)numberOfOpenPinnedTabs;
 - (NSInteger)numberOfOpenNonPinnedTabs;
 - (void)mouseMoved:(NSEvent*)event;
 - (void)setTabTrackingAreasEnabled:(BOOL)enabled;
 - (void)droppingURLsAt:(NSPoint)point
             givesIndex:(NSInteger*)index
            disposition:(WindowOpenDisposition*)disposition;
 - (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
 // ensures that the buttons display the icons when the mouse hovers over
 // them by overriding the _mouseInGroup method.
 @interface CustomWindowControlsView : NSView {
  @private
   BOOL mouseInside_;
 }
 
@@ skipping 1355 matching lines @@
       } else if (newState == kTabCrashed) {
         [tabController setIconImage:sadFaviconImage withToastAnimation:YES];
       } else {
         [tabController setIconImage:throbberImage];
       }
     } else {
       [tabController setIconImage:nil];
     }
   }
 
-  [tabController setMediaState:chrome::GetTabMediaStateForContents(contents)];
+  TabMediaState mediaState = chrome::GetTabMediaStateForContents(contents);
+
+  [self updateWindowMediaState:mediaState on:contents];
+  [tabController setMediaState:mediaState];
 
   [tabController updateVisibility];
 }
 
 // Called when a notification is received from the model that the given tab
 // has been updated. |loading| will be YES when we only want to update the
 // throbber state, not anything else about the (partially) loading tab.
 - (void)tabChangedWithContents:(content::WebContents*)contents
                        atIndex:(NSInteger)modelIndex
                     changeType:(TabStripModelObserver::TabChangeType)change {
@@ skipping 687 matching lines @@
     [[newTabButton_ cell] setImage:Overlay(background, hover, alpha)
                     forButtonState:image_button_cell::kHoverStateBackground];
   } else {
     [[newTabButton_ cell] setImage:nil
                     forButtonState:image_button_cell::kDefaultStateBackground];
     [[newTabButton_ cell] setImage:nil
                     forButtonState:image_button_cell::kHoverStateBackground];
   }
 }
 
+- (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