Render the tab close button as a vector-based icon.

chrome/browser/ui/tabs/tab_utils.cc

 // 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.
 
 #include "chrome/browser/ui/tabs/tab_utils.h"
 
 #include "base/command_line.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/media/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/media_stream_capture_indicator.h"
@@ skipping 145 matching lines @@
 
   if (contents->IsAudioMuted())
     return TAB_MEDIA_STATE_AUDIO_MUTING;
   if (contents->WasRecentlyAudible())
     return TAB_MEDIA_STATE_AUDIO_PLAYING;
 
   return TAB_MEDIA_STATE_NONE;
 }
 
 gfx::Image GetTabMediaIndicatorImage(TabMediaState media_state,
-                                     const ui::ThemeProvider* tp) {
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+                                     SkColor button_color) {
+  ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();

[miu, 2015/09/23 21:15:28]

OOC, why change this?  I realize our style guide forbids passing args to
functions by non-const ref, but isn't it reasonable to use them as locals?  In
this case, the old code was literally declaring that the result from
ui::ResourceBundle::GetSharedInstance() must refer to an object (i.e., not null)
and, as a reference type, |rb| could never be changed to refer to something else
(implicit "pointer constness").

Is there a style guide discussion on using references locally?

[Peter Kasting, 2015/09/23 21:31:04]

The Google style guiding wording is a bit unclear, but I think the common
reading of it is that it also forbids returning non-const refs.  Accordingly, we
have a bug on fixing GetSharedInstance() to do this:

https://code.google.com/p/chromium/issues/detail?id=289640

In comment 0 of that bug, it notes that a first step here is rewriting callers
as I'm doing here.

In the more general case, while the style guide doesn't specifically speak about
using non-const refs as locals, the main argument it gives for not using
non-const ref arguments (that code reads as if this is by-value but is
modifiable as if it's by-pointer) seems to apply to locals as well, and was said
to explicitly outweigh the benefit of assuring the reader that the object
pointed to is non-null.

For this reason, I generally suggest to people that they not use non-const refs
anywhere.
We may have had some, but none comes to mind instantly.

   switch (media_state) {
 #if !defined(OS_MACOSX)
     case TAB_MEDIA_STATE_AUDIO_PLAYING:
     case TAB_MEDIA_STATE_AUDIO_MUTING: {
-      SkColor icon_color = tp->GetColor(ThemeProperties::COLOR_TAB_ICON);
       return gfx::Image(
           gfx::CreateVectorIcon(media_state == TAB_MEDIA_STATE_AUDIO_PLAYING
                                     ? gfx::VectorIconId::TAB_AUDIO
                                     : gfx::VectorIconId::TAB_AUDIO_MUTING,
-                                16, icon_color));
+                                16, button_color));
     }
 #else
     case TAB_MEDIA_STATE_AUDIO_PLAYING:
-      return rb.GetNativeImageNamed(IDR_TAB_AUDIO_INDICATOR);
+      return rb->GetNativeImageNamed(IDR_TAB_AUDIO_INDICATOR);
     case TAB_MEDIA_STATE_AUDIO_MUTING:
-      return rb.GetNativeImageNamed(IDR_TAB_AUDIO_MUTING_INDICATOR);
+      return rb->GetNativeImageNamed(IDR_TAB_AUDIO_MUTING_INDICATOR);
 #endif
     case TAB_MEDIA_STATE_RECORDING:
-      return rb.GetNativeImageNamed(IDR_TAB_RECORDING_INDICATOR);
+      return rb->GetNativeImageNamed(IDR_TAB_RECORDING_INDICATOR);
     case TAB_MEDIA_STATE_CAPTURING:
-      return rb.GetNativeImageNamed(IDR_TAB_CAPTURE_INDICATOR);
+      return rb->GetNativeImageNamed(IDR_TAB_CAPTURE_INDICATOR);
     case TAB_MEDIA_STATE_NONE:
       break;
   }
   NOTREACHED();
-  return rb.GetNativeImageNamed(IDR_SAD_FAVICON);
+  return gfx::Image();
 }
 
 gfx::Image GetTabMediaIndicatorAffordanceImage(TabMediaState media_state,
-                                               const ui::ThemeProvider* tp) {
+                                               SkColor button_color) {
   switch (media_state) {
     case TAB_MEDIA_STATE_AUDIO_PLAYING:
-      return GetTabMediaIndicatorImage(TAB_MEDIA_STATE_AUDIO_MUTING, tp);
+      return GetTabMediaIndicatorImage(TAB_MEDIA_STATE_AUDIO_MUTING,
+                                       button_color);
     case TAB_MEDIA_STATE_AUDIO_MUTING:
     case TAB_MEDIA_STATE_NONE:
     case TAB_MEDIA_STATE_RECORDING:
     case TAB_MEDIA_STATE_CAPTURING:
-      return GetTabMediaIndicatorImage(media_state, tp);
+      return GetTabMediaIndicatorImage(media_state, button_color);
   }
   NOTREACHED();
-  return GetTabMediaIndicatorImage(media_state, tp);
+  return GetTabMediaIndicatorImage(media_state, button_color);
 }
 
 scoped_ptr<gfx::Animation> CreateTabMediaIndicatorFadeAnimation(
     TabMediaState media_state) {
   if (media_state == TAB_MEDIA_STATE_RECORDING ||
       media_state == TAB_MEDIA_STATE_CAPTURING) {
     return TabRecordingIndicatorAnimation::Create();
   }
 
   // Note: While it seems silly to use a one-part MultiAnimation, it's the only
@@ skipping 126 matching lines @@
                      const std::vector<int>& indices) {
   for (std::vector<int>::const_iterator i = indices.begin(); i != indices.end();
        ++i) {
     if (!tab_strip.GetWebContentsAt(*i)->IsAudioMuted())
       return false;
   }
   return true;
 }
 
 }  // namespace chrome