Replace animated tab audio indicator with static tab audio indicator.

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 18 matching lines @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h"
 #include "chrome/browser/ui/cocoa/drag_util.h"
 #import "chrome/browser/ui/cocoa/image_button_cell.h"
 #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"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_model_observer_bridge.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_view.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_view.h"
 #import "chrome/browser/ui/cocoa/tabs/throbber_view.h"
 #import "chrome/browser/ui/cocoa/tabs/throbbing_image_view.h"
 #include "chrome/browser/ui/find_bar/find_bar.h"
 #include "chrome/browser/ui/find_bar/find_bar_controller.h"
@@ skipping 225 matching lines @@
   }) autorelease];
 }
 
 }  // namespace
 
 @interface TabStripController (Private)
 - (void)addSubviewToPermanentList:(NSView*)aView;
 - (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
              regenerateSubviews:(BOOL)doUpdate;
 - (void)animationDidStopForController:(TabController*)controller
                              finished:(BOOL)finished;
 - (NSInteger)indexFromModelIndex:(NSInteger)index;
 - (void)clickNewTabButton:(id)sender;
 - (NSInteger)numberOfOpenTabs;
 - (NSInteger)numberOfOpenMiniTabs;
@@ skipping 210 matching lines @@
     animationContainer_ = new ui::AnimationContainer;
     NSWindow* browserWindow = [view window];
 
     // Important note: any non-tab subviews not added to |permanentSubviews_|
     // (see |-addSubviewToPermanentList:|) will be wiped out.
     permanentSubviews_.reset([[NSMutableArray alloc] init]);
 
     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];
 
     newTabButton_ = [view getNewTabButton];
     [self addSubviewToPermanentList:newTabButton_];
     [newTabButton_ setTarget:self];
     [newTabButton_ setAction:@selector(clickNewTabButton:)];
 
     [self setNewTabImages];
@@ skipping 1029 matching lines @@
   // Either we don't have a valid favicon or there was some issue converting it
   // from an SkBitmap. Either way, just show the default.
   if (!image)
     image = defaultFavicon_.get();
   NSRect frame = NSMakeRect(0, 0, kIconWidthAndHeight, kIconWidthAndHeight);
   NSImageView* view = [[[NSImageView alloc] initWithFrame:frame] autorelease];
   [view setImage:image];
   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
                          atIndex:(NSInteger)modelIndex {
   if (!contents)
     return;
 
   static NSImage* throbberWaitingImage =
       ResourceBundle::GetSharedInstance().GetNativeImageNamed(
           IDR_THROBBER_WAITING).CopyNSImage();
@@ skipping 31 matching lines @@
   if (oldState != newState)
     [tabController setLoadingState:newState];
 
   // While loading, this function is called repeatedly with the same state.
   // To avoid expensive unnecessary view manipulation, only make changes when
   // the state is actually changing.  When loading is complete (kTabDone),
   // every call to this function is significant.
   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]) {
           NSImage* projectorGlow =
               theme->GetNSImageNamed(IDR_TAB_CAPTURE_GLOW);
           NSImage* projector = theme->GetNSImageNamed(IDR_TAB_CAPTURE);
 
           NSRect frame = NSMakeRect(0,
                                     0,
                                     kProjectingIconWidthAndHeight,
@@ skipping 20 matching lines @@
           ThrobbingImageView* recordingView =
               [[[ThrobbingImageView alloc]
                   initWithFrame:frame
                 backgroundImage:favIconMasked
                      throbImage:recording
                      durationMS:kRecordingDurationMs
                   throbPosition:kThrobPositionBottomRight
              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];
         NSRect frame =
             NSMakeRect(0, 0, kIconWidthAndHeight, kIconWidthAndHeight);
         iconView = [ThrobberView toastThrobberViewWithFrame:frame
                                                 beforeImage:oldImage
                                                  afterImage:sadFaviconImage];
       } else {
         NSRect frame =
             NSMakeRect(0, 0, kIconWidthAndHeight, kIconWidthAndHeight);
         iconView = [ThrobberView filmstripThrobberViewWithFrame:frame
                                                           image:throbberImage];
       }
     }
 
     [tabController setIconView:iconView];
     if (iconView && ![tabController projecting]) {
       // See the comment above kTabOverlap for why these DCHECKs exist.
       DCHECK_GE(NSMinX([iconView frame]), kTabOverlap);
       // TODO(thakis): Ideally, this would be true too, but it's not true in
       // some tests.
       //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).

   }
 }
 
 // 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 {
   // Take closing tabs into account.
@@ skipping 589 matching lines @@
 NSView* GetSheetParentViewForWebContents(WebContents* web_contents) {
   // View hierarchy of the contents view:
   // NSView  -- switchView, same for all tabs
   // +- NSView  -- TabContentsController's view
   //    +- TabContentsViewCocoa
   //
   // Changing it? Do not forget to modify
   // -[TabStripController swapInTabAtIndex:] too.
   return [web_contents->GetView()->GetNativeView() superview];
 }