Replace animated tab audio indicator with static tab audio indicator.

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 c99c63fae6c0f9031e5aba28356c75398d822312..962cb405b3708c6727f67be00e567b812ae49615 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
+++ b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
@@ -36,7 +36,6 @@
 #import "chrome/browser/ui/cocoa/new_tab_button.h"
 #import "chrome/browser/ui/cocoa/tab_contents/favicon_util_mac.h"
 #import "chrome/browser/ui/cocoa/tab_contents/tab_contents_controller.h"
-#import "chrome/browser/ui/cocoa/tabs/tab_audio_indicator_view_mac.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_controller.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_projecting_image_view.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_drag_controller.h"
@@ -282,6 +281,7 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
 - (void)regenerateSubviewList;
 - (NSInteger)indexForContentsView:(NSView*)view;
 - (NSImageView*)iconImageViewForContents:(content::WebContents*)contents;
+- (NSView*)audioIndicatorViewForContents:(content::WebContents*)contents;
 - (void)updateFaviconForContents:(content::WebContents*)contents
                          atIndex:(NSInteger)modelIndex;
 - (void)layoutTabsWithAnimation:(BOOL)animate
@@ -512,6 +512,8 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
     ResourceBundle& rb = ResourceBundle::GetSharedInstance();
     defaultFavicon_.reset(
         rb.GetNativeImageNamed(IDR_DEFAULT_FAVICON).CopyNSImage());
+    audioIndicatorImage_.reset(
+        rb.GetNativeImageNamed(IDR_TAB_AUDIO_INDICATOR).CopyNSImage());

[sail, 2013/09/11 19:18:37]

no need to cache this in a member variable, also no need to copy

[miu, 2013/09/11 21:35:06]

Done.

 
     [self setLeftIndentForControls:[[self class] defaultLeftIndentForControls]];
     [self setRightIndentForControls:0];
@@ -1561,6 +1563,19 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
   return view;
 }
 
+// A helper routine for creating an NSImageView to hold the audio indicator icon
+// for |contents|.  Returns nil when the audio indicator should not be shown.
+- (NSView*)audioIndicatorViewForContents:(content::WebContents*)contents {

[sail, 2013/09/11 19:18:37]

This code is hard to follow with one set of conditional here (IsPlayingAudio)
and another in the caller.

I think it would be easier to read if this helper function was removed and
everything moved into the caller code.

[miu, 2013/09/11 21:35:06]

Done.

+  if (!chrome::IsPlayingAudio(contents) || !audioIndicatorImage_)

[sail, 2013/09/11 19:18:37]

don't need the !audioIndicatorImage_ check

+    return nil;
+  NSRect frame;
+  frame.size = [audioIndicatorImage_ size];
+  NSImageView* const image_view =
+      [[[NSImageView alloc] initWithFrame:frame] autorelease];
+  [image_view setImage:audioIndicatorImage_.get()];
+  return image_view;
+}
+
 // Updates the current loading state, replacing the icon view with a favicon,
 // a throbber, the default icon, or nothing at all.
 - (void)updateFaviconForContents:(content::WebContents*)contents
@@ -1612,12 +1627,10 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
   if (newState == kTabDone || oldState != newState ||
       oldHasIcon != newHasIcon) {
     NSView* iconView = nil;
+    BOOL disallowAudioIndicatorView = YES;
     if (newHasIcon) {
       if (newState == kTabDone) {
         NSImageView* imageView = [self iconImageViewForContents:contents];
-        TabAudioIndicatorViewMac* tabAudioIndicatorViewMac =
-            base::mac::ObjCCast<TabAudioIndicatorViewMac>(
-                [tabController iconView]);
 
         ui::ThemeProvider* theme = [[tabStripView_ window] themeProvider];
         if (theme && [tabController projecting]) {
@@ -1658,22 +1671,9 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
              animationContainer:animationContainer_.get()] autorelease];
 
           iconView = recordingView;
-        } else if (chrome::IsPlayingAudio(contents) ||
-                   [tabAudioIndicatorViewMac isAnimating]) {
-          if (!tabAudioIndicatorViewMac) {
-            NSRect frame =
-                NSMakeRect(0, 0, kIconWidthAndHeight, kIconWidthAndHeight);
-            tabAudioIndicatorViewMac = [[[TabAudioIndicatorViewMac alloc]
-                initWithFrame:frame] autorelease];
-            [tabAudioIndicatorViewMac
-                setAnimationContainer:animationContainer_.get()];
-          }
-          [tabAudioIndicatorViewMac
-              setIsPlayingAudio:chrome::IsPlayingAudio(contents)];
-          [tabAudioIndicatorViewMac setBackgroundImage:[imageView image]];
-          iconView = tabAudioIndicatorViewMac;
         } else {
           iconView = imageView;
+          disallowAudioIndicatorView = NO;
         }
       } else if (newState == kTabCrashed) {
         NSImage* oldImage = [[self iconImageViewForContents:contents] image];
@@ -1699,6 +1699,10 @@ NSImage* Overlay(NSImage* ground, NSImage* overlay, CGFloat alpha) {
       //DCHECK_LE(NSMaxX([iconView frame]),
       //          NSWidth([[tabController view] frame]) - kTabOverlap);
     }
+
+    NSView* audioIndicatorView = disallowAudioIndicatorView ?
+        nil : [self audioIndicatorViewForContents:contents];
+    [tabController setAudioIndicatorView:audioIndicatorView];

[sail, 2013/09/11 19:18:37]

does this have to be done after setIconView:? It would be better if the order
didn't matter. Then you can remove |disallowAudioIndicatorView| and move this
into the else clause above.

[miu, 2013/09/11 21:35:06]

Good point.  I got rid of |disallowAudioIndicatorView| and moved the code above.
 However, the [tabController setAudioIndicatorView] call still needs to be made
in all cases (so that the view is set to nil when the audio indicator should be
turned off).

   }
 }